Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Relaxing plane bounds checking allows configurations that the hardware programming function (`hibmc_plane_atomic_update`) does not support, causing the plane to be rendered incorrectly. --
commit 4786b7e1e5ba6fbcc8a7b196d600f9e14eb06bb6 Author: Thomas Zimmermann <[email protected]> drm/hibmc: Use drm_atomic_helper_check_plane_state() This commit replaces custom plane boundary checks with the shared drm_atomic_helper_check_plane_state() helper. > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > index 3066dc9ebc647..7c0b88c774b5e 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > @@ -72,46 +72,28 @@ static int hibmc_get_best_clock_idx(const struct > drm_display_mode *mode) > static int hibmc_plane_atomic_check(struct drm_plane *plane, > struct drm_atomic_commit *state) > { [ ... ] > + ret = drm_atomic_helper_check_plane_state(new_plane_state, > new_crtc_state, > + DRM_PLANE_NO_SCALING, > + DRM_PLANE_NO_SCALING, > + false, true); > + if (ret) > + return ret; > + else if (!new_plane_state->visible) > + return 0; [Severity: High] Does this introduce a mismatch with the hardware update logic? The atomic helper permits planes larger than the CRTC by adjusting the source offsets (src_x and src_y), and also permits completely off-screen planes by setting visible to false. However, looking at hibmc_plane_atomic_update(), it appears the hardware programming function unconditionally scans out from the base address of the framebuffer. It does not seem to check if the plane is visible to skip programming, nor does it appear to account for src_x and src_y offsets. If userspace submits an off-screen plane or a plane with a panning offset, will the hardware incorrectly scan out the unshifted framebuffer, causing display corruption? > if (new_plane_state->fb->pitches[0] % 128 != 0) { > drm_dbg_atomic(plane->dev, "wrong stride with 128-byte > aligned\n"); > return -EINVAL; > } > + > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
