[PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[Gustavo Padovan]

From: Derek Foreman <derek.foreman@collabora.co.uk>

Really just for completeness - old init function ends up making the plane
exactly the same way due to the way the enums are set up.

Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0bdb00b..4cbe286 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	intel_plane->plane = plane;
 	intel_plane->rotation = BIT(DRM_ROTATE_0);
 	possible_crtcs = (1 << pipe);
-	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
-			     &intel_plane_funcs,
-			     plane_formats, num_plane_formats,
-			     false);
+	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
+				       &intel_plane_funcs,
+				       plane_formats, num_plane_formats,
+				       DRM_PLANE_TYPE_OVERLAY);
 	if (ret) {
 		kfree(intel_plane);
 		goto out;
-- 
1.9.3

[PATCH -v2 2/4] drm/i915: trivial: remove unneed set to NULL

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

At this point of the code the obj var is already NULL, so we don't
need to set it again to NULL.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b2e4eac..05937fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8253,7 +8253,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 	if (!obj) {
 		DRM_DEBUG_KMS("cursor off\n");
 		addr = 0;
-		obj = NULL;
 		mutex_lock(&dev->struct_mutex);
 		goto finish;
 	}
-- 
1.9.3

[PATCH -v2 3/4] drm/i915: create struct intel_plane_state

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This new struct will be the storage of src and dst coordinates
between the check and commit stages of a plane update.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4ab0d92..59c1675 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -33,6 +33,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_dp_mst_helper.h>
+#include <drm/drm_rect.h>
 
 /**
  * _wait_for - magic (register) wait macro
@@ -227,6 +228,25 @@ typedef struct dpll {
 	int	p;
 } intel_clock_t;
 
+struct intel_plane_state {
+	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_rect src;
+	struct drm_rect dst;
+	struct drm_rect clip;
+	struct drm_rect orig_src;
+	struct drm_rect orig_dst;
+	bool visible;
+};
+
 struct intel_plane_config {
 	bool tiled;
 	int size;
-- 
1.9.3

Re: [PATCH -v2 3/4] drm/i915: create struct intel_plane_state

[Ville Syrjälä]

On Tue, Sep 02, 2014 at 04:23:46PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> This new struct will be the storage of src and dst coordinates
> between the check and commit stages of a plane update.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_drv.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4ab0d92..59c1675 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -33,6 +33,7 @@
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_dp_mst_helper.h>
> +#include <drm/drm_rect.h>
>  
>  /**
>   * _wait_for - magic (register) wait macro
> @@ -227,6 +228,25 @@ typedef struct dpll {
>  	int	p;
>  } intel_clock_t;
>  
> +struct intel_plane_state {
> +	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;

Seems to me we shouldn't need these non-drm_rect coordinates in
this struct. I'll comment a bit more on the next patch since that's
where this is used.

> +	struct drm_rect src;
> +	struct drm_rect dst;
> +	struct drm_rect clip;
> +	struct drm_rect orig_src;
> +	struct drm_rect orig_dst;
> +	bool visible;
> +};
> +
>  struct intel_plane_config {
>  	bool tiled;
>  	int size;
> -- 
> 1.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH -v2 4/4] drm/i915: split intel_update_plane into check() and commit()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Due to the upcoming atomic modesetting feature we need to separate
some update functions into a check step that can fail and a commit
step that should, ideally, never fail.

This commit splits intel_update_plane() and its commit part can still
fail due to the fb pinning procedure.

v2: use the new struct intel_plane_state to store information
between check and commit stages.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_sprite.c | 236 ++++++++++++++++++++++--------------
 1 file changed, 146 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4cbe286..062eca2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -845,57 +845,28 @@ static bool colorkey_enabled(struct intel_plane *intel_plane)
 }
 
 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_check_sprite_plane(struct drm_plane *plane,
+			 struct intel_plane_state *pstate)
 {
-	struct drm_device *dev = plane->dev;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *intel_crtc = to_intel_crtc(pstate->crtc);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
-	enum pipe pipe = intel_crtc->pipe;
+	struct drm_framebuffer *fb = pstate->fb;
 	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 ret;
-	bool primary_enabled;
-	bool visible;
+	int crtc_x = pstate->crtc_x;
+	int crtc_y = pstate->crtc_y;
+	int crtc_w = pstate->crtc_w;
+	int crtc_h = pstate->crtc_h;
+	int src_x = pstate->src_x;
+	int src_y = pstate->src_y;
+	int src_w = pstate->src_w;
+	int src_h = pstate->src_h;
+	struct drm_rect *src = &pstate->src;
+	struct drm_rect *dst = &pstate->dst;
+	struct drm_rect *clip = &pstate->clip;
 	int hscale, vscale;
 	int max_scale, min_scale;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
-	struct drm_rect src = {
-		/* sample coordinates in 16.16 fixed point */
-		.x1 = src_x,
-		.x2 = src_x + src_w,
-		.y1 = src_y,
-		.y2 = src_y + src_h,
-	};
-	struct drm_rect dst = {
-		/* integer pixels */
-		.x1 = crtc_x,
-		.x2 = crtc_x + crtc_w,
-		.y1 = crtc_y,
-		.y2 = crtc_y + crtc_h,
-	};
-	const struct drm_rect clip = {
-		.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
-		.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
-	};
-	const struct {
-		int crtc_x, crtc_y;
-		unsigned int crtc_w, crtc_h;
-		uint32_t src_x, src_y, src_w, src_h;
-	} orig = {
-		.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,
-	};
 
 	/* Don't modify another pipe's plane */
 	if (intel_plane->pipe != intel_crtc->pipe) {
@@ -927,55 +898,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	max_scale = intel_plane->max_downscale << 16;
 	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
-	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
+	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
 			intel_plane->rotation);
 
