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

[Laurent Pinchart]

On Mon, Sep 07, 2026 at 11:56:01AM +0300, Tomi Valkeinen wrote:
> On 03/07/2026 00:24, Laurent Pinchart wrote:
> > On Mon, Jun 15, 2026 at 04:11:57PM +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, make sure we store the DSI bridge to the
> >> rcdu->dsi[] array, instead of the first bridge in the DSI pipeline
> >> (which can be DCS), by checking the bridge's bridge->type.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> >> ---
> >>   drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 18 ++++++++++++++++--
> >>   drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c   |  1 +
> >>   2 files changed, 17 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> >> index db2088529b48..ac85838ab3b8 100644
> >> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> >> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> >> @@ -89,9 +89,23 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> >>   				drm_bridge_get(bridge);
> >>   
> >>   		if (output == RCAR_DU_OUTPUT_DSI0 ||
> >> -		    output == RCAR_DU_OUTPUT_DSI1)
> >> +		    output == RCAR_DU_OUTPUT_DSI1) {
> >> +			struct drm_bridge *dsi_bridge;
> >> +
> >> +			/*
> >> +			 * 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.
> >> +			 */
> >> +
> >> +			dsi_bridge = bridge->type == DRM_MODE_CONNECTOR_DSI ?
> >> +						     bridge :
> >> +						     bridge->next_bridge;
> > 
> > The indentation looks weird.
> > 
> > 			dsi_bridge = bridge->type == DRM_MODE_CONNECTOR_DSI
> > 				   ? bridge : bridge->next_bridge;
> 
> That looks even weirder, you are associating = and ? there. What about:
> 
> dsi_bridge = bridge->type == DRM_MODE_CONNECTOR_DSI ?
> 	     bridge : bridge->next_bridge;

I like my version better :-)

-- 
Regards,

Laurent Pinchart