All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/8] Skylake Y tiled scanout
@ 2015-02-27 11:15 Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 1/8] drm/i915/skl: Add new displayable tiling formats Tvrtko Ursulin
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Starting with Skylake the display engine can scan out Y tiled objects. (Both
legacy Y tiled, and the new Yf format.)

This series takes the original work by Damien Lespiau and converts it to use the
new frame buffer modifiers instead of object set/get tiling. Some patches needed
to be dropped, some added and some refactored.

Lightly tested with "testdisplay -m -y" and "testdisplay -m --yf".

v2: Rebased on v2 of "i915 fb modifier support, respun".

v3:
   * Part which allows Y tiled fb creation extracted out and moved to the end
     of series.
   * Dropped redundant "drm/i915/skl: Allow Y tiling for sprites".
   * Also see individual change logs.

v4:
   * New to the series - watermark programming updates per BSpec.
   * Addressed review comments in individual patches.

v5:
   * Addressed review comments in individual patches.

Damien Lespiau (4):
  drm/i915/skl: Allow scanning out Y and Yf fbs
  drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling
  drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints
  drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling

Tvrtko Ursulin (4):
  drm/i915/skl: Add new displayable tiling formats
  drm/i915/skl: Updated watermark programming
  drm/i915/skl: Update watermarks for Y tiling
  drm/i915/skl: Allow Y (and Yf) frame buffer creation

 drivers/gpu/drm/i915/intel_display.c | 231 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 drivers/gpu/drm/i915/intel_pm.c      |  97 +++++++++++----
 drivers/gpu/drm/i915/intel_sprite.c  |  24 +++-
 include/uapi/drm/drm_fourcc.h        |  15 +++
 5 files changed, 283 insertions(+), 87 deletions(-)

-- 
2.3.0

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

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

* [PATCH 1/8] drm/i915/skl: Add new displayable tiling formats
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 2/8] drm/i915/skl: Allow scanning out Y and Yf fbs Tvrtko Ursulin
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Starting with SKL display engine can scan out Y, and newly introduced Yf
tiling formats so add the latter to the frame buffer modifier space.

v2: Definitions moved to drm_fourcc.h.
v3: Try to document the format better.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 include/uapi/drm/drm_fourcc.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 1a5a357..e6efac2 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -192,4 +192,19 @@
  */
 #define I915_FORMAT_MOD_Y_TILED	fourcc_mod_code(INTEL, 2)
 
+/*
+ * Intel Yf-tiling layout
+ *
+ * This is a tiled layout using 4Kb tiles in row-major layout.
+ * Within the tile pixels are laid out in 16 256 byte units / sub-tiles which
+ * are arranged in four groups (two wide, two high) with column-major layout.
+ * Each group therefore consits out of four 256 byte units, which are also laid
+ * out as 2x2 column-major.
+ * 256 byte units are made out of four 64 byte blocks of pixels, producing
+ * either a square block or a 2:1 unit.
+ * 64 byte blocks of pixels contain four pixel rows of 16 bytes, where the width
+ * in pixel depends on the pixel depth.
+ */
+#define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
+
 #endif /* DRM_FOURCC_H */
-- 
2.3.0

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

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

* [PATCH 2/8] drm/i915/skl: Allow scanning out Y and Yf fbs
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 1/8] drm/i915/skl: Add new displayable tiling formats Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 3/8] drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling Tvrtko Ursulin
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Damien Lespiau <damien.lespiau@intel.com>

Skylake is able to scannout those tiling formats. We need to allow them
in the ADDFB ioctl and tell the harware about it.

v2: Rebased for addfb2 interface. (Tvrtko Ursulin)
v3: Rebased for fb modifier changes. (Tvrtko Ursulin)
v4: Don't allow Y tiled fbs just yet. (Tvrtko Ursulin)
v5: Check for stride alignment and max pitch. (Tvrtko Ursulin)
v6: Simplify maximum pitch check. (Ville Syrjälä)
v7: Drop the gen9 check since requirements are no different. (Ville Syrjälä)
v8: Gen2 has different X tiling stride. (Ville Syrjälä)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v7)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 121 +++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_sprite.c  |  18 ++++--
 3 files changed, 101 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 94efacb..cf0eadc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2734,6 +2734,40 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	POSTING_READ(reg);
 }
 
+u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
+			      uint32_t pixel_format)
+{
+	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
+
+	/*
+	 * The stride is either expressed as a multiple of 64 bytes
+	 * chunks for linear buffers or in number of tiles for tiled
+	 * buffers.
+	 */
+	switch (fb_modifier) {
+	case DRM_FORMAT_MOD_NONE:
+		return 64;
+	case I915_FORMAT_MOD_X_TILED:
+		if (INTEL_INFO(dev)->gen == 2)
+			return 128;
+		return 512;
+	case I915_FORMAT_MOD_Y_TILED:
+		/* No need to check for old gens and Y tiling since this is
+		 * about the display engine and those will be blocked before
+		 * we get here.
+		 */
+		return 128;
+	case I915_FORMAT_MOD_Yf_TILED:
+		if (bits_per_pixel == 8)
+			return 64;
+		else
+			return 128;
+	default:
+		MISSING_CASE(fb_modifier);
+		return 64;
+	}
+}
+
 static void skylake_update_primary_plane(struct drm_crtc *crtc,
 					 struct drm_framebuffer *fb,
 					 int x, int y)
@@ -2741,10 +2775,9 @@ static void skylake_update_primary_plane(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);
-	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
 	int pipe = intel_crtc->pipe;
-	u32 plane_ctl, stride;
+	u32 plane_ctl, stride_div;
 
 	if (!intel_crtc->primary_enabled) {
 		I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -2788,29 +2821,30 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		BUG();
 	}
 