-	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
+	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
 	BUG_ON(hscale < 0);
 
-	vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale);
+	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
 	BUG_ON(vscale < 0);
 
-	visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale);
+	pstate->visible =  drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
 
-	crtc_x = dst.x1;
-	crtc_y = dst.y1;
-	crtc_w = drm_rect_width(&dst);
-	crtc_h = drm_rect_height(&dst);
+	crtc_x = dst->x1;
+	crtc_y = dst->y1;
+	crtc_w = drm_rect_width(dst);
+	crtc_h = drm_rect_height(dst);
 
-	if (visible) {
+	if (pstate->visible) {
 		/* check again in case clipping clamped the results */
-		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
+		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
 		if (hscale < 0) {
 			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
-			drm_rect_debug_print(&src, true);
-			drm_rect_debug_print(&dst, false);
+			drm_rect_debug_print(src, true);
+			drm_rect_debug_print(dst, false);
 
 			return hscale;
 		}
 
-		vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
+		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 		if (vscale < 0) {
 			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
-			drm_rect_debug_print(&src, true);
-			drm_rect_debug_print(&dst, false);
+			drm_rect_debug_print(src, true);
+			drm_rect_debug_print(dst, false);
 
 			return vscale;
 		}
 
 		/* Make the source viewport size an exact multiple of the scaling factors. */
-		drm_rect_adjust_size(&src,
-				     drm_rect_width(&dst) * hscale - drm_rect_width(&src),
-				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
+		drm_rect_adjust_size(src,
+				     drm_rect_width(dst) * hscale - drm_rect_width(src),
+				     drm_rect_height(dst) * vscale - drm_rect_height(src));
 
-		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
+		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
 				    intel_plane->rotation);
 
 		/* sanity check to make sure the src viewport wasn't enlarged */
-		WARN_ON(src.x1 < (int) src_x ||
-			src.y1 < (int) src_y ||
-			src.x2 > (int) (src_x + src_w) ||
-			src.y2 > (int) (src_y + src_h));
+		WARN_ON(src->x1 < (int) src_x ||
+			src->y1 < (int) src_y ||
+			src->x2 > (int) (src_x + src_w) ||
+			src->y2 > (int) (src_y + src_h));
 
 		/*
 		 * Hardware doesn't handle subpixel coordinates.
@@ -983,10 +954,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		 * increase the source viewport size, because that could
 		 * push the downscaling factor out of bounds.
 		 */
-		src_x = src.x1 >> 16;
-		src_w = drm_rect_width(&src) >> 16;
-		src_y = src.y1 >> 16;
-		src_h = drm_rect_height(&src) >> 16;
+		src_x = src->x1 >> 16;
+		src_w = drm_rect_width(src) >> 16;
+		src_y = src->y1 >> 16;
+		src_h = drm_rect_height(src) >> 16;
 
 		if (format_is_yuv(fb->pixel_format)) {
 			src_x &= ~1;
@@ -1000,12 +971,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 				crtc_w &= ~1;
 
 			if (crtc_w == 0)
-				visible = false;
+				pstate->visible = false;
 		}
 	}
 
 	/* Check size restrictions when scaling */
-	if (visible && (src_w != crtc_w || src_h != crtc_h)) {
+	if (pstate->visible && (src_w != crtc_w || src_h != crtc_h)) {
 		unsigned int width_bytes;
 
 		WARN_ON(!intel_plane->can_scale);
@@ -1013,10 +984,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		/* FIXME interlacing min height is 6 */
 
 		if (crtc_w < 3 || crtc_h < 3)
-			visible = false;
+			pstate->visible = false;
 
 		if (src_w < 3 || src_h < 3)
-			visible = false;
+			pstate->visible = false;
 
 		width_bytes = ((src_x * pixel_size) & 63) + src_w * pixel_size;
 
@@ -1027,17 +998,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		}
 	}
 
-	dst.x1 = crtc_x;
-	dst.x2 = crtc_x + crtc_w;
-	dst.y1 = crtc_y;
-	dst.y2 = crtc_y + crtc_h;
+	dst->x1 = crtc_x;
+	dst->x2 = crtc_x + crtc_w;
+	dst->y1 = crtc_y;
+	dst->y2 = crtc_y + crtc_h;
+
+	pstate->src_x = src_x;
+	pstate->src_y = src_y;
+	pstate->src_w = src_w;
+	pstate->src_h = src_h;
+	pstate->crtc_x = crtc_x;
+	pstate->crtc_y = crtc_y;
+	pstate->crtc_w = crtc_w;
+	pstate->crtc_x = crtc_h;
+
+	return 0;
+}
+
+static int
+intel_commit_sprite_plane(struct drm_plane *plane,
+			  struct intel_plane_state *pstate)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_crtc *crtc = pstate->crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	enum pipe pipe = intel_crtc->pipe;
+	struct drm_framebuffer *fb = pstate->fb;
+	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 crtc_x = pstate->crtc_x;
+	int crtc_y = pstate->crtc_y;
+	int crtc_w = pstate->crtc_w;
+	int crtc_h = pstate->crtc_h;
+	int src_x = pstate->src_x;
+	int src_y = pstate->src_y;
+	int src_w = pstate->src_w;
+	int src_h = pstate->src_h;
+	struct drm_rect *dst = &pstate->dst;
+	struct drm_rect *clip = &pstate->clip;
+	int ret;
+	bool primary_enabled;
 
 	/*
 	 * If the sprite is completely covering the primary plane,
 	 * we can disable the primary and save power.
 	 */
