Re: [PATCH v5 3/7] drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The DRM bridge remains registered and globally accessible after `pm_runtime_disable()` is called, creating a teardown order race during driver removal.
- [Medium] The sleeping function `pm_runtime_get_sync()` is called from an atomic context path (`rcar_dsc_atomic_enable`).
--

commit a8616c71b764dc9fa4ae244e47795371e7fead44
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver

The Renesas DSC Display Stream Compression is a bridge embedded in the
Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to
8k or 400 Mpixel/s. Add rudimentary driver, which currently acts as a
pass-through bridge and allows DSI1 to be operational on R-Car V4H.

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c

[ ... ]

> +static void rcar_dsc_atomic_enable(struct drm_bridge *bridge,
> +				   struct drm_atomic_commit *commit)
> +{
> +	struct rcar_dsc *dsc = bridge_to_rcar_dsc(bridge);
> +
> +	WARN_ON(pm_runtime_get_sync(dsc->dev) < 0);

[Severity: Medium]
Is it safe to call the sleeping function pm_runtime_get_sync() here?

Since rcar_dsc_atomic_enable() is called during non-blocking atomic commits,
doesn't this path execute in atomic context where sleeping is forbidden and
can lead to kernel warnings or deadlocks?

> +}

[ ... ]

> +static int rcar_dsc_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = devm_drm_bridge_add(dev, &dsc->bridge);
> +	if (ret)
> +		goto err_runtime_disable;
> +
> +	return 0;
> +
> +err_runtime_disable:
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static void rcar_dsc_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +}

[Severity: High]
Does this create a teardown order race during driver removal?

Because devm_drm_bridge_add() defers unregistration until after
rcar_dsc_remove() returns, the bridge remains globally accessible to the DRM
subsystem after pm_runtime_disable() has already shut down power management.

If a concurrent modeset calls rcar_dsc_atomic_enable() during this window,
won't pm_runtime_get_sync() return -EACCES and trigger the WARN_ON since
power management is disabled?

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