Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Plane atomic_update callbacks indirectly call sleeping functions (mutex_lock/mutex_lock_interruptible) from an atomic context. -- commit deccb03fba43ed6006d88ae9933240eb52b52fee Author: Thomas Zimmermann <[email protected]> drm/vmwgfx: Remove unused field struct vmwgfx_du_update_plane.old_state This commit removes the unused old_state field from the vmw_du_update_plane structure and updates all caller functions to accommodate this change. > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c [ ... ] > @@ -721,7 +717,6 @@ static void > vmw_sou_primary_plane_atomic_update(struct drm_plane *plane, > struct drm_atomic_commit *state) > { > - struct drm_plane_state *old_state = > drm_atomic_get_old_plane_state(state, plane); > struct drm_plane_state *new_state = > drm_atomic_get_new_plane_state(state, plane); > struct drm_crtc *crtc = new_state->crtc; > struct vmw_fence_obj *fence = NULL; > @@ -734,12 +729,9 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane > *plane, > vmw_framebuffer_to_vfb(new_state->fb); > > if (vfb->bo) > - ret = vmw_sou_plane_update_bo(dev_priv, plane, > - old_state, vfb, &fence); > + ret = vmw_sou_plane_update_bo(dev_priv, plane, vfb, > &fence); > else > - ret = vmw_sou_plane_update_surface(dev_priv, plane, > - old_state, vfb, > - &fence); > + ret = vmw_sou_plane_update_surface(dev_priv, plane, > vfb, &fence); [Severity: High] This is a pre-existing issue, but does this code path end up calling sleeping functions from an atomic context? Looking at the call chain, vmw_sou_primary_plane_atomic_update() is a plane atomic update callback, which runs in atomic context where sleeping is forbidden. However, it calls vmw_sou_plane_update_surface(), which assigns a mutex pointer to the update structure: vmw_sou_plane_update_surface() srf_update.base.mutex = &dev_priv->cmdbuf_mutex; vmw_du_helper_plane_update(&srf_update.base); This closure is then passed down to vmw_du_helper_plane_update(), which calls vmw_validation_prepare(). That function then takes the lock using mutex_lock() or mutex_lock_interruptible(). Could this lead to system instability or deadlocks if the atomic commit runs in a non-blocking path where sleeping is not allowed? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
