Plane support for SNB and IVB

Plane support for SNB and IVB

[Jesse Barnes]

I think the core support is ready now, but I'd like feedback on the
color key ioctl.  It would be good to have a way to disable it at
runtime (w/o color keying enabled, the sprite will always sit on top of
the primary), and potentially add alpha support at some point.

Would 0 as the color key work well enough as a "disable" flag?  Or
should I add a separate field to the ioctl?  Anyone have a preference?

Thanks,
Jesse

[PATCH 1/3] drm/i915: add SNB and IVB video sprite support v5

[Jesse Barnes]

The video sprites support various video surface formats natively and can
handle scaling as well.  So add support for them using the new DRM core
sprite support functions.

v2: use drm specific fourcc header and defines
v3: address Daniel's comments:
  - don't take struct mutex around register access (only needed for
    regs in the GT power well)
  - don't hold struct mutex across vblank waits
  - fix up update_plane API (pass obj instead of GTT offset)
  - add interlaced defines for sprite regs
  - drop unnecessary 'reg' variables
  - comment double buffered reg flushing
  Also fix w/h confusion when writing the scaling reg.
v4: more fixes, address more comments from Daniel, and include Hai's fix
  - prevent divide by zero in scaling calculation (Hai Lan)
  - update to Ville's new DRM_FORMAT_* types
  - fix sprite watermark handling (calc based on CRTC size, separate
    from normal display wm)
  - remove private refcounts now that the fb cleanups handles things
v5: add linear surface support

For this version, I tested DPMS since it came up in the last review;
DPMS off/on works ok when a video player is working under X, but for
power saving we'll probably want to do something smarter.  I'll leave
that for a separate patch on top.  Likewise with the refcounting/fb
layer handling, which are really separate cleanups.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/Makefile        |    1 +
 drivers/gpu/drm/i915/i915_drv.h      |    3 +
 drivers/gpu/drm/i915/i915_reg.h      |  133 ++++++++++
 drivers/gpu/drm/i915/intel_display.c |  174 +++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   28 ++
 drivers/gpu/drm/i915/intel_fb.c      |    6 +
 drivers/gpu/drm/i915/intel_sprite.c  |  452 ++++++++++++++++++++++++++++++++++
 7 files changed, 790 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sprite.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ae6a7c..808b255 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
 	  intel_dvo.o \
 	  intel_ringbuffer.o \
 	  intel_overlay.o \
+	  intel_sprite.o \
 	  intel_opregion.o \
 	  dvo_ch7xxx.o \
 	  dvo_ch7017.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4a9c1b9..0c3078c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -206,6 +206,8 @@ struct drm_i915_display_funcs {
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
 	void (*update_wm)(struct drm_device *dev);
+	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
+				 uint32_t sprite_width, int pixel_size);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     struct drm_display_mode *mode,
 			     struct drm_display_mode *adjusted_mode,
