All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+
@ 2018-05-29 18:28 Ville Syrjala
  2018-05-29 18:28 ` [PATCH 2/5] drm/i915: Fix VLV/CHV sprite src colorkey mask Ville Syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Ville Syrjala @ 2018-05-29 18:28 UTC (permalink / raw)
  To: intel-gfx

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

On SKL+ the dst colorkey must be configured on the lower
plane that contains the colorkey. This is in contrast to
most earlier platforms where the dst colorkey is configured
on the plane above.

The hardware will peform dst keying only between two immediately
adjacent (in zorder) planes. Plane 1 will be keyed against plane 0,
plane 2 againts plane 1, and so on. There is no way to key arbitrary
planes against plane 0. Thus offering dst color keying on plane 2+
is pointless. In fact it can be harmful since enabling dst keying on
more than one plane on the same pipe leads to only the top-most of
the planes performing the keying. For any plane lower in zorder the
dst key enable is simply ignored.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 63 +++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ee23613f9fd4..6164c2ca20c3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	return 0;
 }
 
+static bool has_dst_key_in_primary_plane(struct drm_i915_private *dev_priv)
+{
+	return INTEL_GEN(dev_priv) >= 9;
+}
+
+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_intel_sprite_colorkey *key = &plane_state->ckey;
+
+	*key = *set;
+
+	/*
+	 * We want src key enabled on the
+	 * sprite and not on the primary.
+	 */
+	if (plane->id == PLANE_PRIMARY &&
+	    set->flags & I915_SET_COLORKEY_SOURCE)
+		key->flags = 0;
+
+	/*
+	 * On SKL+ we want dst key enabled on
+	 * the primary and not on the sprite.
+	 */
+	if (plane->id != PLANE_PRIMARY &&
+	    set->flags & I915_SET_COLORKEY_DESTINATION)
+		key->flags = 0;
+}
+
 int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 				    struct drm_file *file_priv)
 {
@@ -1100,6 +1130,16 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
 		return -ENOENT;
 
+	/*
+	 * SKL+ only plane 1 can do destination keying against plane 0.
+	 * Also multiple planes can't do destination keying on the same
+	 * pipe simultaneously.
+	 */
+	if (INTEL_GEN(dev_priv) >= 9 &&
+	    to_intel_plane(plane)->id >= PLANE_SPRITE1 &&
+	    set->flags & I915_SET_COLORKEY_DESTINATION)
+		return -EINVAL;
+
 	drm_modeset_acquire_init(&ctx, 0);
 
 	state = drm_atomic_state_alloc(plane->dev);
@@ -1112,11 +1152,28 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 	while (1) {
 		plane_state = drm_atomic_get_plane_state(state, plane);
 		ret = PTR_ERR_OR_ZERO(plane_state);
-		if (!ret) {
-			to_intel_plane_state(plane_state)->ckey = *set;
-			ret = drm_atomic_commit(state);
+		if (!ret)
+			intel_plane_set_ckey(to_intel_plane_state(plane_state), set);
+
+		/*
+		 * On some platforms we have to configure
+		 * the dst colorkey on the primary plane.
+		 */
+		if (!ret && has_dst_key_in_primary_plane(dev_priv)) {
+			struct intel_crtc *crtc =
+				intel_get_crtc_for_pipe(dev_priv,
+							to_intel_plane(plane)->pipe);
+
+			plane_state = drm_atomic_get_plane_state(state,
+								 crtc->base.primary);
+			ret = PTR_ERR_OR_ZERO(plane_state);
+			if (!ret)
+				intel_plane_set_ckey(to_intel_plane_state(plane_state), set);
 		}
 
+		if (!ret)
+			ret = drm_atomic_commit(state);
+
 		if (ret != -EDEADLK)
 			break;
 
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 2/5] drm/i915: Fix VLV/CHV sprite src colorkey mask
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
@ 2018-05-29 18:28 ` Ville Syrjala
  2018-05-29 18:28 ` [PATCH 3/5] drm/i915: Add basic immutable zpos properties for all planes Ville Syrjala
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2018-05-29 18:28 UTC (permalink / raw)
  To: intel-gfx

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

VLV/CHV sprites use the old plane C layout of the colorkey mask
register. Insted of 8 bit masks for each RGB component in the
lower bits, the register only has the per channel src key enable
bits. On g4x+ sprites those bits are 24,25,26 whereas on the
old plane C and VLV/CHV sprites they are bits 0,1,2. Since
userspace assumes the g4x+ layout let's just shift enable
bits down and ignore the full masks.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6164c2ca20c3..f6be9b1b9c3a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -548,7 +548,7 @@ vlv_update_plane(struct intel_plane *plane,
 	if (key->flags) {
 		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);
+		I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask >> 24);
 	}
 	I915_WRITE_FW(SPSTRIDE(pipe, plane_id), fb->pitches[0]);
 	I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 3/5] drm/i915: Add basic immutable zpos properties for all planes
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
  2018-05-29 18:28 ` [PATCH 2/5] drm/i915: Fix VLV/CHV sprite src colorkey mask Ville Syrjala