-	primary_enabled = !drm_rect_equals(&dst, &clip) || colorkey_enabled(intel_plane);
-	WARN_ON(!primary_enabled && !visible && intel_crtc->active);
+	primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
+	WARN_ON(!primary_enabled && !pstate->visible && intel_crtc->active);
 
 	mutex_lock(&dev->struct_mutex);
 
@@ -1055,14 +1064,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (ret)
 		return ret;
 
-	intel_plane->crtc_x = orig.crtc_x;
-	intel_plane->crtc_y = orig.crtc_y;
-	intel_plane->crtc_w = orig.crtc_w;
-	intel_plane->crtc_h = orig.crtc_h;
-	intel_plane->src_x = orig.src_x;
-	intel_plane->src_y = orig.src_y;
-	intel_plane->src_w = orig.src_w;
-	intel_plane->src_h = orig.src_h;
+	intel_plane->crtc_x = pstate->orig_dst.x1;
+	intel_plane->crtc_y = pstate->orig_dst.y1;
+	intel_plane->crtc_w = drm_rect_width(&pstate->orig_dst);
+	intel_plane->crtc_h = drm_rect_height(&pstate->orig_dst);
+	intel_plane->src_x = pstate->orig_src.x1;
+	intel_plane->src_y = pstate->orig_src.y1;
+	intel_plane->src_w = drm_rect_width(&pstate->orig_src);
+	intel_plane->src_h = drm_rect_height(&pstate->orig_src);
 	intel_plane->obj = obj;
 
 	if (intel_crtc->active) {
@@ -1076,7 +1085,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		if (primary_was_enabled && !primary_enabled)
 			intel_pre_disable_primary(crtc);
 
-		if (visible)
+		if (pstate->visible)
 			intel_plane->update_plane(plane, crtc, fb, obj,
 						  crtc_x, crtc_y, crtc_w, crtc_h,
 						  src_x, src_y, src_w, src_h);
@@ -1109,6 +1118,53 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 }
 
 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)