-	intel_fb = to_intel_framebuffer(fb);
-	obj = intel_fb->obj;
-
-	/*
-	 * The stride is either expressed as a multiple of 64 bytes chunks for
-	 * linear buffers or in number of tiles for tiled buffers.
-	 */
 	switch (fb->modifier[0]) {
 	case DRM_FORMAT_MOD_NONE:
-		stride = fb->pitches[0] >> 6;
 		break;
 	case I915_FORMAT_MOD_X_TILED:
 		plane_ctl |= PLANE_CTL_TILED_X;
-		stride = fb->pitches[0] >> 9;
+		break;
+	case I915_FORMAT_MOD_Y_TILED:
+		plane_ctl |= PLANE_CTL_TILED_Y;
+		break;
+	case I915_FORMAT_MOD_Yf_TILED:
+		plane_ctl |= PLANE_CTL_TILED_YF;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->modifier[0]);
 	}
 
 	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
 	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
 		plane_ctl |= PLANE_CTL_ROTATE_180;
 
+	obj = intel_fb_obj(fb);
+	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
+					       fb->pixel_format);
+
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 
 	DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",
@@ -2823,7 +2857,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	I915_WRITE(PLANE_SIZE(pipe, 0),
 		   (intel_crtc->config->pipe_src_h - 1) << 16 |
 		   (intel_crtc->config->pipe_src_w - 1));
-	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
+	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
 	I915_WRITE(PLANE_SURF(pipe, 0), i915_gem_obj_ggtt_offset(obj));
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
@@ -12683,14 +12717,43 @@ static const struct drm_framebuffer_funcs intel_fb_funcs = {
 	.create_handle = intel_user_framebuffer_create_handle,
 };
 
+static
+u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
+			 uint32_t pixel_format)
+{
+	u32 gen = INTEL_INFO(dev)->gen;
+
+	if (gen >= 9) {
+		/* "The stride in bytes must not exceed the of the size of 8K
+		 *  pixels and 32K bytes."
+		 */
+		 return min(8192*drm_format_plane_cpp(pixel_format, 0), 32768);
+	} else if (gen >= 5 && !IS_VALLEYVIEW(dev)) {
+		return 32*1024;
+	} else if (gen >= 4) {
+		if (fb_modifier == I915_FORMAT_MOD_X_TILED)
+			return 16*1024;
+		else
+			return 32*1024;
+	} else if (gen >= 3) {
+		if (fb_modifier == I915_FORMAT_MOD_X_TILED)
+			return 8*1024;
+		else
+			return 16*1024;
+	} else {
+		/* XXX DSPC is limited to 4k tiled */
+		return 8*1024;
+	}
+}
+
 static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *intel_fb,
 				  struct drm_mode_fb_cmd2 *mode_cmd,
 				  struct drm_i915_gem_object *obj)
 {
 	int aligned_height;
-	int pitch_limit;
 	int ret;
+	u32 pitch_limit, stride_alignment;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
@@ -12716,31 +12779,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	if (mode_cmd->pitches[0] & 63) {
-		DRM_DEBUG("pitch (%d) must be at least 64 byte aligned\n",
-			  mode_cmd->pitches[0]);
+	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
+						     mode_cmd->pixel_format);
+	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
+		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
+			  mode_cmd->pitches[0], stride_alignment);
 		return -EINVAL;
 	}
 
-	if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) {
-		pitch_limit = 32*1024;
-	} else if (INTEL_INFO(dev)->gen >= 4) {
-		if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)
-			pitch_limit = 16*1024;
-		else
-			pitch_limit = 32*1024;
-	} else if (INTEL_INFO(dev)->gen >= 3) {
-		if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)
-			pitch_limit = 8*1024;
-		else
-			pitch_limit = 16*1024;
-	} else
-		/* XXX DSPC is limited to 4k tiled */
-		pitch_limit = 8*1024;
-
+	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
+					   mode_cmd->pixel_format);
 	if (mode_cmd->pitches[0] > pitch_limit) {
-		DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
-			  mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED ?
+		DRM_DEBUG("%s pitch (%u) must be at less than %d\n",
+			  mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ?
 			  "tiled" : "linear",
 			  mode_cmd->pitches[0], pitch_limit);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4997bc9..e21d26c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -903,6 +903,8 @@ int intel_fb_align_height(struct drm_device *dev, int height,
 			  uint64_t fb_format_modifier);
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
 
+u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
+			      uint32_t pixel_format);
 
 /* intel_audio.c */
 void intel_init_audio(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5ae56ec..b7103bd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -189,7 +189,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
-	u32 plane_ctl, stride;
+	u32 plane_ctl, stride_div;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
 	plane_ctl = I915_READ(PLANE_CTL(pipe, plane));
@@ -247,15 +247,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 
 	switch (fb->modifier[0]) {
 	case DRM_FORMAT_MOD_NONE:
-		stride = fb->pitches[0] >> 6;
 		break;
 	case I915_FORMAT_MOD_X_TILED:
 		plane_ctl |= PLANE_CTL_TILED_X;
-		stride = fb->pitches[0] >> 9;
+		break;
+	case I915_FORMAT_MOD_Y_TILED:
+		plane_ctl |= PLANE_CTL_TILED_Y;
+		break;
+	case I915_FORMAT_MOD_Yf_TILED:
+		plane_ctl |= PLANE_CTL_TILED_YF;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->modifier[0]);
 	}
+
 	if (drm_plane->state->rotation == BIT(DRM_ROTATE_180))
 		plane_ctl |= PLANE_CTL_ROTATE_180;
 
@@ -266,6 +271,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 				       pixel_size, true,
 				       src_w != crtc_w || src_h != crtc_h);
 
+	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
+					       fb->pixel_format);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -273,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	crtc_h--;
 
 	I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
-	I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
+	I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
 	I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
 	I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
-- 
2.3.0

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

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

* [PATCH 3/8] drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 1/8] drm/i915/skl: Add new displayable tiling formats Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 2/8] drm/i915/skl: Allow scanning out Y and Yf fbs Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 4/8] drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints Tvrtko Ursulin
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Damien Lespiau <damien.lespiau@intel.com>

We now need the bpp of the fb as Yf tiling has different tile widths
depending on it.

