Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unchecked dereference of optional `destroy_plane_state` function pointer in `drm_simple_kms_plane_reset()`. --
--- Patch [3]: [PATCH 03/60] drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state --- commit 134f3a4eb906f19765fc34b15053dd50db1a56e3 Author: Maxime Ripard <[email protected]> drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state drm_gem_simple_kms_reset_shadow_plane() implements the reset_plane hook for simple-kms shadow-buffered planes. Since the reset hook conflates initial state creation with hardware and software reset during suspend/resume, convert it to the create_plane_state pattern. Replace it with drm_gem_simple_kms_create_shadow_plane_state(), which implements the create_plane_state hook using drm_gem_create_shadow_plane_state(). Update DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS accordingly. > diff --git a/include/drm/drm_gem_atomic_helper.h > b/include/drm/drm_gem_atomic_helper.h > index 43657d8fb1c8c..8a3bb27233db0 100644 > --- a/include/drm/drm_gem_atomic_helper.h > +++ b/include/drm/drm_gem_atomic_helper.h [ ... ] > @@ -150,7 +151,7 @@ void drm_gem_simple_kms_destroy_shadow_plane_state(struct > drm_simple_display_pip > #define DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS \ > .begin_fb_access = drm_gem_simple_kms_begin_shadow_fb_access, \ > .end_fb_access = drm_gem_simple_kms_end_shadow_fb_access, \ > - .reset_plane = drm_gem_simple_kms_reset_shadow_plane, \ > + .create_plane_state = drm_gem_simple_kms_create_shadow_plane_state, \ > .duplicate_plane_state = > drm_gem_simple_kms_duplicate_shadow_plane_state, \ > .destroy_plane_state = drm_gem_simple_kms_destroy_shadow_plane_state [Severity: High] Does converting to the create_plane_state pattern expose a crash in drm_simple_kms_plane_reset() if any drivers omit the destroy_plane_state callback? In drm_simple_kms_plane_reset(), if a driver provides the create_plane_state hook but leaves the destroy_plane_state hook as NULL (relying on the default fallback behavior), the reset path will dereference a NULL function pointer when plane->state is not NULL: drivers/gpu/drm/drm_simple_kms_helper.c:drm_simple_kms_plane_reset() { if (pipe->funcs->create_plane_state) { struct drm_plane_state *state; if (plane->state) { pipe->funcs->destroy_plane_state(pipe, plane->state); plane->state = NULL; } ... } drm_simple_kms_plane_destroy_state() natively supports an absent destroy_plane_state callback by falling back to drm_atomic_helper_plane_destroy_state(), so omitting this callback appears to be a valid configuration that this newly introduced pattern might fail to support. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