@@ -347,6 +349,7 @@ typedef struct drm_i915_private {
 
 	/* overlay */
 	struct intel_overlay *overlay;
+	bool sprite_scaling_enabled;
 
 	/* LVDS info */
 	int backlight_level;  /* restore backlight to this value */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b080cc8..a2ce411 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2459,6 +2459,8 @@
 #define WM3_LP_ILK		0x45110
 #define  WM3_LP_EN		(1<<31)
 #define WM1S_LP_ILK		0x45120
+#define WM2S_LP_IVB		0x45124
+#define WM3S_LP_IVB		0x45128
 #define  WM1S_LP_EN		(1<<31)
 
 /* Memory latency timer register */
@@ -2675,6 +2677,137 @@
 #define _DSPBSURF		0x7119C
 #define _DSPBTILEOFF		0x711A4
 
+/* Sprite A control */
+#define _DVSACNTR		0x72180
+#define   DVS_ENABLE		(1<<31)
+#define   DVS_GAMMA_ENABLE	(1<<30)
+#define   DVS_PIXFORMAT_MASK	(3<<25)
+#define   DVS_FORMAT_YUV422	(0<<25)
+#define   DVS_FORMAT_RGBX101010	(1<<25)
+#define   DVS_FORMAT_RGBX888	(2<<25)
+#define   DVS_FORMAT_RGBX161616	(3<<25)
+#define   DVS_SOURCE_KEY	(1<<22)
+#define   DVS_RGB_ORDER_RGBX	(1<<20)
+#define   DVS_YUV_BYTE_ORDER_MASK (3<<16)
+#define   DVS_YUV_ORDER_YUYV	(0<<16)
+#define   DVS_YUV_ORDER_UYVY	(1<<16)
+#define   DVS_YUV_ORDER_YVYU	(2<<16)
+#define   DVS_YUV_ORDER_VYUY	(3<<16)
+#define   DVS_DEST_KEY		(1<<2)
+#define   DVS_TRICKLE_FEED_DISABLE (1<<14)
+#define   DVS_TILED		(1<<10)
+#define _DVSALINOFF		0x72184
+#define _DVSASTRIDE		0x72188
+#define _DVSAPOS		0x7218c
+#define _DVSASIZE		0x72190
+#define _DVSAKEYVAL		0x72194
+#define _DVSAKEYMSK		0x72198
+#define _DVSASURF		0x7219c
+#define _DVSAKEYMAXVAL		0x721a0
+#define _DVSATILEOFF		0x721a4
+#define _DVSASURFLIVE		0x721ac
+#define _DVSASCALE		0x72204
+#define   DVS_SCALE_ENABLE	(1<<31)
+#define   DVS_FILTER_MASK	(3<<29)
+#define   DVS_FILTER_MEDIUM	(0<<29)
+#define   DVS_FILTER_ENHANCING	(1<<29)
+#define   DVS_FILTER_SOFTENING	(2<<29)
+#define   DVS_VERTICAL_OFFSET_HALF (1<<28) /* must be enabled below */
+#define   DVS_VERTICAL_OFFSET_ENABLE (1<<27)
+#define _DVSAGAMC		0x72300
+
+#define _DVSBCNTR		0x73180
+#define _DVSBLINOFF		0x73184
+#define _DVSBSTRIDE		0x73188
+#define _DVSBPOS		0x7318c
+#define _DVSBSIZE		0x73190
+#define _DVSBKEYVAL		0x73194
+#define _DVSBKEYMSK		0x73198
+#define _DVSBSURF		0x7319c
+#define _DVSBKEYMAXVAL		0x731a0
+#define _DVSBTILEOFF		0x731a4
+#define _DVSBSURFLIVE		0x731ac
+#define _DVSBSCALE		0x73204
+#define _DVSBGAMC		0x73300
+
+#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR)
+#define DVSLINOFF(pipe) _PIPE(pipe, _DVSALINOFF, _DVSBLINOFF)
+#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE)
+#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS)
+#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF)
+#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
+#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
+#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+
+#define _SPRA_CTL		0x70280
+#define   SPRITE_ENABLE			(1<<31)
+#define   SPRITE_GAMMA_ENABLE		(1<<30)
+#define   SPRITE_PIXFORMAT_MASK		(7<<25)
+#define   SPRITE_FORMAT_YUV422		(0<<25)
+#define   SPRITE_FORMAT_RGBX101010	(1<<25)
+#define   SPRITE_FORMAT_RGBX888		(2<<25)
+#define   SPRITE_FORMAT_RGBX161616	(3<<25)
+#define   SPRITE_FORMAT_YUV444		(4<<25)
+#define   SPRITE_FORMAT_XR_BGR101010	(5<<25) /* Extended range */
+#define   SPRITE_CSC_ENABLE		(1<<24)
+#define   SPRITE_SOURCE_KEY		(1<<22)
+#define   SPRITE_RGB_ORDER_RGBX		(1<<20) /* only for 888 and 161616 */
+#define   SPRITE_YUV_TO_RGB_CSC_DISABLE	(1<<19)
+#define   SPRITE_YUV_CSC_FORMAT_BT709	(1<<18) /* 0 is BT601 */
+#define   SPRITE_YUV_BYTE_ORDER_MASK	(3<<16)
+#define   SPRITE_YUV_ORDER_YUYV		(0<<16)
+#define   SPRITE_YUV_ORDER_UYVY		(1<<16)
+#define   SPRITE_YUV_ORDER_YVYU		(2<<16)
+#define   SPRITE_YUV_ORDER_VYUY		(3<<16)
+#define   SPRITE_TRICKLE_FEED_DISABLE	(1<<14)
+#define   SPRITE_INT_GAMMA_ENABLE	(1<<13)
+#define   SPRITE_TILED			(1<<10)
+#define   SPRITE_DEST_KEY		(1<<2)
+#define _SPRA_LINOFF		0x70284
+#define _SPRA_STRIDE		0x70288
+#define _SPRA_POS		0x7028c
+#define _SPRA_SIZE		0x70290
+#define _SPRA_KEYVAL		0x70294
+#define _SPRA_KEYMSK		0x70298
+#define _SPRA_SURF		0x7029c
+#define _SPRA_KEYMAX		0x702a0
+#define _SPRA_TILEOFF		0x702a4
+#define _SPRA_SCALE		0x70304
+#define   SPRITE_SCALE_ENABLE	(1<<31)
+#define   SPRITE_FILTER_MASK	(3<<29)
+#define   SPRITE_FILTER_MEDIUM	(0<<29)
+#define   SPRITE_FILTER_ENHANCING	(1<<29)
+#define   SPRITE_FILTER_SOFTENING	(2<<29)
+#define   SPRITE_VERTICAL_OFFSET_HALF	(1<<28) /* must be enabled below */
+#define   SPRITE_VERTICAL_OFFSET_ENABLE	(1<<27)
+#define _SPRA_GAMC		0x70400
+
+#define _SPRB_CTL		0x71280
+#define _SPRB_LINOFF		0x71284
+#define _SPRB_STRIDE		0x71288
+#define _SPRB_POS		0x7128c
+#define _SPRB_SIZE		0x71290
+#define _SPRB_KEYVAL		0x71294
+#define _SPRB_KEYMSK		0x71298
+#define _SPRB_SURF		0x7129c
+#define _SPRB_KEYMAX		0x712a0
+#define _SPRB_TILEOFF		0x712a4
+#define _SPRB_SCALE		0x71304
+#define _SPRB_GAMC		0x71400
+
+#define SPRCTL(pipe) _PIPE(pipe, _SPRA_CTL, _SPRB_CTL)
+#define SPRLINOFF(pipe) _PIPE(pipe, _SPRA_LINOFF, _SPRB_LINOFF)
+#define SPRSTRIDE(pipe) _PIPE(pipe, _SPRA_STRIDE, _SPRB_STRIDE)
+#define SPRPOS(pipe) _PIPE(pipe, _SPRA_POS, _SPRB_POS)
+#define SPRSIZE(pipe) _PIPE(pipe, _SPRA_SIZE, _SPRB_SIZE)
+#define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL)
+#define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK)
+#define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
+#define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
+#define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
+#define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
+#define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
+
 /* VBIOS regs */
 #define VGACNTRL		0x71400
 # define VGA_DISP_DISABLE			(1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7080eb5..34a7953 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -915,8 +915,8 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 	     pipe_name(pipe));
 }
 
-static void assert_pipe(struct drm_i915_private *dev_priv,
-			enum pipe pipe, bool state)
+void assert_pipe(struct drm_i915_private *dev_priv,
+		 enum pipe pipe, bool state)
 {
 	int reg;
 	u32 val;
@@ -929,8 +929,6 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
 	     "pipe %c assertion failure (expected %s, current %s)\n",
 	     pipe_name(pipe), state_string(state), state_string(cur_state));
 }
-#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
-#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
 
 static void assert_plane_enabled(struct drm_i915_private *dev_priv,
 				 enum plane plane)
@@ -4509,7 +4507,7 @@ static void ironlake_update_wm(struct drm_device *dev)
 	 */
 }
 
-static void sandybridge_update_wm(struct drm_device *dev)
+void sandybridge_update_wm(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
@@ -4569,7 +4567,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
 	I915_WRITE(WM2_LP_ILK, 0);
 	I915_WRITE(WM1_LP_ILK, 0);
 
-	if (!single_plane_enabled(enabled))
+	if (!single_plane_enabled(enabled) ||
+	    dev_priv->sprite_scaling_enabled)
 		return;
 	enabled = ffs(enabled) - 1;
 
@@ -4619,6 +4618,149 @@ static void sandybridge_update_wm(struct drm_device *dev)
 		   cursor_wm);
 }
 