v2: Rebased for the new addfb2 interface. (Tvrtko Ursulin)
v3: Rebased for fb modifier changes. (Tvrtko Ursulin)
v4: Added missing case and 128-bit pixel warning. (Damien Lespiau)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v3)
---
 drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cf0eadc..1db63c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2195,9 +2195,44 @@ intel_fb_align_height(struct drm_device *dev, int height,
 		      uint64_t fb_format_modifier)
 {
 	int tile_height;
+	uint32_t bits_per_pixel;
 
-	tile_height = fb_format_modifier == I915_FORMAT_MOD_X_TILED ?
-		(IS_GEN2(dev) ? 16 : 8) : 1;
+	switch (fb_format_modifier) {
+	case DRM_FORMAT_MOD_NONE:
+		tile_height = 1;
+		break;
+	case I915_FORMAT_MOD_X_TILED:
+		tile_height = IS_GEN2(dev) ? 16 : 8;
+		break;
+	case I915_FORMAT_MOD_Y_TILED:
+		tile_height = 32;
+		break;
+	case I915_FORMAT_MOD_Yf_TILED:
+		bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
+		switch (bits_per_pixel) {
+		default:
+		case 8:
+			tile_height = 64;
+			break;
+		case 16:
+		case 32:
+			tile_height = 32;
+			break;
+		case 64:
+			tile_height = 16;
+			break;
+		case 128:
+			WARN_ONCE(1,
+				  "128-bit pixels are not supported for display!");
+			tile_height = 16;
+			break;
+		}
+		break;
+	default:
+		MISSING_CASE(fb_format_modifier);
+		tile_height = 1;
+		break;
+	}
 
 	return ALIGN(height, tile_height);
 }
-- 
2.3.0

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

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

* [PATCH 4/8] drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 3/8] drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 5/8] drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling Tvrtko Ursulin
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Damien Lespiau <damien.lespiau@intel.com>

1Mb!

v2: Rebased for addfb2 interface. (Tvrtko Ursulin)
v3: Rebased for fb modifier changes. (Tvrtko Ursulin)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1db63c9..cb17b16 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2270,8 +2270,12 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 		}
 		break;
 	case I915_FORMAT_MOD_Y_TILED:
-		WARN(1, "Y tiled bo slipped through, driver bug!\n");
-		return -EINVAL;
+	case I915_FORMAT_MOD_Yf_TILED:
+		if (WARN_ONCE(INTEL_INFO(dev)->gen < 9,
+			  "Y tiling bo slipped through, driver bug!\n"))
+			return -EINVAL;
+		alignment = 1 * 1024 * 1024;
+		break;
 	default:
 		MISSING_CASE(fb->modifier[0]);
 		return -EINVAL;
-- 
2.3.0

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

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

* [PATCH 5/8] drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 4/8] drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 6/8] drm/i915/skl: Updated watermark programming Tvrtko Ursulin
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Damien Lespiau <damien.lespiau@intel.com>

v2: Rebased for addfb2 interface and consolidated a bit. (Tvrtko Ursulin)
v3: Rebased for fb modifier changes. (Tvrtko Ursulin)
v4: Use intel_fb_stride_alignment instead of open coding. (Damien Lespiau)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb17b16..753894d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7755,7 +7755,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 val, base, offset, stride_mult;
+	u32 val, base, offset, stride_mult, tiling;
 	int pipe = crtc->pipe;
 	int fourcc, pixel_format;
 	int aligned_height;
@@ -7774,11 +7774,6 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	if (!(val & PLANE_CTL_ENABLE))
 		goto error;
 
-	if (val & PLANE_CTL_TILED_MASK) {
-		plane_config->tiling = I915_TILING_X;
-		fb->modifier[0] = I915_FORMAT_MOD_X_TILED;
-	}
-
 	pixel_format = val & PLANE_CTL_FORMAT_MASK;
 	fourcc = skl_format_to_fourcc(pixel_format,
 				      val & PLANE_CTL_ORDER_RGBX,
@@ -7786,6 +7781,26 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->pixel_format = fourcc;
 	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
+	tiling = val & PLANE_CTL_TILED_MASK;
+	switch (tiling) {
+	case PLANE_CTL_TILED_LINEAR:
+		fb->modifier[0] = DRM_FORMAT_MOD_NONE;
+		break;
+	case PLANE_CTL_TILED_X:
+		plane_config->tiling = I915_TILING_X;
+		fb->modifier[0] = I915_FORMAT_MOD_X_TILED;
+		break;
+	case PLANE_CTL_TILED_Y:
+		fb->modifier[0] = I915_FORMAT_MOD_Y_TILED;
+		break;
+	case PLANE_CTL_TILED_YF:
+		fb->modifier[0] = I915_FORMAT_MOD_Yf_TILED;
+		break;
+	default:
+		MISSING_CASE(tiling);
+		goto error;
+	}
+
 	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
 	plane_config->base = base;
 
@@ -7796,17 +7811,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->width = ((val >> 0) & 0x1fff) + 1;
 
 	val = I915_READ(PLANE_STRIDE(pipe, 0));
-	switch (plane_config->tiling) {
-	case I915_TILING_NONE:
-		stride_mult = 64;
-		break;
-	case I915_TILING_X:
-		stride_mult = 512;
-		break;
-	default:
-		MISSING_CASE(plane_config->tiling);
-		goto error;
-	}
+	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
+						fb->pixel_format);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
 
 	aligned_height = intel_fb_align_height(dev, fb->height,
-- 
2.3.0

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

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

* [PATCH 6/8] drm/i915/skl: Updated watermark programming
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 5/8] drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling Tvrtko Ursulin
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Recent BSpect updates have changed the watermark calculation to avoid
display flickering in some cases.

v2: Fix check against DDB allocation and tidy the code a bit. (Damien Lespiau)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 54 +++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7dcb5b6..0b18e5d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2609,7 +2609,7 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 	if (latency == 0)
 		return UINT_MAX;
 
-	wm_intermediate_val = latency * pixel_rate * bytes_per_pixel;
+	wm_intermediate_val = latency * pixel_rate * bytes_per_pixel / 512;
 	ret = DIV_ROUND_UP(wm_intermediate_val, 1000);
 
 	return ret;
@@ -2619,15 +2619,18 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
 			       uint32_t latency)
 {
-	uint32_t ret, plane_bytes_per_line, wm_intermediate_val;
+	uint32_t ret;
+	uint32_t plane_bytes_per_line, plane_blocks_per_line;
+	uint32_t wm_intermediate_val;
 
 	if (latency == 0)
 		return UINT_MAX;
 
 	plane_bytes_per_line = horiz_pixels * bytes_per_pixel;
+	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 	wm_intermediate_val = latency * pixel_rate;
 	ret = DIV_ROUND_UP(wm_intermediate_val, pipe_htotal * 1000) *
-				plane_bytes_per_line;
+				plane_blocks_per_line;
 
 	return ret;
 }
