| Message ID | 20230720082604.18618-4-shuijing.li@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add compatible to increase MT8188 audio control | expand |
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>
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>
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> >> > >
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> >>>> >>> >>> >> > >
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> >>>>> >>>> >>>> >>> >> >> >
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 <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)
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)