[PATCH v4] drm/i915: add support for Z-order of planes for VLV.

[PATCH v4] drm/i915: add support for Z-order of planes for VLV.

[yu.dai]

From: "Yu(Alex) Dai" <yu.dai@intel.com>

Add "zorder" property to crtc to control Z-order of sprite and
primary planes. The alpha channel of the planes can be enabled
or disabled during Z-order change.

Signed-off-by: Yu(Alex) Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/i915_reg.h      | 10 ++++
 drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  6 +++
 drivers/gpu/drm/i915/intel_sprite.c  | 95 ++++++++++++++++++++++++++++++++++--
 5 files changed, 163 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05cfcc1..783e0ce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2582,6 +2582,7 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data,
 			       struct drm_file *file);
+extern int i915_set_plane_zorder(struct drm_device *dev, uint64_t zorder);
 
 /* overlay */
 extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f564ce..c9a9993 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3743,6 +3743,16 @@
 #define   SPRITE_INT_GAMMA_ENABLE	(1<<13)
 #define   SPRITE_TILED			(1<<10)
 #define   SPRITE_DEST_KEY		(1<<2)
+#define   SPRITE_FORCE_BOTTOM		(1<<2)
+#define   SPRITE_ZORDER_ENABLE		(1<<0)
+
+#define P1S1S2C1		0
+#define P1S2S1C1		8
+#define S2P1S1C1		1
+#define S2S1P1C1		9
+#define S1P1S2C1		4
+#define S1S2P1C1		6
+
 #define _SPRA_LINOFF		0x70284
 #define _SPRA_STRIDE		0x70288
 #define _SPRA_POS		0x7028c
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6f15627..d4a1f83 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2082,18 +2082,27 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		break;
 	case DRM_FORMAT_XRGB1555:
 	case DRM_FORMAT_ARGB1555:
-		dspcntr |= DISPPLANE_BGRX555;
+		if (intel_crtc->primary_alpha)
+			dspcntr |= DISPPLANE_BGRA555;
+		else
+			dspcntr |= DISPPLANE_BGRX555;
 		break;
 	case DRM_FORMAT_RGB565:
 		dspcntr |= DISPPLANE_BGRX565;
 		break;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
-		dspcntr |= DISPPLANE_BGRX888;
+		if (intel_crtc->primary_alpha)
+			dspcntr |= DISPPLANE_BGRA888;
+		else
+			dspcntr |= DISPPLANE_BGRX888;
 		break;
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_ABGR8888:
-		dspcntr |= DISPPLANE_RGBX888;
+		if (intel_crtc->primary_alpha)
+			dspcntr |= DISPPLANE_RGBA888;
+		else
+			dspcntr |= DISPPLANE_RGBX888;
 		break;
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_ARGB2101010:
@@ -2101,7 +2110,10 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		break;
 	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_ABGR2101010:
-		dspcntr |= DISPPLANE_RGBX101010;
+		if (intel_crtc->primary_alpha)
+			dspcntr |= DISPPLANE_RGBA101010;
+		else
+			dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
 		BUG();
@@ -8258,6 +8270,9 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 
 	intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
 
+	if (intel_crtc->zorder_property)
+		drm_property_destroy(dev, intel_crtc->zorder_property);
+
 	drm_crtc_cleanup(crtc);
 
 	kfree(intel_crtc);
@@ -10160,6 +10175,27 @@ out_config:
 	return ret;
 }
 
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+				   struct drm_property *prop,
+				   uint64_t val)
+{
+	struct intel_crtc *icrtc = to_intel_crtc(crtc);
+	int ret = -ENOENT;
+
+	if (prop && prop == icrtc->zorder_property) {
+		/* Simply return if no change in zorder */
+		if (icrtc->zorder == val)
+			return 0;
+
+		ret = i915_set_plane_zorder(crtc->dev, val);
+
+		if (!ret)
+			icrtc->zorder = val;
+	}
+
+	return ret;
+}
+
 static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.cursor_set = intel_crtc_cursor_set,
 	.cursor_move = intel_crtc_cursor_move,
