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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/6/2023 4:27 PM, Dmitry Baryshkov wrote:
On 07/02/2023 00:40, Abhinav Kumar wrote:


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@xxxxxxxxxx>

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.

This case is checked by the drm_atomic_plane_check().


How does the clipping avoid such a case?

clipping itself does not. However using clipped coordinates from plane_state->src ensures that they already were validated against the FB dimensions. I'll see if I can change the commit message to make it more obvious.


Ah okay, yeah the commit text confused me a bit to look at the clipping code in drm_atomic_helper_check_plane_state().

Yes, please change it to point to the helper which addresses this.


---
  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;




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux