[PATCH 1/2] clk: imx: clk-imx8mp: Allow LDB serializer clock reconfigure parent rate

Marek Vasut marex at denx.de
Sat Oct 12 21:07:13 UTC 2024


On 10/11/24 8:18 AM, Liu Ying wrote:
> On 10/11/2024, Marek Vasut wrote:
>> On 10/10/24 7:22 AM, Liu Ying wrote:
>>> On 10/09/2024, Marek Vasut wrote:
>>>> The media_ldb_root_clk supply LDB serializer. These clock are usually
>>>> shared with the LCDIFv3 pixel clock and supplied by the Video PLL on
>>>> i.MX8MP, but the LDB clock run at either x7 or x14 rate of the LCDIFv3
>>>> pixel clock. Allow the LDB to reconfigure Video PLL as needed, as that
>>>> results in accurate serializer clock.
>>>>
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> ---
>>>> Cc: Abel Vesa <abelvesa at kernel.org>
>>>> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
>>>> Cc: David Airlie <airlied at gmail.com>
>>>> Cc: Fabio Estevam <festevam at gmail.com>
>>>> Cc: Isaac Scott <isaac.scott at ideasonboard.com>
>>>> Cc: Jernej Skrabec <jernej.skrabec at gmail.com>
>>>> Cc: Jonas Karlman <jonas at kwiboo.se>
>>>> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>> Cc: Maxime Ripard <mripard at kernel.org>
>>>> Cc: Michael Turquette <mturquette at baylibre.com>
>>>> Cc: Neil Armstrong <neil.armstrong at linaro.org>
>>>> Cc: Peng Fan <peng.fan at nxp.com>
>>>> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
>>>> Cc: Robert Foss <rfoss at kernel.org>
>>>> Cc: Sascha Hauer <s.hauer at pengutronix.de>
>>>> Cc: Shawn Guo <shawnguo at kernel.org>
>>>> Cc: Simona Vetter <simona at ffwll.ch>
>>>> Cc: Stephen Boyd <sboyd at kernel.org>
>>>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>>>> Cc: dri-devel at lists.freedesktop.org
>>>> Cc: imx at lists.linux.dev
>>>> Cc: kernel at dh-electronics.com
>>>> Cc: linux-arm-kernel at lists.infradead.org
>>>> Cc: linux-clk at vger.kernel.org
>>>> ---
>>>>    drivers/clk/imx/clk-imx8mp.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
>>>> index 516dbd170c8a3..2e61d340b8ab7 100644
>>>> --- a/drivers/clk/imx/clk-imx8mp.c
>>>> +++ b/drivers/clk/imx/clk-imx8mp.c
>>>> @@ -611,7 +611,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
>>>>        hws[IMX8MP_CLK_MEDIA_MIPI_PHY1_REF] = imx8m_clk_hw_composite("media_mipi_phy1_ref", imx8mp_media_mipi_phy1_ref_sels, ccm_base + 0xbd80);
>>>>        hws[IMX8MP_CLK_MEDIA_DISP1_PIX] = imx8m_clk_hw_composite_bus_flags("media_disp1_pix", imx8mp_media_disp_pix_sels, ccm_base + 0xbe00, CLK_SET_RATE_PARENT);
>>>>        hws[IMX8MP_CLK_MEDIA_CAM2_PIX] = imx8m_clk_hw_composite("media_cam2_pix", imx8mp_media_cam2_pix_sels, ccm_base + 0xbe80);
>>>> -    hws[IMX8MP_CLK_MEDIA_LDB] = imx8m_clk_hw_composite("media_ldb", imx8mp_media_ldb_sels, ccm_base + 0xbf00);
>>>> +    hws[IMX8MP_CLK_MEDIA_LDB] = imx8m_clk_hw_composite_bus_flags("media_ldb", imx8mp_media_ldb_sels, ccm_base + 0xbf00, CLK_SET_RATE_PARENT);
>>>
>>> This patch would cause the below in-flight LDB bridge driver
>>> patch[1] fail to do display mode validation upon display modes
>>> read from LVDS to HDMI converter IT6263's DDC I2C bus.
>>
>> Why ?
> 
> Mode validation is affected only for dual LVDS link mode.
> For single LVDS link mode, this patch does open more display
> modes read from the DDC I2C bus.  The reason behind is that
> LVDS serial clock rate/pixel clock rate = 3.5 for dual LVDS
> link mode, while it's 7 for single LVDS link mode.
> 
> In my system, "video_pll1" clock rate is assigned to 1.0395GHz
> in imx8mp.dtsi.  For 1920x1080-60.00Hz with 148.5MHz pixel
> clock rate, "media_ldb" clock rate is 519.75MHz and
> "media_disp2_pix" clock rate is 148.5MHz, which is fine for
> dual LVDS link mode(x3.5).  For newly opened up 1920x1080-59.94Hz
> with 148.352MHz pixel clock rate, "video_pll1" clock rate will
> be changed to 519.232MHz, "media_ldb" clock rate is 519.232MHz
> and "media_disp2_pix" clock rate is wrongly set to 519.232MHz
> too because "media_disp2_pix" clock cannot handle the 3.5
> division ratio from "video_pll1_out" clock running at
> 519.232MHz.  See the below clk_summary.

