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

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

On Fri, Jun 12, 2026 at 02:56:11PM +0300, Tomi Valkeinen wrote:
> On 11/06/2026 03:03, Laurent Pinchart wrote:
> > 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
> 
> It's actually cached in rcar_du_encoder.c, but used in rcar_du_crtc.c.
> 
> If I understand right, you'd like to do the DSC detection in 
> rcar_du_crtc, and skip the DSC, if needed, before calling 
> rcar_mipi_dsi_pclk_enable()?

That would be the idea, yes. Knowledge that there's a DSC in the
pipeline would be in the DU driver, not the DSI driver. I think it would
be better, as the DU driver is the one that should have an overall view
of the display pipeline.

> > 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 ?
> 
> I'm not sure how that would help. Or, I can, as the dsi driver does not 
> set the bridge type, so only DSC would set it. But isn't that even more 
> hacky?
> 
> Or did you rather mean that the DSI driver would set the bridge type, 
> and DSC would not? We can then do:
> 
> 		if (bridge->type != DRM_MODE_CONNECTOR_DSI)
> 			bridge = bridge->next_bridge;

Yes that's what I meant.

> in the crtc driver. This works. It's still a bit hacky, but I think the 
> chances of the code getting it wrong are quite low. If the output port 
> is RCAR_DU_OUTPUT_DSIx, then the next bridge must be rcar-dsi or 
> rcar-dsc, so it's all under our control. Also, it's less code than this 
> patch, so I'll go with that.

Thanks.

-- 
Regards,

Laurent Pinchart