Hi Tomi, Thank you for the patch.
On Fri, May 15, 2026 at 12:09:29PM +0300, Tomi Valkeinen wrote: > Enabling DSI clocks on rcar-du needs some tricks as the DU dot clock is > provided by the DSI. Thus, we call rcar_mipi_dsi_pclk_enable() from the > crtc, when enabling the crtc. > > With DSC (added in upcoming patch) in the pipeline, between the DU and > the DSI, the above call path is broken as the crtc tries to call > rcar_mipi_dsi_pclk_enable() on the DSC. > > Adjust the rcar_mipi_dsi_pclk_enable() so that it detects the DSC, and > in that case gets the next bridge from the DSC, which is the DSI. > > Signed-off-by: Tomi Valkeinen <[email protected]> > --- > drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 36 > +++++++++++++++++++++++-- > 1 file changed, 34 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > index 4ef2e3c129ed..085e229bcb0b 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > @@ -88,6 +88,8 @@ struct dsi_setup_info { > const struct dsi_clk_config *clkset; > }; > > +static const struct drm_bridge_funcs rcar_mipi_dsi_bridge_ops; > + > static inline struct rcar_mipi_dsi * > bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge) > { > @@ -844,15 +846,39 @@ static void rcar_mipi_dsi_atomic_disable(struct > drm_bridge *bridge, > rcar_mipi_dsi_stop_video(dsi); > } > > +/* > + * We need to skip the DSC bridge when we have DSC in between the DU and > + * the DSI. We detect the DSI bridge via bridge->funcs, and assume the > + * next_bridge is the DSI bridge. If this is not the case, the DT data > + * is wrong (so it shouldn't really happen). > + */ > +static struct drm_bridge * > +rcar_mipi_dsi_resolve_bridge(struct drm_bridge *bridge) > +{ > + if (bridge->funcs != &rcar_mipi_dsi_bridge_ops) > + bridge = bridge->next_bridge; > + > + if (!bridge || bridge->funcs != &rcar_mipi_dsi_bridge_ops) > + return NULL; > + > + return bridge; > +} Hmmmm... It's quite a bit of a hack. It would be nicer to do this in rcar_du_crtc.c instead, where we cache the dsi bridge pointer. The question is how to then identify the right bridge, as we won't have access to rcar_mipi_dsi_bridge_ops. Should this driver set the bridge type field to DRM_MODE_CONNECTOR_DSI ? > + > void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge, > struct drm_atomic_state *state) > { > - struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge); > const struct drm_display_mode *mode; > struct drm_connector *connector; > + struct rcar_mipi_dsi *dsi; > struct drm_crtc *crtc; > int ret; > > + bridge = rcar_mipi_dsi_resolve_bridge(bridge); > + if (WARN_ON(!bridge)) > + return; > + > + dsi = bridge_to_rcar_mipi_dsi(bridge); > + > connector = drm_atomic_get_new_connector_for_encoder(state, > bridge->encoder); > crtc = drm_atomic_get_new_connector_state(state, connector)->crtc; > @@ -885,7 +911,13 @@ EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable); > > void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge) > { > - struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge); > + struct rcar_mipi_dsi *dsi; > + > + bridge = rcar_mipi_dsi_resolve_bridge(bridge); > + if (WARN_ON(!bridge)) > + return; > + > + dsi = bridge_to_rcar_mipi_dsi(bridge); > > rcar_mipi_dsi_shutdown(dsi); > rcar_mipi_dsi_clk_disable(dsi); > -- Regards, Laurent Pinchart
