Message-ID:
 <AM7PR04MB7046546A882A8D48E135D84698272@AM7PR04MB7046.eurprd04.prod.outlook.com>
Date: Mon, 18 Nov 2024 03:54:37 +0000
From: Ying Liu <victor.liu@....com>
To: Marek Vasut <marex@...x.de>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-clk@...r.kernel.org"
	<linux-clk@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>
CC: "shawnguo@...nel.org" <shawnguo@...nel.org>, "s.hauer@...gutronix.de"
	<s.hauer@...gutronix.de>, "kernel@...gutronix.de" <kernel@...gutronix.de>,
	"festevam@...il.com" <festevam@...il.com>, "robh@...nel.org"
	<robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "catalin.marinas@....com"
	<catalin.marinas@....com>, "will@...nel.org" <will@...nel.org>,
	"abelvesa@...nel.org" <abelvesa@...nel.org>, Peng Fan <peng.fan@....com>,
	"mturquette@...libre.com" <mturquette@...libre.com>, "sboyd@...nel.org"
	<sboyd@...nel.org>, "andrzej.hajda@...el.com" <andrzej.hajda@...el.com>,
	"neil.armstrong@...aro.org" <neil.armstrong@...aro.org>, "rfoss@...nel.org"
	<rfoss@...nel.org>, Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	"jonas@...boo.se" <jonas@...boo.se>, "jernej.skrabec@...il.com"
	<jernej.skrabec@...il.com>, "maarten.lankhorst@...ux.intel.com"
	<maarten.lankhorst@...ux.intel.com>, "mripard@...nel.org"
	<mripard@...nel.org>, "tzimmermann@...e.de" <tzimmermann@...e.de>,
	"airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch" <simona@...ll.ch>,
	"quic_bjorande@...cinc.com" <quic_bjorande@...cinc.com>,
	"geert+renesas@...der.be" <geert+renesas@...der.be>,
	"dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>, "arnd@...db.de"
	<arnd@...db.de>, "nfraprado@...labora.com" <nfraprado@...labora.com>
Subject: RE: [PATCH v7 2/7] Revert "clk: imx: clk-imx8mp: Allow media_disp
 pixel clock reconfigure parent rate"

Hi Marek,

On 11/15/2024, Marek Vasut wrote:
> On 11/14/24 7:57 AM, Liu Ying wrote:
> > This reverts commit ff06ea04e4cf3ba2f025024776e83bfbdfa05155.
> >
> > media_disp1_pix clock is the pixel clock of the first i.MX8MP LCDIFv3
> > display controller, while media_disp2_pix clock is the pixel clock of
> > the second i.MX8MP LCDIFv3 display controller.  The two display
> > controllers connect with Samsung MIPI DSI controller and LVDS Display
> > Bridge(LDB) respectively.  Since the two display controllers are driven
> > by separate DRM driver instances and the two pixel clocks may be derived
> > from the same video_pll1_out clock(sys_pll3_out clock could be already
> > used to derive audio_axi clock), there is no way to negotiate a dynamically
> > changeable video_pll1_out clock rate to satisfy both of the two display
> > controllers.  In this case, the only solution to drive them with the
> > single video_pll1_out clock is to assign a sensible/unchangeable clock
> > rate for video_pll1_out clock.  Thus, there is no need to set the
> > CLK_SET_RATE_PARENT flag for media_disp{1,2}_pix clocks, drop it then.
> >
> > Fixes: ff06ea04e4cf ("clk: imx: clk-imx8mp: Allow media_disp pixel clock
> reconfigure parent rate")
> > Signed-off-by: Liu Ying <victor.liu@....com>
> Uh, I almost missed this revert between all the LDB patches.
> 
> This revert will break my usecase on MX8MP where I need to operate two
> disparate panels attached to LVDS and TC358767 DSI-to-DP bridge and I
> need accurate pixel clock for both. Not being able to configure accurate
> pixel clock will make the displays not work, so from my side, this is a
> NAK, sorry.

Is your usecase in upstream kernel? If yes, which DT file implements the
usecase?  I guess it's im8mp-dhcom-som.dtsi authored by you, but it only
contains the DT node for TC358767, but not LVDS panel.

Can you please elaborate about the failure case?

You still may assign an accurate PLL rate in DT.
This patch only makes the PLL rate be unchangeable dynamically in
runtime.  That means the existing imx8m-dhcom-som.dtsi would use
IMX8MP_VIDEO_PLL1_OUT(running at 1.0395GHz) as the parent clock
of IMX8MP_CLK_MEDIA_DISP1_PIX (for LCDIF1/DSI), since it includes
imx8mp.dsti.  I assume it should be able to support typical video modes
like 1080p60 video mode with 148.5MHz pixel clock at least with 1.0395GHz
PLL rate.  Granted that less video modes read from DP monitor would
be supported without dynamically changeable PLL rates, this is something
we have to accept because some i.MX8MP platforms(like i.MX8MP EVK)
have to share IMX8MP_VIDEO_PLL1_OUT between LVDS and MIPI DSI
display pipelines.  The missing part is that we need to do mode validation
for the MIPI DSI display pipeline either in samsung-dsim.c or lcdif_kms.c
to filter unsupported video mode out.  Is this missing mode validation
the cause of your failure case?

> 
> There has to be some better solution which still allows the PLL
> reconfiguration to achieve accurate pixel clock.

As I mentioned in cover letter, the only solution to support LVDS and
MIPI DSI displays on all i.MX8MP platforms is to assign a sensible and
unchangeable PLL rate in DT.  Some platforms may use two separate
PLLs for the LVDS and MIPI DSI display pipelines, while some others
have to use only the single IMX8MP_VIDEO_PLL1_OUT because
all other eligible PLLs are used up.  That's all fine, just being platforms
dependent.  The only limitation of the solution is that some platforms
couldn't support some particular LVDS and MIPI DSI displays at the
same time due to lack of PLLs, but this has to be accepted since
the shared IMX8MP_VIDEO_PLL1_OUT case needs to be supported and
the two display pipelines are not aware of each other from kernel's
point of view.

I hope that we can agree on this solution first before spreading
discussions across different threads and eventually the NAK can be
taken back.

Regards,
Liu Ying

Powered by blists - more mailing lists