[PATCH v4 03/13] drm/bridge: fsl-ldb: Use clk_round_rate() to validate "ldb" clock rate

Liu Ying posted 13 patches 4 weeks ago
There is a newer version of this series
[PATCH v4 03/13] drm/bridge: fsl-ldb: Use clk_round_rate() to validate "ldb" clock rate
Posted by Liu Ying 4 weeks ago
Multiple display modes could be read from a display device's EDID.
Use clk_round_rate() to validate the "ldb" clock rate for each mode
in drm_bridge_funcs::mode_valid() to filter unsupported modes out.

Also, since this driver doesn't directly reference pixel clock, use
clk_round_rate() to validate the pixel clock rate against the "ldb"
clock if the "ldb" clock and the pixel clock are sibling in clock
tree.  This is not done in display controller driver because
drm_crtc_helper_funcs::mode_valid() may not decide to do the
validation or not if multiple encoders are connected to the CRTC,
e.g., i.MX93 LCDIF may connect with MIPI DSI controller, LDB and
parallel display output simultaneously.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
Note that this patch depends on a patch in shawnguo/imx/fixes:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241017031146.157996-1-marex@denx.de/

v4:
* No change.

v3:
* No change.

v2:
* Add more comments in fsl-ldb.c and commit message about pixel clock
  rate validation.  (Maxime)

 drivers/gpu/drm/bridge/fsl-ldb.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index b559f3e0bef6..77afc169f0d3 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -11,6 +11,7 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
@@ -64,6 +65,7 @@ struct fsl_ldb_devdata {
 	u32 lvds_ctrl;
 	bool lvds_en_bit;
 	bool single_ctrl_reg;
+	bool ldb_clk_pixel_clk_sibling;
 };
 
 static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
@@ -74,11 +76,13 @@ static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
 	[IMX8MP_LDB] = {
 		.ldb_ctrl = 0x5c,
 		.lvds_ctrl = 0x128,
+		.ldb_clk_pixel_clk_sibling = true,
 	},
 	[IMX93_LDB] = {
 		.ldb_ctrl = 0x20,
 		.lvds_ctrl = 0x24,
 		.lvds_en_bit = true,
+		.ldb_clk_pixel_clk_sibling = true,
 	},
 };
 
@@ -269,11 +273,31 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge,
 		   const struct drm_display_info *info,
 		   const struct drm_display_mode *mode)
 {
+	unsigned long link_freq, pclk_rate, rounded_pclk_rate;
 	struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
 
 	if (mode->clock > (fsl_ldb_is_dual(fsl_ldb) ? 160000 : 80000))
 		return MODE_CLOCK_HIGH;
 
+	/* Validate "ldb" clock rate. */
+	link_freq = fsl_ldb_link_frequency(fsl_ldb, mode->clock);
+	if (link_freq != clk_round_rate(fsl_ldb->clk, link_freq))
+		return MODE_NOCLOCK;
+
+	/*
+	 * Since this driver doesn't directly reference pixel clock and
+	 * display controller driver cannot validate pixel clock due to
+	 * multiple types of encoders connected, use "ldb" clock to
+	 * validate pixel clock rate, if the two clocks are sibling.
+	 */
+	if (fsl_ldb->devdata->ldb_clk_pixel_clk_sibling) {
+		pclk_rate = mode->clock * HZ_PER_KHZ;
+
+		rounded_pclk_rate = clk_round_rate(fsl_ldb->clk, pclk_rate);
+		if (rounded_pclk_rate != pclk_rate)
+			return MODE_NOCLOCK;
+	}
+
 	return MODE_OK;
 }
 
-- 
2.34.1
Re: [PATCH v4 03/13] drm/bridge: fsl-ldb: Use clk_round_rate() to validate "ldb" clock rate
Posted by Maxime Ripard 3 weeks, 5 days ago
On Mon, Oct 28, 2024 at 10:37:30AM +0800, Liu Ying wrote:
> Multiple display modes could be read from a display device's EDID.
> Use clk_round_rate() to validate the "ldb" clock rate for each mode
> in drm_bridge_funcs::mode_valid() to filter unsupported modes out.
> 
> Also, since this driver doesn't directly reference pixel clock, use
> clk_round_rate() to validate the pixel clock rate against the "ldb"
> clock if the "ldb" clock and the pixel clock are sibling in clock
> tree.  This is not done in display controller driver because
> drm_crtc_helper_funcs::mode_valid() may not decide to do the
> validation or not if multiple encoders are connected to the CRTC,
> e.g., i.MX93 LCDIF may connect with MIPI DSI controller, LDB and
> parallel display output simultaneously.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> Note that this patch depends on a patch in shawnguo/imx/fixes:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241017031146.157996-1-marex@denx.de/