@ 2018-05-29 18:28 ` Ville Syrjala
  2018-05-29 18:28 ` [PATCH 4/5] drm/i915: Make primary/sprite zpos configurable on VLV/CHV Ville Syrjala
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2018-05-29 18:28 UTC (permalink / raw)
  To: intel-gfx

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

Add an immutable zpos property for all planes. The normal order
is (bottom to top): primary,sprite(s),cursor.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++--
 drivers/gpu/drm/i915/intel_sprite.c  |  5 ++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8d4c9e249c44..bc3de1b5bb89 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13447,7 +13447,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	unsigned int supported_rotations;
 	unsigned int num_formats;
 	const uint64_t *modifiers;
-	int ret;
+	int ret, zpos;
 
 	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
 	if (!primary) {
@@ -13591,6 +13591,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 						  DRM_COLOR_YCBCR_BT709,
 						  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
+	zpos = 0;
+	drm_plane_create_zpos_immutable_property(&primary->base, zpos);
+
 	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
 	return primary;
@@ -13608,7 +13611,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 {
 	struct intel_plane *cursor = NULL;
 	struct intel_plane_state *state = NULL;
-	int ret;
+	int ret, zpos;
 
 	cursor = kzalloc(sizeof(*cursor), GFP_KERNEL);
 	if (!cursor) {
@@ -13665,6 +13668,9 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 						   DRM_MODE_ROTATE_0 |
 						   DRM_MODE_ROTATE_180);
 
+	zpos = INTEL_INFO(dev_priv)->num_sprites[pipe] + 1;
+	drm_plane_create_zpos_immutable_property(&cursor->base, zpos);
+
 	if (INTEL_GEN(dev_priv) >= 9)
 		state->scaler_id = -1;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f6be9b1b9c3a..a6f4524adff4 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1421,7 +1421,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 	const uint64_t *modifiers;
 	unsigned int supported_rotations;
 	int num_plane_formats;
-	int ret;
+	int ret, zpos;
 
 	intel_plane = kzalloc(sizeof(*intel_plane), GFP_KERNEL);
 	if (!intel_plane) {
@@ -1552,6 +1552,9 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 					  DRM_COLOR_YCBCR_BT709,
 					  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
+	zpos = plane + 1;
+	drm_plane_create_zpos_immutable_property(&intel_plane->base, zpos);
+
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
 	return intel_plane;
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 4/5] drm/i915: Make primary/sprite zpos configurable on VLV/CHV
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
  2018-05-29 18:28 ` [PATCH 2/5] drm/i915: Fix VLV/CHV sprite src colorkey mask Ville Syrjala
  2018-05-29 18:28 ` [PATCH 3/5] drm/i915: Add basic immutable zpos properties for all planes Ville Syrjala
@ 2018-05-29 18:28 ` Ville Syrjala
  2018-05-29 18:28 ` [PATCH 5/5] drm/i915: Implement dst colorkeying for VLV/CHV sprites Ville Syrjala
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2018-05-29 18:28 UTC (permalink / raw)
  To: intel-gfx

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

VLV/CHV can change the z order between the primary and sprite planes
freely. Let's expose that capability by making the zpos property
mutable. The cursor plane is always on top, so the cursor zpos is
left as an immutable property.

The way the hardware is configured is a bit awkward. Both sprite planes
have a "bottom" and "zorder" bits, but the way they interact betweens
the sprites doesn't seem entirely logical. The spec doesn't even list
all 16 possible combinations (just 10 are listed), but I've gone through
them all the discovered the following relationship:

sprite 0	sprite 1	resulting plane order (bottom to top)
B Z		B Z
0 0		0 0		P 0 1	  d0(see note),d1,d01,d01p
0 0		0 1		P 0 1	*
0 0		1 0		1 P 0	* d0
0 0		1 1		1 P 0
0 1		0 0		P 1 0	*
0 1		0 1		P 0 1
0 1		1 0		1 0 P	* dp
0 1		1 1		1 0 P
1 0		0 0		0 P 1	* d1
1 0		0 1		0 1 P	* dp
1 0		1 0		P 0 1
1 0		1 1		P 0 1
1 1		0 0		0 P 1
1 1		0 1		0 1 P
1 1		1 0		P 0 1
1 1		1 1		P 0 1

[B=bottom bit Z=zorder bit, 0=sprite 0, 1=sprite 1, P=primary,
 d0=sprite 0 disabled, d1=sprite 1 disabled, dp=primary disabled,
 d01=both sprites, d01p=both sprites and primary disabled]

Disabling any one or two planes doesn't seem to interfere with the
resulting plane order in an unexpected way. The bottom and zorder
bits of the disabled planes are still effective though, so we must be
careful what we leave in those bits when the plane is disabled.

What we do for enabled sprite planes is set bottom=1 if the plane is
at the bottom, else we set zorder=1 if the plane is just above
the other sprite plane. This will end up picking the configurations
marked with * from the table when both sprite planes are enabled.

For the disabled plane cases we just leave bottom=0 and zorder=0
for any disabled sprite plane. Any enabled sprite plane will still
follow the rule laid out before. For the purposes of that rule,
we'll consider any disabled plane to be stacked on top of the
enabled planes. This will end up picking the configurations marked
d0 when sprite 0 is disabled, d1 when sprite 1 is disabled,
dp when the primary is disabled, and d01 when both sprites are
disabled (and d01p is actually the same thing with just primary
disabled as well).

The use of the 'P 0 1' configuration for the first d0 case doesn't
really make sense based on the above, but given the way the zorder
bits work, there's no nice way to pick the 'P 1 0' (which would be
the logical thing) when sprite 0 is disabled as that would require
setting the zorder bit in the sprite 0 control register, but as
stated we always set both bits to 0 for disabled planes. That way
we don't have to reconfigure already disabled planes to change the
zorder between the currently active planes, and more imporantly we
don't need to consult any plane/crtc state in the .disable_plane()
hook. This slight irregularity doesn't matter in the end as both
of these configuratios will give us the expected visible 'P 1'
result.

v2: Redo the final zpos computation to not require every plane
    on the crtc to be part of the state
v3: More polish

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h           |   2 +
 drivers/gpu/drm/i915/intel_atomic_plane.c |   8 ++
 drivers/gpu/drm/i915/intel_display.c      | 146 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.h      |   9 ++
 drivers/gpu/drm/i915/intel_drv.h          |   7 ++
 drivers/gpu/drm/i915/intel_sprite.c       |  24 ++++-
 6 files changed, 192 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f238b7b33cd9..44eac5ffc2ce 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6265,6 +6265,8 @@ enum {
 #define   SP_ROTATE_180			(1<<15)
 #define   SP_TILED			(1<<10)
 #define   SP_MIRROR			(1<<8) /* CHV pipe B */
+#define   SP_BOTTOM			(1<<2)
+#define   SP_ZORDER			(1<<0)
 #define _SPALINOFF		(VLV_DISPLAY_BASE + 0x72184)
 #define _SPASTRIDE		(VLV_DISPLAY_BASE + 0x72188)
 #define _SPAPOS			(VLV_DISPLAY_BASE + 0x7218c)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 6d068786eb41..a5a3123589e7 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -193,6 +193,14 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 	else
 		crtc_state->nv12_planes &= ~BIT(intel_plane->id);
 
+	/*
+	 * Copy over the zpos to the crtc state so that we don't
+	 * need to add every plane on the crtc to the state to
+	 * recompute the final zpos.
+	 */
+	crtc_state->raw_zpos[intel_plane->id] =
+		intel_plane_raw_zpos(crtc_state, intel_state);
+
 	return intel_plane_atomic_calc_changes(old_crtc_state,
 					       &crtc_state->base,
 					       old_plane_state,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc3de1b5bb89..111cb0c1da54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -30,6 +30,7 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
+#include <linux/sort.h>
 #include <linux/vgaarb.h>
 #include <drm/drm_edid.h>
 #include <drm/drmP.h>
@@ -2760,6 +2761,8 @@ intel_set_plane_visible(struct intel_crtc_state *crtc_state,
 		crtc_state->active_planes &= ~BIT(plane->id);
 	}
 
+	crtc_state->raw_zpos[plane->id] = intel_plane_raw_zpos(crtc_state, plane_state);
+
 	DRM_DEBUG_KMS("%s active planes 0x%x\n",
 		      crtc_state->base.crtc->name,
 		      crtc_state->active_planes);
@@ -3208,8 +3211,8 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
 static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv =
-		to_i915(plane_state->base.plane->dev);
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
@@ -10932,6 +10935,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	struct intel_dpll_hw_state dpll_hw_state;
 	struct intel_shared_dpll *shared_dpll;
 	struct intel_crtc_wm_state wm_state;
+	u8 raw_zpos[I915_MAX_PLANES];
 	bool force_thru, ips_force_disable;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
@@ -10947,6 +10951,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	if (IS_G4X(dev_priv) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		wm_state = crtc_state->wm;
+	memcpy(raw_zpos, crtc_state->raw_zpos, sizeof(raw_zpos));
 
 	/* Keep base drm_crtc_state intact, only clear our extended struct */
 	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
@@ -10961,6 +10966,7 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	if (IS_G4X(dev_priv) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		crtc_state->wm = wm_state;
+	memcpy(crtc_state->raw_zpos, raw_zpos, sizeof(raw_zpos));
 }
 
 static int
@@ -12161,6 +12167,130 @@ static int calc_watermark_data(struct drm_atomic_state *state)
 	return 0;
 }
 
+int intel_plane_raw_zpos(const struct intel_crtc_state *crtc_state,
+			 const struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+
+	/*
+	 * We'll keep the cursor always at the top regardless of
+	 * whether it's enabled or not. This avoids changes to the
+	 * sorted zpos of other planes when turning the cursor on/off,
+	 * and thus we can easily avoid having to add the other
+	 * planes to the atomic state.
+	 */
+	if (plane->id == PLANE_CURSOR)
+		return 0x20;
+
+	/*
+	 * For other planes we stack the disabled planes on top
+	 * of any enabled planes. This simplifies our life by
+	 * allowing us to keep the SP_BOTTOM and SP_ZORDER bits
+	 * cleared for any disabled planes on VLV/CHV. Thus we
+	 * don't have to worry about zpos in .disable_plane().
+	 */
+	if (plane_state->base.visible)
+		return plane_state->base.zpos;
+	else
+		return 0x10;
+}
+
+struct intel_plane_zpos {
+	u32 obj_id;
+	enum plane_id plane_id;
+	u8 zpos;
+};
+
+static int intel_zpos_cmp(const void *a, const void *b)
+{
+	const struct intel_plane_zpos *za = (const struct intel_plane_zpos *)a;
+	const struct intel_plane_zpos *zb = (const struct intel_plane_zpos *)b;
+
+	if (za->zpos != zb->zpos)
+		return za->zpos - zb->zpos;
+	else
+		return za->obj_id - zb->obj_id;
+}
+
+static void vlv_normalize_zpos(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_plane_zpos zpos[I915_MAX_PLANES] = {};
+	struct intel_plane *plane;
+	int i, num_planes;
+
+	i = 0;
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+		zpos[i].obj_id = plane->base.base.id;
+		zpos[i].plane_id = plane->id;
+		zpos[i].zpos = crtc_state->raw_zpos[plane->id];
+		i++;
+	}
+
+	num_planes = i;
+
+	/* sort according to zpos/plane obj id */
+	sort(zpos, num_planes, sizeof(zpos[0]),
+	     intel_zpos_cmp, NULL);
+
+	/* copy over sorted zpos */
+	for (i = 0; i < num_planes; i++) {
+		enum plane_id plane_id = zpos[i].plane_id;
+
+		crtc_state->zpos[plane_id] = i;
+	}
+}
+
+static int vlv_update_zpos(struct drm_i915_private *dev_priv,
+			   struct intel_atomic_state *state)
+{
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+	int i;
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		struct drm_plane *plane;
+		bool sprites_changed;
+
+		vlv_normalize_zpos(new_crtc_state);
+
+		sprites_changed =
+			memcmp(new_crtc_state->zpos,
+			       old_crtc_state->zpos,
+			       sizeof(new_crtc_state->zpos)) != 0;
+
+		if (!sprites_changed)
+			continue;
+
+		drm_for_each_plane_mask(plane, &dev_priv->drm,
+					new_crtc_state->base.plane_mask) {
+			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)
+				continue;
+
+			/*
+			 * zpos is configured via sprite registers, so must
+			 * add them to the state if anything significant
+			 * changed.
+			 */
+			plane_state = drm_atomic_get_plane_state(&state->base, plane);
+			if (IS_ERR(plane_state))
+				return PTR_ERR(plane_state);
+		}
+
+		DRM_DEBUG_KMS("pipe %c zpos sprite %c: %d, sprite %c: %d\n",
+			      pipe_name(crtc->pipe),
+			      sprite_name(crtc->pipe, 0), new_crtc_state->zpos[PLANE_SPRITE0],
+			      sprite_name(crtc->pipe, 1), new_crtc_state->zpos[PLANE_SPRITE1]);
+	}
+
+	return 0;
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -12236,7 +12366,14 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		return ret;
 
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		ret = vlv_update_zpos(dev_priv, intel_state);
+		if (ret)
+			return ret;
+	}
+
 	intel_fbc_choose_crtc(dev_priv, intel_state);
