diff mbox

[RFCv3,13/14] drm/i915: Split cursor update code from cursor ioctl handling

Message ID 1395188579-17191-14-git-send-email-matthew.d.roper@intel.com
State Superseded
Headers show

Commit Message

Matt Roper March 19, 2014, 12:22 a.m. UTC
Legacy cursor ioctls took GEM buffer handles from userspace directly
whereas the new unified plane handling assigns drm_framebuffer's to
cursor planes.  Splitting the code that actually updates the cursor
plane from the code that handles object lookup and reference counting
allows us to share common code between both interfaces.

Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 197 ++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |   2 -
 2 files changed, 134 insertions(+), 65 deletions(-)

Comments

Chris Wilson March 19, 2014, 8:03 a.m. UTC | #1
On Tue, Mar 18, 2014 at 05:22:58PM -0700, Matt Roper wrote:
> Legacy cursor ioctls took GEM buffer handles from userspace directly
> whereas the new unified plane handling assigns drm_framebuffer's to
> cursor planes.  Splitting the code that actually updates the cursor
> plane from the code that handles object lookup and reference counting
> allows us to share common code between both interfaces.

This exposes an internal fb, a bo that was private is now public.

So maybe drm_framebuffer_init_private() and
intel_private_framebuffer_create().
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d43b31d..f661469 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -43,7 +43,8 @@ 
 #include <linux/dma_remapping.h>
 
 static void intel_increase_pllclock(struct drm_crtc *crtc);
-static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
+static void intel_crtc_update_cursor(struct drm_crtc *crtc,
+				     struct drm_framebuffer *fb);
 
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				struct intel_crtc_config *pipe_config);
@@ -56,6 +57,11 @@  static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
 				  struct drm_mode_fb_cmd2 *mode_cmd,
 				  struct drm_i915_gem_object *obj);
+static struct drm_framebuffer *
+intel_user_framebuffer_create(struct drm_device *dev,
+			      struct drm_file *filp,
+			      struct drm_mode_fb_cmd2 *mode_cmd);
+static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb);
 
 typedef struct {
 	int	min, max;
@@ -3709,7 +3715,7 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_pipe(intel_crtc);
 	intel_enable_primary_hw_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
-	intel_crtc_update_cursor(crtc, true);
+	intel_crtc_update_cursor(crtc, crtc->cursor->fb);
 
 	if (intel_crtc->config.has_pch_encoder)
 		ironlake_pch_enable(crtc);
@@ -3753,7 +3759,7 @@  static void haswell_crtc_enable_planes(struct drm_crtc *crtc)
 
 	intel_enable_primary_hw_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
-	intel_crtc_update_cursor(crtc, true);
+	intel_crtc_update_cursor(crtc, crtc->cursor->fb);
 
 	hsw_enable_ips(intel_crtc);
 
@@ -3781,7 +3787,7 @@  static void haswell_crtc_disable_planes(struct drm_crtc *crtc)
 
 	hsw_disable_ips(intel_crtc);
 
-	intel_crtc_update_cursor(crtc, false);
+	intel_crtc_update_cursor(crtc, NULL);
 	intel_disable_planes(crtc);
 	intel_disable_primary_hw_plane(dev_priv, plane, pipe);
 }
@@ -3909,7 +3915,7 @@  static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	if (dev_priv->fbc.plane == plane)
 		intel_disable_fbc(dev);
 
-	intel_crtc_update_cursor(crtc, false);
+	intel_crtc_update_cursor(crtc, NULL);
 	intel_disable_planes(crtc);
 	intel_disable_primary_hw_plane(dev_priv, plane, pipe);
 
@@ -4396,7 +4402,7 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
 	intel_enable_primary_hw_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
-	intel_crtc_update_cursor(crtc, true);
+	intel_crtc_update_cursor(crtc, crtc->cursor->fb);
 
 	intel_update_fbc(dev);
 
@@ -4440,7 +4446,7 @@  static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	/* The fixup needs to happen before cursor is enabled */
 	if (IS_G4X(dev))
 		g4x_fixup_plane(dev_priv, pipe);
-	intel_crtc_update_cursor(crtc, true);
+	intel_crtc_update_cursor(crtc, crtc->cursor->fb);
 
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
@@ -4491,7 +4497,7 @@  static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		intel_disable_fbc(dev);
 
 	intel_crtc_dpms_overlay(intel_crtc, false);
