diff mbox

[29/51] drm/i915: Split clipping and checking from update_plane hook

Message ID 1351188354-24233-30-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 25, 2012, 6:05 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split the update_plane() codepath into two separate steps. The first
step checkis and clips the plane, and the second step actually commits
the changes to the hardware. This allows the atomic modesetting code
to perform all checks before clobering hardware state.

The update_plane() hook is reduced to a thin wrapper calling both check
and commit functions.

Buffer (un)pinning is still being performed in the commit step. This
needs to be changed as well, so that the atomic modesetting code can
try to pin all new buffers before touching the hardware.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |   15 +-
 drivers/gpu/drm/i915/intel_sprite.c |  367 ++++++++++++++++++++++------------
 2 files changed, 247 insertions(+), 135 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 64d87c2..0660d38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -214,6 +214,15 @@  struct intel_crtc {
 	struct intel_pch_pll *pch_pll;
 };
 
+struct intel_plane_coords {
+	/* disabled or fully clipped? */
+	bool visible;
+	/* coordinates clipped against pipe dimensions */
+	int32_t crtc_x, crtc_y;
+	uint32_t crtc_w, crtc_h;
+	uint32_t src_x, src_y, src_w, src_h;
+};
+
 struct intel_plane {
 	struct drm_plane base;
 	enum pipe pipe;
@@ -222,11 +231,7 @@  struct intel_plane {
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
 	void (*update_plane)(struct drm_plane *plane,
 			     struct drm_framebuffer *fb,
-			     struct drm_i915_gem_object *obj,
-			     int crtc_x, int crtc_y,
-			     unsigned int crtc_w, unsigned int crtc_h,
-			     uint32_t x, uint32_t y,
-			     uint32_t src_w, uint32_t src_h);
+			     const struct intel_plane_coords *clip);
 	void (*disable_plane)(struct drm_plane *plane);
 	int (*update_colorkey)(struct drm_plane *plane,
 			       struct drm_intel_sprite_colorkey *key);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cd6777f..fee6f17 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -36,16 +36,173 @@ 
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
+static bool
+format_is_yuv(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_YVYU:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static void intel_clip_plane(const struct drm_plane *plane,
+			     const struct drm_crtc *crtc,
+			     const struct drm_framebuffer *fb,
+			     struct intel_plane_coords *coords)
+{
+	const struct intel_plane *intel_plane = to_intel_plane(plane);
+	const struct drm_display_mode *mode = &crtc->mode;
+	int hscale, vscale;
+	struct drm_region src = {
+		.x1 = coords->src_x,
+		.x2 = coords->src_x + coords->src_w,
+		.y1 = coords->src_y,
+		.y2 = coords->src_y + coords->src_h,
+	};
+	struct drm_region dst = {
+		.x1 = coords->crtc_x,
+		.x2 = coords->crtc_x + coords->crtc_w,
+		.y1 = coords->crtc_y,
+		.y2 = coords->crtc_y + coords->crtc_h,
+	};
+	const struct drm_region clip = {
+		.x2 = mode->hdisplay,
+		.y2 = mode->vdisplay,
+	};
+
+	hscale = drm_calc_hscale(&src, &dst, 1, intel_plane->max_downscale << 16);
+	vscale = drm_calc_vscale(&src, &dst, 1, intel_plane->max_downscale << 16);
+
+	coords->visible = drm_region_clip_scaled(&src, &dst, &clip, hscale, vscale);
+
+	coords->crtc_x = dst.x1;
+	coords->crtc_y = dst.y1;
+	coords->crtc_w = drm_region_width(&dst);
+	coords->crtc_h = drm_region_height(&dst);
+
+	/* HW doesn't seem to like smaller sprite, even when scaling */
+	/* FIXME return an error instead? */
+	if (coords->crtc_w < 3 || coords->crtc_h < 3)
+		coords->visible = false;
+
+	/*
+	 * Hardware doesn't handle subpixel coordinates.
+	 * Round to nearest (macro)pixel boundary.
+	 */
+	if (format_is_yuv(fb->pixel_format)) {
+		coords->src_x = ((src.x1 + 0x10000) >> 17) << 1;
+		coords->src_w = (((src.x2 + 0x10000) >> 17) << 1) - coords->src_x;
+	} else {
+		coords->src_x = (src.x1 + 0x8000) >> 16;
+		coords->src_w = ((src.x2 + 0x8000) >> 16) - coords->src_x;
+	}
+	coords->src_y = (src.y1 + 0x8000) >> 16;
+	coords->src_h = ((src.y2 + 0x8000) >> 16) - coords->src_y;
+
+	/* Account for minimum source size when scaling */
+	if (coords->visible && (coords->src_w != coords->crtc_w || coords->src_h != coords->crtc_h)) {
+		unsigned int min_w = format_is_yuv(fb->pixel_format) ? 4 : 3;
+
+		if (coords->src_w < min_w) {
+			coords->src_w = min_w;
+			if (coords->src_x > fb->width - coords->src_w)
+				coords->src_x = fb->width - coords->src_w;
+		}
+
+		/* FIXME interlacing */
+		if (coords->src_h < 3) {
+			coords->src_h = 3;
+			if (coords->src_y > fb->height - coords->src_h)
+				coords->src_y = fb->height - coords->src_h;
+		}
+	}
+}
+
+int intel_check_plane(const struct drm_plane *plane,
+		      const struct drm_crtc *crtc,
+		      const struct drm_framebuffer *fb,
+		      struct intel_plane_coords *coords)
+{
+	const struct intel_plane *intel_plane = to_intel_plane(plane);
+
+	if (fb) {
+		/* FIXME copy-pasted. refactor common code in drm_crtc.c */
+		uint32_t fb_width = fb->width << 16;
+		uint32_t fb_height = fb->height << 16;
+		int i;
+
+		for (i = 0; i < plane->format_count; i++) {
+			if (plane->format_types[i] == fb->pixel_format)
+				break;
+		}
+		if (i == plane->format_count)
+			return -EINVAL;
+
+		if (coords->src_w > fb_width ||
+		    coords->src_x > fb_width - coords->src_w ||
+		    coords->src_h > fb_height ||
+		    coords->src_y > fb_height - coords->src_h)
+			return -ENOSPC;
+
+		if (coords->crtc_w > INT_MAX ||
+		    coords->crtc_x > INT_MAX - (int32_t) coords->crtc_w ||
+		    coords->crtc_h > INT_MAX ||
+		    coords->crtc_y > INT_MAX - (int32_t) coords->crtc_h)
+			return -ERANGE;
+
+		if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384)
+			return -EINVAL;
+	}
+
+	if (crtc) {
+		const struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+		/* Don't modify another pipe's plane */
+		if (intel_plane->pipe != intel_crtc->pipe)
+			return -EINVAL;
+	}
+
+	if (!fb || !crtc || !crtc->enabled) {
+		coords->visible = false;
+		return 0;
+	}
+
+	intel_clip_plane(plane, crtc, fb, coords);
+
+	/* Check size restrictions when scaling */
+	if (coords->visible && (coords->src_w != coords->crtc_w || coords->src_h != coords->crtc_h)) {
+		unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
+
+		if (coords->src_w > 2048 || coords->src_h > 2048 ||
+		    coords->src_w * cpp > 4096 - 64 || fb->pitches[0] > 4096)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void
-ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
-		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+ivb_update_plane(struct drm_plane *plane,
+		 struct drm_framebuffer *fb,
+		 const struct intel_plane_coords *coords)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	const struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
+	int crtc_x = coords->crtc_x;
+	int crtc_y = coords->crtc_y;
+	unsigned int crtc_w = coords->crtc_w;
+	unsigned int crtc_h = coords->crtc_h;
+	uint32_t x = coords->src_x;
+	uint32_t y = coords->src_y;
+	uint32_t src_w = coords->src_w;
+	uint32_t src_h = coords->src_h;
 	int pipe = intel_plane->pipe;
 	u32 sprctl, sprscale = 0;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
@@ -210,15 +367,22 @@  ivb_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key)
 }
 
 static void
-ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
-		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+ilk_update_plane(struct drm_plane *plane,
+		 struct drm_framebuffer *fb,
+		 const struct intel_plane_coords *coords)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	const struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
+	int crtc_x = coords->crtc_x;
+	int crtc_y = coords->crtc_y;
+	unsigned int crtc_w = coords->crtc_w;
+	unsigned int crtc_h = coords->crtc_h;
+	uint32_t x = coords->src_x;
+	uint32_t y = coords->src_y;
+	uint32_t src_w = coords->src_w;
+	uint32_t src_h = coords->src_h;
 	int pipe = intel_plane->pipe;
 	u32 dvscntr, dvsscale;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
@@ -397,129 +561,68 @@  ilk_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key)
 		key->flags = I915_SET_COLORKEY_NONE;
 }
 
