diff mbox series

[v6,07/42] drm/mediatek: mtk_dpi: Add support for DPI input clock from HDMI

Message ID 20250211113409.1517534-8-angelogioacchino.delregno@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add support for MT8195/88 DPI, HDMIv2 and DDCv2 | expand

Commit Message

AngeloGioacchino Del Regno Feb. 11, 2025, 11:33 a.m. UTC
On some SoCs, like MT8195 and MT8188, the DPI instance that is
reserved to the HDMI Transmitter uses a different clock topology.

In this case, the DPI is clocked by the HDMI IP, and this outputs
its clock to the MM input of dpi_pixel_clk, which is essential to
enable register access to the DPI IP.

Add a `clocked_by_hdmi` member to struct mtk_dpi_conf, and check
it to avoid enabling the DPI clocks in the mediatek-drm internal
.start() callback (and avoid disabing in the .stop() component
callback): this will make sure that the clock configuration
sequence is respected during display pipeline setup by following
the bridge ops between DPI and HDMI, where the HDMI driver is
expected to enable the clocks in the bridge's pre_enable(), and
DPI in the enable() cb.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

CK Hu (胡俊光) Feb. 14, 2025, 2:09 a.m. UTC | #1
On Tue, 2025-02-11 at 12:33 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> On some SoCs, like MT8195 and MT8188, the DPI instance that is
> reserved to the HDMI Transmitter uses a different clock topology.
> 
> In this case, the DPI is clocked by the HDMI IP, and this outputs
> its clock to the MM input of dpi_pixel_clk, which is essential to
> enable register access to the DPI IP.
> 
> Add a `clocked_by_hdmi` member to struct mtk_dpi_conf, and check
> it to avoid enabling the DPI clocks in the mediatek-drm internal
> .start() callback (and avoid disabing in the .stop() component
> callback): this will make sure that the clock configuration
> sequence is respected during display pipeline setup by following
> the bridge ops between DPI and HDMI, where the HDMI driver is
> expected to enable the clocks in the bridge's pre_enable(), and
> DPI in the enable() cb.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index ad07005ad56e..6493c7e2fae4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -146,6 +146,8 @@ struct mtk_dpi_factor {
>   * @input_2p_en_bit: Enable bit of two pixel per round feature
>   * @pixels_per_iter: Quantity of transferred pixels per iteration.
>   * @edge_cfg_in_mmsys: If the edge configuration for DPI's output needs to be set in MMSYS.
> + * @clocked_by_hdmi: HDMI IP outputs clock to dpi_pixel_clk input clock, needed
> + *                  for DPI registers access.
>   */
>  struct mtk_dpi_conf {
>         const struct mtk_dpi_factor *dpi_factor;
> @@ -167,6 +169,7 @@ struct mtk_dpi_conf {
>         u32 input_2p_en_bit;
>         u32 pixels_per_iter;
>         bool edge_cfg_in_mmsys;
> +       bool clocked_by_hdmi;
>  };
> 
>  static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
> @@ -587,7 +590,9 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
>         struct videomode vm = { 0 };
> 
>         drm_display_mode_to_videomode(mode, &vm);
> -       mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
> +
> +       if (!dpi->conf->clocked_by_hdmi)
> +               mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
> 
>         dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING;
>         dpi_pol.de_pol = MTK_DPI_POLARITY_RISING;
> @@ -922,14 +927,16 @@ void mtk_dpi_start(struct device *dev)
>  {
>         struct mtk_dpi *dpi = dev_get_drvdata(dev);
> 
> -       mtk_dpi_power_on(dpi);
> +       if (!dpi->conf->clocked_by_hdmi)
> +               mtk_dpi_power_on(dpi);

mtk_dpi_bridge_enable() also call mtk_dpi_power_on().
Add this checking in mtk_dpi_bridge_enable() also.

>  }
> 
>  void mtk_dpi_stop(struct device *dev)
>  {
>         struct mtk_dpi *dpi = dev_get_drvdata(dev);
> 
> -       mtk_dpi_power_off(dpi);
> +       if (!dpi->conf->clocked_by_hdmi)
> +               mtk_dpi_power_off(dpi);

mtk_dpi_bridge_disable() also call mtk_dpi_power_off().
Add this checking in mtk_dpi_bridge_disable() also.

Because the clock is from hdmi, I think the clock define in DPI node in device tree should be removed.
Also change the binding document and let the clock not required in MT8195.

Regards,
CK

>  }
> 
>  unsigned int mtk_dpi_encoder_index(struct device *dev)
> --
> 2.48.1
>
AngeloGioacchino Del Regno Feb. 17, 2025, 2:40 p.m. UTC | #2
Il 14/02/25 03:09, CK Hu (胡俊光) ha scritto:
> On Tue, 2025-02-11 at 12:33 +0100, AngeloGioacchino Del Regno wrote:
>> External email : Please do not click links or open attachments until you have verified the sender or the content.
>>
>>
>> On some SoCs, like MT8195 and MT8188, the DPI instance that is
>> reserved to the HDMI Transmitter uses a different clock topology.
>>
>> In this case, the DPI is clocked by the HDMI IP, and this outputs
>> its clock to the MM input of dpi_pixel_clk, which is essential to
>> enable register access to the DPI IP.
>>
>> Add a `clocked_by_hdmi` member to struct mtk_dpi_conf, and check
>> it to avoid enabling the DPI clocks in the mediatek-drm internal
>> .start() callback (and avoid disabing in the .stop() component
>> callback): this will make sure that the clock configuration
>> sequence is respected during display pipeline setup by following
>> the bridge ops between DPI and HDMI, where the HDMI driver is
>> expected to enable the clocks in the bridge's pre_enable(), and
>> DPI in the enable() cb.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_dpi.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index ad07005ad56e..6493c7e2fae4 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -146,6 +146,8 @@ struct mtk_dpi_factor {
>>    * @input_2p_en_bit: Enable bit of two pixel per round feature
>>    * @pixels_per_iter: Quantity of transferred pixels per iteration.
>>    * @edge_cfg_in_mmsys: If the edge configuration for DPI's output needs to be set in MMSYS.
>> + * @clocked_by_hdmi: HDMI IP outputs clock to dpi_pixel_clk input clock, needed
>> + *                  for DPI registers access.
>>    */
>>   struct mtk_dpi_conf {
>>          const struct mtk_dpi_factor *dpi_factor;
>> @@ -167,6 +169,7 @@ struct mtk_dpi_conf {
>>          u32 input_2p_en_bit;
>>          u32 pixels_per_iter;
>>          bool edge_cfg_in_mmsys;
>> +       bool clocked_by_hdmi;
>>   };
>>
>>   static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
>> @@ -587,7 +590,9 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
>>          struct videomode vm = { 0 };
>>
>>          drm_display_mode_to_videomode(mode, &vm);
>> -       mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
>> +
>> +       if (!dpi->conf->clocked_by_hdmi)
>> +               mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
>>
>>          dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING;
>>          dpi_pol.de_pol = MTK_DPI_POLARITY_RISING;
>> @@ -922,14 +927,16 @@ void mtk_dpi_start(struct device *dev)
>>   {
>>          struct mtk_dpi *dpi = dev_get_drvdata(dev);
>>
>> -       mtk_dpi_power_on(dpi);
>> +       if (!dpi->conf->clocked_by_hdmi)
>> +               mtk_dpi_power_on(dpi);
> 
> mtk_dpi_bridge_enable() also call mtk_dpi_power_on().
> Add this checking in mtk_dpi_bridge_enable() also.
> 

That's wanted.

>>   }
>>
>>   void mtk_dpi_stop(struct device *dev)
>>   {
>>          struct mtk_dpi *dpi = dev_get_drvdata(dev);
>>
>> -       mtk_dpi_power_off(dpi);
>> +       if (!dpi->conf->clocked_by_hdmi)
>> +               mtk_dpi_power_off(dpi);
> 
> mtk_dpi_bridge_disable() also call mtk_dpi_power_off().
> Add this checking in mtk_dpi_bridge_disable() also.
> 

That's also wanted.

> Because the clock is from hdmi, I think the clock define in DPI node in device tree should be removed.
> Also change the binding document and let the clock not required in MT8195.

No, because the clock is from HDMI, but there's a gate that needs to be
enabled in order to enable the clock output to the DPI hardware.

The clock being *generated from* HDMI IP doesn't mean that there's no
gate in DPI - and that's the actual problem here that we're solving
with clocked_by_hdmi. We are preventing the clock to be ungated to DPI
while it is wrong or unstable.

I didn't miss any check, all that was on purpose.

Regards,
Angelo

> 
> Regards,
> CK
> 
>>   }
>>
>>   unsigned int mtk_dpi_encoder_index(struct device *dev)
>> --
>> 2.48.1
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index ad07005ad56e..6493c7e2fae4 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -146,6 +146,8 @@  struct mtk_dpi_factor {
  * @input_2p_en_bit: Enable bit of two pixel per round feature
  * @pixels_per_iter: Quantity of transferred pixels per iteration.
  * @edge_cfg_in_mmsys: If the edge configuration for DPI's output needs to be set in MMSYS.
+ * @clocked_by_hdmi: HDMI IP outputs clock to dpi_pixel_clk input clock, needed
+ *		     for DPI registers access.
  */
 struct mtk_dpi_conf {
 	const struct mtk_dpi_factor *dpi_factor;
@@ -167,6 +169,7 @@  struct mtk_dpi_conf {
 	u32 input_2p_en_bit;
 	u32 pixels_per_iter;
 	bool edge_cfg_in_mmsys;
+	bool clocked_by_hdmi;
 };
 
 static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
@@ -587,7 +590,9 @@  static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
 	struct videomode vm = { 0 };
 
 	drm_display_mode_to_videomode(mode, &vm);
-	mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
+
+	if (!dpi->conf->clocked_by_hdmi)
+		mtk_dpi_set_pixel_clk(dpi, &vm, mode->clock);
 
 	dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING;
 	dpi_pol.de_pol = MTK_DPI_POLARITY_RISING;
@@ -922,14 +927,16 @@  void mtk_dpi_start(struct device *dev)
 {
 	struct mtk_dpi *dpi = dev_get_drvdata(dev);
 
-	mtk_dpi_power_on(dpi);
+	if (!dpi->conf->clocked_by_hdmi)
+		mtk_dpi_power_on(dpi);
 }
 
 void mtk_dpi_stop(struct device *dev)
 {
 	struct mtk_dpi *dpi = dev_get_drvdata(dev);
 
-	mtk_dpi_power_off(dpi);
+	if (!dpi->conf->clocked_by_hdmi)
+		mtk_dpi_power_off(dpi);
 }
 
 unsigned int mtk_dpi_encoder_index(struct device *dev)