+static bool
+sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
+			      uint32_t sprite_width, int pixel_size,
+			      const struct intel_watermark_params *display,
+			      int display_latency_ns, int *sprite_wm)
+{
+	struct drm_crtc *crtc;
+	int clock;
+	int entries, tlb_miss;
+
+	crtc = intel_get_crtc_for_plane(dev, plane);
+	if (crtc->fb == NULL || !crtc->enabled) {
+		*sprite_wm = display->guard_size;
+		return false;
+	}
+
+	clock = crtc->mode.clock;
+
+	/* Use the small buffer method to calculate the sprite watermark */
+	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
+	tlb_miss = display->fifo_size*display->cacheline_size -
+		sprite_width * 8;
+	if (tlb_miss > 0)
+		entries += tlb_miss;
+	entries = DIV_ROUND_UP(entries, display->cacheline_size);
+	*sprite_wm = entries + display->guard_size;
+	if (*sprite_wm > (int)display->max_wm)
+		*sprite_wm = display->max_wm;
+
+	return true;
+}
+
+static bool
+sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
+				uint32_t sprite_width, int pixel_size,
+				const struct intel_watermark_params *display,
+				int latency_ns, int *sprite_wm)
+{
+	struct drm_crtc *crtc;
+	unsigned long line_time_us;
+	int clock;
+	int line_count, line_size;
+	int small, large;
+	int entries;
+
+	if (!latency_ns) {
+		*sprite_wm = 0;
+		return false;
+	}
+
+	crtc = intel_get_crtc_for_plane(dev, plane);
+	clock = crtc->mode.clock;
+
+	line_time_us = (sprite_width * 1000) / clock;
+	line_count = (latency_ns / line_time_us + 1000) / 1000;
+	line_size = sprite_width * pixel_size;
+
+	/* Use the minimum of the small and large buffer method for primary */
+	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
+	large = line_count * line_size;
+
+	entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
+	*sprite_wm = entries + display->guard_size;
+
+	return *sprite_wm > 0x3ff ? false : true;
+}
+
+static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
+					 uint32_t sprite_width, int pixel_size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
+	int sprite_wm, reg;
+	int ret;
+
+	switch (pipe) {
+	case 0:
+		reg = WM0_PIPEA_ILK;
+		break;
+	case 1:
+		reg = WM0_PIPEB_ILK;
+		break;
+	case 2:
+		reg = WM0_PIPEC_IVB;
+		break;
+	default:
+		return; /* bad pipe */
+	}
+
+	ret = sandybridge_compute_sprite_wm(dev, pipe, sprite_width, pixel_size,
+					    &sandybridge_display_wm_info,
+					    latency, &sprite_wm);
+	if (!ret) {
+		DRM_DEBUG_KMS("failed to compute sprite wm for pipe %d\n",
+			      pipe);
+		return;
+	}
+
+	I915_WRITE(reg, I915_READ(reg) | (sprite_wm << WM0_PIPE_SPRITE_SHIFT));
+	DRM_DEBUG_KMS("sprite watermarks For pipe %d - %d\n", pipe, sprite_wm);
+
+
+	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
+					      pixel_size,
+					      &sandybridge_display_srwm_info,
+					      SNB_READ_WM1_LATENCY() * 500,
+					      &sprite_wm);
+	if (!ret) {
+		DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %d\n",
+			      pipe);
+		return;
+	}
+	I915_WRITE(WM1S_LP_ILK, sprite_wm);
+
+	/* Only IVB has two more LP watermarks for sprite */
+	if (!IS_IVYBRIDGE(dev))
+		return;
+
+	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
+					      pixel_size,
+					      &sandybridge_display_srwm_info,
+					      SNB_READ_WM2_LATENCY() * 500,
+					      &sprite_wm);
+	if (!ret) {
+		DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %d\n",
+			      pipe);
+		return;
+	}
+	I915_WRITE(WM2S_LP_IVB, sprite_wm);
+
+	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
+					      pixel_size,
+					      &sandybridge_display_srwm_info,
+					      SNB_READ_WM3_LATENCY() * 500,
+					      &sprite_wm);
+	if (!ret) {
+		DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %d\n",
+			      pipe);
+		return;
+	}
+	I915_WRITE(WM3S_LP_IVB, sprite_wm);
+}
+
 /**
  * intel_update_watermarks - update FIFO watermark values based on current modes
  *
@@ -4659,6 +4801,16 @@ static void intel_update_watermarks(struct drm_device *dev)
 		dev_priv->display.update_wm(dev);
 }
 
+void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
+				    uint32_t sprite_width, int pixel_size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (dev_priv->display.update_sprite_wm)
+		dev_priv->display.update_sprite_wm(dev, pipe, sprite_width,
+						   pixel_size);
+}
+
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
 	if (i915_panel_use_ssc >= 0)
@@ -8515,6 +8667,7 @@ static void intel_init_display(struct drm_device *dev)
 		} else if (IS_GEN6(dev)) {
 			if (SNB_READ_WM0_LATENCY()) {
 				dev_priv->display.update_wm = sandybridge_update_wm;
+				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
@@ -8528,6 +8681,7 @@ static void intel_init_display(struct drm_device *dev)
 			dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 			if (SNB_READ_WM0_LATENCY()) {
 				dev_priv->display.update_wm = sandybridge_update_wm;
+				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
@@ -8711,7 +8865,7 @@ static void i915_disable_vga(struct drm_device *dev)
 void intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int i;
+	int i, ret;
 
 	drm_mode_config_init(dev);
 
@@ -8741,6 +8895,12 @@ void intel_modeset_init(struct drm_device *dev)
 
 	for (i = 0; i < dev_priv->num_pipe; i++) {
 		intel_crtc_init(dev, i);
+		if (HAS_PCH_SPLIT(dev)) {
+			ret = intel_plane_init(dev, i);
+			if (ret)
+				DRM_ERROR("plane %d init failed: %d\n",
+					  i, ret);
+		}
 	}
 
 	/* Just disable it once at startup */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 23c5622..089cdde 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -176,10 +176,27 @@ struct intel_crtc {
 	bool use_pll_a;
 };
 
+struct intel_plane {
+	struct drm_plane base;
+	enum pipe pipe;
+	struct drm_i915_gem_object *obj;
+	int max_downscale;
+	u32 lut_r[1024], lut_g[1024], lut_b[1024];
+	void (*update_plane)(struct drm_plane *plane,
+			     struct drm_framebuffer *fb,
+			     struct drm_i915_gem_object *obj,
+			     int crtc_x, int crtc_y,
+			     unsigned int crtc_w, unsigned int crtc_h,
+			     uint32_t x, uint32_t y,
+			     uint32_t src_w, uint32_t src_h);
+	void (*disable_plane)(struct drm_plane *plane);
+};
+
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
 #define to_intel_connector(x) container_of(x, struct intel_connector, base)
 #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
 #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
