Re: [PATCH v3 4/7] drm/rcar-du: dsi: Support DSC in the pipeline

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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 <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  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