+
 	return calc_watermark_data(state);
 }
 
@@ -13592,7 +13729,10 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 						  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
 	zpos = 0;
-	drm_plane_create_zpos_immutable_property(&primary->base, zpos);
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		drm_plane_create_zpos_property(&primary->base, zpos, 0, 2);
+	else
+		drm_plane_create_zpos_immutable_property(&primary->base, zpos);
 
 	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 2ef31617614a..09bbd9a3efd6 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -333,6 +333,15 @@ struct intel_link_m_n {
 	     (__i)++) \
 		for_each_if(plane)
 
+#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if(crtc)
+
 void intel_link_compute_m_n(int bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fd6256632482..994b9473b817 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -890,6 +890,10 @@ struct intel_crtc_state {
 	u8 active_planes;
 	u8 nv12_planes;
 
+	/* VLV/CHV zpos for each plane */
+	u8 raw_zpos[I915_MAX_PLANES]; /* raw property value (or 0xff for disabled planes) */
+	u8 zpos[I915_MAX_PLANES]; /* final zpos for each plane */
+
 	/* HDMI scrambling status */
 	bool hdmi_scrambling;
 
@@ -1457,6 +1461,9 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 			    const char *context);
 
 /* intel_display.c */
+int intel_plane_raw_zpos(const struct intel_crtc_state *crtc_state,
+			 const struct intel_plane_state *plane_state);
+void intel__enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a6f4524adff4..228eba582d44 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -511,6 +511,22 @@ static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	return sprctl;
 }
 