+#define to_intel_plane(x) container_of(x, struct intel_plane, base)
 
 #define DIP_HEADER_SIZE	5
 
@@ -289,6 +306,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 extern bool intel_dpd_is_edp(struct drm_device *dev);
 extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
 extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
+extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
 
 /* intel_panel.c */
 extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
@@ -379,9 +397,19 @@ extern int intel_overlay_attrs(struct drm_device *dev, void *data,
 extern void intel_fb_output_poll_changed(struct drm_device *dev);
 extern void intel_fb_restore_mode(struct drm_device *dev);
 
+extern void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
+			bool state);
+#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
+#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
+
 extern void intel_init_clock_gating(struct drm_device *dev);
 extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
 
+/* For use by IVB LP watermark workaround in intel_sprite.c */
+extern void sandybridge_update_wm(struct drm_device *dev);
+extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
+					   uint32_t sprite_width,
+					   int pixel_size);
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index dc1db4f..068b086 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -270,8 +270,14 @@ void intel_fb_restore_mode(struct drm_device *dev)
 {
 	int ret;
 	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_plane *plane;
 
 	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
 	if (ret)
 		DRM_DEBUG("failed to restore crtc mode\n");
+
+	/* Be sure to shut off any planes that may be active */
+	list_for_each_entry(plane, &config->plane_list, head)
+		plane->funcs->disable_plane(plane);
 }
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
new file mode 100644
index 0000000..18eb7b8
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -0,0 +1,452 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *   Jesse Barnes <jbarnes@virtuousgeek.org>
+ *
+ * New plane/sprite handling.
+ *
+ * The older chips had a separate interface for programming plane related
+ * registers; newer ones are much simpler and we can use the new DRM plane
+ * support.
+ */
+#include "drmP.h"
+#include "drm_crtc.h"
+#include "drm_fourcc.h"
+#include "intel_drv.h"
+#include "i915_drm.h"
+#include "i915_drv.h"
+
+static void
+ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
+		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
+		 unsigned int crtc_w, unsigned int crtc_h,
+		 uint32_t x, uint32_t y,
+		 uint32_t src_w, uint32_t src_h)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	int pipe = intel_plane->pipe;
+	u32 sprctl, sprscale = 0;
+	int pixel_size;
+
+	sprctl = I915_READ(SPRCTL(pipe));
+
+	/* Mask out pixel format bits in case we change it */
+	sprctl &= ~(SPRITE_DEST_KEY | SPRITE_SOURCE_KEY);
+	sprctl &= ~SPRITE_PIXFORMAT_MASK;
+	sprctl &= ~SPRITE_RGB_ORDER_RGBX;
+	sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
+
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XBGR8888:
+		sprctl |= SPRITE_FORMAT_RGBX888;
+		pixel_size = 4;
+		break;
+	case DRM_FORMAT_XRGB8888:
+		sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
+		pixel_size = 4;
+		break;
+	case DRM_FORMAT_YUYV:
+		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_YVYU:
+		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YVYU;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_UYVY:
+		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_UYVY;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_VYUY:
+		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
+		pixel_size = 2;
+		break;
+	default:
+		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
+		sprctl |= DVS_FORMAT_RGBX888;
+		pixel_size = 4;
+		break;
+	}
+
+	if (obj->tiling_mode != I915_TILING_NONE)
+		sprctl |= SPRITE_TILED;
+
+	/* must disable */
+	sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
+	sprctl |= SPRITE_ENABLE;
+
+	/* Sizes are 0 based */
+	src_w--;
+	src_h--;
+	crtc_w--;
+	crtc_h--;
+
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size);
+
+	/*
+	 * IVB workaround: must disable low power watermarks for at least
+	 * one frame before enabling scaling.  LP watermarks can be re-enabled
+	 * when scaling is disabled.
+	 */
+	if (crtc_w != src_w || crtc_h != src_h) {
+		dev_priv->sprite_scaling_enabled = true;
+		sandybridge_update_wm(dev);
+		intel_wait_for_vblank(dev, pipe);
+		sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
+	} else {
+		dev_priv->sprite_scaling_enabled = false;
+		/* potentially re-enable LP watermarks */
+		sandybridge_update_wm(dev);
+	}
+
+	I915_WRITE(SPRSTRIDE(pipe), fb->pitch);
+	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
+	if (obj->tiling_mode != I915_TILING_NONE) {
+		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
+	} else {
+		unsigned long offset;
+
+		offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
+		I915_WRITE(SPRLINOFF(pipe), offset);
+	}
+	I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
+	I915_WRITE(SPRSCALE(pipe), sprscale);
+	I915_WRITE(SPRCTL(pipe), sprctl);
+	I915_WRITE(SPRSURF(pipe), obj->gtt_offset);
+	POSTING_READ(SPRSURF(pipe));
+}
+
+static void
+ivb_disable_plane(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	int pipe = intel_plane->pipe;
+
+	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
+	/* Can't leave the scaler enabled... */
+	I915_WRITE(SPRSCALE(pipe), 0);
+	/* Activate double buffered register update */
+	I915_WRITE(SPRSURF(pipe), 0);
+	POSTING_READ(SPRSURF(pipe));
+}
+
+static void
+snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
+		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
+		 unsigned int crtc_w, unsigned int crtc_h,
+		 uint32_t x, uint32_t y,
+		 uint32_t src_w, uint32_t src_h)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	int pipe = intel_plane->pipe, pixel_size;
+	u32 dvscntr, dvsscale = 0;
+
+	dvscntr = I915_READ(DVSCNTR(pipe));
+
+	/* Mask out pixel format bits in case we change it */
+	dvscntr &= ~(DVS_DEST_KEY | DVS_SOURCE_KEY);
+	dvscntr &= ~DVS_PIXFORMAT_MASK;
+	dvscntr &= ~DVS_RGB_ORDER_RGBX;
+	dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
+
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XBGR8888:
+		dvscntr |= DVS_FORMAT_RGBX888;
+		pixel_size = 4;
+		break;
+	case DRM_FORMAT_XRGB8888:
+		dvscntr |= DVS_FORMAT_RGBX888 | DVS_RGB_ORDER_RGBX;
+		pixel_size = 4;
+		break;
+	case DRM_FORMAT_YUYV:
+		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_YVYU:
+		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YVYU;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_UYVY:
+		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_UYVY;
+		pixel_size = 2;
+		break;
+	case DRM_FORMAT_VYUY:
+		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
+		pixel_size = 2;
+		break;
+	default:
+		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
+		dvscntr |= DVS_FORMAT_RGBX888;
+		pixel_size = 4;
+		break;
+	}
+
+	if (obj->tiling_mode != I915_TILING_NONE)
+		dvscntr |= DVS_TILED;
+
+	/* must disable */
+	dvscntr |= DVS_TRICKLE_FEED_DISABLE;
+	dvscntr |= DVS_ENABLE;
+
+	/* Sizes are 0 based */
+	src_w--;
+	src_h--;
+	crtc_w--;
+	crtc_h--;
+
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size);
+
+	if (crtc_w != src_w || crtc_h != src_h)
+		dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
+
+	I915_WRITE(DVSSTRIDE(pipe), fb->pitch);
+	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
+	if (obj->tiling_mode != I915_TILING_NONE) {
+		I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
+	} else {
+		unsigned long offset;
+
+		offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
+		I915_WRITE(DVSLINOFF(pipe), offset);
+	}
+	I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
+	I915_WRITE(DVSSCALE(pipe), dvsscale);
+	I915_WRITE(DVSCNTR(pipe), dvscntr);
+	I915_WRITE(DVSSURF(pipe), obj->gtt_offset);
+	POSTING_READ(DVSSURF(pipe));
+}
+
+static void
+snb_disable_plane(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	int pipe = intel_plane->pipe;
+
+	I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
+	/* Disable the scaler */
+	I915_WRITE(DVSSCALE(pipe), 0);
+	/* Flush double buffered register updates */
+	I915_WRITE(DVSSURF(pipe), 0);
+	POSTING_READ(DVSSURF(pipe));
+}
+
+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 drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_framebuffer *intel_fb;
+	struct drm_i915_gem_object *obj, *old_obj;
+	int pipe = intel_plane->pipe;
+	int ret = 0;
+	int x = src_x >> 16, y = src_y >> 16;
+	int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
+	bool disable_primary = false;
+
+	intel_fb = to_intel_framebuffer(fb);
+	obj = intel_fb->obj;
+
+	old_obj = intel_plane->obj;
+
+	/* Pipe must be running... */
+	if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
+		return -EINVAL;
+
+	if (crtc_x >= primary_w || crtc_y >= primary_h)
+		return -EINVAL;
+
+	/* Don't modify another pipe's plane */
+	if (intel_plane->pipe != intel_crtc->pipe)
+		return -EINVAL;
+
+	/*
+	 * Clamp the width & height into the visible area.  Note we don't
+	 * try to scale the source if part of the visible region is offscreen.
+	 * The caller must handle that by adjusting source offset and size.
+	 */
+	if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) {
+		crtc_w += crtc_x;
+		crtc_x = 0;
+	}
+	if ((crtc_x + crtc_w) <= 0) /* Nothing to display */
+		goto out;
+	if ((crtc_x + crtc_w) > primary_w)
+		crtc_w = primary_w - crtc_x;
+
+	if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) {
+		crtc_h += crtc_y;
+		crtc_y = 0;
+	}
+	if ((crtc_y + crtc_h) <= 0) /* Nothing to display */
+		goto out;
+	if (crtc_y + crtc_h > primary_h)
+		crtc_h = primary_h - crtc_y;
+
+	if (!crtc_w || !crtc_h) /* Again, nothing to display */
+		goto out;
+
+	/*
+	 * We can take a larger source and scale it down, but
+	 * only so much...  16x is the max on SNB.
+	 */
+	if (((src_w * src_h) / (crtc_w * crtc_h)) > intel_plane->max_downscale)
+		return -EINVAL;
+
+	/*
+	 * If the sprite is completely covering the primary plane,
+	 * we can disable the primary and save power.
+	 */
+	if ((crtc_x == 0) && (crtc_y == 0) &&
+	    (crtc_w == primary_w) && (crtc_h == primary_h))
+		disable_primary = true;
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+	if (ret) {
+		DRM_ERROR("failed to pin object\n");
+		goto out_unlock;
+	}
+
+	intel_plane->obj = obj;
+
+	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
+				  crtc_w, crtc_h, x, y, src_w, src_h);
+
+	/* Unpin old obj after new one is active to avoid ugliness */
+	if (old_obj) {
+		/*
+		 * It's fairly common to simply update the position of
+		 * an existing object.  In that case, we don't need to
+		 * wait for vblank to avoid ugliness, we only need to
+		 * do the pin & ref bookkeeping.
+		 */
+		if (old_obj != obj) {
+			mutex_unlock(&dev->struct_mutex);
+			intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
+			mutex_lock(&dev->struct_mutex);
+		}
+		i915_gem_object_unpin(old_obj);
+	}
+
+out_unlock:
+	mutex_unlock(&dev->struct_mutex);
+out:
+	return ret;
+}
+
+static int
+intel_disable_plane(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	int ret = 0;
+
+	intel_plane->disable_plane(plane);
+
+	if (!intel_plane->obj)
+		goto out;
+
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_object_unpin(intel_plane->obj);
+	intel_plane->obj = NULL;
+	mutex_unlock(&dev->struct_mutex);
+out:
+
+	return ret;
+}
+
+static void intel_destroy_plane(struct drm_plane *plane)
+{
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	intel_disable_plane(plane);
+	drm_plane_cleanup(plane);
+	kfree(intel_plane);
+}
+
+static const struct drm_plane_funcs intel_plane_funcs = {
+	.update_plane = intel_update_plane,
+	.disable_plane = intel_disable_plane,
+	.destroy = intel_destroy_plane,
+};
+
+static uint32_t snb_plane_formats[] = {
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+};
+
+int
+intel_plane_init(struct drm_device *dev, enum pipe pipe)
+{
+	struct intel_plane *intel_plane;
+	unsigned long possible_crtcs;
+	int ret;
+
+	if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
+		DRM_ERROR("new plane code only for SNB+\n");
+		return -ENODEV;
+	}
+
+	intel_plane = kzalloc(sizeof(struct intel_plane), GFP_KERNEL);
+	if (!intel_plane)
+		return -ENOMEM;
+
+	if (IS_GEN6(dev)) {
+		intel_plane->max_downscale = 16;
+		intel_plane->update_plane = snb_update_plane;
+		intel_plane->disable_plane = snb_disable_plane;
+	} else if (IS_GEN7(dev)) {
+		intel_plane->max_downscale = 2;
+		intel_plane->update_plane = ivb_update_plane;
+		intel_plane->disable_plane = ivb_disable_plane;
+	}
+
+	intel_plane->pipe = pipe;
+	possible_crtcs = (1 << pipe);
+	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
+			     &intel_plane_funcs, snb_plane_formats,
+			     ARRAY_SIZE(snb_plane_formats));
+	if (ret)
+		kfree(intel_plane);
+
+	return ret;
+}
+
-- 
1.7.4.1

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

