[PATCH v3 21/27] drm/msm/dpu: simplify dpu_plane_validate_src()

[Abhinav Kumar]

On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
> Since the driver uses clipped src coordinates, there is no need to check
> against the fb coordinates. Remove corresponding checks and inline
> dpu_plane_validate_src().
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>

Can you please explain how the clipping in 
drm_atomic_helper_check_plane_state() can allow us to remove checking 
the fb co-ordinates?

The clipping is done using the mode parameters.

So lets say the FB being used is smaller than the source buffer by an 
incorrect usermode setting.

Then the src sspp shall try to fetch the full image of src rectangle 
size from a FB which isnt that big leading to a fault.

How does the clipping avoid such a case?

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 30 ++++++++---------------
>   1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index ecf5402ab61a..0986e740b978 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -894,25 +894,6 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
>   				old_pstate->needs_dirtyfb);
>   }
>   
> -static bool dpu_plane_validate_src(struct drm_rect *src,
> -				   struct drm_rect *fb_rect,
> -				   uint32_t min_src_size)
> -{
> -	/* Ensure fb size is supported */
> -	if (drm_rect_width(fb_rect) > MAX_IMG_WIDTH ||
> -	    drm_rect_height(fb_rect) > MAX_IMG_HEIGHT)
> -		return false;
> -
> -	/* Ensure src rect is above the minimum size */
> -	if (drm_rect_width(src) < min_src_size ||
> -	    drm_rect_height(src) < min_src_size)
> -		return false;
> -
> -	/* Ensure src is fully encapsulated in fb */
> -	return drm_rect_intersect(fb_rect, src) &&
> -		drm_rect_equals(fb_rect, src);
> -}
> -
>   static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
>   						const struct dpu_sspp_sub_blks *sblk,
>   						struct drm_rect src, const struct dpu_format *fmt)
> @@ -998,6 +979,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	fb_rect.x2 = new_plane_state->fb->width;
>   	fb_rect.y2 = new_plane_state->fb->height;
>   
> +	/* Ensure fb size is supported */
> +	if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH ||
> +	    drm_rect_height(&fb_rect) > MAX_IMG_HEIGHT) {
> +		DPU_DEBUG_PLANE(pdpu, "invalid framebuffer " DRM_RECT_FMT "\n",
> +				DRM_RECT_ARG(&fb_rect));
> +		return -E2BIG;
> +	}
> +
>   	max_linewidth = pdpu->catalog->caps->max_linewidth;
>   
>   	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
> @@ -1012,7 +1001,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   		return -EINVAL;
>   
>   	/* check src bounds */
> -	} else if (!dpu_plane_validate_src(&pipe_cfg->src_rect, &fb_rect, min_src_size)) {
> +	} else if (drm_rect_width(&pipe_cfg->src_rect) < min_src_size ||
> +		   drm_rect_height(&pipe_cfg->src_rect) < min_src_size) {
>   		DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
>   				DRM_RECT_ARG(&pipe_cfg->src_rect));
>   		return -E2BIG;