+static u32 vlv_sprctl_zpos(struct intel_plane *plane,
+			   const struct intel_crtc_state *crtc_state)
+{
+	enum plane_id plane_id = plane->id;
+	enum plane_id other_plane_id = plane_id == PLANE_SPRITE0 ?
+		PLANE_SPRITE1 : PLANE_SPRITE0;
+
+	if (crtc_state->zpos[plane_id] == 0)
+		return SP_BOTTOM;
+	else if (crtc_state->zpos[plane_id] ==
+		 crtc_state->zpos[other_plane_id] + 1)
+		return SP_ZORDER;
+
+	return 0;
+}
+
 static void
 vlv_update_plane(struct intel_plane *plane,
 		 const struct intel_crtc_state *crtc_state,
@@ -538,6 +554,9 @@ vlv_update_plane(struct intel_plane *plane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	/* final zpos value is computed after vlv_sprite_ctl() */
+	sprctl |= vlv_sprctl_zpos(plane, crtc_state);
+
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	vlv_update_clrc(plane_state);
@@ -1553,7 +1572,10 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 					  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
 	zpos = plane + 1;
-	drm_plane_create_zpos_immutable_property(&intel_plane->base, zpos);
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		drm_plane_create_zpos_property(&intel_plane->base, zpos, 0, 2);
+	else
+		drm_plane_create_zpos_immutable_property(&intel_plane->base, zpos);
 
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 5/5] drm/i915: Implement dst colorkeying for VLV/CHV sprites
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-05-29 18:28 ` [PATCH 4/5] drm/i915: Make primary/sprite zpos configurable on VLV/CHV Ville Syrjala
@ 2018-05-29 18:28 ` Ville Syrjala
  2018-05-29 18:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2018-05-29 18:28 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@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@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-05-29 18:28 ` [PATCH 5/5] drm/i915: Implement dst colorkeying for VLV/CHV sprites Ville Syrjala
@ 2018-05-29 18:34 ` Patchwork
  2018-05-29 18:50 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-05-29 18:34 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
URL   : https://patchwork.freedesktop.org/series/43902/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ab49b9e98274 drm/i915: Fix sprite destination colorkeying on SKL+
b08400aa9188 drm/i915: Fix VLV/CHV sprite src colorkey mask
c99414ba31cf drm/i915: Add basic immutable zpos properties for all planes
0a5bc4c8cb26 drm/i915: Make primary/sprite zpos configurable on VLV/CHV
-:90: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#90: FILE: drivers/gpu/drm/i915/i915_reg.h:6268:
+#define   SP_BOTTOM			(1<<2)
                    			  ^

-:91: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#91: FILE: drivers/gpu/drm/i915/i915_reg.h:6269:
+#define   SP_ZORDER			(1<<0)
                    			  ^

-:336: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__state' - possible side-effects?
#336: FILE: drivers/gpu/drm/i915/intel_display.h:336:
+#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if(crtc)

-:336: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'crtc' - possible side-effects?
#336: FILE: drivers/gpu/drm/i915/intel_display.h:336:
+#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if(crtc)

-:336: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects?
#336: FILE: drivers/gpu/drm/i915/intel_display.h:336:
+#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if(crtc)

-:340: WARNING:LONG_LINE: line over 100 characters
#340: FILE: drivers/gpu/drm/i915/intel_display.h:340:
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \

-:341: WARNING:LONG_LINE: line over 100 characters
#341: FILE: drivers/gpu/drm/i915/intel_display.h:341:
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \

total: 0 errors, 2 warnings, 5 checks, 299 lines checked
c844de7ddf13 drm/i915: Implement dst colorkeying for VLV/CHV sprites
-:31: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#31: FILE: drivers/gpu/drm/i915/i915_reg.h:6003:
+#define   DISPPLANE_SRC_KEY_WINDOW_ENABLE	(1<<23) /* planes A/B */
                                          	  ^

total: 0 errors, 0 warnings, 1 checks, 259 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-05-29 18:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Patchwork
@ 2018-05-29 18:50 ` Patchwork
  2018-05-29 20:01 ` ✓ Fi.CI.IGT: " Patchwork
  2018-06-05  8:10 ` [PATCH 1/5] " Lisovskiy, Stanislav
  7 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-05-29 18:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
URL   : https://patchwork.freedesktop.org/series/43902/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4254 -> Patchwork_9141 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/43902/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9141 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-kbl-7567u:       PASS -> DMESG-WARN (fdo#105602)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-skl-6770hq:      PASS -> FAIL (fdo#103928, fdo#100368)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)
      fi-glk-j4005:       PASS -> FAIL (fdo#104724, fdo#103167)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload-inject:
      fi-glk-j4005:       DMESG-WARN (fdo#106248) -> PASS

    igt@gem_exec_fence@await-hang-default:
      fi-glk-j4005:       DMESG-WARN (fdo#105719) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cnl-psr:         DMESG-WARN (fdo#104951) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248


== Participating hosts (45 -> 39) ==

  Missing    (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-cfl-u2 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4254 -> Patchwork_9141

  CI_DRM_4254: 59d0ee3539be08fb2551cc59719e79305e3115aa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4499: f560ae5a464331f03f0a669ed46b8c9e56526187 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9141: c844de7ddf13da9b20987dde73372281747864d0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c844de7ddf13 drm/i915: Implement dst colorkeying for VLV/CHV sprites
0a5bc4c8cb26 drm/i915: Make primary/sprite zpos configurable on VLV/CHV
c99414ba31cf drm/i915: Add basic immutable zpos properties for all planes
b08400aa9188 drm/i915: Fix VLV/CHV sprite src colorkey mask
ab49b9e98274 drm/i915: Fix sprite destination colorkeying on SKL+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9141/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
                   ` (5 preceding siblings ...)
  2018-05-29 18:50 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-29 20:01 ` Patchwork
  2018-06-05  8:10 ` [PATCH 1/5] " Lisovskiy, Stanislav
  7 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-05-29 20:01 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+
URL   : https://patchwork.freedesktop.org/series/43902/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4254_full -> Patchwork_9141_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9141_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9141_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/43902/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9141_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          SKIP -> PASS

    igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_9141_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-hsw:          PASS -> FAIL (fdo#105189)

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      shard-apl:          DMESG-FAIL (fdo#106560) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#106509, fdo#105454) -> PASS

    igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
      shard-glk:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@plain-flip-fb-recreate:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS +2

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-pgflip-blt:
      shard-glk:          FAIL (fdo#103167, fdo#104724) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4254 -> Patchwork_9141

  CI_DRM_4254: 59d0ee3539be08fb2551cc59719e79305e3115aa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4499: f560ae5a464331f03f0a669ed46b8c9e56526187 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9141: c844de7ddf13da9b20987dde73372281747864d0 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9141/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+
  2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
                   ` (6 preceding siblings ...)
  2018-05-29 20:01 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-06-05  8:10 ` Lisovskiy, Stanislav
  2018-06-08 18:29   ` Ville Syrjälä
  7 siblings, 1 reply; 10+ messages in thread
From: Lisovskiy, Stanislav @ 2018-06-05  8:10 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Tue, 2018-05-29 at 21:28 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On SKL+ the dst colorkey must be configured on the lower
> plane that contains the colorkey. This is in contrast to
> most earlier platforms where the dst colorkey is configured
> on the plane above.
> 
> The hardware will peform dst keying only between two immediately
> adjacent (in zorder) planes. Plane 1 will be keyed against plane 0,
> plane 2 againts plane 1, and so on. There is no way to key arbitrary
> planes against plane 0. Thus offering dst color keying on plane 2+
> is pointless. In fact it can be harmful since enabling dst keying on
> more than one plane on the same pipe leads to only the top-most of
> the planes performing the keying. For any plane lower in zorder the
> dst key enable is simply ignored.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 63
> +++++++++++++++++++++++++++++++++++--
>  1 file changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index ee23613f9fd4..6164c2ca20c3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
>  	return 0;
>  }
>  
> +static bool has_dst_key_in_primary_plane(struct drm_i915_private
> *dev_priv)
> +{
> +	return INTEL_GEN(dev_priv) >= 9;
> +}
> +
> +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_intel_sprite_colorkey *key = &plane_state->ckey;
> +
> +	*key = *set;
> +
> +	/*
> +	 * We want src key enabled on the
> +	 * sprite and not on the primary.
> +	 */
> +	if (plane->id == PLANE_PRIMARY &&
> +	    set->flags & I915_SET_COLORKEY_SOURCE)
> +		key->flags = 0;
> +
> +	/*
> +	 * On SKL+ we want dst key enabled on
> +	 * the primary and not on the sprite.
> +	 */
> +	if (plane->id != PLANE_PRIMARY &&
> +	    set->flags & I915_SET_COLORKEY_DESTINATION)
> +		key->flags = 0;
> +}
> +
>  int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void
> *data,
>  				    struct drm_file *file_priv)
>  {
> @@ -1100,6 +1130,16 @@ int intel_sprite_set_colorkey_ioctl(struct
> drm_device *dev, void *data,
>  	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
>  		return -ENOENT;
>  
> +	/*
> +	 * SKL+ only plane 1 can do destination keying against plane
> 0.
> +	 * Also multiple planes can't do destination keying on the
> same
> +	 * pipe simultaneously.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 9 &&
> +	    to_intel_plane(plane)->id >= PLANE_SPRITE1 &&
> +	    set->flags & I915_SET_COLORKEY_DESTINATION)
> +		return -EINVAL;
> +
>  	drm_modeset_acquire_init(&ctx, 0);
>  
>  	state = drm_atomic_state_alloc(plane->dev);
> @@ -1112,11 +1152,28 @@ int intel_sprite_set_colorkey_ioctl(struct
> drm_device *dev, void *data,
>  	while (1) {
>  		plane_state = drm_atomic_get_plane_state(state,
> plane);
>  		ret = PTR_ERR_OR_ZERO(plane_state);
> -		if (!ret) {
> -			to_intel_plane_state(plane_state)->ckey =
> *set;
> -			ret = drm_atomic_commit(state);
> +		if (!ret)
> +			intel_plane_set_ckey(to_intel_plane_state(pl
> ane_state), set);
> +
> +		/*
> +		 * On some platforms we have to configure
> +		 * the dst colorkey on the primary plane.
> +		 */
> +		if (!ret && has_dst_key_in_primary_plane(dev_priv))
> {
> +			struct intel_crtc *crtc =
> +				intel_get_crtc_for_pipe(dev_priv,
> +							to_intel_pla
> ne(plane)->pipe);
> +
> +			plane_state =
> drm_atomic_get_plane_state(state,
> +								 crt
> c->base.primary);
> +			ret = PTR_ERR_OR_ZERO(plane_state);
> +			if (!ret)
> +				intel_plane_set_ckey(to_intel_plane_
> state(plane_state), set);
>  		}
>  
> +		if (!ret)
> +			ret = drm_atomic_commit(state);
> +
>  		if (ret != -EDEADLK)
>  			break;
>  
-- 
Best Regards,

Lisovskiy Stanislav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+
  2018-06-05  8:10 ` [PATCH 1/5] " Lisovskiy, Stanislav
@ 2018-06-08 18:29   ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2018-06-08 18:29 UTC (permalink / raw)
  To: Lisovskiy, Stanislav; +Cc: intel-gfx

On Tue, Jun 05, 2018 at 08:10:36AM +0000, Lisovskiy, Stanislav wrote:
> On Tue, 2018-05-29 at 21:28 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On SKL+ the dst colorkey must be configured on the lower
> > plane that contains the colorkey. This is in contrast to
> > most earlier platforms where the dst colorkey is configured
> > on the plane above.
> > 
> > The hardware will peform dst keying only between two immediately
> > adjacent (in zorder) planes. Plane 1 will be keyed against plane 0,
> > plane 2 againts plane 1, and so on. There is no way to key arbitrary
> > planes against plane 0. Thus offering dst color keying on plane 2+
> > is pointless. In fact it can be harmful since enabling dst keying on
> > more than one plane on the same pipe leads to only the top-most of
> > the planes performing the keying. For any plane lower in zorder the
> > dst key enable is simply ignored.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 63
> > +++++++++++++++++++++++++++++++++++--
> >  1 file changed, 60 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index ee23613f9fd4..6164c2ca20c3 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane
> > *plane,
> >  	return 0;
> >  }
> >  
> > +static bool has_dst_key_in_primary_plane(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	return INTEL_GEN(dev_priv) >= 9;
> > +}
> > +
> > +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_intel_sprite_colorkey *key = &plane_state->ckey;
> > +
> > +	*key = *set;
> > +
> > +	/*
> > +	 * We want src key enabled on the
> > +	 * sprite and not on the primary.
> > +	 */
> > +	if (plane->id == PLANE_PRIMARY &&
> > +	    set->flags & I915_SET_COLORKEY_SOURCE)
> > +		key->flags = 0;
> > +
> > +	/*
> > +	 * On SKL+ we want dst key enabled on
> > +	 * the primary and not on the sprite.
> > +	 */
> > +	if (plane->id != PLANE_PRIMARY &&

The 'INTEL_GEN(dev_priv) >= 9' check that was supposed to be here ended
up in the wrong patch. I moved it over while applying this to avoid
breaking the pre-SKL sprites. Only caught it when glancing at the
resulting code before pushing. Might be time to write some real
colorkeying igts...

Patch pushed to dinq. Thanks for the review.

> > +	    set->flags & I915_SET_COLORKEY_DESTINATION)
> > +		key->flags = 0;
> > +}
> > +
-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-06-08 18:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-29 18:28 [PATCH 1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Ville Syrjala
2018-05-29 18:28 ` [PATCH 2/5] drm/i915: Fix VLV/CHV sprite src colorkey mask Ville Syrjala
2018-05-29 18:28 ` [PATCH 3/5] drm/i915: Add basic immutable zpos properties for all planes Ville Syrjala
2018-05-29 18:28 ` [PATCH 4/5] drm/i915: Make primary/sprite zpos configurable on VLV/CHV Ville Syrjala
2018-05-29 18:28 ` [PATCH 5/5] drm/i915: Implement dst colorkeying for VLV/CHV sprites Ville Syrjala
2018-05-29 18:34 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915: Fix sprite destination colorkeying on SKL+ Patchwork
2018-05-29 18:50 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-29 20:01 ` ✓ Fi.CI.IGT: " Patchwork
2018-06-05  8:10 ` [PATCH 1/5] " Lisovskiy, Stanislav
2018-06-08 18:29   ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.