Re: [PATCH 1/3] drm/i915: add SNB and IVB video sprite support v5

[Daniel Vetter]

On Wed, Dec 07, 2011 at 12:29:21PM -0800, Jesse Barnes wrote:
> The video sprites support various video surface formats natively and can
> handle scaling as well.  So add support for them using the new DRM core
> sprite support functions.
> 
> v2: use drm specific fourcc header and defines
> v3: address Daniel's comments:
>   - don't take struct mutex around register access (only needed for
>     regs in the GT power well)
>   - don't hold struct mutex across vblank waits
>   - fix up update_plane API (pass obj instead of GTT offset)
>   - add interlaced defines for sprite regs
>   - drop unnecessary 'reg' variables
>   - comment double buffered reg flushing
>   Also fix w/h confusion when writing the scaling reg.
> v4: more fixes, address more comments from Daniel, and include Hai's fix
>   - prevent divide by zero in scaling calculation (Hai Lan)
>   - update to Ville's new DRM_FORMAT_* types
>   - fix sprite watermark handling (calc based on CRTC size, separate
>     from normal display wm)
>   - remove private refcounts now that the fb cleanups handles things
> v5: add linear surface support
> 
> For this version, I tested DPMS since it came up in the last review;
> DPMS off/on works ok when a video player is working under X, but for
> power saving we'll probably want to do something smarter.  I'll leave
> that for a separate patch on top.  Likewise with the refcounting/fb
> layer handling, which are really separate cleanups.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

