[Intel-gfx] [PATCH 5/5] drm/i915: Implement dst colorkeying for VLV/CHV sprites

Ville Syrjala ville.syrjala at linux.intel.com
Tue May 29 18:28:04 UTC 2018


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

On VLV/CHV we can use the "src key window" bit on the primary plane to
implemnt sprite dst colorkeying. This is exactly the same mechanism that
would be used on gen2-4 for sprite C dst colorkeying as well.

The slight complication with this approach is that we have to adjust the
zpos of the planes since we're still technically doing src color keying
on the primary, hence the primary must sit above the sprites. But since
we now have working zpos controls on VLV/CHV we can do this pretty
easily.

Working dst colorkeying makes for a much nicer sprite Xv experience.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h           |  9 +++-
 drivers/gpu/drm/i915/intel_atomic_plane.c |  3 ++
 drivers/gpu/drm/i915/intel_display.c      | 89 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h          |  3 ++
 drivers/gpu/drm/i915/intel_sprite.c       | 21 +++++---
 5 files changed, 113 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 44eac5ffc2ce..07ec95906869 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6000,8 +6000,8 @@ enum {
 #define   DISPPLANE_SEL_PIPE_SHIFT		24
 #define   DISPPLANE_SEL_PIPE_MASK		(3<<DISPPLANE_SEL_PIPE_SHIFT)
 #define   DISPPLANE_SEL_PIPE(pipe)		((pipe)<<DISPPLANE_SEL_PIPE_SHIFT)
+#define   DISPPLANE_SRC_KEY_WINDOW_ENABLE	(1<<23) /* planes A/B */
 #define   DISPPLANE_SRC_KEY_ENABLE		(1<<22)
-#define   DISPPLANE_SRC_KEY_DISABLE		0
 #define   DISPPLANE_LINE_DOUBLE			(1<<20)
 #define   DISPPLANE_NO_LINE_DOUBLE		0
 #define   DISPPLANE_STEREO_POLARITY_FIRST	0
@@ -6014,8 +6014,12 @@ enum {
 #define _DSPAADDR				0x70184
 #define _DSPASTRIDE				0x70188
 #define _DSPAPOS				0x7018C /* reserved */
+#define _DSPAKEYVAL				0x70194 /* planes A/B */
+#define _DSPAMINKEYVAL				0x70194 /* plane C */
+#define _DSPAKEYMSK				0x70198
 #define _DSPASIZE				0x70190
 #define _DSPASURF				0x7019C /* 965+ only */
+#define _DSPAMAXKEYVAL				0x701A0 /* plane C */
 #define _DSPATILEOFF				0x701A4 /* 965+ only */
 #define _DSPAOFFSET				0x701A4 /* HSW */
 #define _DSPASURFLIVE				0x701AC
@@ -6024,6 +6028,9 @@ enum {
 #define DSPADDR(plane)		_MMIO_PIPE2(plane, _DSPAADDR)
 #define DSPSTRIDE(plane)	_MMIO_PIPE2(plane, _DSPASTRIDE)
 #define DSPPOS(plane)		_MMIO_PIPE2(plane, _DSPAPOS)
+#define DSPKEYVAL(plane)	_MMIO_PIPE2(plane, _DSPAKEYVAL)
+#define DSPMINKEYVAL(plane)	_MMIO_PIPE2(plane, _DSPAMINKEYVAL)
+#define DSPKEYMSK(plane)	_MMIO_PIPE2(plane, _DSPAKEYMSK)
 #define DSPSIZE(plane)		_MMIO_PIPE2(plane, _DSPASIZE)
 #define DSPSURF(plane)		_MMIO_PIPE2(plane, _DSPASURF)
 #define DSPTILEOFF(plane)	_MMIO_PIPE2(plane, _DSPATILEOFF)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index a5a3123589e7..b29ae8cded06 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -200,6 +200,9 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 	 */
 	crtc_state->raw_zpos[intel_plane->id] =
 		intel_plane_raw_zpos(crtc_state, intel_state);
+	crtc_state->raw_dst_colorkey[intel_plane->id] =
+		intel_state->base.visible &&
+		intel_state->ckey.flags & I915_SET_COLORKEY_DESTINATION;
 
 	return intel_plane_atomic_calc_changes(old_crtc_state,
 					       &crtc_state->base,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 111cb0c1da54..b9a85bfe5d69 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3314,6 +3314,7 @@ static void i9xx_update_plane(struct intel_plane *plane,
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(i9xx_plane);
@@ -3329,8 +3330,17 @@ static void i9xx_update_plane(struct intel_plane *plane,
 	else
 		dspaddr_offset = linear_offset;
 
+	if (crtc_state->dst_colorkey[plane->id])
+		dspcntr |= DISPPLANE_SRC_KEY_ENABLE |
+			DISPPLANE_SRC_KEY_WINDOW_ENABLE;
+
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	if (crtc_state->dst_colorkey[plane->id]) {
+		I915_WRITE_FW(DSPKEYVAL(i9xx_plane), key->min_value);
+		I915_WRITE_FW(DSPKEYMSK(i9xx_plane), key->channel_mask);
+	}
+
 	if (INTEL_GEN(dev_priv) < 4) {
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
@@ -10936,6 +10946,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	struct intel_shared_dpll *shared_dpll;
 	struct intel_crtc_wm_state wm_state;
 	u8 raw_zpos[I915_MAX_PLANES];
+	u8 raw_dst_colorkey[I915_MAX_PLANES];
 	bool force_thru, ips_force_disable;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
@@ -10952,6 +10963,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		wm_state = crtc_state->wm;
 	memcpy(raw_zpos, crtc_state->raw_zpos, sizeof(raw_zpos));
+	memcpy(raw_dst_colorkey, crtc_state->raw_dst_colorkey, sizeof(raw_dst_colorkey));
 
 	/* Keep base drm_crtc_state intact, only clear our extended struct */
 	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
@@ -10967,6 +10979,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		crtc_state->wm = wm_state;
 	memcpy(crtc_state->raw_zpos, raw_zpos, sizeof(raw_zpos));
+	memcpy(crtc_state->raw_dst_colorkey, raw_dst_colorkey, sizeof(raw_dst_colorkey));
 }
 
 static int
@@ -12242,6 +12255,55 @@ static void vlv_normalize_zpos(struct intel_crtc_state *crtc_state)
 	}
 }
 
+static int vlv_fixup_colorkey_zpos(struct intel_crtc_state *crtc_state)
+{
+	/* disable dst colorkey if the primary is above the sprite */
+	crtc_state->dst_colorkey[PLANE_SPRITE0] =
+		crtc_state->raw_dst_colorkey[PLANE_SPRITE0] &&
+		crtc_state->zpos[PLANE_SPRITE0] > crtc_state->zpos[PLANE_PRIMARY];
+
+	crtc_state->dst_colorkey[PLANE_SPRITE1] =
+		crtc_state->raw_dst_colorkey[PLANE_SPRITE1] &&
+		crtc_state->zpos[PLANE_SPRITE1] > crtc_state->zpos[PLANE_PRIMARY];
+
+	/* lift the primary above the sprites that have dst colorkey enabled */
+	if (crtc_state->dst_colorkey[PLANE_SPRITE0] &&
+	    crtc_state->dst_colorkey[PLANE_SPRITE1]) {
+		/*
+		 * FIXME should make sure both planes have
+		 * the same dst colorkey value+mask.
+		 */
+		crtc_state->zpos[PLANE_SPRITE0]--;
+		crtc_state->zpos[PLANE_SPRITE1]--;
+		crtc_state->zpos[PLANE_PRIMARY] = 2;
+	} else if (crtc_state->dst_colorkey[PLANE_SPRITE0]) {
+		if (crtc_state->zpos[PLANE_SPRITE1] <
+		    crtc_state->zpos[PLANE_SPRITE0]) {
+			DRM_DEBUG_KMS("Sprite 0 dst colorkey conflicts with zpos\n");
+			return -EINVAL;
+		}
+
+		swap(crtc_state->zpos[PLANE_PRIMARY],
+		     crtc_state->zpos[PLANE_SPRITE0]);
+	} else if (crtc_state->dst_colorkey[PLANE_SPRITE1]) {
+		if (crtc_state->zpos[PLANE_SPRITE0] <
+		    crtc_state->zpos[PLANE_SPRITE1]) {
+			DRM_DEBUG_KMS("Sprite 1 dst colorkey conflicts with zpos\n");
+			return -EINVAL;
+		}
+
+		swap(crtc_state->zpos[PLANE_PRIMARY],
+		     crtc_state->zpos[PLANE_SPRITE1]);
+	}
+
+	/* enable the dst colorkey on the primary if either sprite needs it */
+	crtc_state->dst_colorkey[PLANE_PRIMARY] =
+		crtc_state->dst_colorkey[PLANE_SPRITE0] ||
+		crtc_state->dst_colorkey[PLANE_SPRITE1];
+
+	return 0;
+}
+
 static int vlv_update_zpos(struct drm_i915_private *dev_priv,
 			   struct intel_atomic_state *state)
 {
@@ -12251,16 +12313,26 @@ static int vlv_update_zpos(struct drm_i915_private *dev_priv,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		struct drm_plane *plane;
+		bool primary_changed;
 		bool sprites_changed;
+		int ret;
 
 		vlv_normalize_zpos(new_crtc_state);
 
+		ret = vlv_fixup_colorkey_zpos(new_crtc_state);
+		if (ret)
+			return ret;
+
+		primary_changed =
+			new_crtc_state->dst_colorkey[PLANE_PRIMARY] !=
+			old_crtc_state->dst_colorkey[PLANE_PRIMARY];
+
 		sprites_changed =
 			memcmp(new_crtc_state->zpos,
 			       old_crtc_state->zpos,
 			       sizeof(new_crtc_state->zpos)) != 0;
 
-		if (!sprites_changed)
+		if (!primary_changed && !sprites_changed)
 			continue;
 
 		drm_for_each_plane_mask(plane, &dev_priv->drm,
@@ -12268,14 +12340,23 @@ static int vlv_update_zpos(struct drm_i915_private *dev_priv,
 			struct drm_plane_state *plane_state;
 			enum plane_id plane_id = to_intel_plane(plane)->id;
 
-			if (plane_id == PLANE_CURSOR ||
-			    plane_id == PLANE_PRIMARY)
+			if (plane_id == PLANE_CURSOR)
 				continue;
 
+			if (plane_id == PLANE_PRIMARY) {
+				if (!primary_changed)
+					continue;
+			} else {
+				if (!sprites_changed)
+					continue;
+			}
+
 			/*
 			 * zpos is configured via sprite registers, so must
 			 * add them to the state if anything significant
-			 * changed.
+			 * changed. dst colorkeying is configured via the primary
+			 * plane registers, so similarly we may need to reconfigure
+			 * it as well.
 			 */
 			plane_state = drm_atomic_get_plane_state(&state->base, plane);
 			if (IS_ERR(plane_state))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 994b9473b817..415e8ac0b6e7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -894,6 +894,9 @@ struct intel_crtc_state {
 	u8 raw_zpos[I915_MAX_PLANES]; /* raw property value (or 0xff for disabled planes) */
 	u8 zpos[I915_MAX_PLANES]; /* final zpos for each plane */
 
+	u8 raw_dst_colorkey[I915_MAX_PLANES]; /* dst colorkey yes/no? */
+	u8 dst_colorkey[I915_MAX_PLANES]; /* dst colorkey yes/no? */
+
 	/* HDMI scrambling status */
 	bool hdmi_scrambling;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 228eba582d44..21e379ab37ce 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -564,7 +564,7 @@ vlv_update_plane(struct intel_plane *plane,
 	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
 		chv_update_csc(plane_state);
 
-	if (key->flags) {
+	if (key->flags & I915_SET_COLORKEY_SOURCE) {
 		I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);
 		I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value);
 		I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask >> 24);
@@ -1092,13 +1092,15 @@ intel_check_sprite_plane(struct intel_plane *plane,
 
 static bool has_dst_key_in_primary_plane(struct drm_i915_private *dev_priv)
 {
-	return INTEL_GEN(dev_priv) >= 9;
+	return INTEL_GEN(dev_priv) >= 9 ||
+		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv);
 }
 
 static void intel_plane_set_ckey(struct intel_plane_state *plane_state,
 				 const struct drm_intel_sprite_colorkey *set)
 {
 	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 
 	*key = *set;
@@ -1114,8 +1116,13 @@ static void intel_plane_set_ckey(struct intel_plane_state *plane_state,
 	/*
 	 * On SKL+ we want dst key enabled on
 	 * the primary and not on the sprite.
+	 *
+	 * On VLV/CHV we want to keep the dst key flag even on the
+	 * sprites to make the crtc_state->dst_colorkey[] stuff work.
+	 * vlv_update_plane() won't do anything with the dst key flag
+	 * anyway, so leaving it on doesn't hurt.
 	 */
-	if (plane->id != PLANE_PRIMARY &&
+	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_PRIMARY &&
 	    set->flags & I915_SET_COLORKEY_DESTINATION)
 		key->flags = 0;
 }
@@ -1141,10 +1148,6 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 	if ((set->flags & (I915_SET_COLORKEY_DESTINATION | I915_SET_COLORKEY_SOURCE)) == (I915_SET_COLORKEY_DESTINATION | I915_SET_COLORKEY_SOURCE))
 		return -EINVAL;
 
-	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	    set->flags & I915_SET_COLORKEY_DESTINATION)
-		return -EINVAL;
-
 	plane = drm_plane_find(dev, file_priv, set->plane_id);
 	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
 		return -ENOENT;
@@ -1177,6 +1180,10 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 		/*
 		 * On some platforms we have to configure
 		 * the dst colorkey on the primary plane.
+		 *
+		 * FIXME this won't work properly for multiple
+		 * sprite planes on VLV/CHV if each is trying to
+		 * use a different colorkey value/mask.
 		 */
 		if (!ret && has_dst_key_in_primary_plane(dev_priv)) {
 			struct intel_crtc *crtc =
-- 
2.16.1



More information about the Intel-gfx mailing list