@@ -10167,6 +10203,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property,
 };
 
 static void intel_cpu_pll_init(struct drm_device *dev)
@@ -10274,6 +10311,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc;
+	struct drm_property *prop = 0;
 	int i;
 
 	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
@@ -10306,6 +10344,18 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
+
+	intel_crtc->primary_alpha = false;
+	intel_crtc->sprite0_alpha = true;
+	intel_crtc->sprite1_alpha = true;
+
+	if (IS_VALLEYVIEW(dev))
+		prop = drm_property_create(dev, 0, "zorder", 1);
+
+	if (prop)
+		drm_object_attach_property(&intel_crtc->base.base, prop, 0);
+
+	intel_crtc->zorder_property = prop;
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4ffc02..ba6685a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -371,6 +371,9 @@ struct intel_crtc {
 	bool new_enabled;
 
 	uint32_t ddi_pll_sel;
+	bool primary_alpha;
+	bool sprite0_alpha;
+	bool sprite1_alpha;
 
 	/* reset counter value when the last flip was submitted */
 	unsigned int reset_counter;
@@ -384,6 +387,9 @@ struct intel_crtc {
 		/* watermarks currently being used  */
 		struct intel_pipe_wm active;
 	} wm;
+
+	struct drm_property *zorder_property;
+	uint64_t zorder;
 };
 
 struct intel_plane_wm_parameters {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..9e80f38 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -37,6 +37,77 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
+static int vlv_set_plane_zorder(struct drm_device *dev, uint64_t order)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+	int s1_zorder, s1_bottom, s2_zorder, s2_bottom;
+	int pipe = (order >> 31) & 0x1;
+	int z_order = order & 0x000F;
+	struct intel_crtc *intel_crtc =
+			to_intel_crtc(dev_priv->plane_to_crtc_mapping[pipe]);
+
+	/* Bit 31 for pipe and 0~3 for order */
+	if (order & ~0x8000000F)
+		return -EINVAL;
+
+	s1_zorder = (order >> 3) & 0x1;
+	s1_bottom = (order >> 2) & 0x1;
+	s2_zorder = (order >> 1) & 0x1;
+	s2_bottom = (order >> 0) & 0x1;
+
+	/* Clear the older Z-order */
+	val = I915_READ(SPCNTR(pipe, 0));
+	val &= ~(SPRITE_FORCE_BOTTOM | SPRITE_ZORDER_ENABLE);
+	I915_WRITE(SPCNTR(pipe, 0), val);
+
+	val = I915_READ(SPCNTR(pipe, 1));
+	val &= ~(SPRITE_FORCE_BOTTOM | SPRITE_ZORDER_ENABLE);
+	I915_WRITE(SPCNTR(pipe, 1), val);
+
+	/* Program new Z-order */
+	val = I915_READ(SPCNTR(pipe, 0));
+	if (s1_zorder)
+		val |= SPRITE_ZORDER_ENABLE;
+	if (s1_bottom)
+		val |= SPRITE_FORCE_BOTTOM;
+	I915_WRITE(SPCNTR(pipe, 0), val);
+
+	val = I915_READ(SPCNTR(pipe, 1));
+	if (s2_zorder)
+		val |= SPRITE_ZORDER_ENABLE;
+	if (s2_bottom)
+		val |= SPRITE_FORCE_BOTTOM;
+	I915_WRITE(SPCNTR(pipe, 1), val);
+
+	if (z_order != P1S1S2C1 && z_order != P1S2S1C1)
+		intel_crtc->primary_alpha = true;
+	else
+		intel_crtc->primary_alpha = false;
+
+	if (z_order != S1P1S2C1 && z_order != S1S2P1C1)
+		intel_crtc->sprite0_alpha = true;
+	else
+		intel_crtc->sprite0_alpha = false;
+
+	if (z_order != S2P1S1C1 && z_order != S2S1P1C1)
+		intel_crtc->sprite1_alpha = true;
+	else
+		intel_crtc->sprite1_alpha = false;
+
+	return 0;
+}
+
+int i915_set_plane_zorder(struct drm_device *dev, uint64_t val)
+{
+	int ret = -ENOENT;
+
+	if (IS_VALLEYVIEW(dev))
+		ret = vlv_set_plane_zorder(dev, val);
+
+	return ret;
+}
+
 static void
 vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 		 struct drm_framebuffer *fb,
