Re: [PATCH v6 3/6] drm/rcar-du: dsi: Support DSC in the pipeline

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing validation of the assumed DSI bridge pointer and type can lead to a NULL pointer dereference or type confusion in rcar_mipi_dsi_pclk_enable().

Pre-existing issues:
- [High] The function rcar_mipi_dsi_pclk_enable calls sleeping functions but is executed from the CRTC atomic_enable callback, violating atomic context rules.
--

commit daca709dc803d35328aef5637d1cd674b65a8f0a
Author: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Subject: drm/rcar-du: dsi: Support DSC in the pipeline

This patch modifies the DU encoder initialization to support a DSC block 
between the DU and the DSI. It ensures the DSI bridge is correctly identified 
and stored in the rcdu->dsi[] array by checking the bridge type.

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

[Severity: High]
This is a pre-existing issue, but the call path mentioned here executes in
an atomic context and appears to call sleeping functions.

When the display hardware is enabled via a non-blocking modeset, the
rcar_du_crtc_atomic_enable() callback runs in atomic context. It calls
rcar_mipi_dsi_pclk_enable(), which sequentially calls clk_prepare_enable()
(which takes a mutex via the clk API) and rcar_mipi_dsi_startup() which
performs a direct sleep:

drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c:rcar_mipi_dsi_startup()
    usleep_range(400, 500);

Does this violate the atomic context rules by calling sleeping functions
(mutexes and usleep_range) during the atomic CRTC enable callback?

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
[ ... ]
> @@ -90,9 +90,22 @@ 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;
> +
>  			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] =
> -				drm_bridge_get(bridge);
> +				drm_bridge_get(dsi_bridge);

[Severity: High]
If the bridge is not a DSI bridge (e.g., it is a DSC block), this grabs
bridge->next_bridge without validating if it is non-NULL or if it actually
represents an rcar_mipi_dsi bridge.

If the device tree configures an intermediate bridge before DSI that does not
have a next_bridge set, or if it points to a non-DSI bridge,
bridge->next_bridge will evaluate to NULL or the wrong bridge. This unchecked
value is then saved to rcdu->dsi[].

Later, on the modeset path, rcar_du_crtc_atomic_enable() passes this pointer
to rcar_mipi_dsi_pclk_enable(), which directly dereferences bridge->encoder
and casts it to struct rcar_mipi_dsi:

drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c:rcar_mipi_dsi_pclk_enable()
    struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
    ...
    connector = drm_atomic_get_new_connector_for_encoder(state,
                                                         bridge->encoder);

Could this lead to a NULL pointer dereference or type confusion if an
unsupported bridge topology is loaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-rcar-du-dsc-v6-0-f8ef711a7494@ideasonboard.com?part=3