@@ -2707,41 +2710,49 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 	}
 }
 
-static bool skl_compute_plane_wm(struct skl_pipe_wm_parameters *p,
+static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
+				 struct skl_pipe_wm_parameters *p,
 				 struct intel_plane_wm_parameters *p_params,
 				 uint16_t ddb_allocation,
-				 uint32_t mem_value,
+				 int level,
 				 uint16_t *out_blocks, /* out */
 				 uint8_t *out_lines /* out */)
 {
-	uint32_t method1, method2, plane_bytes_per_line, res_blocks, res_lines;
-	uint32_t result_bytes;
+	uint32_t latency = dev_priv->wm.skl_latency[level];
+	uint32_t method1, method2;
+	uint32_t plane_bytes_per_line, plane_blocks_per_line;
+	uint32_t res_blocks, res_lines;
+	uint32_t selected_result;
 
-	if (mem_value == 0 || !p->active || !p_params->enabled)
+	if (latency == 0 || !p->active || !p_params->enabled)
 		return false;
 
 	method1 = skl_wm_method1(p->pixel_rate,
 				 p_params->bytes_per_pixel,
-				 mem_value);
+				 latency);
 	method2 = skl_wm_method2(p->pixel_rate,
 				 p->pipe_htotal,
 				 p_params->horiz_pixels,
 				 p_params->bytes_per_pixel,
-				 mem_value);
+				 latency);
 
 	plane_bytes_per_line = p_params->horiz_pixels *
 					p_params->bytes_per_pixel;
+	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
 	/* For now xtile and linear */
-	if (((ddb_allocation * 512) / plane_bytes_per_line) >= 1)
-		result_bytes = min(method1, method2);
+	if ((ddb_allocation / plane_blocks_per_line) >= 1)
+		selected_result = min(method1, method2);
 	else
-		result_bytes = method1;
+		selected_result = method1;
+
+	res_blocks = selected_result + 1;
+	res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
 
-	res_blocks = DIV_ROUND_UP(result_bytes, 512) + 1;
-	res_lines = DIV_ROUND_UP(result_bytes, plane_bytes_per_line);
+	if (level >= 1 && level <= 7)
+		res_blocks++;
 
-	if (res_blocks > ddb_allocation || res_lines > 31)
+	if (res_blocks >= ddb_allocation || res_lines > 31)
 		return false;
 
 	*out_blocks = res_blocks;
@@ -2758,23 +2769,24 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
 				 int num_planes,
 				 struct skl_wm_level *result)
 {
-	uint16_t latency = dev_priv->wm.skl_latency[level];
 	uint16_t ddb_blocks;
 	int i;
 
 	for (i = 0; i < num_planes; i++) {
 		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
 
-		result->plane_en[i] = skl_compute_plane_wm(p, &p->plane[i],
+		result->plane_en[i] = skl_compute_plane_wm(dev_priv,
+						p, &p->plane[i],
 						ddb_blocks,
-						latency,
+						level,
 						&result->plane_res_b[i],
 						&result->plane_res_l[i]);
 	}
 
 	ddb_blocks = skl_ddb_entry_size(&ddb->cursor[pipe]);
-	result->cursor_en = skl_compute_plane_wm(p, &p->cursor, ddb_blocks,
-						 latency, &result->cursor_res_b,
+	result->cursor_en = skl_compute_plane_wm(dev_priv, p, &p->cursor,
+						 ddb_blocks, level,
+						 &result->cursor_res_b,
 						 &result->cursor_res_l);
 }
 
-- 
2.3.0

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

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

* [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 6/8] drm/i915/skl: Updated watermark programming Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 15:12   ` Tvrtko Ursulin
  2015-02-27 11:15 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
  2015-02-27 15:27 ` [PATCH v5 0/8] Skylake Y tiled scanout Daniel Vetter
  8 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Display watermarks need different programming for different tiling
modes.

Set the relevant flag so this happens during the plane commit and
add relevant data into a structure made available to the watermark
computation code.

v2: Pass in tiling info to sprite plane updates as well.
v3: Rebased for plane handling changes.
v4: Handle fb == NULL when plane is disabled.
v5: Refactored for addfb2 interface.
v6: Refactored for fb modifier changes.
v7: Updated for atomic commit by only updating watermarks when tiling changes.
v8: BSpec watermark calculation updates.
v9: Restrict scope of y_tile_minimum variable. (Damien Lespiau)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Acked-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  6 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 55 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_sprite.c  |  6 ++++
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 753894d..4056fad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12022,6 +12022,12 @@ intel_check_primary_plane(struct drm_plane *plane,
 			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
 
 		intel_crtc->atomic.update_fbc = true;
+
+		/* Update watermarks on tiling changes. */
+		if (!plane->state->fb || !state->base.fb ||
+		    plane->state->fb->modifier[0] !=
+		    state->base.fb->modifier[0])
+			intel_crtc->atomic.update_wm = true;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e21d26c..c6db290 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -501,6 +501,7 @@ struct intel_plane_wm_parameters {
 	uint8_t bytes_per_pixel;
 	bool enabled;
 	bool scaled;
+	u64 tiling;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0b18e5d..39516aa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2617,7 +2617,7 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 
 static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
-			       uint32_t latency)
+			       uint64_t tiling, uint32_t latency)
 {
 	uint32_t ret;
 	uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -2627,7 +2627,16 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 		return UINT_MAX;
 
 	plane_bytes_per_line = horiz_pixels * bytes_per_pixel;
-	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+
+	if (tiling == I915_FORMAT_MOD_Y_TILED ||
+	    tiling == I915_FORMAT_MOD_Yf_TILED) {
+		plane_bytes_per_line *= 4;
+		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+		plane_blocks_per_line /= 4;
+	} else {
+		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+	}
+
 	wm_intermediate_val = latency * pixel_rate;
 	ret = DIV_ROUND_UP(wm_intermediate_val, pipe_htotal * 1000) *
 				plane_blocks_per_line;
@@ -2679,6 +2688,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	struct drm_plane *plane;
+	struct drm_framebuffer *fb;
 	int i = 1; /* Index for sprite planes start */
 
 	p->active = intel_crtc_active(crtc);
@@ -2694,6 +2704,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 			crtc->primary->fb->bits_per_pixel / 8;
 		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
 		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
+		p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
+		fb = crtc->primary->fb;
+		/*
+		 * Framebuffer can be NULL on plane disable, but it does not
+		 * matter for watermarks if we assume no tiling in that case.
+		 */
+		if (fb)
+			p->plane[0].tiling = fb->modifier[0];
 
 		p->cursor.enabled = true;
 		p->cursor.bytes_per_pixel = 4;
@@ -2734,23 +2752,34 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 p->pipe_htotal,
 				 p_params->horiz_pixels,
 				 p_params->bytes_per_pixel,
+				 p_params->tiling,
 				 latency);
 
 	plane_bytes_per_line = p_params->horiz_pixels *
 					p_params->bytes_per_pixel;
 	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
-	/* For now xtile and linear */
-	if ((ddb_allocation / plane_blocks_per_line) >= 1)
-		selected_result = min(method1, method2);
-	else
-		selected_result = method1;
+	if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+	    p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
+		uint32_t y_tile_minimum = plane_blocks_per_line * 4;
+		selected_result = max(method2, y_tile_minimum);
+	} else {
+		if ((ddb_allocation / plane_blocks_per_line) >= 1)
+			selected_result = min(method1, method2);
+		else
+			selected_result = method1;
+	}
 
 	res_blocks = selected_result + 1;
 	res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
 
-	if (level >= 1 && level <= 7)
-		res_blocks++;
+	if (level >= 1 && level <= 7) {
+		if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+		    p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
+			res_lines += 4;
+		else
+			res_blocks++;
+	}
 
 	if (res_blocks >= ddb_allocation || res_lines > 31)
 		return false;
@@ -3179,12 +3208,20 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
 		     int pixel_size, bool enabled, bool scaled)
 {
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane->fb;
 
 	intel_plane->wm.enabled = enabled;
 	intel_plane->wm.scaled = scaled;
 	intel_plane->wm.horiz_pixels = sprite_width;
 	intel_plane->wm.vert_pixels = sprite_height;
 	intel_plane->wm.bytes_per_pixel = pixel_size;
+	intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
+	/*
+	 * Framebuffer can be NULL on plane disable, but it does not
+	 * matter for watermarks if we assume no tiling in that case.
+	 */
+	if (fb)
+		intel_plane->wm.tiling = fb->modifier[0];
 
 	skl_update_wm(crtc);
 }
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b7103bd..29ec206 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1256,6 +1256,12 @@ finish:
 
 		if (!intel_crtc->primary_enabled && !state->hides_primary)
 			intel_crtc->atomic.post_enable_primary = true;
