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;
