On 10/09/2024, Marek Vasut wrote: > The LDB serializer clock operate at either x7 or x14 rate of the input
Isn't it either x7 or 3.5x? s/operate/operates/ > LCDIFv3 scanout engine clock. Make sure the serializer clock and their > upstream Video PLL are configured early in .mode_set to the x7 or x14 > rate of pixel clock, before LCDIFv3 .atomic_enable is called which would > configure the Video PLL to low x1 rate, which is unusable. > > With this patch in place, the clock tree is correctly configured. The > example below is for a 71.1 MHz pixel clock panel, the LDB serializer > clock is then 497.7 MHz: > > video_pll1_ref_sel 1 1 0 24000000 0 0 50000 > video_pll1 1 1 0 497700000 0 0 50000 > video_pll1_bypass 1 1 0 497700000 0 0 50000 > video_pll1_out 2 2 0 497700000 0 0 50000 > media_ldb 1 1 0 497700000 0 0 50000 > media_ldb_root_clk 1 1 0 497700000 0 0 50000 > media_disp2_pix 1 1 0 71100000 0 0 50000 > media_disp2_pix_root_clk 1 1 0 71100000 0 0 50000 > > Signed-off-by: Marek Vasut <[email protected]> > --- > Cc: Abel Vesa <[email protected]> > Cc: Andrzej Hajda <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Fabio Estevam <[email protected]> > Cc: Isaac Scott <[email protected]> > Cc: Jernej Skrabec <[email protected]> > Cc: Jonas Karlman <[email protected]> > Cc: Laurent Pinchart <[email protected]> > Cc: Maarten Lankhorst <[email protected]> > Cc: Maxime Ripard <[email protected]> > Cc: Michael Turquette <[email protected]> > Cc: Neil Armstrong <[email protected]> > Cc: Peng Fan <[email protected]> > Cc: Pengutronix Kernel Team <[email protected]> > Cc: Robert Foss <[email protected]> > Cc: Sascha Hauer <[email protected]> > Cc: Shawn Guo <[email protected]> > Cc: Simona Vetter <[email protected]> > Cc: Stephen Boyd <[email protected]> > Cc: Thomas Zimmermann <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > --- > drivers/gpu/drm/bridge/fsl-ldb.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c > b/drivers/gpu/drm/bridge/fsl-ldb.c > index 0e4bac7dd04ff..a3a31467fcc85 100644 > --- a/drivers/gpu/drm/bridge/fsl-ldb.c > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c > @@ -278,6 +278,16 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge, > return MODE_OK; > } > > +static void fsl_ldb_mode_set(struct drm_bridge *bridge, > + const struct drm_display_mode *mode, > + const struct drm_display_mode *adj) > +{ > + struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge); > + unsigned long requested_link_freq = fsl_ldb_link_frequency(fsl_ldb, > mode->clock); > + > + clk_set_rate(fsl_ldb->clk, requested_link_freq); The mode_set callback won't be called when only crtc_state->active is changed from false to true in an atomic commit, e.g., blanking the emulated fbdev first and then unblanking it. So, in this case, the clk_set_rate() in fsl_ldb_atomic_enable() is still called after those from mxsfb_kms or lcdif_kms. Also, it doesn't look neat to call clk_set_rate() from both mode_set callback and atomic_enable callback. The idea is to assign a reasonable PLL clock rate in DT to make display drivers' life easier, especially for i.MX8MP where LDB, Samsung MIPI DSI may use a single PLL at the same time. > +} > + > static const struct drm_bridge_funcs funcs = { > .attach = fsl_ldb_attach, > .atomic_enable = fsl_ldb_atomic_enable, > @@ -287,6 +297,7 @@ static const struct drm_bridge_funcs funcs = { > .atomic_get_input_bus_fmts = fsl_ldb_atomic_get_input_bus_fmts, > .atomic_reset = drm_atomic_helper_bridge_reset, > .mode_valid = fsl_ldb_mode_valid, > + .mode_set = fsl_ldb_mode_set, > }; > > static int fsl_ldb_probe(struct platform_device *pdev) -- Regards, Liu Ying