+
+		/* Update watermarks on tiling changes. */
+		if (!plane->state->fb || !state->base.fb ||
+		    plane->state->fb->modifier[0] !=
+		    state->base.fb->modifier[0])
+			intel_crtc->atomic.update_wm = true;
 	}
 
 	return 0;
-- 
2.3.0

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

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

* [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling Tvrtko Ursulin
@ 2015-02-27 11:15 ` Tvrtko Ursulin
  2015-02-27 15:27 ` [PATCH v5 0/8] Skylake Y tiled scanout Daniel Vetter
  8 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 11:15 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

By this patch all underlying bits have been implemented and this
patch actually enables the feature.

v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter)
v3: Rearrange validation checks per code review comments. (Daniel Vetter)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v1)
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4056fad..1dd0326 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12825,8 +12825,21 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		}
 	}
 
-	if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) {
-		DRM_DEBUG("hardware does not support tiling Y\n");
+	/* Passed in modifier sanity checking. */
+	switch (mode_cmd->modifier[0]) {
+	case I915_FORMAT_MOD_Y_TILED:
+	case I915_FORMAT_MOD_Yf_TILED:
+		if (INTEL_INFO(dev)->gen < 9) {
+			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
+				  mode_cmd->modifier[0]);
+			return -EINVAL;
+		}
+	case DRM_FORMAT_MOD_NONE:
+	case I915_FORMAT_MOD_X_TILED:
+		break;
+	default:
+		DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
+				mode_cmd->modifier[0]);
 		return -EINVAL;
 	}
 
-- 
2.3.0

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

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

* [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling
  2015-02-27 11:15 ` [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling Tvrtko Ursulin
@ 2015-02-27 15:12   ` Tvrtko Ursulin
  0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27 15:12 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Display watermarks need different programming for different tiling
modes.

Set the relevant flag so this happens during the plane commit and
add relevant data into a structure made available to the watermark
computation code.

v2: Pass in tiling info to sprite plane updates as well.
v3: Rebased for plane handling changes.
v4: Handle fb == NULL when plane is disabled.
v5: Refactored for addfb2 interface.
v6: Refactored for fb modifier changes.
v7: Updated for atomic commit by only updating watermarks when tiling changes.
v8: BSpec watermark calculation updates.
v9: Restrict scope of y_tile_minimum variable. (Damien Lespiau)
v10: Get fb from plane state otherwise we are working on old state.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Acked-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v9)
---
 drivers/gpu/drm/i915/intel_display.c |  6 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 55 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_sprite.c  |  6 ++++
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 753894d..4056fad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12022,6 +12022,12 @@ intel_check_primary_plane(struct drm_plane *plane,
 			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
 
 		intel_crtc->atomic.update_fbc = true;
+
+		/* Update watermarks on tiling changes. */
+		if (!plane->state->fb || !state->base.fb ||
+		    plane->state->fb->modifier[0] !=
+		    state->base.fb->modifier[0])
+			intel_crtc->atomic.update_wm = true;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e21d26c..c6db290 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -501,6 +501,7 @@ struct intel_plane_wm_parameters {
 	uint8_t bytes_per_pixel;
 	bool enabled;
 	bool scaled;
+	u64 tiling;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0b18e5d..542cf68 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2617,7 +2617,7 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 
 static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
-			       uint32_t latency)
+			       uint64_t tiling, uint32_t latency)
 {
 	uint32_t ret;
 	uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -2627,7 +2627,16 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 		return UINT_MAX;
 
 	plane_bytes_per_line = horiz_pixels * bytes_per_pixel;
-	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+
+	if (tiling == I915_FORMAT_MOD_Y_TILED ||
+	    tiling == I915_FORMAT_MOD_Yf_TILED) {
+		plane_bytes_per_line *= 4;
+		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+		plane_blocks_per_line /= 4;
+	} else {
+		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
+	}
+
 	wm_intermediate_val = latency * pixel_rate;
 	ret = DIV_ROUND_UP(wm_intermediate_val, pipe_htotal * 1000) *
 				plane_blocks_per_line;
@@ -2679,6 +2688,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	struct drm_plane *plane;
+	struct drm_framebuffer *fb;
 	int i = 1; /* Index for sprite planes start */
 
 	p->active = intel_crtc_active(crtc);
@@ -2694,6 +2704,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 			crtc->primary->fb->bits_per_pixel / 8;
 		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
 		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
+		p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
+		fb = crtc->primary->state->fb;
+		/*
+		 * Framebuffer can be NULL on plane disable, but it does not
+		 * matter for watermarks if we assume no tiling in that case.
+		 */
+		if (fb)
+			p->plane[0].tiling = fb->modifier[0];
 
 		p->cursor.enabled = true;
 		p->cursor.bytes_per_pixel = 4;
@@ -2734,23 +2752,34 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 p->pipe_htotal,
 				 p_params->horiz_pixels,
 				 p_params->bytes_per_pixel,
+				 p_params->tiling,
 				 latency);
 
 	plane_bytes_per_line = p_params->horiz_pixels *
 					p_params->bytes_per_pixel;
 	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