+{
+	struct intel_plane_state pstate;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int ret;
+
+	pstate.crtc = crtc;
+	pstate.fb = fb;
+	pstate.crtc_x = crtc_x;
+	pstate.crtc_y = crtc_y;
+	pstate.crtc_w = crtc_w;
+	pstate.crtc_h = crtc_h;
+	pstate.src_x = src_x;
+	pstate.src_y = src_y;
+	pstate.src_w = src_w;
+	pstate.src_h = src_h;
+	pstate.src.x1 = src_x,
+	pstate.src.x2 = src_x + src_w,
+	pstate.src.y1 = src_y,
+	pstate.src.y2 = src_y + src_h,
+	pstate.dst.x1 = crtc_x,
+	pstate.dst.x2 = crtc_x + crtc_w,
+	pstate.dst.y1 = crtc_y,
+	pstate.dst.y2 = crtc_y + crtc_h,
+	pstate.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
+	pstate.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
+	pstate.orig_src.x1 = src_x,
+	pstate.orig_src.x2 = src_x + src_w,
+	pstate.orig_src.y1 = src_y,
+	pstate.orig_src.y2 = src_y + src_h,
+	pstate.orig_dst.x1 = crtc_x,
+	pstate.orig_dst.x2 = crtc_x + crtc_w,
+	pstate.orig_dst.y1 = crtc_y,
+	pstate.orig_dst.y2 = crtc_y + crtc_h,
+
+	ret = intel_check_sprite_plane(plane, &pstate);
+	if (ret)
+		return ret;
+
+	return intel_commit_sprite_plane(plane, &pstate);
+}
+
+static int
 intel_disable_plane(struct drm_plane *plane)
 {
 	struct drm_device *dev = plane->dev;
-- 
1.9.3

Re: [PATCH -v2 4/4] drm/i915: split intel_update_plane into check() and commit()

[Ville Syrjälä]

On Tue, Sep 02, 2014 at 04:23:47PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Due to the upcoming atomic modesetting feature we need to separate
> some update functions into a check step that can fail and a commit
> step that should, ideally, never fail.
> 
> This commit splits intel_update_plane() and its commit part can still
> fail due to the fb pinning procedure.
> 
> v2: use the new struct intel_plane_state to store information
> between check and commit stages.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 236 ++++++++++++++++++++++--------------
>  1 file changed, 146 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4cbe286..062eca2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -845,57 +845,28 @@ static bool colorkey_enabled(struct intel_plane *intel_plane)
>  }
>  
>  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_check_sprite_plane(struct drm_plane *plane,
> +			 struct intel_plane_state *pstate)

pstate looks a bit funny. Makes me think on cpu pstates. Maybe just call
it 'state' everywhere?