I didn't bother to recheck the regs and and the wm stuff looks like the
usual magic ;-) Otherwise you've implemented way more of my review
comments than I could possibly still remember, so it must be good.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/3] drm/i915: add SNB and IVB video sprite support v5

[Jesse Barnes]

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

On Mon, 12 Dec 2011 22:56:34 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Dec 07, 2011 at 12:29:21PM -0800, Jesse Barnes wrote:
> > The video sprites support various video surface formats natively and can
> > handle scaling as well.  So add support for them using the new DRM core
> > sprite support functions.
> > 
> > v2: use drm specific fourcc header and defines
> > v3: address Daniel's comments:
> >   - don't take struct mutex around register access (only needed for
> >     regs in the GT power well)
> >   - don't hold struct mutex across vblank waits
> >   - fix up update_plane API (pass obj instead of GTT offset)
> >   - add interlaced defines for sprite regs
> >   - drop unnecessary 'reg' variables
> >   - comment double buffered reg flushing
> >   Also fix w/h confusion when writing the scaling reg.
> > v4: more fixes, address more comments from Daniel, and include Hai's fix
> >   - prevent divide by zero in scaling calculation (Hai Lan)
> >   - update to Ville's new DRM_FORMAT_* types
> >   - fix sprite watermark handling (calc based on CRTC size, separate
> >     from normal display wm)
> >   - remove private refcounts now that the fb cleanups handles things
> > v5: add linear surface support
> > 
> > For this version, I tested DPMS since it came up in the last review;
> > DPMS off/on works ok when a video player is working under X, but for
> > power saving we'll probably want to do something smarter.  I'll leave
> > that for a separate patch on top.  Likewise with the refcounting/fb
> > layer handling, which are really separate cleanups.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> I didn't bother to recheck the regs and and the wm stuff looks like the
> usual magic ;-) Otherwise you've implemented way more of my review
> comments than I could possibly still remember, so it must be good.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

The one thing I kept out was the "disable planes in generic code"
call.  The TI guys are working on using planes to represent all planes,
not just additional overlays, so calling disable from generic code
seemed unfriendly.

Keith, this one is ready for -next.  I'll clean up the ioctl now and
re-post; without it the overlays will always sit on top without
blending, so the patch is still safe.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 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 1/3] drm/i915: add SNB and IVB video sprite support v5

[Jesse Barnes]

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

On Mon, 12 Dec 2011 22:56:34 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Dec 07, 2011 at 12:29:21PM -0800, Jesse Barnes wrote:
> > The video sprites support various video surface formats natively and can
> > handle scaling as well.  So add support for them using the new DRM core
> > sprite support functions.
> > 
> > v2: use drm specific fourcc header and defines
> > v3: address Daniel's comments:
> >   - don't take struct mutex around register access (only needed for
> >     regs in the GT power well)
> >   - don't hold struct mutex across vblank waits
> >   - fix up update_plane API (pass obj instead of GTT offset)
> >   - add interlaced defines for sprite regs
> >   - drop unnecessary 'reg' variables
> >   - comment double buffered reg flushing
> >   Also fix w/h confusion when writing the scaling reg.
> > v4: more fixes, address more comments from Daniel, and include Hai's fix
> >   - prevent divide by zero in scaling calculation (Hai Lan)
> >   - update to Ville's new DRM_FORMAT_* types
> >   - fix sprite watermark handling (calc based on CRTC size, separate
> >     from normal display wm)
> >   - remove private refcounts now that the fb cleanups handles things
> > v5: add linear surface support
> > 
> > For this version, I tested DPMS since it came up in the last review;
> > DPMS off/on works ok when a video player is working under X, but for
> > power saving we'll probably want to do something smarter.  I'll leave
> > that for a separate patch on top.  Likewise with the refcounting/fb
> > layer handling, which are really separate cleanups.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> I didn't bother to recheck the regs and and the wm stuff looks like the
> usual magic ;-) Otherwise you've implemented way more of my review
> comments than I could possibly still remember, so it must be good.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I take it back, I need to re-post this one minus a few lines now that
I'm updating the color key support.  New series on its way.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 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

[PATCH 2/3] drm/i915: add destination color key support

[Jesse Barnes]

Add new ioctls for getting and setting the current destination color
key.  This allows for simple overlay display control by matching a color
key value in the primary plane before blending the overlay on top.