-	/* For now xtile and linear */
-	if ((ddb_allocation / plane_blocks_per_line) >= 1)
-		selected_result = min(method1, method2);
-	else
-		selected_result = method1;
+	if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+	    p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
+		uint32_t y_tile_minimum = plane_blocks_per_line * 4;
+		selected_result = max(method2, y_tile_minimum);
+	} else {
+		if ((ddb_allocation / plane_blocks_per_line) >= 1)
+			selected_result = min(method1, method2);
+		else
+			selected_result = method1;
+	}
 
 	res_blocks = selected_result + 1;
 	res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
 
-	if (level >= 1 && level <= 7)
-		res_blocks++;
+	if (level >= 1 && level <= 7) {
+		if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
+		    p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
+			res_lines += 4;
+		else
+			res_blocks++;
+	}
 
 	if (res_blocks >= ddb_allocation || res_lines > 31)
 		return false;
@@ -3179,12 +3208,20 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
 		     int pixel_size, bool enabled, bool scaled)
 {
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane->state->fb;
 
 	intel_plane->wm.enabled = enabled;
 	intel_plane->wm.scaled = scaled;
 	intel_plane->wm.horiz_pixels = sprite_width;
 	intel_plane->wm.vert_pixels = sprite_height;
 	intel_plane->wm.bytes_per_pixel = pixel_size;
+	intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
+	/*
+	 * Framebuffer can be NULL on plane disable, but it does not
+	 * matter for watermarks if we assume no tiling in that case.
+	 */
+	if (fb)
+		intel_plane->wm.tiling = fb->modifier[0];
 
 	skl_update_wm(crtc);
 }
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b7103bd..29ec206 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1256,6 +1256,12 @@ finish:
 
 		if (!intel_crtc->primary_enabled && !state->hides_primary)
 			intel_crtc->atomic.post_enable_primary = true;
+
+		/* Update watermarks on tiling changes. */
+		if (!plane->state->fb || !state->base.fb ||
+		    plane->state->fb->modifier[0] !=
+		    state->base.fb->modifier[0])
+			intel_crtc->atomic.update_wm = true;
 	}
 
 	return 0;
-- 
2.3.0

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

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

* Re: [PATCH v5 0/8] Skylake Y tiled scanout
  2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
                   ` (7 preceding siblings ...)
  2015-02-27 11:15 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
@ 2015-02-27 15:27 ` Daniel Vetter
  8 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-02-27 15:27 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Fri, Feb 27, 2015 at 11:15:16AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Starting with Skylake the display engine can scan out Y tiled objects. (Both
> legacy Y tiled, and the new Yf format.)
> 
> This series takes the original work by Damien Lespiau and converts it to use the
> new frame buffer modifiers instead of object set/get tiling. Some patches needed
> to be dropped, some added and some refactored.
> 
> Lightly tested with "testdisplay -m -y" and "testdisplay -m --yf".
> 
> v2: Rebased on v2 of "i915 fb modifier support, respun".
> 
> v3:
>    * Part which allows Y tiled fb creation extracted out and moved to the end
>      of series.
>    * Dropped redundant "drm/i915/skl: Allow Y tiling for sprites".
>    * Also see individual change logs.
> 
> v4:
>    * New to the series - watermark programming updates per BSpec.
>    * Addressed review comments in individual patches.
> 
> v5:
>    * Addressed review comments in individual patches.

Merged entire series, thanks.
-Daniel

