Re: [PATCH v2 33/37] drm/omap: venc: 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] The atomic_enable and atomic_disable bridge callbacks invoke functions that sleep, violating the DRM atomic context rules.
--

--- Patch [33]: [PATCH v2 33/37] drm/omap: venc: Switch to atomic bridge callbacks ---
commit d92bb384d028f7e7472ce6ad66e7c5996b3db815
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: venc: Switch to atomic bridge callbacks

The venc 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/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c
> index 9b5d53dc361e6..5416c668a2055 100644
> --- a/drivers/gpu/drm/omapdrm/dss/venc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/venc.c
[ ... ]
> @@ -614,14 +615,16 @@ static void venc_bridge_mode_set(struct drm_bridge *bridge,
>  	dispc_set_tv_pclk(venc->dss->dispc, 13500000);
>  }
>  
> -static void venc_bridge_enable(struct drm_bridge *bridge)
> +static void venc_bridge_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_commit *commit)
>  {
>  	struct venc_device *venc = drm_bridge_to_venc(bridge);
>  
>  	venc_power_on(venc);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does calling venc_power_on()
here violate DRM atomic context rules?

The atomic_enable callback can be reached from drm_atomic_helper_commit_tail()
during non-blocking atomic commits, which can be initiated by untrusted
userspace via DRM/KMS operations. This path constitutes an atomic context
where sleeping is strictly forbidden.

However, venc_power_on() internally calls sleeping functions like
regulator_enable() and pm_runtime_get_sync() (via venc_runtime_get()):

drivers/gpu/drm/omapdrm/dss/venc.c:venc_power_on() {
	...
	r = venc_runtime_get(venc);
	...
	r = regulator_enable(venc->vdda_dac_reg);
	...
}

Calling sleeping functions in this context could cause kernel warnings like
BUG: scheduling while atomic, system instability, or potential deadlocks.

>  
> -static void venc_bridge_disable(struct drm_bridge *bridge)
> +static void venc_bridge_disable(struct drm_bridge *bridge,
> +				struct drm_atomic_commit *commit)
>  {
>  	struct venc_device *venc = drm_bridge_to_venc(bridge);
>  
>  	venc_power_off(venc);
>  }

[Severity: High]
This is a pre-existing issue, but does venc_power_off() also call sleeping
functions from an atomic context?

Similar to venc_bridge_enable() above, this atomic_disable callback is executed
from the atomic commit path. If venc_power_off() sleeps, this could also cause
scheduling while atomic warnings and instability.

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