Re: [PATCH v2 30/37] drm/omap: dpi: Switch to atomic bridge callbacks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced resource teardown on bridge enable error paths leads to PM runtime underflow and kernel panic.
--

--- Patch [30]: [PATCH v2 30/37] drm/omap: dpi: Switch to atomic bridge callbacks ---
commit 6fda2bd3a893061ddf4b74080bd409f4c3cc6741
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: dpi: Switch to atomic bridge callbacks

The dpi bridge uses the legacy non-atomic enable and disable bridge
callbacks.  In order to remove the legacy bridge callback support from the
DRM bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index 9f86db774c395..419915e338163 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c

[ ... ]

> @@ -482,7 +483,8 @@ static void dpi_bridge_mode_set(struct drm_bridge *bridge,
>  	dpi->pixelclock = adjusted_mode->clock * 1000;
>  }
>  
> -static void dpi_bridge_enable(struct drm_bridge *bridge)
> +static void dpi_bridge_enable(struct drm_bridge *bridge,
> +			      struct drm_atomic_commit *commit)
>  {
>  	struct dpi_data *dpi = drm_bridge_to_dpi(bridge);
>  	int r;

[Severity: High]
This is a pre-existing issue, but does this early return on failure lead to
unbalanced resource teardown and a potential kernel panic?

Looking at dpi_bridge_enable() in drivers/gpu/drm/omapdrm/dss/dpi.c, if
initialization fails, the function cleans up and returns early:

	if (dpi->vdds_dsi_reg) {
		r = regulator_enable(dpi->vdds_dsi_reg);
		if (r)
			return;
	}

Because the callback returns void, the DRM core considers the bridge enabled.
During pipeline teardown, dpi_bridge_disable() executes and unconditionally
drops a PM runtime reference:

	dispc_runtime_put(dpi->dss->dispc);

Since the failed dpi_bridge_enable() never acquired (or already cleaned up)
this PM runtime reference, does this extra put operation incorrectly drop the
reference held by the CRTC (omap_crtc_atomic_enable())?

If this drops the DISPC PM runtime usage count to 0 and immediately powers
down the dispc hardware, could the later call to drm_crtc_vblank_off() from
omap_crtc_atomic_disable() access powered-down DISPC registers and trigger
an L3 bus error or imprecise external abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=30