> 
> Damien Lespiau (4):
>   drm/i915/skl: Allow scanning out Y and Yf fbs
>   drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling
>   drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints
>   drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling
> 
> Tvrtko Ursulin (4):
>   drm/i915/skl: Add new displayable tiling formats
>   drm/i915/skl: Updated watermark programming
>   drm/i915/skl: Update watermarks for Y tiling
>   drm/i915/skl: Allow Y (and Yf) frame buffer creation
> 
>  drivers/gpu/drm/i915/intel_display.c | 231 ++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>  drivers/gpu/drm/i915/intel_pm.c      |  97 +++++++++++----
>  drivers/gpu/drm/i915/intel_sprite.c  |  24 +++-
>  include/uapi/drm/drm_fourcc.h        |  15 +++
>  5 files changed, 283 insertions(+), 87 deletions(-)
> 
> -- 
> 2.3.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-27  9:45     ` Tvrtko Ursulin
@ 2015-02-27 14:20       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-02-27 14:20 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Fri, Feb 27, 2015 at 09:45:27AM +0000, Tvrtko Ursulin wrote:
> 
> On 02/26/2015 04:44 PM, Daniel Vetter wrote:
> >On Wed, Feb 25, 2015 at 04:47:24PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>By this patch all underlying bits have been implemented and this
> >>patch actually enables the feature.
> >>
> >>v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter)
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v1)
> >>---
> >>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
> >>  1 file changed, 21 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index 1d50934..3232ddd 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -12783,6 +12783,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >>
> >>  	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> >>+		/* Passed in modifier sanity checking. */
> >>+		switch (mode_cmd->modifier[0]) {
> >>+		case DRM_FORMAT_MOD_NONE:
> >>+		case I915_FORMAT_MOD_X_TILED:
> >>+		case I915_FORMAT_MOD_Y_TILED:
> >>+		case I915_FORMAT_MOD_Yf_TILED:
> >>+			break;
> >>+		default:
> >>+			DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
> >>+				  mode_cmd->modifier[0]);
> >>+			return -EINVAL;
> >>+		}
> >>+
> >>  		/* Enforce that fb modifier and tiling mode match, but only for
> >>  		 * X-tiled. This is needed for FBC. */
> >>  		if (!!(obj->tiling_mode == I915_TILING_X) !=
> >>@@ -12790,6 +12803,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >>  			DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
> >>  			return -EINVAL;
> >>  		}
> >>+
> >>+		if (INTEL_INFO(dev)->gen < 9 &&
> >>+		    (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> >>+		     mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
> >>+			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
> >>+				  mode_cmd->modifier[0]);
> >>+			return -EINVAL;
> >>+		}
> >>  	} else {
> >>  		if (obj->tiling_mode == I915_TILING_X)
> >>  			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> >>@@ -12799,11 +12820,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >>  		}
> >>  	}
> >>
> >>-	if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) {
> >>-		DRM_DEBUG("hardware does not support tiling Y\n");
> >>-		return -EINVAL;
> >>-	}
> >
> >My idea was actually to put the switch here (reduces one indent level) and
> >put the per-gen stuff into the switch statement too. I.e.
> >
> >     	/* Passed in modifier sanity checking. */
> >     	switch (mode_cmd->modifier[0]) {
> >     	case I915_FORMAT_MOD_Y_TILED:
> >     	case I915_FORMAT_MOD_Yf_TILED:
> >		if (INTEL_INFO(dev)->gen < 9) {
> >			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
> >				  mode_cmd->modifier[0]);
> >			return -EINVAL;
> >
> >		}
> >     	case DRM_FORMAT_MOD_NONE:
> >     	case I915_FORMAT_MOD_X_TILED:
> >     		break;
> >     	default:
> >     		DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
> >     			  mode_cmd->modifier[0]);
> >     		return -EINVAL;
> >     	}
> >
> >That gives us a natural place for extensions later on if we need to add
> >more special cases.
> 
> Yes I agree this patch being only on v2 was way disproportionate compared to
> some others from this series. ;)

Well I've obviously failed to explain properly in my review to v1 to avoid
a v3 ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-26 16:44   ` Daniel Vetter
@ 2015-02-27  9:45     ` Tvrtko Ursulin
  2015-02-27 14:20       ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-27  9:45 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel-gfx


On 02/26/2015 04:44 PM, Daniel Vetter wrote:
> On Wed, Feb 25, 2015 at 04:47:24PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> By this patch all underlying bits have been implemented and this
>> patch actually enables the feature.
>>
>> v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v1)
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
>>   1 file changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1d50934..3232ddd 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12783,6 +12783,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
>>   	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>>
>>   	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
>> +		/* Passed in modifier sanity checking. */
>> +		switch (mode_cmd->modifier[0]) {
>> +		case DRM_FORMAT_MOD_NONE:
>> +		case I915_FORMAT_MOD_X_TILED:
>> +		case I915_FORMAT_MOD_Y_TILED:
>> +		case I915_FORMAT_MOD_Yf_TILED:
>> +			break;
>> +		default:
>> +			DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
>> +				  mode_cmd->modifier[0]);
>> +			return -EINVAL;
>> +		}
>> +
>>   		/* Enforce that fb modifier and tiling mode match, but only for
>>   		 * X-tiled. This is needed for FBC. */
>>   		if (!!(obj->tiling_mode == I915_TILING_X) !=
>> @@ -12790,6 +12803,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
>>   			DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
>>   			return -EINVAL;
>>   		}
>> +
>> +		if (INTEL_INFO(dev)->gen < 9 &&
>> +		    (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>> +		     mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
>> +			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
>> +				  mode_cmd->modifier[0]);
>> +			return -EINVAL;
>> +		}
>>   	} else {
>>   		if (obj->tiling_mode == I915_TILING_X)
>>   			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
>> @@ -12799,11 +12820,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
>>   		}
>>   	}
>>
>> -	if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) {
>> -		DRM_DEBUG("hardware does not support tiling Y\n");
>> -		return -EINVAL;
>> -	}
>
> My idea was actually to put the switch here (reduces one indent level) and
> put the per-gen stuff into the switch statement too. I.e.
>
>      	/* Passed in modifier sanity checking. */
>      	switch (mode_cmd->modifier[0]) {
>      	case I915_FORMAT_MOD_Y_TILED:
>      	case I915_FORMAT_MOD_Yf_TILED:
> 		if (INTEL_INFO(dev)->gen < 9) {
> 			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
> 				  mode_cmd->modifier[0]);
> 			return -EINVAL;
>
> 		}
>      	case DRM_FORMAT_MOD_NONE:
>      	case I915_FORMAT_MOD_X_TILED:
>      		break;
>      	default:
>      		DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
>      			  mode_cmd->modifier[0]);
>      		return -EINVAL;
>      	}
>
> That gives us a natural place for extensions later on if we need to add
> more special cases.

Yes I agree this patch being only on v2 was way disproportionate 
compared to some others from this series. ;)

Regards,

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

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

* Re: [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-25 16:47 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
  2015-02-26 14:55   ` shuang.he
