Re: [PATCH v2 02/61] drm/gem-atomic-helper: Create drm_gem_create_shadow_plane_state()

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced `drm_gem_create_shadow_plane_state` callback is not added to the `DRM_GEM_SHADOW_PLANE_FUNCS` macro.
--

--- Patch [2]: [PATCH v2 02/61] drm/gem-atomic-helper: Create drm_gem_create_shadow_plane_state() ---
commit 04bcd76bbde0146f1fe95d41bdf27b75eecf6aaa
Author: Maxime Ripard <[email protected]>

drm/gem-atomic-helper: Create drm_gem_create_shadow_plane_state()
    
The GEM shadow-plane helpers only expose reset functions that handle
both the initial state allocation and the reset path. The new
atomic_create_state hook requires a function that only allocates and
initializes a pristine state. Create new helpers to allocate and
initialize a shadow plane state from scratch.

> diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
> index 3e01c619a25e0..43657d8fb1c8c 100644
> --- a/include/drm/drm_gem_atomic_helper.h
> +++ b/include/drm/drm_gem_atomic_helper.h
> @@ -95,6 +95,9 @@ void __drm_gem_reset_shadow_plane(struct drm_plane *plane,
>  				  struct drm_shadow_plane_state *shadow_plane_state);
>  
>  void drm_gem_reset_shadow_plane(struct drm_plane *plane);
> +void __drm_gem_shadow_plane_state_init(struct drm_plane *plane,
> +				       struct drm_shadow_plane_state *shadow_plane_state);
> +struct drm_plane_state *drm_gem_create_shadow_plane_state(struct drm_plane *plane);

[Severity: Medium]
Should the DRM_GEM_SHADOW_PLANE_FUNCS macro in this file be updated to include
the newly introduced drm_gem_create_shadow_plane_state for the
.atomic_create_state hook?

Currently, the macro only sets .reset, .atomic_duplicate_state, and
.atomic_destroy_state:

include/drm/drm_gem_atomic_helper.h:DRM_GEM_SHADOW_PLANE_FUNCS {
#define DRM_GEM_SHADOW_PLANE_FUNCS \
	.reset = drm_gem_reset_shadow_plane, \
	.atomic_duplicate_state = drm_gem_duplicate_shadow_plane_state, \
	.atomic_destroy_state = drm_gem_destroy_shadow_plane_state
}

Without adding .atomic_create_state to this macro, any driver using it to
initialize their drm_plane_funcs will have .atomic_create_state left as NULL.

If a driver attempts to migrate to using drm_mode_config_create_initial_state()
or relies on .atomic_create_state for initial allocation without noticing this
omission, plane->state would remain NULL, which can lead to crashes from a
NULL pointer dereference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.