@@ -50,10 +121,19 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	int pipe = intel_plane->pipe;
 	int plane = intel_plane->plane;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 sprctl;
+	bool alpha = true;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
+	if (plane && intel_crtc->sprite1_alpha)
+		alpha = true;
+	else if (!plane && intel_crtc->sprite0_alpha)
+		alpha = true;
+	else
+		alpha = false;
+
 	sprctl = I915_READ(SPCNTR(pipe, plane));
 
 	/* Mask out pixel format bits in case we change it */
@@ -81,19 +161,28 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 		sprctl |= SP_FORMAT_BGRX8888;
 		break;
 	case DRM_FORMAT_ARGB8888:
-		sprctl |= SP_FORMAT_BGRA8888;
+		if (alpha)
+			sprctl |= SP_FORMAT_BGRA8888;
+		else
+			sprctl |= SP_FORMAT_BGRX8888;
 		break;
 	case DRM_FORMAT_XBGR2101010:
 		sprctl |= SP_FORMAT_RGBX1010102;
 		break;
 	case DRM_FORMAT_ABGR2101010:
-		sprctl |= SP_FORMAT_RGBA1010102;
+		if (alpha)
+			sprctl |= SP_FORMAT_RGBA1010102;
+		else
+			sprctl |= SP_FORMAT_RGBX1010102;
 		break;
 	case DRM_FORMAT_XBGR8888:
 		sprctl |= SP_FORMAT_RGBX8888;
 		break;
 	case DRM_FORMAT_ABGR8888:
