Re: [PATCH v4 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 4 potential issue(s) to consider:
- [High] `WARN_ON(pm_runtime_get_sync(dsc->dev))` incorrectly warns on success and mishandles errors.
- [High] Mixing devm-managed bridge registration with manual runtime PM manipulation creates race conditions during probe and removal.
- [Low] The driver calls `of_drm_get_bridge_by_endpoint()`, which does not exist anywhere in the kernel, causing a build failure.
- [Low] Dead Kconfig dependency prevents `DRM_RCAR_DSC` from ever compiling under `COMPILE_TEST`.
--

commit 6b399ab87555498e65f4bb3171342804aa8feb3a
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/Kconfig b/drivers/gpu/drm/renesas/rcar-du/Kconfig
> index 840305fdeb49a..80bd770ae0f68 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/renesas/rcar-du/Kconfig
[ ... ]
> @@ -25,6 +25,18 @@ config DRM_RCAR_CMM
>  	def_tristate DRM_RCAR_DU
>  	depends on DRM_RCAR_USE_CMM
>  
> +config DRM_RCAR_USE_DSC
> +	bool "R-Car DU DSC Encoder Support"
> +	depends on DRM_BRIDGE && OF
> +	depends on DRM_RCAR_DU || COMPILE_TEST
> +	default DRM_RCAR_DU
> +	help
> +	  Enable support for the R-Car Display Unit embedded DSC encoder.
> +
> +config DRM_RCAR_DSC
> +	def_tristate DRM_RCAR_DU
> +	depends on DRM_RCAR_USE_DSC
> +

[Severity: Low]
Will this Kconfig setup prevent compile testing?

Since DRM_RCAR_USE_DSC depends on DRM_RCAR_DU || COMPILE_TEST, it can be
enabled when DRM_RCAR_DU=n. However, DRM_RCAR_DSC defaults to
def_tristate DRM_RCAR_DU.

This means when DRM_RCAR_DU=n during a COMPILE_TEST, DRM_RCAR_DSC will be
forced to 'n' and rcar_dsc.c will never actually be compiled.

> 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
> index 0000000000000..5a2ff31491360
> --- /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));
> +}

[Severity: High]
Does this WARN_ON trigger on success?

pm_runtime_get_sync() returns 1 when the device is already active
(RPM_ACTIVE). By wrapping it directly in WARN_ON(), wouldn't this trigger
a false-positive stack trace on every successful modeset?

[ ... ]
> +static int rcar_dsc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_dsc *dsc;
> +	int ret;
[ ... ]
> +	dsc->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node,
> +								1, 0);

[Severity: Low]
Does this function exist?

Calling of_drm_get_bridge_by_endpoint() causes a build failure because it
doesn't exist anywhere in the kernel. Was this intended to be
devm_drm_of_get_bridge()?

> +	if (IS_ERR(dsc->bridge.next_bridge))
> +		return PTR_ERR(dsc->bridge.next_bridge);
> +
> +	dsc->bridge.of_node = dev->of_node;
> +
> +	ret = devm_drm_bridge_add(dev, &dsc->bridge);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static void rcar_dsc_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +}

[Severity: High]
Can these devres and manual pm_runtime calls race?

In rcar_dsc_probe(), devm_drm_bridge_add() exposes the bridge to the DRM
subsystem before pm_runtime_enable() is called.

On teardown, rcar_dsc_remove() manually calls pm_runtime_disable(), but the
devres-managed bridge isn't removed until devres_release_all() runs later.

Doesn't this create a window where the bridge is fully accessible via DRM
but its PM runtime is disabled, potentially causing hardware access faults
during concurrent operations?

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