I still believe that the root cause of this issue is your clock tree and
driver setup, and since I've asked for explanations and didn't get any,
I don't really see how we can move forward here.

Maxime
Re: [PATCH v4 03/13] drm/bridge: fsl-ldb: Use clk_round_rate() to validate "ldb" clock rate
Posted by Liu Ying 3 weeks, 4 days ago
Hi Maxime,

On 10/30/2024, Maxime Ripard wrote:
> On Mon, Oct 28, 2024 at 10:37:30AM +0800, Liu Ying wrote:
>> Multiple display modes could be read from a display device's EDID.
>> Use clk_round_rate() to validate the "ldb" clock rate for each mode
>> in drm_bridge_funcs::mode_valid() to filter unsupported modes out.
>>
>> Also, since this driver doesn't directly reference pixel clock, use
>> clk_round_rate() to validate the pixel clock rate against the "ldb"
>> clock if the "ldb" clock and the pixel clock are sibling in clock
>> tree.  This is not done in display controller driver because
>> drm_crtc_helper_funcs::mode_valid() may not decide to do the
>> validation or not if multiple encoders are connected to the CRTC,
>> e.g., i.MX93 LCDIF may connect with MIPI DSI controller, LDB and
>> parallel display output simultaneously.
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>> Note that this patch depends on a patch in shawnguo/imx/fixes:
>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241017031146.157996-1-marex@denx.de/
> 
> I still believe that the root cause of this issue is your clock tree and
> driver setup, and since I've asked for explanations and didn't get any,
> I don't really see how we can move forward here.

Since you asked for a description at *somewhere* in another thread[1],
can you please suggest a place where this could happen?

[1] https://lore.kernel.org/imx/47d92ae0-c71a-4c18-9ad7-432c0f70a31f@nxp.com/T/#m587e6a25bdab542d5d99abbf01caaca89495b1d5

> 
> Maxime

-- 
Regards,
Liu Ying
Re: [PATCH v4 03/13] drm/bridge: fsl-ldb: Use clk_round_rate() to validate "ldb" clock rate
Posted by Liu Ying 6 days, 10 hours ago
On 10/31/2024, Liu Ying wrote:
> Hi Maxime,
> 
> On 10/30/2024, Maxime Ripard wrote:
>> On Mon, Oct 28, 2024 at 10:37:30AM +0800, Liu Ying wrote:
>>> Multiple display modes could be read from a display device's EDID.
>>> Use clk_round_rate() to validate the "ldb" clock rate for each mode
>>> in drm_bridge_funcs::mode_valid() to filter unsupported modes out.
>>>
>>> Also, since this driver doesn't directly reference pixel clock, use
>>> clk_round_rate() to validate the pixel clock rate against the "ldb"
>>> clock if the "ldb" clock and the pixel clock are sibling in clock
>>> tree.  This is not done in display controller driver because
>>> drm_crtc_helper_funcs::mode_valid() may not decide to do the
>>> validation or not if multiple encoders are connected to the CRTC,
>>> e.g., i.MX93 LCDIF may connect with MIPI DSI controller, LDB and
>>> parallel display output simultaneously.
>>>
>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>> ---
>>> Note that this patch depends on a patch in shawnguo/imx/fixes:
>>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241017031146.157996-1-marex@denx.de/
>>
>> I still believe that the root cause of this issue is your clock tree and
>> driver setup, and since I've asked for explanations and didn't get any,
>> I don't really see how we can move forward here.
> 
> Since you asked for a description at *somewhere* in another thread[1],
> can you please suggest a place where this could happen?

I have written a description in the cover letter of this patch series(v7):

https://patchwork.freedesktop.org/series/139266/#rev7

If you don't mind, please provide review comments there, thanks.

> 
> [1] https://lore.kernel.org/imx/47d92ae0-c71a-4c18-9ad7-432c0f70a31f@nxp.com/T/#m587e6a25bdab542d5d99abbf01caaca89495b1d5
> 
>>
>> Maxime
> 

-- 
Regards,
Liu Ying