Shouldn't this patch help exactly with that ?

It should allow you to set video_pll1_out to whatever is necessary by 
LDB first, fixate that frequency, and the LCDIFv3 would then be forced 
to use /7 divider from faster Video PLL1 , right ?

>      video_pll1_ref_sel               1       1        0        24000000    0          0     50000      Y      deviceless                      no_connection_id
>         video_pll1                    1       1        0        519232000   0          0     50000      Y         deviceless                      no_connection_id
>            video_pll1_bypass          1       1        0        519232000   0          0     50000      Y            deviceless                      no_connection_id
>               video_pll1_out          2       2        0        519232000   0          0     50000      Y               deviceless                      no_connection_id
>                  media_ldb            1       1        0        519232000   0          0     50000      Y                  deviceless                      no_connection_id
>                     media_ldb_root_clk 1       1        0        519232000   0          0     50000      Y                     32ec0000.blk-ctrl:bridge at 5c     ldb
>                                                                                                                               deviceless                      no_connection_id
>                  media_disp1_pix      0       0        0        129808000   0          0     50000      N                  deviceless                      no_connection_id
>                     media_disp1_pix_root_clk 0       0        0        129808000   0          0     50000      N                     32e80000.display-controller     pix
>                                                                                                                               32ec0000.blk-ctrl               disp1
>                                                                                                                               deviceless                      no_connection_id
>                  media_disp2_pix      1       1        0        519232000   0          0     50000      Y                  deviceless                      no_connection_id
>                     media_disp2_pix_root_clk 1       1        0        519232000   0          0     50000      Y                     32e90000.display-controller     pix
>                                                                                                                               32ec0000.blk-ctrl               disp2
>                                                                                                                               deviceless                      no_connection_id
> 
> Single LVDS link mode is not affected because "media_disp2_pix"
> clock can handle the 7 division ratio.
> 
> To support the dual LVDS link mode, "video_pll1" clock rate needs
> to be x2 "media_ldb" clock rate so that "media_disp2_pix" clock
> can use 7 division ratio to achieve the /3.5 clock rate comparing
> to "media_ldb" clock rate.  However, "video_pll1" is not seen by
> LDB driver thus not directly controlled by it.  This is another
> reason why assigning a reasonable "video_pll1" clock rate in DT
> makes sense.

I agree that _right_now_, the DT clock assignment is the only option.
I would like to see that DT part disappear and instead would prefer if 
the LDB/LCDIF could figure out the clock tree configuration themselves.

>> Also, please Cc me on fsl-ldb.c patches.
> 
> Ok, will do.  BTW, if MAINTAINERS is updated, then you'll always
> receive fsl-ldb.c patches.

Thanks

>>> Unsupported display modes cannot be ruled out.  Note that
>>> "media_ldb" is derived from "video_pll1_out" by default as the
>>> parent is set in imx8mp.dtsi.  And, the only 4 rates supported
>>> by "video_pll1" are listed in imx_pll1443x_tbl[] - 1.0395GHz,
>>> 650MHz, 594MHz and 519.75MHz.
>> I disagree with this part, since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates") the 1443x PLLs can be configured to arbitrary rates which for video PLL is desirable as those should produce accurate clock.
> 
> Kinda ack - that commit does open up many more clock rates.
> But, the commit just says dynamic rates, not arbitrary rates or
> any rate.
I am fine with that.


More information about the dri-devel mailing list