Re: [PATCH v4 1/6] drm/hibmc: Use drm_atomic_helper_check_plane_state()

[sashiko-bot@kernel.org]

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 <tzimmermann@suse.de>

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/20260618123142.92298-1-tzimmermann@suse.de?part=1