-		sprctl |= SP_FORMAT_RGBA8888;
+		if (alpha)
+			sprctl |= SP_FORMAT_RGBA8888;
+		else
+			sprctl |= SP_FORMAT_RGBX8888;
 		break;
 	default:
 		/*
-- 
1.8.5.2

Re: [PATCH v4] drm/i915: add support for Z-order of planes for VLV.

[Chris Wilson]

On Mon, Mar 03, 2014 at 02:12:28PM -0800, yu.dai@intel.com wrote:
> From: "Yu(Alex) Dai" <yu.dai@intel.com>
> 
> Add "zorder" property to crtc to control Z-order of sprite and
> primary planes. The alpha channel of the planes can be enabled
> or disabled during Z-order change.

Can I just say that is an abonimal user interface. You expect the client
to encode a hw specific value into a CRTC property that affects global
state. and given the two properties on the two CRTCs, which should userspace
believe?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v4] drm/i915: add support for Z-order of planes for VLV.

[Yu Dai]

[-- Attachment #1.1: Type: text/plain, Size: 1346 bytes --]

Chris,

This looks like a hw specific value which is difficult to understand. 
However, the definition of these values are just a list of available 
options of z-order. On Intel VLV, there is only 6 options for the three 
planes Primary, Sprite A and Sprite B. Cursor is always on top. For 
example, "P1S1S2C1" means z-order (from bottom to top) of Primary -> 
Sprite A -> Sprite B -> Cursor.

#define P1S1S2C1        0
#define P1S2S1C1        8
#define S2P1S1C1        1
#define S2S1P1C1        9
#define S1P1S2C1        4
#define S1S2P1C1        6

In theory, if hw supports, each CRTC may have their own plane z-order if 
user intentionally wants that happens. The pipe info (0 or 1) is packed 
into z-order value with extra bit (bit 31).

Thanks,
Alex

On 14-03-04 12:20 AM, Chris Wilson wrote:
> On Mon, Mar 03, 2014 at  02:12:28PM -0800, yu.dai@intel.com wrote:
 >> From: "Yu(Alex) Dai" <yu.dai@intel.com>
 >>
 >> Add "zorder" property to crtc to control Z-order of sprite and
 >> primary planes. The alpha channel of the planes can be enabled or
 >> disabled during Z-order change.
 >
 > Can I just say that is an abonimal user interface. You expect the
 > client to encode a hw specific value into a CRTC property that
 > affects global state. and given the two properties on the two CRTCs,
 > which should userspace believe? -Chris
 >



[-- Attachment #1.2: Type: text/html, Size: 2347 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v4] drm/i915: add support for Z-order of planes for VLV.

[Matt Roper]

On Tue, Mar 04, 2014 at 05:37:39PM -0800, Yu Dai wrote:
> Chris,
> 
> This looks like a hw specific value which is difficult to
> understand. However, the definition of these values are just a list
> of available options of z-order. On Intel VLV, there is only 6
> options for the three planes Primary, Sprite A and Sprite B. Cursor
> is always on top. For example, "P1S1S2C1" means z-order (from bottom
> to top) of Primary -> Sprite A -> Sprite B -> Cursor.
> 
> #define P1S1S2C1        0
> #define P1S2S1C1        8
> #define S2P1S1C1        1
> #define S2S1P1C1        9
> #define S1P1S2C1        4
> #define S1S2P1C1        6
> 
> In theory, if hw supports, each CRTC may have their own plane
> z-order if user intentionally wants that happens. The pipe info (0
> or 1) is packed into z-order value with extra bit (bit 31).
> 
> Thanks,
> Alex

If you want this to go into the upstream kernel, then it winds up being
an interface that pretty much has to be supported forever.  A
userspace-facing encoding like this is very closely tied to the specific
number of pipes and planes, so it isn't going to work in a consistent
manner across different Intel hardware platforms (current and future).
Ideally we'd like to have an interface that we can share with drivers
for other vendors' hardware as well so that general purpose userspace
(e.g., a Weston wayland compositor) can make use of this without having
to worry too much about what kind of hardware it's running on.  As Ville
mentioned earlier, if you want userspace to program a specific encoding
like this, you need to advertise a list of enum values that are valid
for that hardware, and the enum list has to be somehow
parseable/understandable by userspace.

Personally I still prefer the approach of giving every plane its own
z-order rank property and then letting the platform-specific code that
actually writes the registers sort all the planes' zorders and figure
out what the appropriate register values should be.  Granted, this kind
of assumes that we've fully moved to the atomic flip API, but then the
"check/calculate" step of the process would catch any illegal plane
orderings (e.g., two planes listed with the same z-order rank, or an
overall ordering that the hardware simply can't support) and compute the
appropriate z-order bits for the CRTC.  I have a patchset under progress
on dri-devel that unifies the plane interface (exposing primary and
cursor planes in the same manner sprite/overlay planes are today), so
primary and cursor would also have a zorder rank value and could be
sorted along with the sprites.

The main obstacle I see with the per-plane rank approach is that the
display server or compositor still needs a way to figure out which plane
orderings are actually valid.  It's possible to quickly submit a few
possible orderings that the compositor might be interested in using to
the atomic flip API at compositor startup with the "test only" flag set
to figure out which ones are possible.  However this does add a small
amount of startup overhead which may be undesirable in some settings.


Matt

> On 14-03-04 12:20 AM, Chris Wilson wrote:
> >On Mon, Mar 03, 2014 at  02:12:28PM -0800, yu.dai@intel.com wrote:
> >> From: "Yu(Alex) Dai" <yu.dai@intel.com>
> >>
> >> Add "zorder" property to crtc to control Z-order of sprite and
> >> primary planes. The alpha channel of the planes can be enabled or
> >> disabled during Z-order change.
> >
> > Can I just say that is an abonimal user interface. You expect the
> > client to encode a hw specific value into a CRTC property that
> > affects global state. and given the two properties on the two CRTCs,
> > which should userspace believe? -Chris
> >
> 
> 

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


-- 
Matt Roper
Graphics Software Engineer
ISG Platform Enabling & Development
Intel Corporation
(916) 356-2795