>  {
> -	struct drm_device *dev = plane->dev;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(pstate->crtc);
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> -	enum pipe pipe = intel_crtc->pipe;
> +	struct drm_framebuffer *fb = pstate->fb;
>  	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 ret;
> -	bool primary_enabled;
> -	bool visible;
> +	int crtc_x = pstate->crtc_x;
> +	int crtc_y = pstate->crtc_y;
> +	int crtc_w = pstate->crtc_w;
> +	int crtc_h = pstate->crtc_h;
> +	int src_x = pstate->src_x;
> +	int src_y = pstate->src_y;
> +	int src_w = pstate->src_w;
> +	int src_h = pstate->src_h;

AFAICS we don't need to initialize this stuff here. We already populate
it all before we need it further down. So I think the only thing we need
is add a bit of code to write the src_? coordinates back into
pstate->src (at the end of check() like you already do for the dst
coordinates). To avoid warning about unitialized variables that
probably needs to be wrapped in an 'if (visible)' check.

> +	struct drm_rect *src = &pstate->src;
> +	struct drm_rect *dst = &pstate->dst;
> +	struct drm_rect *clip = &pstate->clip;

Please keep 'clip' const here to prevent silly accidents.

>  	int hscale, vscale;
>  	int max_scale, min_scale;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	struct drm_rect src = {
> -		/* sample coordinates in 16.16 fixed point */
> -		.x1 = src_x,
> -		.x2 = src_x + src_w,
> -		.y1 = src_y,
> -		.y2 = src_y + src_h,
> -	};
> -	struct drm_rect dst = {
> -		/* integer pixels */
> -		.x1 = crtc_x,
> -		.x2 = crtc_x + crtc_w,
> -		.y1 = crtc_y,
> -		.y2 = crtc_y + crtc_h,
> -	};
> -	const struct drm_rect clip = {
> -		.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> -		.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> -	};
> -	const struct {
> -		int crtc_x, crtc_y;
> -		unsigned int crtc_w, crtc_h;
> -		uint32_t src_x, src_y, src_w, src_h;
> -	} orig = {
> -		.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,
> -	};
>  
>  	/* Don't modify another pipe's plane */
>  	if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -927,55 +898,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	max_scale = intel_plane->max_downscale << 16;
>  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>  
> -	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> +	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>  			intel_plane->rotation);
>  
> -	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> +	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
>  	BUG_ON(hscale < 0);
>  
> -	vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
>  	BUG_ON(vscale < 0);
>  
> -	visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale);
> +	pstate->visible =  drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>  
> -	crtc_x = dst.x1;
> -	crtc_y = dst.y1;
> -	crtc_w = drm_rect_width(&dst);
> -	crtc_h = drm_rect_height(&dst);
> +	crtc_x = dst->x1;
> +	crtc_y = dst->y1;
> +	crtc_w = drm_rect_width(dst);
> +	crtc_h = drm_rect_height(dst);
>  
> -	if (visible) {
> +	if (pstate->visible) {
>  		/* check again in case clipping clamped the results */
> -		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> +		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>  		if (hscale < 0) {
>  			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> -			drm_rect_debug_print(&src, true);
> -			drm_rect_debug_print(&dst, false);
> +			drm_rect_debug_print(src, true);
> +			drm_rect_debug_print(dst, false);
>  
>  			return hscale;
>  		}
>  
> -		vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
> +		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>  		if (vscale < 0) {
>  			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> -			drm_rect_debug_print(&src, true);
> -			drm_rect_debug_print(&dst, false);
> +			drm_rect_debug_print(src, true);
> +			drm_rect_debug_print(dst, false);
>  
>  			return vscale;
>  		}
>  
>  		/* Make the source viewport size an exact multiple of the scaling factors. */
> -		drm_rect_adjust_size(&src,
> -				     drm_rect_width(&dst) * hscale - drm_rect_width(&src),
> -				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
> +		drm_rect_adjust_size(src,
> +				     drm_rect_width(dst) * hscale - drm_rect_width(src),
> +				     drm_rect_height(dst) * vscale - drm_rect_height(src));
>  
> -		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> +		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
>  				    intel_plane->rotation);
>  
>  		/* sanity check to make sure the src viewport wasn't enlarged */
> -		WARN_ON(src.x1 < (int) src_x ||
> -			src.y1 < (int) src_y ||
> -			src.x2 > (int) (src_x + src_w) ||
> -			src.y2 > (int) (src_y + src_h));
> +		WARN_ON(src->x1 < (int) src_x ||
> +			src->y1 < (int) src_y ||
> +			src->x2 > (int) (src_x + src_w) ||
> +			src->y2 > (int) (src_y + src_h));
>  
>  		/*
>  		 * Hardware doesn't handle subpixel coordinates.
> @@ -983,10 +954,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		 * increase the source viewport size, because that could
>  		 * push the downscaling factor out of bounds.
>  		 */
> -		src_x = src.x1 >> 16;
> -		src_w = drm_rect_width(&src) >> 16;
> -		src_y = src.y1 >> 16;
> -		src_h = drm_rect_height(&src) >> 16;
> +		src_x = src->x1 >> 16;
> +		src_w = drm_rect_width(src) >> 16;
> +		src_y = src->y1 >> 16;
> +		src_h = drm_rect_height(src) >> 16;
>  
>  		if (format_is_yuv(fb->pixel_format)) {
>  			src_x &= ~1;
> @@ -1000,12 +971,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  				crtc_w &= ~1;
>  
>  			if (crtc_w == 0)
> -				visible = false;
> +				pstate->visible = false;
>  		}
>  	}
>  
>  	/* Check size restrictions when scaling */
> -	if (visible && (src_w != crtc_w || src_h != crtc_h)) {
> +	if (pstate->visible && (src_w != crtc_w || src_h != crtc_h)) {
>  		unsigned int width_bytes;
>  
>  		WARN_ON(!intel_plane->can_scale);
> @@ -1013,10 +984,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		/* FIXME interlacing min height is 6 */
>  
>  		if (crtc_w < 3 || crtc_h < 3)
> -			visible = false;
> +			pstate->visible = false;
>  
>  		if (src_w < 3 || src_h < 3)
> -			visible = false;
> +			pstate->visible = false;
>  
>  		width_bytes = ((src_x * pixel_size) & 63) + src_w * pixel_size;
>  
> @@ -1027,17 +998,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		}
>  	}
>  
> -	dst.x1 = crtc_x;
> -	dst.x2 = crtc_x + crtc_w;
> -	dst.y1 = crtc_y;
> -	dst.y2 = crtc_y + crtc_h;
> +	dst->x1 = crtc_x;
> +	dst->x2 = crtc_x + crtc_w;
> +	dst->y1 = crtc_y;
> +	dst->y2 = crtc_y + crtc_h;
> +
> +	pstate->src_x = src_x;
> +	pstate->src_y = src_y;
> +	pstate->src_w = src_w;
> +	pstate->src_h = src_h;
> +	pstate->crtc_x = crtc_x;
> +	pstate->crtc_y = crtc_y;
> +	pstate->crtc_w = crtc_w;
> +	pstate->crtc_x = crtc_h;
> +
> +	return 0;
> +}
> +
> +static int
> +intel_commit_sprite_plane(struct drm_plane *plane,
> +			  struct intel_plane_state *pstate)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_crtc *crtc = pstate->crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	enum pipe pipe = intel_crtc->pipe;
> +	struct drm_framebuffer *fb = pstate->fb;
> +	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 crtc_x = pstate->crtc_x;
> +	int crtc_y = pstate->crtc_y;
> +	int crtc_w = pstate->crtc_w;
> +	int crtc_h = pstate->crtc_h;
> +	int src_x = pstate->src_x;
> +	int src_y = pstate->src_y;
> +	int src_w = pstate->src_w;
> +	int src_h = pstate->src_h;

