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 <[email protected]> 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/[email protected]?part=33