-	intel_crtc_update_cursor(crtc, false);
+	intel_crtc_update_cursor(crtc, NULL);
 	intel_disable_planes(crtc);
 	intel_disable_primary_hw_plane(dev_priv, plane, pipe);
 
@@ -7739,7 +7745,7 @@  static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc,
-				     bool on)
+				     struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7750,32 +7756,36 @@  static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	u32 base = 0, pos = 0;
 	bool visible;
 
-	if (on)
-		base = intel_crtc->cursor_addr;
+	if (!intel_crtc->active)
+		return;
 
-	if (x >= intel_crtc->config.pipe_src_w)
-		base = 0;
+	if (fb != NULL) {
+		base = intel_crtc->cursor_addr;
 
-	if (y >= intel_crtc->config.pipe_src_h)
-		base = 0;
+		if (x >= intel_crtc->config.pipe_src_w)
+			base = 0;
 
-	if (x < 0) {
-		if (x + intel_crtc->cursor_width <= 0)
+		if (y >= intel_crtc->config.pipe_src_h)
 			base = 0;
 
-		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
-		x = -x;
-	}
-	pos |= x << CURSOR_X_SHIFT;
+		if (x < 0) {
+			if (x + fb->width <= 0)
+				base = 0;
 
-	if (y < 0) {
-		if (y + intel_crtc->cursor_height <= 0)
-			base = 0;
+			pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
+			x = -x;
+		}
+		pos |= x << CURSOR_X_SHIFT;
 
-		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
-		y = -y;
+		if (y < 0) {
+			if (y + fb->height <= 0)
+				base = 0;
+
+			pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
+			y = -y;
+		}
+		pos |= y << CURSOR_Y_SHIFT;
 	}
-	pos |= y << CURSOR_Y_SHIFT;
 
 	visible = base != 0;
 	if (!visible && !intel_crtc->cursor_visible)
@@ -7793,41 +7803,46 @@  static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	}
 }
 
-static int intel_crtc_cursor_set(struct drm_crtc *crtc,
-				 struct drm_file *file,
-				 uint32_t handle,
-				 uint32_t width, uint32_t height)
+/*
+ * Common cursor-setting code shared by both the legacy cursor ioctls (which
+ * take a GEM bo handle directly from userspace) and the new unified plane
+ * interface (which takes drm_framebuffer's).  By the time we get here, the
+ * desired new buffer will be in the form of a DRM framebuffer, regardless
+ * of which userspace API was used.
+ */
+static int cursor_set_common(struct drm_crtc *crtc,
+			     struct drm_plane *cursor,
+			     struct drm_framebuffer *fb)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = cursor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_i915_gem_object *obj;
-	uint32_t addr;
+	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+	struct drm_i915_gem_object *obj = NULL;
+	struct drm_i915_gem_object *cursor_bo = NULL;
+	uint32_t addr = 0;
 	int ret;
 