v2: remove unnecessary mutex acquire/release around reg accesses

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_dma.c     |    2 +
 drivers/gpu/drm/i915/i915_reg.h     |    2 +
 drivers/gpu/drm/i915/intel_drv.h    |    8 ++
 drivers/gpu/drm/i915/intel_sprite.c |  138 +++++++++++++++++++++++++++++++++++
 include/drm/i915_drm.h              |   16 ++++
 5 files changed, 166 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a9533c5..bc7ee4f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2295,6 +2295,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_DESTKEY, intel_sprite_set_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_DESTKEY, intel_sprite_get_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a2ce411..860b250 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2738,6 +2738,8 @@
 #define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
 #define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
 #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+#define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
+#define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
 
 #define _SPRA_CTL		0x70280
 #define   SPRITE_ENABLE			(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 089cdde..e0407cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -190,6 +190,8 @@ struct intel_plane {
 			     uint32_t x, uint32_t y,
 			     uint32_t src_w, uint32_t src_h);
 	void (*disable_plane)(struct drm_plane *plane);
+	int (*update_destkey)(struct drm_plane *plane, u32 value);
+	u32 (*get_destkey)(struct drm_plane *plane);
 };
 
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
@@ -412,4 +414,10 @@ extern void sandybridge_update_wm(struct drm_device *dev);
 extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 					   uint32_t sprite_width,
 					   int pixel_size);
+
+extern int intel_sprite_set_destkey(struct drm_device *dev, void *data,
+				     struct drm_file *file_priv);
+extern int intel_sprite_get_destkey(struct drm_device *dev, void *data,
+				     struct drm_file *file_priv);
+
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 18eb7b8..a51edc9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -96,6 +96,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	/* must disable */
 	sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
 	sprctl |= SPRITE_ENABLE;
+	sprctl |= SPRITE_DEST_KEY;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -154,6 +155,41 @@ ivb_disable_plane(struct drm_plane *plane)
 	POSTING_READ(SPRSURF(pipe));
 }
 
+static int
+ivb_update_destkey(struct drm_plane *plane, u32 value)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane;
+	int ret = 0;
+
+	if (value > 0xffffff)
+		return -EINVAL;
+
+	intel_plane = to_intel_plane(plane);
+
+	I915_WRITE(SPRKEYVAL(intel_plane->pipe), value);
+	I915_WRITE(SPRKEYMSK(intel_plane->pipe), 0xffffff);
+	POSTING_READ(SPRKEYMSK(intel_plane->pipe));
+
+	return ret;
+}
+
+static u32
+ivb_get_destkey(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane;
+	u32 value;
+
+	intel_plane = to_intel_plane(plane);
+
+	value = I915_READ(SPRKEYVAL(intel_plane->pipe));
+
+	return value;
+}
+
 static void
 snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
@@ -259,6 +295,41 @@ snb_disable_plane(struct drm_plane *plane)
 }
 
 static int
+snb_update_destkey(struct drm_plane *plane, u32 value)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane;
+	int ret = 0;
+
+	if (value > 0xffffff)
+		return -EINVAL;
+
+	intel_plane = to_intel_plane(plane);
+
+	I915_WRITE(DVSKEYVAL(intel_plane->pipe), value);
+	I915_WRITE(DVSKEYMSK(intel_plane->pipe), 0xffffff);
+	POSTING_READ(DVSKEYMSK(intel_plane->pipe));
+
+	return ret;
+}
+
+static u32
+snb_get_destkey(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane;
+	u32 value;
+
+	intel_plane = to_intel_plane(plane);
+
+	value = I915_READ(DVSKEYVAL(intel_plane->pipe));
+
+	return value;
+}
+
+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,
@@ -398,6 +469,69 @@ static void intel_destroy_plane(struct drm_plane *plane)
 	kfree(intel_plane);
 }
 
+int intel_sprite_set_destkey(struct drm_device *dev, void *data,
+			      struct drm_file *file_priv)
+{
+	struct drm_intel_set_sprite_destkey *set = data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_object *obj;
+	struct drm_plane *plane;
+	struct intel_plane *intel_plane;
+	int ret = 0;
+
+	if (!dev_priv)
+		return -EINVAL;
+
+	if (set->value > 0xffffff)
+		return -EINVAL;
+
+	mutex_lock(&dev->mode_config.mutex);
+
+	obj = drm_mode_object_find(dev, set->plane_id, DRM_MODE_OBJECT_PLANE);
+	if (!obj) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	plane = obj_to_plane(obj);
+	intel_plane = to_intel_plane(plane);
+	ret = intel_plane->update_destkey(plane, set->value);
+
+out_unlock:
+	mutex_unlock(&dev->mode_config.mutex);
+	return ret;
+}
+
+int intel_sprite_get_destkey(struct drm_device *dev, void *data,
+			      struct drm_file *file_priv)
+{
+	struct drm_intel_get_sprite_destkey *get = data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_object *obj;
+	struct drm_plane *plane;
+	struct intel_plane *intel_plane;
+	int ret = 0;
+
+	if (!dev_priv)
+		return -EINVAL;
+
+	mutex_lock(&dev->mode_config.mutex);
+
+	obj = drm_mode_object_find(dev, get->plane_id, DRM_MODE_OBJECT_PLANE);
+	if (!obj) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	plane = obj_to_plane(obj);
+	intel_plane = to_intel_plane(plane);
+	get->value = intel_plane->get_destkey(plane);
+
+out_unlock:
+	mutex_unlock(&dev->mode_config.mutex);
+	return ret;
+}
+
 static const struct drm_plane_funcs intel_plane_funcs = {
 	.update_plane = intel_update_plane,
 	.disable_plane = intel_disable_plane,
@@ -433,10 +567,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
 		intel_plane->max_downscale = 16;
 		intel_plane->update_plane = snb_update_plane;
 		intel_plane->disable_plane = snb_disable_plane;
+		intel_plane->update_destkey = snb_update_destkey;
+		intel_plane->get_destkey = snb_get_destkey;
 	} else if (IS_GEN7(dev)) {
 		intel_plane->max_downscale = 2;
 		intel_plane->update_plane = ivb_update_plane;
 		intel_plane->disable_plane = ivb_disable_plane;
+		intel_plane->update_destkey = ivb_update_destkey;
+		intel_plane->get_destkey = ivb_get_destkey;
 	}
 
 	intel_plane->pipe = pipe;
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 28c0d11..71674b3 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -198,6 +198,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_OVERLAY_PUT_IMAGE	0x27
 #define DRM_I915_OVERLAY_ATTRS	0x28
 #define DRM_I915_GEM_EXECBUFFER2	0x29