As I mentioned on the previous patch I don't think we need the
coordinates stored twice in the plane config.

Just do:
 crtc_x = pstate->dst.x1;
 ...
 crtc_w = drm_rect_width(&pstate->dst);
 ...

etc.

> +	struct drm_rect *dst = &pstate->dst;
> +	struct drm_rect *clip = &pstate->clip;
> +	int ret;
> +	bool primary_enabled;
>  
>  	/*
>  	 * If the sprite is completely covering the primary plane,
>  	 * we can disable the primary and save power.
>  	 */
> -	primary_enabled = !drm_rect_equals(&dst, &clip) || colorkey_enabled(intel_plane);
> -	WARN_ON(!primary_enabled && !visible && intel_crtc->active);
> +	primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
> +	WARN_ON(!primary_enabled && !pstate->visible && intel_crtc->active);
>  
>  	mutex_lock(&dev->struct_mutex);
>  
> @@ -1055,14 +1064,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	if (ret)
>  		return ret;
>  
> -	intel_plane->crtc_x = orig.crtc_x;
> -	intel_plane->crtc_y = orig.crtc_y;
> -	intel_plane->crtc_w = orig.crtc_w;
> -	intel_plane->crtc_h = orig.crtc_h;
> -	intel_plane->src_x = orig.src_x;
> -	intel_plane->src_y = orig.src_y;
> -	intel_plane->src_w = orig.src_w;
> -	intel_plane->src_h = orig.src_h;
> +	intel_plane->crtc_x = pstate->orig_dst.x1;
> +	intel_plane->crtc_y = pstate->orig_dst.y1;
> +	intel_plane->crtc_w = drm_rect_width(&pstate->orig_dst);
> +	intel_plane->crtc_h = drm_rect_height(&pstate->orig_dst);
> +	intel_plane->src_x = pstate->orig_src.x1;
> +	intel_plane->src_y = pstate->orig_src.y1;
> +	intel_plane->src_w = drm_rect_width(&pstate->orig_src);
> +	intel_plane->src_h = drm_rect_height(&pstate->orig_src);
>  	intel_plane->obj = obj;
>  
>  	if (intel_crtc->active) {
> @@ -1076,7 +1085,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		if (primary_was_enabled && !primary_enabled)
>  			intel_pre_disable_primary(crtc);
>  
> -		if (visible)
> +		if (pstate->visible)
>  			intel_plane->update_plane(plane, crtc, fb, obj,
>  						  crtc_x, crtc_y, crtc_w, crtc_h,
>  						  src_x, src_y, src_w, src_h);
> @@ -1109,6 +1118,53 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  }
>  
>  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)
> +{
> +	struct intel_plane_state pstate;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int ret;
> +
> +	pstate.crtc = crtc;
> +	pstate.fb = fb;
> +	pstate.crtc_x = crtc_x;
> +	pstate.crtc_y = crtc_y;
> +	pstate.crtc_w = crtc_w;
> +	pstate.crtc_h = crtc_h;
> +	pstate.src_x = src_x;
> +	pstate.src_y = src_y;
> +	pstate.src_w = src_w;
> +	pstate.src_h = src_h;
> +	pstate.src.x1 = src_x,
> +	pstate.src.x2 = src_x + src_w,
> +	pstate.src.y1 = src_y,
> +	pstate.src.y2 = src_y + src_h,
> +	pstate.dst.x1 = crtc_x,
> +	pstate.dst.x2 = crtc_x + crtc_w,
> +	pstate.dst.y1 = crtc_y,
> +	pstate.dst.y2 = crtc_y + crtc_h,
> +	pstate.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> +	pstate.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,

You need to initialize clip x1 and y1 too since pstate isn't zero
initialized.

> +	pstate.orig_src.x1 = src_x,
> +	pstate.orig_src.x2 = src_x + src_w,
> +	pstate.orig_src.y1 = src_y,
> +	pstate.orig_src.y2 = src_y + src_h,
> +	pstate.orig_dst.x1 = crtc_x,
> +	pstate.orig_dst.x2 = crtc_x + crtc_w,
> +	pstate.orig_dst.y1 = crtc_y,
> +	pstate.orig_dst.y2 = crtc_y + crtc_h,

s/,/;/ in a lots of places here.

But I think we can simplify this a bit like so:

pstate.orig.src... = ..;
pstate.orig.dst... = ..;

pstate.src = pstate.orig.src;
pstate.dst = pstate.orig.dst;

> +
> +	ret = intel_check_sprite_plane(plane, &pstate);
> +	if (ret)
> +		return ret;
> +
> +	return intel_commit_sprite_plane(plane, &pstate);
> +}
> +
> +static int
>  intel_disable_plane(struct drm_plane *plane)
>  {
>  	struct drm_device *dev = plane->dev;
> -- 
> 1.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[Ville Syrjälä]

On Tue, Sep 02, 2014 at 04:23:44PM -0300, Gustavo Padovan wrote:
> From: Derek Foreman <derek.foreman@collabora.co.uk>
> 
> Really just for completeness - old init function ends up making the plane
> exactly the same way due to the way the enums are set up.
> 
> Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0bdb00b..4cbe286 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  	intel_plane->plane = plane;
>  	intel_plane->rotation = BIT(DRM_ROTATE_0);
>  	possible_crtcs = (1 << pipe);
> -	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
> -			     &intel_plane_funcs,
> -			     plane_formats, num_plane_formats,
> -			     false);
> +	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> +				       &intel_plane_funcs,
> +				       plane_formats, num_plane_formats,
> +				       DRM_PLANE_TYPE_OVERLAY);
>  	if (ret) {
>  		kfree(intel_plane);
>  		goto out;
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[Daniel Vetter]

On Wed, Sep 03, 2014 at 11:18:27AM +0300, Ville Syrjälä wrote:
> On Tue, Sep 02, 2014 at 04:23:44PM -0300, Gustavo Padovan wrote:
> > From: Derek Foreman <derek.foreman@collabora.co.uk>
> > 
> > Really just for completeness - old init function ends up making the plane
> > exactly the same way due to the way the enums are set up.
> > 
> > Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 0bdb00b..4cbe286 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  	intel_plane->plane = plane;
> >  	intel_plane->rotation = BIT(DRM_ROTATE_0);
> >  	possible_crtcs = (1 << pipe);
> > -	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
> > -			     &intel_plane_funcs,
> > -			     plane_formats, num_plane_formats,
> > -			     false);
> > +	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> > +				       &intel_plane_funcs,
> > +				       plane_formats, num_plane_formats,
> > +				       DRM_PLANE_TYPE_OVERLAY);
> >  	if (ret) {
> >  		kfree(intel_plane);
> >  		goto out;
> > -- 
> > 1.9.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[Daniel Vetter]

On Wed, Sep 03, 2014 at 11:18:27AM +0300, Ville Syrjälä wrote:
> On Tue, Sep 02, 2014 at 04:23:44PM -0300, Gustavo Padovan wrote:
> > From: Derek Foreman <derek.foreman@collabora.co.uk>
> > 
> > Really just for completeness - old init function ends up making the plane
> > exactly the same way due to the way the enums are set up.
> > 
> > Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch. And patch 2 is already merged from
the previous round.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function

[Daniel Vetter]

On Wed, Sep 03, 2014 at 10:38:17AM +0200, Daniel Vetter wrote:
> On Wed, Sep 03, 2014 at 11:18:27AM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 02, 2014 at 04:23:44PM -0300, Gustavo Padovan wrote:
> > > From: Derek Foreman <derek.foreman@collabora.co.uk>
> > > 
> > > Really just for completeness - old init function ends up making the plane
> > > exactly the same way due to the way the enums are set up.
> > > 
> > > Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Queued for -next, thanks for the patch. And patch 2 is already merged from
> the previous round.

Actually I can't merge this patch since it's not authered by you, but is
missing your signed-off-by line. Please resend with that fixed, thanks.

If you handle a patch for the linux kernel, you _always_ have to add your
sob in accordance with the Developer's Certificate of Origin.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: init sprites with univeral plane init function

[Gustavo Padovan]

From: Derek Foreman <derek.foreman@collabora.co.uk>

Really just for completeness - old init function ends up making the plane
exactly the same way due to the way the enums are set up.

Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0bdb00b..4cbe286 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	intel_plane->plane = plane;
 	intel_plane->rotation = BIT(DRM_ROTATE_0);
 	possible_crtcs = (1 << pipe);
-	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
-			     &intel_plane_funcs,
-			     plane_formats, num_plane_formats,
-			     false);
+	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
+				       &intel_plane_funcs,
+				       plane_formats, num_plane_formats,
+				       DRM_PLANE_TYPE_OVERLAY);
 	if (ret) {
 		kfree(intel_plane);
 		goto out;
-- 
1.9.3

Re: [PATCH] drm/i915: init sprites with univeral plane init function

[Daniel Vetter]

On Wed, Sep 03, 2014 at 10:38:20AM -0300, Gustavo Padovan wrote:
> From: Derek Foreman <derek.foreman@collabora.co.uk>
> 
> Really just for completeness - old init function ends up making the plane
> exactly the same way due to the way the enums are set up.
> 
> Signed-off-by: Derek Foreman <derek.foreman@collabora.co.uk>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0bdb00b..4cbe286 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  	intel_plane->plane = plane;
>  	intel_plane->rotation = BIT(DRM_ROTATE_0);
>  	possible_crtcs = (1 << pipe);
> -	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
> -			     &intel_plane_funcs,
> -			     plane_formats, num_plane_formats,
> -			     false);
> +	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> +				       &intel_plane_funcs,
> +				       plane_formats, num_plane_formats,
> +				       DRM_PLANE_TYPE_OVERLAY);
>  	if (ret) {
>  		kfree(intel_plane);
>  		goto out;
> -- 
> 1.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch