Re: [PATCH v3 5/5] drm: renesas: rz-du: Add support for RZ/T2H SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Register access is performed unconditionally even if hardware clock enablement fails, risking bus errors or system crashes.
--

commit a456e1dcca8021206b701fc39c90471891f25115
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

drm: renesas: rz-du: Add support for RZ/T2H SoC

This commit adds support for the RZ/T2H (R9A09G077) SoC, which includes a
DU with a DPI interface. It requires an explicit assertion of a DPI
output-enable signal during CRTC startup.

> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> index 2b772a11c7eec..017d5f26bc965 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c

[ ... ]

> @@ -217,8 +218,12 @@ static void rzg2l_du_crtc_put(struct rzg2l_du_crtc *rcrtc)
>  static void rzg2l_du_start_stop(struct rzg2l_du_crtc *rcrtc, bool start)
>  {
>  	struct rzg2l_du_device *rcdu = rcrtc->dev;
> +	u32 val = DU_MCR0_DI_EN;
>  
> -	writel(start ? DU_MCR0_DI_EN : 0, rcdu->mmio + DU_MCR0);
> +	if (start && rzg2l_du_has(rcdu, RZG2L_DU_FEATURE_DPIO_OE))
> +		val |= DU_MCR0_DPI_EN;
> +
> +	writel(start ? val : 0, rcdu->mmio + DU_MCR0);
>  }
>  
>  static void rzg2l_du_crtc_start(struct rzg2l_du_crtc *rcrtc)

This isn't a bug introduced by this patch, but this modifies the register
write logic inside an unsafe hardware path.

In rzg2l_du_crtc_atomic_enable(), rzg2l_du_crtc_get(rcrtc) is called to enable
clocks and deassert the reset controller, but its return value is ignored:

drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c:rzg2l_du_crtc_atomic_enable() {
	...
	rzg2l_du_crtc_get(rcrtc);

	rzg2l_du_crtc_start(rcrtc);
}

If clock enablement fails, the driver unconditionally proceeds to
rzg2l_du_crtc_start(), which accesses the DU_MCR0 MMIO register via
rzg2l_du_start_stop() above.

Could accessing MMIO registers without the corresponding clocks enabled result
in an external asynchronous abort or bus lockup, leading to a system crash?

Although atomic_enable cannot return an error to the DRM core, should it at
least skip subsequent hardware register accesses to avoid crashing the system
if rzg2l_du_crtc_get() fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512144104.761531-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=5