-	/* if we want to turn off the cursor ignore width and height */
-	if (!handle) {
+	BUG_ON(cursor != crtc->cursor);
+	BUG_ON(cursor->possible_crtcs != drm_crtc_mask(crtc));
+
+	if (fb == NULL) {
 		DRM_DEBUG_KMS("cursor off\n");
-		addr = 0;
-		obj = NULL;
 		mutex_lock(&dev->struct_mutex);
 		goto finish;
 	}
 
+	obj = intel_fb->obj;
+
 	/* Currently we only support 64x64 cursors */
-	if (width != 64 || height != 64) {
+	if (fb->width != 64 || fb->height != 64) {
 		DRM_ERROR("we currently only support 64x64 cursors\n");
 		return -EINVAL;
 	}
 
-	obj = to_intel_bo(drm_gem_object_lookup(dev, file, handle));
-	if (&obj->base == NULL)
-		return -ENOENT;
-
-	if (obj->base.size < width * height * 4) {
-		DRM_DEBUG_KMS("buffer is to small\n");
-		ret = -ENOMEM;
-		goto fail;
+	if (obj->base.size < fb->width * fb->height * 4) {
+		DRM_DEBUG_KMS("Cursor buffer is too small\n");
+		return -ENOMEM;
 	}
 
 	/* we only need to pin inside GTT if cursor is non-phy */
@@ -7876,35 +7891,87 @@  static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 	}
 
 	if (IS_GEN2(dev))
-		I915_WRITE(CURSIZE, (height << 12) | width);
+		I915_WRITE(CURSIZE, (fb->height << 12) | fb->width);
 
- finish:
-	if (intel_crtc->cursor_bo) {
+finish:
+	if (cursor->fb) {
+		cursor_bo = to_intel_framebuffer(cursor->fb)->obj;
 		if (INTEL_INFO(dev)->cursor_needs_physical) {
-			if (intel_crtc->cursor_bo != obj)
-				i915_gem_detach_phys_object(dev, intel_crtc->cursor_bo);
+			if (cursor_bo != obj)
+				i915_gem_detach_phys_object(dev, cursor_bo);
 		} else
-			i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
-		drm_gem_object_unreference(&intel_crtc->cursor_bo->base);
+			i915_gem_object_unpin_from_display_plane(cursor_bo);
 	}
 
 	mutex_unlock(&dev->struct_mutex);
 
 	intel_crtc->cursor_addr = addr;
-	intel_crtc->cursor_bo = obj;
-	intel_crtc->cursor_width = width;
-	intel_crtc->cursor_height = height;
-
-	if (intel_crtc->active)
-		intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
 
 	return 0;
+
 fail_unpin:
 	i915_gem_object_unpin_from_display_plane(obj);
 fail_locked:
 	mutex_unlock(&dev->struct_mutex);
-fail:
-	drm_gem_object_unreference_unlocked(&obj->base);
+	return ret;
+}
+
+static int intel_crtc_cursor_set(struct drm_crtc *crtc,
+				 struct drm_file *file,
+				 uint32_t handle,
+				 uint32_t width, uint32_t height)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_plane *cursor = crtc->cursor;
+	struct drm_framebuffer *fb = NULL;
+	struct drm_framebuffer *old_fb = NULL;
+	struct drm_i915_gem_object *new_obj = NULL;
+	struct drm_i915_gem_object *old_obj = NULL;
+	struct drm_mode_fb_cmd2 cmd = {
+		.width = width,
+		.height = height,
+		.pixel_format = DRM_FORMAT_ARGB8888,
+	};
+	int ret = 0;
+
+	/* i915 cursor support may be disabled for debugging */
+	if (cursor == NULL)
+		return -ENXIO;
+
+	old_fb = cursor->fb;
+
+	/* Wrap the buffer handle we got in a drm framebuffer */
+	if (handle != 0) {
+		cmd.handles[0] = handle;
+		fb = intel_user_framebuffer_create(dev, file, &cmd);
+		if (IS_ERR(fb))
+			return PTR_ERR(fb);
+	}
+
+	new_obj = fb ? to_intel_framebuffer(fb)->obj : NULL;
+	old_obj = old_fb ? to_intel_framebuffer(old_fb)->obj : NULL;
+
+	/* If this buffer is already the cursor image, no work is needed */
+	if (new_obj == old_obj) {
+		DRM_DEBUG_KMS("Cursor is already set with this object\n");
+		old_fb = NULL;
+		goto done;
+	}
+
+	ret = cursor_set_common(crtc, cursor, fb);
+	if (ret != 0) {
+		old_fb = NULL;
+	} else {
+		intel_crtc_update_cursor(crtc, fb);
+		cursor->fb = fb;
+		fb = NULL;
+	}
+
+done:
+	if (fb)
+		intel_user_framebuffer_destroy(fb);
+	if (old_fb)
+		intel_user_framebuffer_destroy(old_fb);
 	return ret;
 }
 
@@ -7912,11 +7979,15 @@  static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	/* i915 cursor support may be disabled for debugging */
+	if (crtc->cursor == NULL)
+		return -ENXIO;
+
 	intel_crtc->cursor_x = clamp_t(int, x, SHRT_MIN, SHRT_MAX);
 	intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX);
 
 	if (intel_crtc->active)
-		intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
+		intel_crtc_update_cursor(crtc, crtc->cursor->fb);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 770f80d..76950bb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -364,10 +364,8 @@  struct intel_crtc {
 	 * handled in the hw itself (with the TILEOFF register). */
 	unsigned long dspaddr_offset;
 
-	struct drm_i915_gem_object *cursor_bo;
 	uint32_t cursor_addr;
 	int16_t cursor_x, cursor_y;
-	int16_t cursor_width, cursor_height;
 	bool cursor_visible;
 
 	struct intel_plane_config plane_config;