drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++- drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)
Due to the difference of HW, different dividers need to be set.
Signed-off-by: Shuijing Li <shuijing.li@mediatek.com>
Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
Changes in v3:
Separate these two things into two different patches.
per suggestion from the previous thread:
https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/
Changes in v2:
- change the variables' name to be more descriptive
- add a comment that describes the function of mtk_dp_audio_sample_arrange
- reduce indentation by doing the inverse check
- add a definition of some bits
- add support for mediatek, mt8188-edp-tx
per suggestion from the previous thread:
https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/
---
drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++-
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index d8cda83d6fef..8e1a13ab2ba2 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -140,6 +140,7 @@ struct mtk_dp_data {
const struct mtk_dp_efuse_fmt *efuse_fmt;
bool audio_supported;
bool audio_pkt_in_hblank_area;
+ u16 audio_m_div2_bit;
};
static const struct mtk_dp_efuse_fmt mt8195_edp_efuse_fmt[MTK_DP_CAL_MAX] = {
@@ -648,7 +649,7 @@ static void mtk_dp_audio_sdp_asp_set_channels(struct mtk_dp *mtk_dp,
static void mtk_dp_audio_set_divider(struct mtk_dp *mtk_dp)
{
mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_30BC,
- AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
+ mtk_dp->data->audio_m_div2_bit,
AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MASK);
}
@@ -2636,6 +2637,7 @@ static const struct mtk_dp_data mt8188_edp_data = {
.efuse_fmt = mt8195_edp_efuse_fmt,
.audio_supported = false,
.audio_pkt_in_hblank_area = false,
+ .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
};
static const struct mtk_dp_data mt8188_dp_data = {
@@ -2644,6 +2646,7 @@ static const struct mtk_dp_data mt8188_dp_data = {
.efuse_fmt = mt8195_dp_efuse_fmt,
.audio_supported = true,
.audio_pkt_in_hblank_area = true,
+ .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
};
static const struct mtk_dp_data mt8195_edp_data = {
@@ -2652,6 +2655,7 @@ static const struct mtk_dp_data mt8195_edp_data = {
.efuse_fmt = mt8195_edp_efuse_fmt,
.audio_supported = false,
.audio_pkt_in_hblank_area = false,
+ .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
};
static const struct mtk_dp_data mt8195_dp_data = {
@@ -2660,6 +2664,7 @@ static const struct mtk_dp_data mt8195_dp_data = {
.efuse_fmt = mt8195_dp_efuse_fmt,
.audio_supported = true,
.audio_pkt_in_hblank_area = false,
+ .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
};
static const struct of_device_id mtk_dp_of_match[] = {
diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
index f38d6ff12afe..6d7f0405867e 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
+++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
@@ -162,6 +162,7 @@
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8)
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8)
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8)
+#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8)
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8)
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8)
#define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8)
--
2.40.1Hi, Shuijing:
On Thu, 2023-07-20 at 16:26 +0800, Shuijing Li wrote:
> Due to the difference of HW, different dividers need to be set.
>
> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> Changes in v3:
> Separate these two things into two different patches.
> per suggestion from the previous thread:
>
https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/
> Changes in v2:
> - change the variables' name to be more descriptive
> - add a comment that describes the function of
> mtk_dp_audio_sample_arrange
> - reduce indentation by doing the inverse check
> - add a definition of some bits
> - add support for mediatek, mt8188-edp-tx
> per suggestion from the previous thread:
>
https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++-
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> index d8cda83d6fef..8e1a13ab2ba2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -140,6 +140,7 @@ struct mtk_dp_data {
> const struct mtk_dp_efuse_fmt *efuse_fmt;
> bool audio_supported;
> bool audio_pkt_in_hblank_area;
> + u16 audio_m_div2_bit;
> };
>
> static const struct mtk_dp_efuse_fmt
> mt8195_edp_efuse_fmt[MTK_DP_CAL_MAX] = {
> @@ -648,7 +649,7 @@ static void
> mtk_dp_audio_sdp_asp_set_channels(struct mtk_dp *mtk_dp,
> static void mtk_dp_audio_set_divider(struct mtk_dp *mtk_dp)
> {
> mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_30BC,
> - AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> + mtk_dp->data->audio_m_div2_bit,
> AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MASK);
> }
>
> @@ -2636,6 +2637,7 @@ static const struct mtk_dp_data mt8188_edp_data
> = {
> .efuse_fmt = mt8195_edp_efuse_fmt,
> .audio_supported = false,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit =
> MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8188_dp_data = {
> @@ -2644,6 +2646,7 @@ static const struct mtk_dp_data mt8188_dp_data
> = {
> .efuse_fmt = mt8195_dp_efuse_fmt,
> .audio_supported = true,
> .audio_pkt_in_hblank_area = true,
> + .audio_m_div2_bit =
> MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8195_edp_data = {
> @@ -2652,6 +2655,7 @@ static const struct mtk_dp_data mt8195_edp_data
> = {
> .efuse_fmt = mt8195_edp_efuse_fmt,
> .audio_supported = false,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8195_dp_data = {
> @@ -2660,6 +2664,7 @@ static const struct mtk_dp_data mt8195_dp_data
> = {
> .efuse_fmt = mt8195_dp_efuse_fmt,
> .audio_supported = true,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct of_device_id mtk_dp_of_match[] = {
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> index f38d6ff12afe..6d7f0405867e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -162,6 +162,7 @@
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8)
> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 <<
> 8)
For AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_*, add prefix 'MT8195_'.
And list all configuration for MT8188.
Regards,
CK
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8)
On 20/07/2023 10:26, Shuijing Li wrote:
> Due to the difference of HW, different dividers need to be set.
>
> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> Changes in v3:
> Separate these two things into two different patches.
> per suggestion from the previous thread:
> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/
> Changes in v2:
> - change the variables' name to be more descriptive
> - add a comment that describes the function of mtk_dp_audio_sample_arrange
> - reduce indentation by doing the inverse check
> - add a definition of some bits
> - add support for mediatek, mt8188-edp-tx
> per suggestion from the previous thread:
> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++-
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index d8cda83d6fef..8e1a13ab2ba2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -140,6 +140,7 @@ struct mtk_dp_data {
> const struct mtk_dp_efuse_fmt *efuse_fmt;
> bool audio_supported;
> bool audio_pkt_in_hblank_area;
> + u16 audio_m_div2_bit;
> };
>
> static const struct mtk_dp_efuse_fmt mt8195_edp_efuse_fmt[MTK_DP_CAL_MAX] = {
> @@ -648,7 +649,7 @@ static void mtk_dp_audio_sdp_asp_set_channels(struct mtk_dp *mtk_dp,
> static void mtk_dp_audio_set_divider(struct mtk_dp *mtk_dp)
> {
> mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_30BC,
> - AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> + mtk_dp->data->audio_m_div2_bit,
> AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MASK);
> }
>
> @@ -2636,6 +2637,7 @@ static const struct mtk_dp_data mt8188_edp_data = {
> .efuse_fmt = mt8195_edp_efuse_fmt,
> .audio_supported = false,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8188_dp_data = {
> @@ -2644,6 +2646,7 @@ static const struct mtk_dp_data mt8188_dp_data = {
> .efuse_fmt = mt8195_dp_efuse_fmt,
> .audio_supported = true,
> .audio_pkt_in_hblank_area = true,
> + .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8195_edp_data = {
> @@ -2652,6 +2655,7 @@ static const struct mtk_dp_data mt8195_edp_data = {
> .efuse_fmt = mt8195_edp_efuse_fmt,
> .audio_supported = false,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct mtk_dp_data mt8195_dp_data = {
> @@ -2660,6 +2664,7 @@ static const struct mtk_dp_data mt8195_dp_data = {
> .efuse_fmt = mt8195_dp_efuse_fmt,
> .audio_supported = true,
> .audio_pkt_in_hblank_area = false,
> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
> };
>
> static const struct of_device_id mtk_dp_of_match[] = {
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> index f38d6ff12afe..6d7f0405867e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -162,6 +162,7 @@
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8)
> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8)
IMO, it's a bit weird to have SoC specific define in the generic header.
Are you sure this bit is only available for MT8188 ?
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8)
> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8)
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
--
Regards,
Alexandre
Il 20/07/23 12:14, Alexandre Mergnat ha scritto:
>
>
> On 20/07/2023 10:26, Shuijing Li wrote:
>> Due to the difference of HW, different dividers need to be set.
>>
>> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com>
>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
>> ---
>> Changes in v3:
>> Separate these two things into two different patches.
>> per suggestion from the previous thread:
>> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/
>> Changes in v2:
>> - change the variables' name to be more descriptive
>> - add a comment that describes the function of mtk_dp_audio_sample_arrange
>> - reduce indentation by doing the inverse check
>> - add a definition of some bits
>> - add support for mediatek, mt8188-edp-tx
>> per suggestion from the previous thread:
>> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/
>> ---
>> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++-
>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 +
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
>> index d8cda83d6fef..8e1a13ab2ba2 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
>> @@ -140,6 +140,7 @@ struct mtk_dp_data {
>> const struct mtk_dp_efuse_fmt *efuse_fmt;
>> bool audio_supported;
>> bool audio_pkt_in_hblank_area;
>> + u16 audio_m_div2_bit;
>> };
>> static const struct mtk_dp_efuse_fmt mt8195_edp_efuse_fmt[MTK_DP_CAL_MAX] = {
>> @@ -648,7 +649,7 @@ static void mtk_dp_audio_sdp_asp_set_channels(struct mtk_dp
>> *mtk_dp,
>> static void mtk_dp_audio_set_divider(struct mtk_dp *mtk_dp)
>> {
>> mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_30BC,
>> - AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
>> + mtk_dp->data->audio_m_div2_bit,
>> AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MASK);
>> }
>> @@ -2636,6 +2637,7 @@ static const struct mtk_dp_data mt8188_edp_data = {
>> .efuse_fmt = mt8195_edp_efuse_fmt,
>> .audio_supported = false,
>> .audio_pkt_in_hblank_area = false,
>> + .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
>> };
>> static const struct mtk_dp_data mt8188_dp_data = {
>> @@ -2644,6 +2646,7 @@ static const struct mtk_dp_data mt8188_dp_data = {
>> .efuse_fmt = mt8195_dp_efuse_fmt,
>> .audio_supported = true,
>> .audio_pkt_in_hblank_area = true,
>> + .audio_m_div2_bit = MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
>> };
>> static const struct mtk_dp_data mt8195_edp_data = {
>> @@ -2652,6 +2655,7 @@ static const struct mtk_dp_data mt8195_edp_data = {
>> .efuse_fmt = mt8195_edp_efuse_fmt,
>> .audio_supported = false,
>> .audio_pkt_in_hblank_area = false,
>> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
>> };
>> static const struct mtk_dp_data mt8195_dp_data = {
>> @@ -2660,6 +2664,7 @@ static const struct mtk_dp_data mt8195_dp_data = {
>> .efuse_fmt = mt8195_dp_efuse_fmt,
>> .audio_supported = true,
>> .audio_pkt_in_hblank_area = false,
>> + .audio_m_div2_bit = AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2,
>> };
>> static const struct of_device_id mtk_dp_of_match[] = {
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> index f38d6ff12afe..6d7f0405867e 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
>> @@ -162,6 +162,7 @@
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8)
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8)
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8)
>> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8)
>
> IMO, it's a bit weird to have SoC specific define in the generic header.
> Are you sure this bit is only available for MT8188 ?
>
Eh, the P0_DIV2 bit is 5<<8 for MT8195, while for 8188 it's 4<<8, clearly :-)
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8)
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8)
>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8)
>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
>
On 20/07/2023 13:54, AngeloGioacchino Del Regno wrote: > Il 20/07/23 12:14, Alexandre Mergnat ha scritto: >> >> >> On 20/07/2023 10:26, Shuijing Li wrote: >>> Due to the difference of HW, different dividers need to be set. >>> >>> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com> >>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> >>> --- >>> Changes in v3: >>> Separate these two things into two different patches. >>> per suggestion from the previous thread: >>> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/ >>> Changes in v2: >>> - change the variables' name to be more descriptive >>> - add a comment that describes the function of >>> mtk_dp_audio_sample_arrange >>> - reduce indentation by doing the inverse check >>> - add a definition of some bits >>> - add support for mediatek, mt8188-edp-tx >>> per suggestion from the previous thread: >>> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/ >>> --- >>> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++- >>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 + >>> 2 files changed, 7 insertions(+), 1 deletion(-) >>> ... >>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>> index f38d6ff12afe..6d7f0405867e 100644 >>> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>> @@ -162,6 +162,7 @@ >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8) >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8) >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8) >>> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8) >> >> IMO, it's a bit weird to have SoC specific define in the generic header. >> Are you sure this bit is only available for MT8188 ? >> > > Eh, the P0_DIV2 bit is 5<<8 for MT8195, while for 8188 it's 4<<8, > clearly :-) > Ok then, to avoid this kind of issue for other SoCs in the future, is that make sense for you to do a SoC specific header file beside the generic one? >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8) >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8) >>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8) >> >> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >> > > -- Regards, Alexandre
Il 20/07/23 14:07, Alexandre Mergnat ha scritto: > > > On 20/07/2023 13:54, AngeloGioacchino Del Regno wrote: >> Il 20/07/23 12:14, Alexandre Mergnat ha scritto: >>> >>> >>> On 20/07/2023 10:26, Shuijing Li wrote: >>>> Due to the difference of HW, different dividers need to be set. >>>> >>>> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com> >>>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> >>>> --- >>>> Changes in v3: >>>> Separate these two things into two different patches. >>>> per suggestion from the previous thread: >>>> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/ >>>> Changes in v2: >>>> - change the variables' name to be more descriptive >>>> - add a comment that describes the function of mtk_dp_audio_sample_arrange >>>> - reduce indentation by doing the inverse check >>>> - add a definition of some bits >>>> - add support for mediatek, mt8188-edp-tx >>>> per suggestion from the previous thread: >>>> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/ >>>> --- >>>> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++- >>>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 + >>>> 2 files changed, 7 insertions(+), 1 deletion(-) >>>> > ... >>>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>> index f38d6ff12afe..6d7f0405867e 100644 >>>> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>> @@ -162,6 +162,7 @@ >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8) >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8) >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8) >>>> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8) >>> >>> IMO, it's a bit weird to have SoC specific define in the generic header. >>> Are you sure this bit is only available for MT8188 ? >>> >> >> Eh, the P0_DIV2 bit is 5<<8 for MT8195, while for 8188 it's 4<<8, clearly :-) >> > > Ok then, to avoid this kind of issue for other SoCs in the future, is that make > sense for you to do a SoC specific header file beside the generic one? > For just one definition? That's a bit overkill :-) >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8) >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8) >>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8) >>> >>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>> >> >> >
On 20/07/2023 14:08, AngeloGioacchino Del Regno wrote: > Il 20/07/23 14:07, Alexandre Mergnat ha scritto: >> >> >> On 20/07/2023 13:54, AngeloGioacchino Del Regno wrote: >>> Il 20/07/23 12:14, Alexandre Mergnat ha scritto: >>>> >>>> >>>> On 20/07/2023 10:26, Shuijing Li wrote: >>>>> Due to the difference of HW, different dividers need to be set. >>>>> >>>>> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com> >>>>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> >>>>> --- >>>>> Changes in v3: >>>>> Separate these two things into two different patches. >>>>> per suggestion from the previous thread: >>>>> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/ >>>>> Changes in v2: >>>>> - change the variables' name to be more descriptive >>>>> - add a comment that describes the function of >>>>> mtk_dp_audio_sample_arrange >>>>> - reduce indentation by doing the inverse check >>>>> - add a definition of some bits >>>>> - add support for mediatek, mt8188-edp-tx >>>>> per suggestion from the previous thread: >>>>> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/ >>>>> --- >>>>> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++- >>>>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 + >>>>> 2 files changed, 7 insertions(+), 1 deletion(-) >>>>> >> ... >>>>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>> index f38d6ff12afe..6d7f0405867e 100644 >>>>> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>> @@ -162,6 +162,7 @@ >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8) >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8) >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8) >>>>> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8) >>>> >>>> IMO, it's a bit weird to have SoC specific define in the generic >>>> header. >>>> Are you sure this bit is only available for MT8188 ? >>>> >>> >>> Eh, the P0_DIV2 bit is 5<<8 for MT8195, while for 8188 it's 4<<8, >>> clearly :-) >>> >> >> Ok then, to avoid this kind of issue for other SoCs in the future, is >> that make sense for you to do a SoC specific header file beside the >> generic one? >> > > For just one definition? That's a bit overkill :-) > You're right, but we must start somewhere ^^, and show the proper way for future patches. Actually, I gave my Reviewed-by because it's only one definition. This will be fixed later (I hope). >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8) >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8) >>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8) >>>> >>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> >>> >>> >> > > -- Regards, Alexandre
Il 20/07/23 14:29, Alexandre Mergnat ha scritto: > > > On 20/07/2023 14:08, AngeloGioacchino Del Regno wrote: >> Il 20/07/23 14:07, Alexandre Mergnat ha scritto: >>> >>> >>> On 20/07/2023 13:54, AngeloGioacchino Del Regno wrote: >>>> Il 20/07/23 12:14, Alexandre Mergnat ha scritto: >>>>> >>>>> >>>>> On 20/07/2023 10:26, Shuijing Li wrote: >>>>>> Due to the difference of HW, different dividers need to be set. >>>>>> >>>>>> Signed-off-by: Shuijing Li <shuijing.li@mediatek.com> >>>>>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> >>>>>> --- >>>>>> Changes in v3: >>>>>> Separate these two things into two different patches. >>>>>> per suggestion from the previous thread: >>>>>> https://lore.kernel.org/lkml/e2ad22bcba31797f38a12a488d4246a01bf0cb2e.camel@mediatek.com/ >>>>>> Changes in v2: >>>>>> - change the variables' name to be more descriptive >>>>>> - add a comment that describes the function of mtk_dp_audio_sample_arrange >>>>>> - reduce indentation by doing the inverse check >>>>>> - add a definition of some bits >>>>>> - add support for mediatek, mt8188-edp-tx >>>>>> per suggestion from the previous thread: >>>>>> https://lore.kernel.org/lkml/ac0fcec9-a2fe-06cc-c727-189ef7babe9c@collabora.com/ >>>>>> --- >>>>>> drivers/gpu/drm/mediatek/mtk_dp.c | 7 ++++++- >>>>>> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 1 + >>>>>> 2 files changed, 7 insertions(+), 1 deletion(-) >>>>>> >>> ... >>>>>> b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>>> index f38d6ff12afe..6d7f0405867e 100644 >>>>>> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>>> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h >>>>>> @@ -162,6 +162,7 @@ >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_2 (1 << 8) >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_4 (2 << 8) >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MUL_8 (3 << 8) >>>>>> +#define MT8188_AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (4 << 8) >>>>> >>>>> IMO, it's a bit weird to have SoC specific define in the generic header. >>>>> Are you sure this bit is only available for MT8188 ? >>>>> >>>> >>>> Eh, the P0_DIV2 bit is 5<<8 for MT8195, while for 8188 it's 4<<8, clearly :-) >>>> >>> >>> Ok then, to avoid this kind of issue for other SoCs in the future, is that make >>> sense for you to do a SoC specific header file beside the generic one? >>> >> >> For just one definition? That's a bit overkill :-) >> > > You're right, but we must start somewhere ^^, and show the proper way for future > patches. Actually, I gave my Reviewed-by because it's only one definition. This > will be fixed later (I hope). > I'm confident that *if* and when "a bunch" of SoC-specific definitions will appear, those will be splitted in different headers. :-) >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_2 (5 << 8) >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_4 (6 << 8) >>>>>> #define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_DIV_8 (7 << 8) >>>>> >>>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>> >>>> >>>> >>> >> >> >
Il 20/07/23 10:26, Shuijing Li ha scritto: > Due to the difference of HW, different dividers need to be set. > > Signed-off-by: Shuijing Li <shuijing.li@mediatek.com> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
© 2016 - 2024 Red Hat, Inc.