Due to the difference of HW, different dividers need to be set.
Signed-off-by: Shuijing Li <[email protected]>
Signed-off-by: Jitao Shi <[email protected]>
---
Changes in v3:
Separate these two things into two different patches.
per suggestion from the previous thread:
https://lore.kernel.org/lkml/[email protected]/
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/[email protected]/
---
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.1
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 <[email protected]>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> Changes in v3:
> Separate these two things into two different patches.
> per suggestion from the previous thread:
> https://lore.kernel.org/lkml/[email protected]/
> 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/[email protected]/
> ---
> 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 <[email protected]>
--
Regards,
Alexandre
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 <[email protected]>
> Signed-off-by: Jitao Shi <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
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 <[email protected]>
>> Signed-off-by: Jitao Shi <[email protected]>
>> ---
>> Changes in v3:
>> Separate these two things into two different patches.
>> per suggestion from the previous thread:
>> https://lore.kernel.org/lkml/[email protected]/
>> 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/[email protected]/
>> ---
>> 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 <[email protected]>
>
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 <[email protected]>
>>>> Signed-off-by: Jitao Shi <[email protected]>
>>>> ---
>>>> Changes in v3:
>>>> Separate these two things into two different patches.
>>>> per suggestion from the previous thread:
>>>> https://lore.kernel.org/lkml/[email protected]/
>>>> 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/[email protected]/
>>>> ---
>>>> 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 <[email protected]>
>>>
>>
>>
>
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 <[email protected]>
>>> Signed-off-by: Jitao Shi <[email protected]>
>>> ---
>>> Changes in v3:
>>> Separate these two things into two different patches.
>>> per suggestion from the previous thread:
>>> https://lore.kernel.org/lkml/[email protected]/
>>> 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/[email protected]/
>>> ---
>>> 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 <[email protected]>
>>
>
>
--
Regards,
Alexandre
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 <[email protected]>
>>>>> Signed-off-by: Jitao Shi <[email protected]>
>>>>> ---
>>>>> Changes in v3:
>>>>> Separate these two things into two different patches.
>>>>> per suggestion from the previous thread:
>>>>> https://lore.kernel.org/lkml/[email protected]/
>>>>> 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/[email protected]/
>>>>> ---
>>>>> 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 <[email protected]>
>>>>
>>>
>>>
>>
>
>
--
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 <[email protected]>
>>>>>> Signed-off-by: Jitao Shi <[email protected]>
>>>>>> ---
>>>>>> Changes in v3:
>>>>>> Separate these two things into two different patches.
>>>>>> per suggestion from the previous thread:
>>>>>> https://lore.kernel.org/lkml/[email protected]/
>>>>>> 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/[email protected]/
>>>>>> ---
>>>>>> 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 <[email protected]>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
Hi, 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 <[email protected]>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> Changes in v3:
> Separate these two things into two different patches.
> per suggestion from the previous thread:
>
https://lore.kernel.org/lkml/[email protected]/
> 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/[email protected]/
> ---
> 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)