On Wed, 17 Apr 2024 at 02:57, Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> wrote: > > Ordering issues here cause an uninitalized (default STANDALONE) > usecase to be programmed (which appears to be a MUX) in some cases > when msm_dsi_host_register() is called, leading to the slave PLL in > bonded-DSI mode to source from a clock parent (dsi1vco) that is off. > > This should seemingly not be a problem as the actual dispcc clocks from > DSI1 that are muxed in the clock tree of DSI0 are way further down, this > bit still seems to have an effect on them somehow and causes the right > side of the panel controlled by DSI1 to not function. > > In an ideal world this code is refactored to no longer have such > error-prone calls "across subsystems", and instead model the "PLL src" > register field as a regular mux so that changing the clock parents > programmatically or in DTS via `assigned-clock-parents` has the > desired effect. > But for the avid reader, the clocks that we *are* muxing into DSI0's > tree are way further down, so if this bit turns out to be a simple mux > between dsiXvco and out_div, that shouldn't have any effect as this > whole tree is off anyway. > > Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> > --- > drivers/gpu/drm/msm/dsi/dsi_manager.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c > index af2a287cb3bd..17f43b3c0494 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > @@ -85,6 +85,17 @@ static int dsi_mgr_setup_components(int id) > msm_dsi : other_dsi; > struct msm_dsi *slave_link_dsi = IS_MASTER_DSI_LINK(id) ? > other_dsi : msm_dsi; > + > + /* PLL0 is to drive both 2 DSI link clocks in bonded DSI mode. > + * > + * Set the usecase before calling msm_dsi_host_register() to prevent it from > + * enabling and configuring the usecase (which is just a mux bit) first. > + */ > + msm_dsi_phy_set_usecase(clk_master_dsi->phy, > + MSM_DSI_PHY_MASTER); > + msm_dsi_phy_set_usecase(clk_slave_dsi->phy, > + MSM_DSI_PHY_SLAVE); > + > /* Register slave host first, so that slave DSI device > * has a chance to probe, and do not block the master > * DSI device's probe. > @@ -100,10 +111,6 @@ static int dsi_mgr_setup_components(int id) > return ret; > > /* PLL0 is to drive both 2 DSI link clocks in bonded DSI mode. */ > - msm_dsi_phy_set_usecase(clk_master_dsi->phy, > - MSM_DSI_PHY_MASTER); > - msm_dsi_phy_set_usecase(clk_slave_dsi->phy, > - MSM_DSI_PHY_SLAVE); > msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); > msm_dsi_host_set_phy_mode(other_dsi->host, other_dsi->phy); Please move msm_dsi_host_set_phy_mode() calls too. Also please update the non-bonded case. > } > > -- > 2.44.0 > -- With best wishes Dmitry