-static bool
-format_is_yuv(uint32_t format)
-{
-	switch (format) {
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-	case DRM_FORMAT_YVYU:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static int
-intel_update_plane(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)
+intel_disable_plane(struct drm_plane *plane)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	struct drm_i915_gem_object *old_obj = intel_plane->obj;
-	int pipe = intel_plane->pipe;
 	int ret = 0;
-	int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
-	bool disable_primary = false;
-	bool visible;
-	int hscale, vscale;
-	int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
-	struct drm_region src = {
-		.x1 = src_x,
-		.x2 = src_x + src_w,
-		.y1 = src_y,
-		.y2 = src_y + src_h,
-	};
-	struct drm_region dst = {
-		.x1 = crtc_x,
-		.x2 = crtc_x + crtc_w,
-		.y1 = crtc_y,
-		.y2 = crtc_y + crtc_h,
-	};
-	const struct drm_region clip = {
-		.x2 = crtc->mode.hdisplay,
-		.y2 = crtc->mode.vdisplay,
-	};
 
-	/* Don't modify another pipe's plane */
-	if (intel_plane->pipe != intel_crtc->pipe)
-		return -EINVAL;
-
-	if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384)
-		return -EINVAL;
-
-	hscale = drm_calc_hscale(&src, &dst, 1, intel_plane->max_downscale << 16);
-	vscale = drm_calc_vscale(&src, &dst, 1, intel_plane->max_downscale << 16);
-
-	visible = drm_region_clip_scaled(&src, &dst, &clip, hscale, vscale);
-
-	crtc_x = dst.x1;
-	crtc_y = dst.y1;
-	crtc_w = drm_region_width(&dst);
-	crtc_h = drm_region_height(&dst);
+	if (plane->crtc)
+		intel_enable_primary(plane->crtc);
 
-	/* HW doesn't seem to like smaller sprite, even when scaling */
-	/* FIXME return an error instead? */
-	if (crtc_w < 3 || crtc_h < 3)
-		visible = false;
+	intel_plane->disable_plane(plane);
 
-	/*
-	 * Hardware doesn't handle subpixel coordinates.
-	 * Round to nearest (macro)pixel boundary.
-	 */
-	if (format_is_yuv(fb->pixel_format)) {
-		src_x = ((src.x1 + 0x10000) >> 17) << 1;
-		src_w = (((src.x2 + 0x10000) >> 17) << 1) - src_x;
-	} else {
-		src_x = (src.x1 + 0x8000) >> 16;
-		src_w = ((src.x2 + 0x8000) >> 16) - src_x;
-	}
-	src_y = (src.y1 + 0x8000) >> 16;
-	src_h = ((src.y2 + 0x8000) >> 16) - src_y;
+	if (!intel_plane->obj)
+		goto out;
 
-	/* Account for minimum source size when scaling */
-	if (visible && (src_w != crtc_w || src_h != crtc_h)) {
-		unsigned int min_w = format_is_yuv(fb->pixel_format) ? 4 : 3;
+	mutex_lock(&dev->struct_mutex);
+	intel_unpin_fb_obj(intel_plane->obj);
+	intel_plane->obj = NULL;
+	mutex_unlock(&dev->struct_mutex);
+out:
 
-		if (src_w < min_w) {
-			src_w = min_w;
-			if (src_x > fb->width - src_w)
-				src_x = fb->width - src_w;
-		}
+	return ret;
+}
 
-		/* FIXME interlacing */
-		if (src_h < 3) {
-			src_h = 3;
-			if (src_y > fb->height - src_h)
-				src_y = fb->height - src_h;
-		}
-	}
+int
+intel_commit_plane(struct drm_plane *plane,
+		   struct drm_crtc *crtc,
+		   struct drm_framebuffer *fb,
+		   const struct intel_plane_coords *coords)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_framebuffer *intel_fb;
+	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_object *old_obj = intel_plane->obj;
+	int pipe = intel_plane->pipe;
+	int ret;
+	int primary_w, primary_h;
+	bool disable_primary = false;
 