+#define DRM_I915_GET_SPRITE_DESTKEY	0x2a
+#define DRM_I915_SET_SPRITE_DESTKEY	0x2b
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -239,6 +241,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_MADVISE	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MADVISE, struct drm_i915_gem_madvise)
 #define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
 #define DRM_IOCTL_I915_OVERLAY_ATTRS	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
+#define DRM_IOCTL_I915_SET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_set_sprite_destkey)
+#define DRM_IOCTL_I915_GET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_get_sprite_destkey)
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.
@@ -844,4 +848,16 @@ struct drm_intel_overlay_attrs {
 	__u32 gamma5;
 };
 
+/* Set the destination color key on a given sprite */
+struct drm_intel_set_sprite_destkey {
+	__u32 plane_id;
+	__u32 value;
+};
+
+/* Get the current destination color key on a given sprite */
+struct drm_intel_get_sprite_destkey {
+	__u32 plane_id;
+	__u32 value;
+};
+
 #endif				/* _I915_DRM_H_ */
-- 
1.7.4.1

Re: [PATCH 2/3] drm/i915: add destination color key support

[Daniel Vetter]

On Wed, Dec 07, 2011 at 12:29:22PM -0800, Jesse Barnes wrote:
> Add new ioctls for getting and setting the current destination color
> key.  This allows for simple overlay display control by matching a color
> key value in the primary plane before blending the overlay on top.
> 
> v2: remove unnecessary mutex acquire/release around reg accesses
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

[snip]

> +/* Set the destination color key on a given sprite */
> +struct drm_intel_set_sprite_destkey {
> +	__u32 plane_id;
> +	__u32 value;
> +};
> +
> +/* Get the current destination color key on a given sprite */
> +struct drm_intel_get_sprite_destkey {
> +	__u32 plane_id;
> +	__u32 value;
> +};

As explained in my top-reply, I think we want a __u32 flags here to
enable the colorkey (and maybe switch between dst/src color key). Nothing
else to complain about here.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH 3/3] drm/i915: track sprite coverage and disable primary plane if possible

[Jesse Barnes]

To save power when the sprite is full screen, we can disable the primary
plane on the same pipe.  Track the sprite status and enable/disable the
primary opportunistically.

v2: remove primary plane enable/disable hooks; they're identical

Signed-off-by: Jesse Barnes
---
 drivers/gpu/drm/i915/intel_drv.h    |    1 +
 drivers/gpu/drm/i915/intel_sprite.c |   41 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e0407cd..0340b4c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -180,6 +180,7 @@ struct intel_plane {
 	struct drm_plane base;
 	enum pipe pipe;
 	struct drm_i915_gem_object *obj;
+	bool primary_disabled;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
 	void (*update_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a51edc9..976eee3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -329,6 +329,28 @@ snb_get_destkey(struct drm_plane *plane)
 	return value;
 }
 
+static void
+intel_enable_primary(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int reg = DSPCNTR(intel_crtc->plane);
+
+	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
+}
+
+static void
+intel_disable_primary(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int reg = DSPCNTR(intel_crtc->plane);
+
+	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
+}
+
 static int
 intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
@@ -415,9 +437,23 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	intel_plane->obj = obj;
 
+	/*
+	 * Be sure to re-enable the primary before the sprite is no longer
+	 * covering it fully.
+	 */
+	if (!disable_primary && intel_plane->primary_disabled) {
+		intel_enable_primary(crtc);
+		intel_plane->primary_disabled = false;
+	}
+
 	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
 				  crtc_w, crtc_h, x, y, src_w, src_h);
 
+	if (disable_primary) {
+		intel_disable_primary(crtc);
+		intel_plane->primary_disabled = true;
+	}
+
 	/* Unpin old obj after new one is active to avoid ugliness */
 	if (old_obj) {
 		/*
@@ -447,6 +483,11 @@ intel_disable_plane(struct drm_plane *plane)
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int ret = 0;
 
+	if (intel_plane->primary_disabled) {
+		intel_enable_primary(plane->crtc);
+		intel_plane->primary_disabled = false;
+	}
+
 	intel_plane->disable_plane(plane);
 
 	if (!intel_plane->obj)
-- 
1.7.4.1

Re: [PATCH 3/3] drm/i915: track sprite coverage and disable primary plane if possible

[Daniel Vetter]

On Wed, Dec 07, 2011 at 12:29:23PM -0800, Jesse Barnes wrote:
> To save power when the sprite is full screen, we can disable the primary
> plane on the same pipe.  Track the sprite status and enable/disable the
> primary opportunistically.
> 
> v2: remove primary plane enable/disable hooks; they're identical
> 
> Signed-off-by: Jesse Barnes

Minor nitpick on patch-splitting: You've left the disable_primary
calculations in the 1st patch. But since I didn't notice it there, I can't
be that bad ;-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: Plane support for SNB and IVB

[Hai Lan]

2011/12/8 Jesse Barnes <jbarnes@virtuousgeek.org>
>
> I think the core support is ready now, but I'd like feedback on the
> color key ioctl.  It would be good to have a way to disable it at
> runtime (w/o color keying enabled, the sprite will always sit on top of
> the primary), and potentially add alpha support at some point.
>
> Would 0 as the color key work well enough as a "disable" flag?  Or
> should I add a separate field to the ioctl?  Anyone have a preference?
>
Do you mean whether to use colorkey value 0 as a disabled flag? I
don't think we can use 0 as disabled colorkey because sometimes we
need to set the colorkey as 0.
So I think it's better to use a separate field of ioctl.

Thanks and best regards.

Hai Lan

Re: Plane support for SNB and IVB

[Daniel Vetter]

On Wed, Dec 07, 2011 at 12:29:20PM -0800, Jesse Barnes wrote:
> I think the core support is ready now, but I'd like feedback on the
> color key ioctl.  It would be good to have a way to disable it at
> runtime (w/o color keying enabled, the sprite will always sit on top of
> the primary), and potentially add alpha support at some point.
> 
> Would 0 as the color key work well enough as a "disable" flag?  Or
> should I add a separate field to the ioctl?  Anyone have a preference?

0 is black and I can image a legit color key with "black bars". Any other
value makes the interface kinda ad-hoc. So a separate field preferred.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48