@ 2015-02-26 16:44   ` Daniel Vetter
  2015-02-27  9:45     ` Tvrtko Ursulin
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-02-26 16:44 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Feb 25, 2015 at 04:47:24PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> By this patch all underlying bits have been implemented and this
> patch actually enables the feature.
> 
> v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v1)
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1d50934..3232ddd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12783,6 +12783,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>  
>  	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> +		/* Passed in modifier sanity checking. */
> +		switch (mode_cmd->modifier[0]) {
> +		case DRM_FORMAT_MOD_NONE:
> +		case I915_FORMAT_MOD_X_TILED:
> +		case I915_FORMAT_MOD_Y_TILED:
> +		case I915_FORMAT_MOD_Yf_TILED:
> +			break;
> +		default:
> +			DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
> +				  mode_cmd->modifier[0]);
> +			return -EINVAL;
> +		}
> +
>  		/* Enforce that fb modifier and tiling mode match, but only for
>  		 * X-tiled. This is needed for FBC. */
>  		if (!!(obj->tiling_mode == I915_TILING_X) !=
> @@ -12790,6 +12803,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  			DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
>  			return -EINVAL;
>  		}
> +
> +		if (INTEL_INFO(dev)->gen < 9 &&
> +		    (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> +		     mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
> +			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
> +				  mode_cmd->modifier[0]);
> +			return -EINVAL;
> +		}
>  	} else {
>  		if (obj->tiling_mode == I915_TILING_X)
>  			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> @@ -12799,11 +12820,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		}
>  	}
>  
> -	if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) {
> -		DRM_DEBUG("hardware does not support tiling Y\n");
> -		return -EINVAL;
> -	}

My idea was actually to put the switch here (reduces one indent level) and
put the per-gen stuff into the switch statement too. I.e.

    	/* Passed in modifier sanity checking. */
    	switch (mode_cmd->modifier[0]) {
    	case I915_FORMAT_MOD_Y_TILED:
    	case I915_FORMAT_MOD_Yf_TILED:
		if (INTEL_INFO(dev)->gen < 9) {
			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
				  mode_cmd->modifier[0]);
			return -EINVAL;

		}
    	case DRM_FORMAT_MOD_NONE:
    	case I915_FORMAT_MOD_X_TILED:
    		break;
    	default:
    		DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
    			  mode_cmd->modifier[0]);
    		return -EINVAL;
    	}

That gives us a natural place for extensions later on if we need to add
more special cases.
-Daniel

> -
>  	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
>  						     mode_cmd->pixel_format);
>  	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> -- 
> 2.3.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-25 16:47 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
@ 2015-02-26 14:55   ` shuang.he
  2015-02-26 16:44   ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-02-26 14:55 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, tvrtko.ursulin

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5826
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              281/281              279/281
ILK                                  308/308              308/308
SNB                 -1              326/326              325/326
IVB                                  380/380              380/380
BYT                                  294/294              294/294
HSW                                  387/421              387/421
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_minor-normal-sync      DMESG_WARN(3)PASS(1)      DMESG_WARN(1)PASS(1)
*PNV  igt_gen3_render_tiledx_blits      NO_RESULT(1)PASS(5)      CRASH(1)PASS(1)
 SNB  igt_kms_rotation_crc_primary-rotation      DMESG_WARN(12)PASS(3)      DMESG_WARN(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(12)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation
  2015-02-25 16:47 [PATCH v4 " Tvrtko Ursulin
@ 2015-02-25 16:47 ` Tvrtko Ursulin
  2015-02-26 14:55   ` shuang.he
  2015-02-26 16:44   ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-02-25 16:47 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

By this patch all underlying bits have been implemented and this
patch actually enables the feature.

v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> (v1)
---
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1d50934..3232ddd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12783,6 +12783,19 @@ static int intel_framebuffer_init(struct drm_device *dev,
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
 	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
+		/* Passed in modifier sanity checking. */
+		switch (mode_cmd->modifier[0]) {
+		case DRM_FORMAT_MOD_NONE:
+		case I915_FORMAT_MOD_X_TILED:
+		case I915_FORMAT_MOD_Y_TILED:
+		case I915_FORMAT_MOD_Yf_TILED:
+			break;
+		default:
+			DRM_ERROR("Unsupported fb modifier 0x%llx!\n",
+				  mode_cmd->modifier[0]);
+			return -EINVAL;
+		}
+
 		/* Enforce that fb modifier and tiling mode match, but only for
 		 * X-tiled. This is needed for FBC. */
 		if (!!(obj->tiling_mode == I915_TILING_X) !=
@@ -12790,6 +12803,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
 			DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
 			return -EINVAL;
 		}
+
+		if (INTEL_INFO(dev)->gen < 9 &&
+		    (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+		     mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
+			DRM_DEBUG("Unsupported tiling 0x%llx!\n",
+				  mode_cmd->modifier[0]);
+			return -EINVAL;
+		}
 	} else {
 		if (obj->tiling_mode == I915_TILING_X)
 			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
@@ -12799,11 +12820,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		}
 	}
 
-	if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) {
-		DRM_DEBUG("hardware does not support tiling Y\n");
-		return -EINVAL;
-	}
-
 	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
 						     mode_cmd->pixel_format);
 	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
-- 
2.3.0

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

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

end of thread, other threads:[~2015-02-27 15:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-27 11:15 [PATCH v5 0/8] Skylake Y tiled scanout Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 1/8] drm/i915/skl: Add new displayable tiling formats Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 2/8] drm/i915/skl: Allow scanning out Y and Yf fbs Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 3/8] drm/i915/skl: Adjust intel_fb_align_height() for Yb/Yf tiling Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 4/8] drm/i915/skl: Teach pin_and_fence_fb_obj() about Y tiling constraints Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 5/8] drm/i915/skl: Adjust get_plane_config() to support Yb/Yf tiling Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 6/8] drm/i915/skl: Updated watermark programming Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling Tvrtko Ursulin
2015-02-27 15:12   ` Tvrtko Ursulin
2015-02-27 11:15 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
2015-02-27 15:27 ` [PATCH v5 0/8] Skylake Y tiled scanout Daniel Vetter
  -- strict thread matches above, loose matches on Subject: below --
2015-02-25 16:47 [PATCH v4 " Tvrtko Ursulin
2015-02-25 16:47 ` [PATCH 8/8] drm/i915/skl: Allow Y (and Yf) frame buffer creation Tvrtko Ursulin
2015-02-26 14:55   ` shuang.he
2015-02-26 16:44   ` Daniel Vetter
2015-02-27  9:45     ` Tvrtko Ursulin
2015-02-27 14:20       ` Daniel Vetter

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.