[PATCH] drm: Check fb against plane size rather than CRTC mode for pageflip

* [PATCH] drm: Check fb against plane size rather than CRTC mode for pageflip
@ 2015-10-07  0:02 Matt Roper
  0 siblings, 0 replies; 3+ messages in thread
From: Matt Roper @ 2015-10-07  0:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tvrtko Ursulin

The legacy pageflip ioctl calls drm_crtc_check_viewport() to determine
whether the framebuffer being flipped is big enough to fill the display
it is being flipped to.  However some drivers support "windowing" of
their primary planes (i.e., a primary plane that does not cover the
entire CRTC dimensions); in such situations we can wind up rejecting
valid pageflips of buffers that are smaller than the display mode, but
still large enough to fill the entire primary plane.

What we really want to be comparing against for pageflips is the size of
the primary plane, which can be found in crtc->primary->state for atomic
drivers (and drivers in the process of transitioning to atomic).  There
are no non-atomic drivers that support primary plane windowing at the
moment, so we'll continue to use the current behavior of looking at the
CRTC mode size on drivers that don't have a crtc->primary->state.  We'll
also continue to use the existing logic for SetCrtc, which is the other
callsite for drm_crtc_check_viewport(), since legacy modesets reprogram
the primary plane and remove windowing.

Note that the existing code was checking a crtc->invert_dimensions field
to determine whether the width/height of the mode needed to be swapped.
A bonus of checking plane size is that the source width/height we get
already take rotation into account so that check is no longer necessary
when using the plane size.

Testcase: igt/universal-plane-gen9-features-pipe-#
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 87 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e600a5f..35cd4dc 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2534,9 +2534,76 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
 }
 EXPORT_SYMBOL(drm_crtc_get_hv_timing);
 
+static int check_viewport(int hdisplay,
+			  int vdisplay,
+			  int x,
+			  int y,
+			  bool invert_dimensions,
+			  const struct drm_framebuffer *fb)
+{
+	if (invert_dimensions)
+		swap(hdisplay, vdisplay);
+
+	if (hdisplay > fb->width ||
+	    vdisplay > fb->height ||
+	    x > fb->width - hdisplay ||
+	    y > fb->height - vdisplay) {
+		DRM_DEBUG_KMS("Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n",
+			      fb->width, fb->height, hdisplay, vdisplay, x, y,
+			      invert_dimensions ? " (inverted)" : "");
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
+/**
+ * drm_plane_check_viewport - Checks that a framebuffer is big enough for the
+ *     plane's viewport
+ * @plane: Plane that framebuffer will be displayed on
+ * @x: x panning
+ * @y: y panning
+ * @fb: framebuffer to check size of
+ *
+ * Atomic drivers (or transitioning drivers that support proper plane state)
+ * may call this function on any plane.  Non-atomic drivers may only call this
+ * for the primary plane while the CRTC is active (we'll assume that the
+ * primary plane covers the entire CRTC in that case).
+ */
+int drm_plane_check_viewport(const struct drm_plane *plane,
+			     int x,
+			     int y,
+			     const struct drm_framebuffer *fb)
+
+{
+	struct drm_crtc *crtc = plane->crtc;
+	int hdisplay, vdisplay;
+
+	if (WARN_ON(plane->state == NULL &&
+		    plane->type != DRM_PLANE_TYPE_PRIMARY))
+		return -EINVAL;
+
+	/*
+	 * Non-atomic drivers may not have valid plane state to look at.  But
+	 * those drivers also don't support windowing of the primary plane, so
+	 * we can fall back to looking at the mode of the owning CRTC.
+	 */
+	if (plane->state) {
+		hdisplay = plane->state->src_w >> 16;
+		vdisplay = plane->state->src_h >> 16;
+	} else if (WARN_ON(!crtc)) {
+		hdisplay = vdisplay = 0;
+	} else {
+		drm_crtc_get_hv_timing(&crtc->mode, &hdisplay, &vdisplay);
+	}
+
+	return check_viewport(hdisplay, vdisplay, x, y, false, fb);
+}
+EXPORT_SYMBOL(drm_plane_check_viewport);
+
 /**
  * drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
- *     CRTC viewport
+ *     CRTC viewport when running in the specified mode
  * @crtc: CRTC that framebuffer will be displayed on
  * @x: x panning
  * @y: y panning
@@ -2553,20 +2620,8 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
 
 	drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
 
-	if (crtc->invert_dimensions)
-		swap(hdisplay, vdisplay);
-
-	if (hdisplay > fb->width ||
-	    vdisplay > fb->height ||
-	    x > fb->width - hdisplay ||
-	    y > fb->height - vdisplay) {
-		DRM_DEBUG_KMS("Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n",
-			      fb->width, fb->height, hdisplay, vdisplay, x, y,
-			      crtc->invert_dimensions ? " (inverted)" : "");
-		return -ENOSPC;
-	}
-
-	return 0;
+	return check_viewport(hdisplay, vdisplay, x, y,
+			      crtc->invert_dimensions, fb);
 }
 EXPORT_SYMBOL(drm_crtc_check_viewport);
 
@@ -5181,7 +5236,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);
+	ret = drm_plane_check_viewport(crtc->primary, crtc->x, crtc->y, fb);
 	if (ret)
 		goto out;
 
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread