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

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

Hi,

On 15/06/2026 12:19, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Jun 15, 2026 at 09:28:09AM +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.
>>
>> To solve this problem, add checks to rcar_du_crtc_atomic_enable() and
>> rcar_du_crtc_atomic_disable() to detect the DSC via the bridge type, and
>> skip the DCS bridge when needed.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c  | 18 ++++++++++++++++++
>>   drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c |  1 +
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
>> index 6c07c406f725..f14fd89f9104 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
>> @@ -754,6 +754,15 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>>   	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
>>   		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
>>   
>> +		/*
>> +		 * When we have a DSC block between the DU and the DSI,
>> +		 * the "bridge" points to the DSC. Detect the DSC by looking
>> +		 * at the bridge type, and skip the DSC if the bridge is not
>> +		 * the DSI bridge.
>> +		 */
>> +		if (bridge->type != DRM_MODE_CONNECTOR_DSI)
>> +			bridge = bridge->next_bridge;
>> +
> 
> Is there a reason you don't do this in rcar_du_encoder_init() instead ?
> It would avoid running this code on every enable/disable.
That works too.

Both options feel a bit confusing, in different ways. Your suggestion is 
less code and, I think, more correct as rcdu->dsi[] will always point to 
a DSI bridge (although that itself is a bit confusing, as one could 
expect it to point to a neighboring bridge from DU...), so I'll switch 
to that.

  Tomi