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

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

Hi,

On 11/06/2026 03:03, Laurent Pinchart wrote:
> 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

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()?

> 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;

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.

  Tomi