[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5fb80bf6-96be-4654-bd54-dc4f1d5136ae@nxp.com>
Date: Mon, 30 Sep 2024 15:55:30 +0800
From: Liu Ying <victor.liu@....com>
To: Maxime Ripard <mripard@...nel.org>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, andrzej.hajda@...el.com,
neil.armstrong@...aro.org, rfoss@...nel.org,
Laurent.pinchart@...asonboard.com, jonas@...boo.se,
jernej.skrabec@...il.com, maarten.lankhorst@...ux.intel.com,
tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, shawnguo@...nel.org,
s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
catalin.marinas@....com, will@...nel.org, quic_bjorande@...cinc.com,
geert+renesas@...der.be, dmitry.baryshkov@...aro.org, arnd@...db.de,
nfraprado@...labora.com, o.rempel@...gutronix.de, y.moog@...tec.de
Subject: Re: [PATCH 4/8] drm/bridge: fsl-ldb: Use clk_round_rate() to validate
"ldb" clock rate
On 09/30/2024, Maxime Ripard wrote:
> On Mon, Sep 30, 2024 at 01:28:59PM GMT, 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, if the "ldb" clock and the pixel clock are sibling in clock
>> tree, use clk_round_rate() to validate the pixel clock rate against
>> the "ldb" clock. 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@....com>
>> ---
>> drivers/gpu/drm/bridge/fsl-ldb.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
>> index b559f3e0bef6..ee8471c86617 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,29 @@ 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;
>> +
>> + /*
>> + * 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;
>> + }
>> +
>
> I guess this is to workaround the fact that the parent rate would be
> changed, and thus the sibling rate as well? This should be documented in
> a comment if so.
This is to workaround the fact that the display controller driver
(lcdif_kms.c) cannot do the mode validation against pixel clock, as
the commit message mentions.
The parent clock is IMX8MP_VIDEO_PLL1_OUT and it's clock rate is not
supposed to be changed any more once IMX8MP_VIDEO_PLL1 clock rate is
set by using DT assigned-clock-rates property. For i.MX8MP EVK, the
clock rate is assigned to 1039500000Hz in imx8mp.dtsi in media_blk_ctrl
node.
>
> Maxime
--
Regards,
Liu Ying
Powered by blists - more mailing lists