-	/* Check size restrictions when scaling */
-	if (visible && (src_w != crtc_w || src_h != crtc_h)) {
-		if (src_w > 2048 || src_h > 2048 ||
-		    src_w * cpp > 4096 - 64 || fb->pitches[0] > 4096)
-			return -EINVAL;
+	if (!coords->visible) {
+		intel_disable_plane(plane);
+		return 0;
 	}
 
+	/* FIXME this should happen anymore I suppose */
 	/* Pipe must be running... */
 	if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
 		return 0;
 
+	intel_fb = to_intel_framebuffer(fb);
+	obj = intel_fb->obj;
+	primary_w = crtc->mode.hdisplay;
+	primary_h = crtc->mode.vdisplay;
+
 	/*
 	 * If the sprite is completely covering the primary plane,
 	 * we can disable the primary and save power.
 	 */
-	if ((crtc_x == 0) && (crtc_y == 0) &&
-	    (crtc_w == primary_w) && (crtc_h == primary_h))
+	if ((coords->crtc_x == 0) && (coords->crtc_y == 0) &&
+	    (coords->crtc_w == primary_w) && (coords->crtc_h == primary_h))
 		disable_primary = true;
 
 	mutex_lock(&dev->struct_mutex);
@@ -537,9 +640,8 @@  intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (!disable_primary)
 		intel_enable_primary(crtc);
 
-	if (visible) {
-		intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
-					  crtc_w, crtc_h, src_x, src_y, src_w, src_h);
+	if (coords->visible) {
+		intel_plane->update_plane(plane, fb, coords);
 
 		if (disable_primary)
 			intel_disable_primary(crtc);
@@ -568,26 +670,31 @@  out_unlock:
 }
 
 static int
-intel_disable_plane(struct drm_plane *plane)
+intel_update_plane(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)
 {
-	struct drm_device *dev = plane->dev;
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	int ret = 0;
-
-	if (plane->crtc)
-		intel_enable_primary(plane->crtc);
-	intel_plane->disable_plane(plane);
+	int ret;
+	struct intel_plane_coords coords = {
+		.crtc_x = crtc_x,
+		.crtc_y = crtc_y,
+		.crtc_w = crtc_w,
+		.crtc_h = crtc_h,
+		.src_x = src_x,
+		.src_y = src_y,
+		.src_w = src_w,
+		.src_h = src_h,
+	};
 
-	if (!intel_plane->obj)
-		goto out;
+	ret = intel_check_plane(plane, crtc, fb, &coords);
+	if (ret)
+		return ret;
 
-	mutex_lock(&dev->struct_mutex);
-	intel_unpin_fb_obj(intel_plane->obj);
-	intel_plane->obj = NULL;
-	mutex_unlock(&dev->struct_mutex);
-out:
+	intel_commit_plane(plane, crtc, fb, &coords);
 
-	return ret;
+	return 0;
 }
 
 static void intel_destroy_plane(struct drm_plane *plane)