[PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()

[Matt Roper <matthew.d.roper@intel.com>]

Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.

A few of the checks here were also updated based on suggestions by
Ville Syrjälä.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/drm_plane_helper.c | 148 +++++++++++++++++++++++++++++--------
 include/drm/drm_plane_helper.h     |   9 +++
 2 files changed, 128 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 }
 
 /**
+ * drm_primary_helper_check_update() - Check primary plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *                doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid.  Drivers that provide
+ * their own primary plane handling may still wish to call this function to
+ * avoid duplication of error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_primary_helper_check_update(struct drm_plane *plane,
+				    struct drm_crtc *crtc,
+				    struct drm_framebuffer *fb,
+				    int crtc_x, int crtc_y,
+				    unsigned int crtc_w, unsigned int crtc_h,
+				    uint32_t src_x, uint32_t src_y,
+				    uint32_t src_w, uint32_t src_h,
+				    bool can_scale,
+				    bool can_position)
+{
+	struct drm_rect dest = {
+		.x1 = crtc_x,
+		.y1 = crtc_y,
+		.x2 = crtc_x + crtc_w,
+		.y2 = crtc_y + crtc_h,
+	};
+	struct drm_rect src = {
+		.x1 = src_x >> 16,
+		.y1 = src_y >> 16,
+		.x2 = (src_x + src_w) >> 16,
+		.y2 = (src_y + src_h) >> 16,
+	};
+	const struct drm_rect clip = {
+		.x2 = crtc->mode.hdisplay,
+		.y2 = crtc->mode.vdisplay,
+	};
+	int hscale, vscale;
+	int visible;
+
+	if (!crtc->enabled) {
+		DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
+		return -EINVAL;
+	}
+
+	/* setplane API takes shifted source rectangle values; unshift them */
+	src_x >>= 16;
+	src_y >>= 16;
+	src_w >>= 16;
+	src_h >>= 16;
+
+	/* check scaling */
+	if (!can_scale && (crtc_w != src_w || crtc_h != src_h)) {
+		DRM_DEBUG_KMS("Can't scale primary plane\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Drivers that can scale should perform their own min/max scale
+	 * factor checks.
+	 */
+	hscale = drm_rect_calc_hscale(&src, &dest, 0, INT_MAX);
+	vscale = drm_rect_calc_vscale(&src, &dest, 0, INT_MAX);
+	visible = drm_rect_clip_scaled(&src, &dest, &clip, hscale, vscale);
+	if (!visible)
+		/*
+		 * Primary plane isn't visible; some drivers can handle this
+		 * so we just return success here.  Drivers that can't
+		 * (including those that use the primary plane helper's
+		 * update function) will return an error from their
+		 * update_plane handler.
+		 */
+		return 0;
+
+	if (!can_position && !drm_rect_equals(&dest, &clip)) {
+		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_primary_helper_check_update);
+
+/**
  * drm_primary_helper_update() - Helper for primary plane update
  * @plane: plane object to update
  * @crtc: owning CRTC of owning plane
@@ -113,6 +209,12 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
 		.x = src_x >> 16,
 		.y = src_y >> 16,
 	};
+	struct drm_rect src = {
+		.x1 = src_x >> 16,
+		.y1 = src_y >> 16,
+		.x2 = (src_x + src_w) >> 16,
+		.y2 = (src_y + src_h) >> 16,
+	};
 	struct drm_rect dest = {
 		.x1 = crtc_x,
 		.y1 = crtc_y,
@@ -124,40 +226,28 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
 		.y2 = crtc->mode.vdisplay,
 	};
 	struct drm_connector **connector_list;
+	int visible;
+	int hscale, vscale;
 	int num_connectors, ret;
 
-	if (!crtc->enabled) {
-		DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
-		return -EINVAL;
-	}
-
-	/* Disallow subpixel positioning */
-	if ((src_x | src_y | src_w | src_h) & SUBPIXEL_MASK) {
-		DRM_DEBUG_KMS("Primary plane does not support subpixel positioning\n");
-		return -EINVAL;
-	}
-
-	/* Disallow scaling */
-	src_w >>= 16;
-	src_h >>= 16;
-	if (crtc_w != src_w || crtc_h != src_h) {
-		DRM_DEBUG_KMS("Can't scale primary plane\n");
-		return -EINVAL;
-	}
-
-	/* Make sure primary plane covers entire CRTC */
-	drm_rect_intersect(&dest, &clip);
-	if (dest.x1 != 0 || dest.y1 != 0 ||
-	    dest.x2 != crtc->mode.hdisplay || dest.y2 != crtc->mode.vdisplay) {
-		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
-		return -EINVAL;
-	}
-
-	/* Framebuffer must be big enough to cover entire plane */
-	ret = drm_crtc_check_viewport(crtc, crtc_x, crtc_y, &crtc->mode, fb);
+	ret = drm_primary_helper_check_update(plane, crtc, fb,
+					      crtc_x, crtc_y, crtc_w, crtc_h,
+					      src_x, src_y, src_w, src_h,
+					      false, false);
 	if (ret)
 		return ret;
 
+	hscale = drm_rect_calc_hscale(&src, &dest, 0, INT_MAX);
+	vscale = drm_rect_calc_vscale(&src, &dest, 0, INT_MAX);
+	visible = drm_rect_clip_scaled(&src, &dest, &clip, hscale, vscale);
+	if (!visible)
+		/*
+		 * Primary plane isn't visible.  Note that unless a driver
+		 * provides their own disable function, this will just
+		 * wind up returning -EINVAL to userspace.
+		 */
+		return plane->funcs->disable_plane(plane);
+
 	/* Find current connectors for CRTC */
 	num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
 	BUG_ON(num_connectors == 0);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 09824be..373201a 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -31,6 +31,15 @@
  * planes.
  */
 
+extern int drm_primary_helper_check_update(struct drm_plane *plane,
+					   struct drm_crtc *crtc,
+					   struct drm_framebuffer *fb,
+					   int crtc_x, int crtc_y,
+					   unsigned int crtc_w, unsigned int crtc_h,
+					   uint32_t src_x, uint32_t src_y,
+					   uint32_t src_w, uint32_t src_h,
+					   bool can_scale,
+					   bool can_position);
 extern int drm_primary_helper_update(struct drm_plane *plane,
 				     struct drm_crtc *crtc,
 				     struct drm_framebuffer *fb,
-- 
1.8.5.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel