Re: [PATCH 35/60] drm/vboxvideo: Convert to atomic_create_state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver acquires a mutex in plane atomic update and disable callbacks, which execute in atomic context where sleeping is forbidden.
--

--- Patch [35]: [PATCH 35/60] drm/vboxvideo: Convert to atomic_create_state ---
commit b805e15405279473a42dd26a1b3da549e8eee553
Author: Maxime Ripard <mripard@kernel.org>

drm/vboxvideo: Convert to atomic_create_state

The plane only initializes a pristine state in its reset hook
using drm_atomic_helper_plane_reset(), which is equivalent to what
atomic_create_state expects. Convert to it.

> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index 8e4e5fc9d3c5a..a30e9265ecb17 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -498,7 +498,7 @@ static const struct drm_plane_funcs vbox_primary_plane_funcs = {
>  	.update_plane	= drm_atomic_helper_update_plane,
>  	.disable_plane	= drm_atomic_helper_disable_plane,
>  	.destroy	= drm_plane_helper_destroy,
> -	.reset		= drm_atomic_helper_plane_reset,
> +	.atomic_create_state = drm_atomic_helper_plane_create_state,

[Severity: High]
This isn't a bug introduced by this patch, but does this driver sleep in
atomic context during plane updates?

Looking at other functions in this file, the atomic update and disable
callbacks appear to acquire a mutex:

drivers/gpu/drm/vboxvideo/vbox_mode.c:vbox_primary_atomic_update() {
    ...
    mutex_lock(&vbox->hw_mutex);
    ...
}

Similarly, vbox_cursor_atomic_update() and vbox_cursor_atomic_disable()
also call mutex_lock(&vbox->hw_mutex), and vbox_primary_atomic_disable()
calls vbox_crtc_set_base_and_mode() which acquires the same mutex.

Since the DRM core executes plane atomic update and disable callbacks in an
atomic context where sleeping functions are forbidden, could this cause
kernel warnings about sleeping in an invalid context, or potential deadlocks
during non-blocking commits?

>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org?part=35