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@xxxxxxx> > 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