[PATCH 00/12] drm/i915: Fix up the CCS code

[PATCH 00/12] drm/i915: Fix up the CCS code

[ville.syrjala]

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

Looks like we ended up with a stale version of my CCS code in dinq.
This series contains the remainder in smaller chunks. I also ended
up adding a bunch of extra cleanup etc. on top.

The most important thing we need to get in is the change
to the fb->offsets[] interpretation since that's ABI territory.
The hash mode apparently doesn't require the nasty virtual address
alignment tricks so that shouldn't have any ABI issues after all.

Entire series available here:
git://github.com/vsyrjala/linux.git ccs_fixes

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>

Ville Syrjälä (12):
  drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
    offset
  drm/i915: Skip fence alignemnt check for the CCS plane
  drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
    CCS
  drm/i915: Add a comment exlaining CCS hsub/vsub
  drm/i915: Nuke a pointless unreachable()
  drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
  drm/i915: Clean up the sprite modifier checks
  drm/i915: Add CCS capability for sprites
  drm/i915: Allow up to 32KB stride on SKL+ "sprites"
  drm: Fix modifiers_property kernel doc
  drm: Check that the plane supports the request format+modifier combo
  drm/i915: Remove the pipe/plane ID checks from
    skl_check_ccs_aux_surface()

 drivers/gpu/drm/drm_atomic.c           |   8 +-
 drivers/gpu/drm/drm_crtc.c             |   8 +-
 drivers/gpu/drm/drm_crtc_internal.h    |   4 +-
 drivers/gpu/drm/drm_plane.c            |  31 +++++--
 drivers/gpu/drm/i915/i915_reg.h        |   8 +-
 drivers/gpu/drm/i915/intel_display.c   | 145 +++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h       |   2 +
 drivers/gpu/drm/i915/intel_engine_cs.c |  13 +++
 drivers/gpu/drm/i915/intel_pm.c        |  27 +++---
 drivers/gpu/drm/i915/intel_sprite.c    | 102 +++++++++++++++--------
 include/drm/drm_mode_config.h          |   2 +-
 11 files changed, 217 insertions(+), 133 deletions(-)

-- 
2.13.0

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

[PATCH 01/12] drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear offset

[ville.syrjala]

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

Userspace wants to treat fb->offsets[] as raw byte offsets into the gem
bo. Adjust the kernel code to match.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 108 +++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad74d1d11dbe..6f47eaca07fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2288,17 +2288,13 @@ void intel_add_fb_offsets(int *x, int *y,
 	}
 }
 
-/*
- * Input tile dimensions and pitch must already be
- * rotated to match x and y, and in pixel units.
- */
-static u32 _intel_adjust_tile_offset(int *x, int *y,
-				     unsigned int tile_width,
-				     unsigned int tile_height,
-				     unsigned int tile_size,
-				     unsigned int pitch_tiles,
-				     u32 old_offset,
-				     u32 new_offset)
+static u32 __intel_adjust_tile_offset(int *x, int *y,
+				      unsigned int tile_width,
+				      unsigned int tile_height,
+				      unsigned int tile_size,
+				      unsigned int pitch_tiles,
+				      u32 old_offset,
+				      u32 new_offset)
 {
 	unsigned int pitch_pixels = pitch_tiles * tile_width;
 	unsigned int tiles;
@@ -2319,18 +2315,13 @@ static u32 _intel_adjust_tile_offset(int *x, int *y,
 	return new_offset;
 }
 
-/*
- * Adjust the tile offset by moving the difference into
- * the x/y offsets.
- */
-static u32 intel_adjust_tile_offset(int *x, int *y,
-				    const struct intel_plane_state *state, int plane,
-				    u32 old_offset, u32 new_offset)
+static u32 _intel_adjust_tile_offset(int *x, int *y,
+				     const struct drm_framebuffer *fb, int plane,
+				     unsigned int rotation,
+				     u32 old_offset, u32 new_offset)
 {
-	const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
-	const struct drm_framebuffer *fb = state->base.fb;
+	const struct drm_i915_private *dev_priv = to_i915(fb->dev);
 	unsigned int cpp = fb->format->cpp[plane];
-	unsigned int rotation = state->base.rotation;
 	unsigned int pitch = intel_fb_pitch(fb, plane, rotation);
 
 	WARN_ON(new_offset > old_offset);
@@ -2349,9 +2340,9 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
 			pitch_tiles = pitch / (tile_width * cpp);
 		}
 
-		_intel_adjust_tile_offset(x, y, tile_width, tile_height,
-					  tile_size, pitch_tiles,
-					  old_offset, new_offset);
+		__intel_adjust_tile_offset(x, y, tile_width, tile_height,
+					   tile_size, pitch_tiles,
+					   old_offset, new_offset);
 	} else {
 		old_offset += *y * pitch + *x * cpp;
 
@@ -2363,6 +2354,19 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
 }
 
 /*
+ * Adjust the tile offset by moving the difference into
+ * the x/y offsets.
+ */
+static u32 intel_adjust_tile_offset(int *x, int *y,
+				    const struct intel_plane_state *state, int plane,
+				    u32 old_offset, u32 new_offset)
+{
+	return _intel_adjust_tile_offset(x, y, state->base.fb, plane,
+					 state->base.rotation,
+					 old_offset, new_offset);
+}
+
+/*
  * Computes the linear offset to the base tile and adjusts
  * x, y. bytes per pixel is assumed to be a power-of-two.
  *
@@ -2413,9 +2417,9 @@ static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
 		offset = (tile_rows * pitch_tiles + tiles) * tile_size;
 		offset_aligned = offset & ~alignment;
 
-		_intel_adjust_tile_offset(x, y, tile_width, tile_height,
-					  tile_size, pitch_tiles,
-					  offset, offset_aligned);
+		__intel_adjust_tile_offset(x, y, tile_width, tile_height,
+					   tile_size, pitch_tiles,
+					   offset, offset_aligned);
 	} else {
 		offset = *y * pitch + *x * cpp;
 		offset_aligned = offset & ~alignment;
@@ -2447,16 +2451,24 @@ u32 intel_compute_tile_offset(int *x, int *y,
 					  rotation, alignment);
 }
 
-/* Convert the fb->offset[] linear offset into x/y offsets */
-static void intel_fb_offset_to_xy(int *x, int *y,
-				  const struct drm_framebuffer *fb, int plane)
+/* Convert the fb->offset[] into x/y offsets */
+static int intel_fb_offset_to_xy(int *x, int *y,
+				 const struct drm_framebuffer *fb, int plane)
 {
-	unsigned int cpp = fb->format->cpp[plane];
-	unsigned int pitch = fb->pitches[plane];
-	u32 linear_offset = fb->offsets[plane];
+	struct drm_i915_private *dev_priv = to_i915(fb->dev);
+
+	if (fb->modifier != DRM_FORMAT_MOD_LINEAR &&
+	    fb->offsets[plane] % intel_tile_size(dev_priv))
+		return -EINVAL;
 
-	*y = linear_offset / pitch;
-	*x = linear_offset % pitch / cpp;
+	*x = 0;
+	*y = 0;
+
+	_intel_adjust_tile_offset(x, y,
+				  fb, plane, DRM_MODE_ROTATE_0,
+				  fb->offsets[plane], 0);
+
+	return 0;
 }
 
 static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
@@ -2523,12 +2535,18 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
 		unsigned int cpp, size;
 		u32 offset;
 		int x, y;
+		int ret;
 
 		cpp = fb->format->cpp[i];
 		width = drm_framebuffer_plane_width(fb->width, fb, i);
 		height = drm_framebuffer_plane_height(fb->height, fb, i);
 
-		intel_fb_offset_to_xy(&x, &y, fb, i);
+		ret = intel_fb_offset_to_xy(&x, &y, fb, i);
+		if (ret) {
+			DRM_DEBUG_KMS("bad fb plane %d offset: 0x%x\n",
+				      i, fb->offsets[i]);
+			return ret;
+		}
 
 		if ((fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
 		     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) && i == 1) {
@@ -2539,11 +2557,13 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
 			int ccs_x, ccs_y;
 
 			intel_tile_dims(fb, i, &tile_width, &tile_height);
+			tile_width *= hsub;
+			tile_height *= vsub;
 
-			ccs_x = (x * hsub) % (tile_width * hsub);
-			ccs_y = (y * vsub) % (tile_height * vsub);
-			main_x = intel_fb->normal[0].x % (tile_width * hsub);
-			main_y = intel_fb->normal[0].y % (tile_height * vsub);
+			ccs_x = (x * hsub) % tile_width;
+			ccs_y = (y * vsub) % tile_height;
+			main_x = intel_fb->normal[0].x % tile_width;
+			main_y = intel_fb->normal[0].y % tile_height;
 
 			/*
 			 * CCS doesn't have its own x/y offset register, so the intra CCS tile
@@ -2632,10 +2652,10 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
 			 * We only keep the x/y offsets, so push all of the
 			 * gtt offset into the x/y offsets.
 			 */
-			_intel_adjust_tile_offset(&x, &y,
-						  tile_width, tile_height,
-						  tile_size, pitch_tiles,
-						  gtt_offset_rotated * tile_size, 0);
+			__intel_adjust_tile_offset(&x, &y,
+						   tile_width, tile_height,
+						   tile_size, pitch_tiles,
+						   gtt_offset_rotated * tile_size, 0);
 
 			gtt_offset_rotated += rot_info->plane[i].width * rot_info->plane[i].height;
 
-- 
2.13.0

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

[PATCH 02/12] drm/i915: Skip fence alignemnt check for the CCS plane

[ville.syrjala]

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

The CCS won't have the same stride as the main surface anyway so trying
to guard against the fence stride not matching the CCS stride is
not sensible. Just skip the fence vs. fb alignment check for the aux
plane.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6f47eaca07fb..a8809bc64475 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2589,7 +2589,7 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
 		 * fb layout agrees with the fence layout. We already check that the
 		 * fb stride matches the fence stride elsewhere.
 		 */
-		if (i915_gem_object_is_tiled(intel_fb->obj) &&
+		if (i == 0 && i915_gem_object_is_tiled(intel_fb->obj) &&
 		    (x + width) * cpp > fb->pitches[i]) {
 			DRM_DEBUG_KMS("bad fb plane %d offset: 0x%x\n",
 				      i, fb->offsets[i]);
-- 
2.13.0

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

[PATCH 03/12] drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS

[ville.syrjala]

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

Use the LLC/eLLC hotspot avoidance mode for CCS on LLC machines. This is
reported to give better performance.

Testing has indicated that we don't need to enforce any massive 2 or 4
MiB alignment for all compressed resources even though there are still
plenty of stale comments in the spec suggesting that we do.

We do need to make sure every hardware unit that deals with the
compressed data uses the same hash mode.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h        |  8 +++++++-
 drivers/gpu/drm/i915/intel_engine_cs.c | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_pm.c        | 27 +++++++++++++--------------
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c59c590e45c4..aa354874c2c1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6909,7 +6909,7 @@ enum {
 # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
 
 #define CHICKEN_PAR1_1		_MMIO(0x42080)
-#define  SKL_RC_HASH_OUTSIDE	(1 << 15)
+#define  SKL_DE_COMPRESSED_HASH_MODE	(1 << 15)
 #define  DPA_MASK_VBLANK_SRD	(1 << 15)
 #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
 #define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
@@ -6982,6 +6982,7 @@ enum {
 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
 # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
 #define COMMON_SLICE_CHICKEN2			_MMIO(0x7014)
+# define GEN9_PBE_COMPRESSED_HASH_SELECTION	(1<<13)
 # define GEN9_DISABLE_GATHER_AT_SET_SHADER_COMMON_SLICE (1<<12)
 # define GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION (1<<8)
 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
@@ -8071,6 +8072,7 @@ enum {
 #define   GEN8_SAMPLER_POWER_BYPASS_DIS	(1<<1)
 
 #define GEN9_HALF_SLICE_CHICKEN7	_MMIO(0xe194)
+#define   GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR	(1<<8)
 #define   GEN9_ENABLE_YV12_BUGFIX	(1<<4)
 #define   GEN9_ENABLE_GPGPU_PREEMPTION	(1<<2)
 
@@ -9371,4 +9373,8 @@ enum skl_power_gate {
 #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL  0x67F1427F /*    "        " */
 #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT  0x5FF101FF /*    "        " */
 
+#define MMCD_MISC_CTRL		_MMIO(0x4ddc) /* skl+ */
+#define  MMCD_PCLA		(1 << 31)
+#define  MMCD_HOTSPOT_EN	(1 << 27)
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a6ac9d0a4156..61d9d79452c4 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -812,6 +812,19 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine)
 		I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
 			   ECOCHK_DIS_TLB);
 
+	if (HAS_LLC(dev_priv)) {
+		/* WaCompressedResourceSamplerPbeMediaNewHashMode:skl,kbl
+		 *
+		 * Must match Display Engine. See
+		 * WaCompressedResourceDisplayNewHashMode.
+		 */
+		WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
+				  GEN9_PBE_COMPRESSED_HASH_SELECTION);
+		WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
+				  GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR);
+		WA_SET_BIT(MMCD_MISC_CTRL, MMCD_PCLA | MMCD_HOTSPOT_EN);
+	}
+
 	/* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk,cfl */
 	/* WaDisablePartialInstShootdown:skl,bxt,kbl,glk,cfl */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d5ff0b9f999f..45be01ce8e68 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -58,24 +58,23 @@
 
 static void gen9_init_clock_gating(struct drm_i915_private *dev_priv)
 {
+	if (HAS_LLC(dev_priv)) {
+		/*
+		 * WaCompressedResourceDisplayNewHashMode:skl,kbl
+		 * Display WA#0390: skl,kbl
+		 *
+		 * Must match Sampler, Pixel Back End, and Media. See
+		 * WaCompressedResourceSamplerPbeMediaNewHashMode.
+		 */
+		I915_WRITE(CHICKEN_PAR1_1,
+			   I915_READ(CHICKEN_PAR1_1) |
+			   SKL_DE_COMPRESSED_HASH_MODE);
+	}
+
 	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */
 	I915_WRITE(CHICKEN_PAR1_1,
 		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
 
-	/*
-	 * Display WA#0390: skl,bxt,kbl,glk
-	 *
-	 * Must match Sampler, Pixel Back End, and Media
-	 * (0xE194 bit 8, 0x7014 bit 13, 0x4DDC bits 27 and 31).
-	 *
-	 * Including bits outside the page in the hash would
-	 * require 2 (or 4?) MiB alignment of resources. Just
-	 * assume the defaul hashing mode which only uses bits
-	 * within the page.
-	 */
-	I915_WRITE(CHICKEN_PAR1_1,
-		   I915_READ(CHICKEN_PAR1_1) & ~SKL_RC_HASH_OUTSIDE);
-
 	I915_WRITE(GEN8_CONFIG0,
 		   I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES);
 
-- 
2.13.0

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

[PATCH 04/12] drm/i915: Add a comment exlaining CCS hsub/vsub

[ville.syrjala]

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

Let's document why we claim hsub==8,vsub==16 for CCS even though the
memory layout would suggest that we use 16x8 instead.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8809bc64475..27c1a4cbce7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2484,6 +2484,13 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
 	}
 }
 
+/*
+ * 1 byte of CCS actually corresponds to 16x8 pixels on the main
+ * surface, and the memory layout for the CCS tile is 64x64 bytes.
+ * But since we're pretending the CCS tile is 128 bytes wide we
+ * adjust hsub/vsub here accordingly to 8x16 so that the
+ * bytes<->x/y conversions come out correct.
+ */
 static const struct drm_format_info ccs_formats[] = {
 	{ .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 8, .vsub = 16, },
 	{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 8, .vsub = 16, },
-- 
2.13.0

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

[PATCH 05/12] drm/i915: Nuke a pointless unreachable()

[ville.syrjala]

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

The unreachable() is very much unreachable and the compiler knows
that, so there's no point in having it.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 27c1a4cbce7d..ba351cbef93b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13078,8 +13078,6 @@ static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
 		return i965_mod_supported(format, modifier);
 	else
 		return i8xx_mod_supported(format, modifier);
-
-	unreachable();
 }
 
 static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
-- 
2.13.0

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

[PATCH 06/12] drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites

[ville.syrjala]

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

Y/Yf somehow dropped out from the SKL+ sprite modifier list. Add them
in.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 524933b01483..4b146634de3c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1080,6 +1080,8 @@ static uint32_t skl_plane_formats[] = {
 };
 
 static const uint64_t skl_plane_format_modifiers[] = {
+	I915_FORMAT_MOD_Yf_TILED,
+	I915_FORMAT_MOD_Y_TILED,
 	I915_FORMAT_MOD_X_TILED,
 	DRM_FORMAT_MOD_LINEAR,
 	DRM_FORMAT_MOD_INVALID
-- 
2.13.0

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

[PATCH 07/12] drm/i915: Clean up the sprite modifier checks

[ville.syrjala]

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

Split the g4x and snb cases into separate functions to match how we deal
with all other platforms. Also sort the switch cases to match the format
lists we've declared earlier, to ease comparisons.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 48 ++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4b146634de3c..7610f84f3525 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1087,12 +1087,9 @@ static const uint64_t skl_plane_format_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
-static bool g4x_sprite_plane_format_mod_supported(struct drm_plane *plane,
-						  uint32_t format,
-						  uint64_t modifier)
+static bool g4x_mod_supported(uint32_t format, uint64_t modifier)
 {
 	switch (format) {
-	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
@@ -1107,22 +1104,38 @@ static bool g4x_sprite_plane_format_mod_supported(struct drm_plane *plane,
 	}
 }
 
-static bool vlv_sprite_plane_format_mod_supported(struct drm_plane *plane,
-						  uint32_t format,
-						  uint64_t modifier)
+static bool snb_mod_supported(uint32_t format, uint64_t modifier)
 {
 	switch (format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+		if (modifier == DRM_FORMAT_MOD_LINEAR ||
+		    modifier == I915_FORMAT_MOD_X_TILED)
+			return true;
+		/* fall through */
+	default:
+		return false;
+	}
+}
+
+static bool vlv_mod_supported(uint32_t format, uint64_t modifier)
+{
+	switch (format) {
 	case DRM_FORMAT_RGB565:
-	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_ABGR2101010:
-	case DRM_FORMAT_XBGR8888:
-	case DRM_FORMAT_ABGR8888:
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_YVYU:
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
 		if (modifier == DRM_FORMAT_MOD_LINEAR ||
 		    modifier == I915_FORMAT_MOD_X_TILED)
 			return true;
@@ -1132,11 +1145,8 @@ static bool vlv_sprite_plane_format_mod_supported(struct drm_plane *plane,
 	}
 }
 
-static bool skl_sprite_plane_format_mod_supported(struct drm_plane *plane,
-						  uint32_t format,
-						  uint64_t modifier)
+static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 {
-	/* This is the same as primary plane since SKL has universal planes */
 	switch (format) {
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR8888:
@@ -1177,13 +1187,13 @@ static bool intel_sprite_plane_format_mod_supported(struct drm_plane *plane,
 		return false;
 
 	if (INTEL_GEN(dev_priv) >= 9)
-		return skl_sprite_plane_format_mod_supported(plane, format, modifier);
+		return skl_mod_supported(format, modifier);
 	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		return vlv_sprite_plane_format_mod_supported(plane, format, modifier);
+		return vlv_mod_supported(format, modifier);
+	else if (INTEL_GEN(dev_priv) >= 6)
+		return snb_mod_supported(format, modifier);
 	else
-		return g4x_sprite_plane_format_mod_supported(plane, format, modifier);
-
-	unreachable();
+		return g4x_mod_supported(format, modifier);
 }
 
 static const struct drm_plane_funcs intel_sprite_plane_funcs = {
-- 
2.13.0

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

[PATCH 08/12] drm/i915: Add CCS capability for sprites

[ville.syrjala]

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

Allow sprites to scan out compressed framebuffers.

Since different platforms have a different set of planes that
support CCS let's add a small helper to determine whether a
specific plane supports CCS or not. Currently that information
is spread around in many places, and not all the pieces of
code even agree with each other.

In addition to allowing sprites to scan out compressed fbs,
the other fix here is that we stop rejecting them on pipe C
on CNL.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 24 ++++--------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_sprite.c  | 49 +++++++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ba351cbef93b..d52a5e01aaf9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3120,6 +3120,7 @@ static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 {
 	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	int src_x = plane_state->base.src.x1 >> 16;
@@ -3130,17 +3131,8 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 	int y = src_y / vsub;
 	u32 offset;
 
-	switch (plane->id) {
-	case PLANE_PRIMARY:
-	case PLANE_SPRITE0:
-		break;
-	default:
-		DRM_DEBUG_KMS("RC support only on plane 1 and 2\n");
-		return -EINVAL;
-	}
-
-	if (crtc->pipe == PIPE_C) {
-		DRM_DEBUG_KMS("No RC support on pipe C\n");
+	if (!skl_plane_has_ccs(dev_priv, crtc->pipe, plane->id)) {
+		DRM_DEBUG_KMS("No RC support on %s\n", plane->base.name);
 		return -EINVAL;
 	}
 
@@ -13279,17 +13271,11 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
 
-	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 9) {
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
-		modifiers = skl_format_modifiers_ccs;
 
-		primary->update_plane = skylake_update_primary_plane;
-		primary->disable_plane = skylake_disable_primary_plane;
-	} else if (INTEL_GEN(dev_priv) >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
-		if (pipe < PIPE_C)
+		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
 		else
 			modifiers = skl_format_modifiers_noccs;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f60995fe455c..b2bec0b2e582 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1890,6 +1890,8 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
 void intel_pipe_update_start(struct intel_crtc *crtc);
 void intel_pipe_update_end(struct intel_crtc *crtc);
+bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
+		       enum pipe pipe, enum plane_id plane_id);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7610f84f3525..345873be14c3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1079,7 +1079,17 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
-static const uint64_t skl_plane_format_modifiers[] = {
+static const uint64_t skl_plane_format_modifiers_noccs[] = {
+	I915_FORMAT_MOD_Yf_TILED,
+	I915_FORMAT_MOD_Y_TILED,
+	I915_FORMAT_MOD_X_TILED,
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+static const uint64_t skl_plane_format_modifiers_ccs[] = {
+	I915_FORMAT_MOD_Yf_TILED_CCS,
+	I915_FORMAT_MOD_Y_TILED_CCS,
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
 	I915_FORMAT_MOD_X_TILED,
@@ -1152,6 +1162,10 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_ARGB8888:
 	case DRM_FORMAT_ABGR8888:
+		if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
+		    modifier == I915_FORMAT_MOD_Y_TILED_CCS)
+			return true;
+		/* fall through */
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_XBGR2101010:
@@ -1207,6 +1221,23 @@ static const struct drm_plane_funcs intel_sprite_plane_funcs = {
         .format_mod_supported = intel_sprite_plane_format_mod_supported,
 };
 
+bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
+		       enum pipe pipe, enum plane_id plane_id)
+{
+	if (plane_id == PLANE_CURSOR)
+		return false;
+
+	if (INTEL_GEN(dev_priv) >= 10)
+		return true;
+
+	if (IS_GEMINILAKE(dev_priv))
+		return pipe != PIPE_C;
+
+	return pipe != PIPE_C &&
+		(plane_id == PLANE_PRIMARY ||
+		 plane_id == PLANE_SPRITE0);
+}
+
 struct intel_plane *
 intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 			  enum pipe pipe, int plane)
@@ -1233,7 +1264,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 	}
 	intel_plane->base.state = &state->base;
 
-	if (INTEL_GEN(dev_priv) >= 10) {
+	if (INTEL_GEN(dev_priv) >= 9) {
 		intel_plane->can_scale = true;
 		state->scaler_id = -1;
 
@@ -1242,17 +1273,11 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
-		modifiers = skl_plane_format_modifiers;
-	} else if (INTEL_GEN(dev_priv) >= 9) {
-		intel_plane->can_scale = true;
-		state->scaler_id = -1;
-
-		intel_plane->update_plane = skl_update_plane;
-		intel_plane->disable_plane = skl_disable_plane;
 
-		plane_formats = skl_plane_formats;
-		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
-		modifiers = skl_plane_format_modifiers;
+		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
+			modifiers = skl_plane_format_modifiers_ccs;
+		else
+			modifiers = skl_plane_format_modifiers_noccs;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		intel_plane->can_scale = false;
 		intel_plane->max_downscale = 1;
-- 
2.13.0

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

[PATCH 09/12] drm/i915: Allow up to 32KB stride on SKL+ "sprites"

[ville.syrjala]

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

SKL+ "sprites" no longer have 16KB max stride limit that earlier
platforms had. Bump up the limit to 32KB.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 345873be14c3..ec9964adc0b0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -786,6 +786,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	struct drm_rect *src = &state->base.src;
 	struct drm_rect *dst = &state->base.dst;
 	const struct drm_rect *clip = &state->clip;
+	int max_stride = INTEL_GEN(dev_priv) >= 9 ? 32768 : 16384;
 	int hscale, vscale;
 	int max_scale, min_scale;
 	bool can_scale;
@@ -806,7 +807,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	}
 
 	/* FIXME check all gen limits */
-	if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) {
+	if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > max_stride) {
 		DRM_DEBUG_KMS("Unsuitable framebuffer for plane\n");
 		return -EINVAL;
 	}
-- 
2.13.0

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

[PATCH 10/12] drm: Fix modifiers_property kernel doc

[ville.syrjala]

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

The member is called 'modifiers_property' instead of 'modifiers'. Adjust
the kernel docs to match.

Cc: dri-devel@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_mode_config.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..6040c4b73e6d 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -758,7 +758,7 @@ struct drm_mode_config {
 	bool allow_fb_modifiers;
 
 	/**
-	 * @modifiers: Plane property to list support modifier/format
+	 * @modifiers_property: Plane property to list support modifier/format
 	 * combination.
 	 */
 	struct drm_property *modifiers_property;
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/12] drm: Check that the plane supports the request format+modifier combo

[ville.syrjala]

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

Currently we only check that the plane supports the pixel format of the
fb we're about to feed to it. Extend it to check also the modifier, and
more specifically that the combination of the format and modifier is
supported.

Cc: dri-devel@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  8 +++++---
 drivers/gpu/drm/drm_crtc.c          |  8 +++++---
 drivers/gpu/drm/drm_crtc_internal.h |  4 ++--
 drivers/gpu/drm/drm_plane.c         | 31 +++++++++++++++++++++++++------
 4 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2fd383d7253a..51cd05a7360b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -884,12 +884,14 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
 	}
 
 	/* Check whether this plane supports the fb pixel format. */
-	ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
+	ret = drm_plane_check_pixel_format(plane, state->fb->format->format,
+					   state->fb->modifier);
 	if (ret) {
 		struct drm_format_name_buf format_name;
-		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
+		DRM_DEBUG_ATOMIC("Invalid pixel format %s, modifier 0x%llx\n",
 		                 drm_get_format_name(state->fb->format->format,
-		                                     &format_name));
+		                                     &format_name),
+				 state->fb->modifier);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5af25ce5bf7c..dd54deb75c0d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -625,12 +625,14 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 		 */
 		if (!crtc->primary->format_default) {
 			ret = drm_plane_check_pixel_format(crtc->primary,
-							   fb->format->format);
+							   fb->format->format,
+							   fb->modifier);
 			if (ret) {
 				struct drm_format_name_buf format_name;
-				DRM_DEBUG_KMS("Invalid pixel format %s\n",
+				DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
 				              drm_get_format_name(fb->format->format,
-				                                  &format_name));
+				                                  &format_name),
+					      fb->modifier);
 				goto out;
 			}
 		}
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index a43582076b20..81865841b656 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -194,8 +194,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 /* drm_plane.c */
 int drm_plane_register_all(struct drm_device *dev);
 void drm_plane_unregister_all(struct drm_device *dev);
-int drm_plane_check_pixel_format(const struct drm_plane *plane,
-				 u32 format);
+int drm_plane_check_pixel_format(struct drm_plane *plane,
+				 u32 format, u64 modifier);
 
 /* drm_bridge.c */
 void drm_bridge_detach(struct drm_bridge *bridge);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 7a00351d5b5d..c63a81e32e23 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -555,16 +555,33 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 	return 0;
 }
 
-int drm_plane_check_pixel_format(const struct drm_plane *plane, u32 format)
+int drm_plane_check_pixel_format(struct drm_plane *plane,
+				 u32 format, u64 modifier)
 {
 	unsigned int i;
 
 	for (i = 0; i < plane->format_count; i++) {
 		if (format == plane->format_types[i])
-			return 0;
+			break;
+	}
+	if (i == plane->format_count)
+		return -EINVAL;
+
+	if (!plane->modifier_count)
+		return 0;
+
+	for (i = 0; i < plane->modifier_count; i++) {
+		if (modifier == plane->modifiers[i])
+			break;
 	}
+	if (i == plane->modifier_count)
+		return -EINVAL;
 
-	return -EINVAL;
+	if (plane->funcs->format_mod_supported &&
+	    !plane->funcs->format_mod_supported(plane, format, modifier))
+		return -EINVAL;
+
+	return 0;
 }
 
 /*
@@ -609,12 +626,14 @@ static int __setplane_internal(struct drm_plane *plane,
 	}
 
 	/* Check whether this plane supports the fb pixel format. */
-	ret = drm_plane_check_pixel_format(plane, fb->format->format);
+	ret = drm_plane_check_pixel_format(plane, fb->format->format,
+					   fb->modifier);
 	if (ret) {
 		struct drm_format_name_buf format_name;
-		DRM_DEBUG_KMS("Invalid pixel format %s\n",
+		DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
 		              drm_get_format_name(fb->format->format,
-		                                  &format_name));
+		                                  &format_name),
+			      fb->modifier);
 		goto out;
 	}
 
-- 
2.13.0

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

[PATCH 12/12] drm/i915: Remove the pipe/plane ID checks from skl_check_ccs_aux_surface()

[ville.syrjala]

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

The core now checks that the plane supports the fb's format+modifier
combination, so we can drop the related checks from
skl_check_ccs_aux_surface(). These checks were specific to
SKL/KBL/BXT anyway.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d52a5e01aaf9..ba12ebd8ae6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3119,9 +3119,6 @@ static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 
 static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 {
-	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-	struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	int src_x = plane_state->base.src.x1 >> 16;
 	int src_y = plane_state->base.src.y1 >> 16;
@@ -3131,11 +3128,6 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 	int y = src_y / vsub;
 	u32 offset;
 
-	if (!skl_plane_has_ccs(dev_priv, crtc->pipe, plane->id)) {
-		DRM_DEBUG_KMS("No RC support on %s\n", plane->base.name);
-		return -EINVAL;
-	}
-
 	if (plane_state->base.rotation & ~(DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180)) {
 		DRM_DEBUG_KMS("RC support only with 0/180 degree rotation %x\n",
 			      plane_state->base.rotation);
-- 
2.13.0

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

✓ Fi.CI.BAT: success for drm/i915: Fix up the CCS code

[Patchwork]

== Series Details ==

Series: drm/i915: Fix up the CCS code
URL   : https://patchwork.freedesktop.org/series/29308/
State : success

== Summary ==

Series 29308v1 drm/i915: Fix up the CCS code
https://patchwork.freedesktop.org/api/1.0/series/29308/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#102392 +2

fdo#102392 https://bugs.freedesktop.org/show_bug.cgi?id=102392

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:453s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:435s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:357s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:550s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:254s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:528s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:522s
fi-byt-n2820     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:517s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:436s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:609s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:447s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:425s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:504s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7560u     total:210  pass:205  dwarn:1   dfail:0   fail:0   skip:3  
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:603s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:535s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:468s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:477s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:485s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:405s

650ffdaee44b186cd59cbaf893d4177bc0a78aa1 drm-tip: 2017y-08m-24d-14h-29m-45s UTC integration manifest
98e730858092 drm/i915: Remove the pipe/plane ID checks from skl_check_ccs_aux_surface()
0a4a48834e23 drm: Check that the plane supports the request format+modifier combo
027ccca2d8e8 drm: Fix modifiers_property kernel doc
9444cc1bd712 drm/i915: Allow up to 32KB stride on SKL+ "sprites"
878da5624fc8 drm/i915: Add CCS capability for sprites
5b4e7bd649b2 drm/i915: Clean up the sprite modifier checks
ae345cacbe26 drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
3ad385207cb8 drm/i915: Nuke a pointless unreachable()
91e2802bfaed drm/i915: Add a comment exlaining CCS hsub/vsub
25f698d358cf drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS
52144a3d7fed drm/i915: Skip fence alignemnt check for the CCS plane
2628f6f3d9c8 drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear offset

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: Fix up the CCS code

[Patchwork]

== Series Details ==

Series: drm/i915: Fix up the CCS code
URL   : https://patchwork.freedesktop.org/series/29308/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2230 pass:1232 dwarn:0   dfail:0   fail:16  skip:982 time:9457s

== Logs ==

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

Re: [PATCH 05/12] drm/i915: Nuke a pointless unreachable()

[Ben Widawsky]

This was specifically requested by Emil.

On 17-08-24 22:10:53, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>The unreachable() is very much unreachable and the compiler knows
>that, so there's no point in having it.
>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: Jason Ekstrand <jason@jlekstrand.net>
>Cc: Daniel Stone <daniels@collabora.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/intel_display.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index 27c1a4cbce7d..ba351cbef93b 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -13078,8 +13078,6 @@ static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
> 		return i965_mod_supported(format, modifier);
> 	else
> 		return i8xx_mod_supported(format, modifier);
>-
>-	unreachable();
> }
>
> static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>-- 
>2.13.0
>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/12] drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS

[Ben Widawsky]

On 17-08-24 22:10:51, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Use the LLC/eLLC hotspot avoidance mode for CCS on LLC machines. This is
>reported to give better performance.
>

Not seeing in the diff how this only hits eLLC machines. Am I misreading when
this is needed.

>Testing has indicated that we don't need to enforce any massive 2 or 4
>MiB alignment for all compressed resources even though there are still
>plenty of stale comments in the spec suggesting that we do.
>
>We do need to make sure every hardware unit that deals with the
>compressed data uses the same hash mode.
>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: Jason Ekstrand <jason@jlekstrand.net>
>Cc: Daniel Stone <daniels@collabora.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h        |  8 +++++++-
> drivers/gpu/drm/i915/intel_engine_cs.c | 13 +++++++++++++
> drivers/gpu/drm/i915/intel_pm.c        | 27 +++++++++++++--------------
> 3 files changed, 33 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index c59c590e45c4..aa354874c2c1 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -6909,7 +6909,7 @@ enum {
> # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
>
> #define CHICKEN_PAR1_1		_MMIO(0x42080)
>-#define  SKL_RC_HASH_OUTSIDE	(1 << 15)
>+#define  SKL_DE_COMPRESSED_HASH_MODE	(1 << 15)
> #define  DPA_MASK_VBLANK_SRD	(1 << 15)
> #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> #define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
>@@ -6982,6 +6982,7 @@ enum {
> # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
> # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
> #define COMMON_SLICE_CHICKEN2			_MMIO(0x7014)
>+# define GEN9_PBE_COMPRESSED_HASH_SELECTION	(1<<13)
> # define GEN9_DISABLE_GATHER_AT_SET_SHADER_COMMON_SLICE (1<<12)
> # define GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION (1<<8)
> # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>@@ -8071,6 +8072,7 @@ enum {
> #define   GEN8_SAMPLER_POWER_BYPASS_DIS	(1<<1)
>
> #define GEN9_HALF_SLICE_CHICKEN7	_MMIO(0xe194)
>+#define   GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR	(1<<8)
> #define   GEN9_ENABLE_YV12_BUGFIX	(1<<4)
> #define   GEN9_ENABLE_GPGPU_PREEMPTION	(1<<2)
>
>@@ -9371,4 +9373,8 @@ enum skl_power_gate {
> #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL  0x67F1427F /*    "        " */
> #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT  0x5FF101FF /*    "        " */
>
>+#define MMCD_MISC_CTRL		_MMIO(0x4ddc) /* skl+ */
>+#define  MMCD_PCLA		(1 << 31)
>+#define  MMCD_HOTSPOT_EN	(1 << 27)
>+
> #endif /* _I915_REG_H_ */
>diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>index a6ac9d0a4156..61d9d79452c4 100644
>--- a/drivers/gpu/drm/i915/intel_engine_cs.c
>+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>@@ -812,6 +812,19 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine)
> 		I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
> 			   ECOCHK_DIS_TLB);
>
>+	if (HAS_LLC(dev_priv)) {
>+		/* WaCompressedResourceSamplerPbeMediaNewHashMode:skl,kbl
>+		 *
>+		 * Must match Display Engine. See
>+		 * WaCompressedResourceDisplayNewHashMode.
>+		 */
>+		WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
>+				  GEN9_PBE_COMPRESSED_HASH_SELECTION);
>+		WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
>+				  GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR);
>+		WA_SET_BIT(MMCD_MISC_CTRL, MMCD_PCLA | MMCD_HOTSPOT_EN);
>+	}
>+
> 	/* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk,cfl */
> 	/* WaDisablePartialInstShootdown:skl,bxt,kbl,glk,cfl */
> 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>index d5ff0b9f999f..45be01ce8e68 100644
>--- a/drivers/gpu/drm/i915/intel_pm.c
>+++ b/drivers/gpu/drm/i915/intel_pm.c
>@@ -58,24 +58,23 @@
>
> static void gen9_init_clock_gating(struct drm_i915_private *dev_priv)
> {
>+	if (HAS_LLC(dev_priv)) {
>+		/*
>+		 * WaCompressedResourceDisplayNewHashMode:skl,kbl
>+		 * Display WA#0390: skl,kbl
>+		 *
>+		 * Must match Sampler, Pixel Back End, and Media. See
>+		 * WaCompressedResourceSamplerPbeMediaNewHashMode.
>+		 */
>+		I915_WRITE(CHICKEN_PAR1_1,
>+			   I915_READ(CHICKEN_PAR1_1) |
>+			   SKL_DE_COMPRESSED_HASH_MODE);
>+	}
>+
> 	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */
> 	I915_WRITE(CHICKEN_PAR1_1,
> 		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
>
>-	/*
>-	 * Display WA#0390: skl,bxt,kbl,glk
>-	 *
>-	 * Must match Sampler, Pixel Back End, and Media
>-	 * (0xE194 bit 8, 0x7014 bit 13, 0x4DDC bits 27 and 31).
>-	 *
>-	 * Including bits outside the page in the hash would
>-	 * require 2 (or 4?) MiB alignment of resources. Just
>-	 * assume the defaul hashing mode which only uses bits
>-	 * within the page.
>-	 */
>-	I915_WRITE(CHICKEN_PAR1_1,
>-		   I915_READ(CHICKEN_PAR1_1) & ~SKL_RC_HASH_OUTSIDE);
>-
> 	I915_WRITE(GEN8_CONFIG0,
> 		   I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES);
>
>-- 
>2.13.0
>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/12] drm/i915: Skip fence alignemnt check for the CCS plane

[Ben Widawsky]

On 17-08-24 22:10:50, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>The CCS won't have the same stride as the main surface anyway so trying
>to guard against the fence stride not matching the CCS stride is
>not sensible. Just skip the fence vs. fb alignment check for the aux
>plane.
>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: Jason Ekstrand <jason@jlekstrand.net>
>Cc: Daniel Stone <daniels@collabora.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index 6f47eaca07fb..a8809bc64475 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -2589,7 +2589,7 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> 		 * fb layout agrees with the fence layout. We already check that the
> 		 * fb stride matches the fence stride elsewhere.
> 		 */
>-		if (i915_gem_object_is_tiled(intel_fb->obj) &&
>+		if (i == 0 && i915_gem_object_is_tiled(intel_fb->obj) &&
> 		    (x + width) * cpp > fb->pitches[i]) {
> 			DRM_DEBUG_KMS("bad fb plane %d offset: 0x%x\n",
> 				      i, fb->offsets[i]);

Patch 1 is too much churn for my eyes to review, but it's
Acked-by: Ben Widawsky <ben@bwidawsk.net>

This one is
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/12] drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites

[Daniel Stone]

On 24 August 2017 at 20:10,  <ville.syrjala@linux.intel.com> wrote:
> Y/Yf somehow dropped out from the SKL+ sprite modifier list. Add them
> in.

There's no 'somehow':
https://lists.freedesktop.org/archives/intel-gfx/2017-August/134932.html

I would prefer to not see this pushed whilst it doesn't actually work.

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

Re: [PATCH 06/12] drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites

[Ville Syrjälä]

On Fri, Aug 25, 2017 at 10:40:28AM +0100, Daniel Stone wrote:
> On 24 August 2017 at 20:10,  <ville.syrjala@linux.intel.com> wrote:
> > Y/Yf somehow dropped out from the SKL+ sprite modifier list. Add them
> > in.
> 
> There's no 'somehow':
> https://lists.freedesktop.org/archives/intel-gfx/2017-August/134932.html
> 
> I would prefer to not see this pushed whilst it doesn't actually work.

Works fine here. Well, I should say it works just as well as it does for
the primary plane. There are no plane specific checks in the wm/ddb code
IIRC so if something is broken for sprites then it's most likely equally
broken for the primary plane.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/12] drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS

[Ville Syrjälä]

On Thu, Aug 24, 2017 at 09:55:54PM -0700, Ben Widawsky wrote:
> On 17-08-24 22:10:51, Ville Syrjälä wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Use the LLC/eLLC hotspot avoidance mode for CCS on LLC machines. This is
> >reported to give better performance.
> >
> 
> Not seeing in the diff how this only hits eLLC machines. Am I misreading when
> this is needed.

It's enabled on all LLC machines, not just those that have eLLC.

> 
> >Testing has indicated that we don't need to enforce any massive 2 or 4
> >MiB alignment for all compressed resources even though there are still
> >plenty of stale comments in the spec suggesting that we do.
> >
> >We do need to make sure every hardware unit that deals with the
> >compressed data uses the same hash mode.
> >
> >Cc: Ben Widawsky <ben@bwidawsk.net>
> >Cc: Jason Ekstrand <jason@jlekstrand.net>
> >Cc: Daniel Stone <daniels@collabora.com>
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/i915_reg.h        |  8 +++++++-
> > drivers/gpu/drm/i915/intel_engine_cs.c | 13 +++++++++++++
> > drivers/gpu/drm/i915/intel_pm.c        | 27 +++++++++++++--------------
> > 3 files changed, 33 insertions(+), 15 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >index c59c590e45c4..aa354874c2c1 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -6909,7 +6909,7 @@ enum {
> > # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
> >
> > #define CHICKEN_PAR1_1		_MMIO(0x42080)
> >-#define  SKL_RC_HASH_OUTSIDE	(1 << 15)
> >+#define  SKL_DE_COMPRESSED_HASH_MODE	(1 << 15)
> > #define  DPA_MASK_VBLANK_SRD	(1 << 15)
> > #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> > #define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
> >@@ -6982,6 +6982,7 @@ enum {
> > # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
> > # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
> > #define COMMON_SLICE_CHICKEN2			_MMIO(0x7014)
> >+# define GEN9_PBE_COMPRESSED_HASH_SELECTION	(1<<13)
> > # define GEN9_DISABLE_GATHER_AT_SET_SHADER_COMMON_SLICE (1<<12)
> > # define GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION (1<<8)
> > # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
> >@@ -8071,6 +8072,7 @@ enum {
> > #define   GEN8_SAMPLER_POWER_BYPASS_DIS	(1<<1)
> >
> > #define GEN9_HALF_SLICE_CHICKEN7	_MMIO(0xe194)
> >+#define   GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR	(1<<8)
> > #define   GEN9_ENABLE_YV12_BUGFIX	(1<<4)
> > #define   GEN9_ENABLE_GPGPU_PREEMPTION	(1<<2)
> >
> >@@ -9371,4 +9373,8 @@ enum skl_power_gate {
> > #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL  0x67F1427F /*    "        " */
> > #define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT  0x5FF101FF /*    "        " */
> >
> >+#define MMCD_MISC_CTRL		_MMIO(0x4ddc) /* skl+ */
> >+#define  MMCD_PCLA		(1 << 31)
> >+#define  MMCD_HOTSPOT_EN	(1 << 27)
> >+
> > #endif /* _I915_REG_H_ */
> >diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >index a6ac9d0a4156..61d9d79452c4 100644
> >--- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >@@ -812,6 +812,19 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine)
> > 		I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
> > 			   ECOCHK_DIS_TLB);
> >
> >+	if (HAS_LLC(dev_priv)) {
> >+		/* WaCompressedResourceSamplerPbeMediaNewHashMode:skl,kbl
> >+		 *
> >+		 * Must match Display Engine. See
> >+		 * WaCompressedResourceDisplayNewHashMode.
> >+		 */
> >+		WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
> >+				  GEN9_PBE_COMPRESSED_HASH_SELECTION);
> >+		WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
> >+				  GEN9_SAMPLER_HASH_COMPRESSED_READ_ADDR);
> >+		WA_SET_BIT(MMCD_MISC_CTRL, MMCD_PCLA | MMCD_HOTSPOT_EN);
> >+	}
> >+
> > 	/* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk,cfl */
> > 	/* WaDisablePartialInstShootdown:skl,bxt,kbl,glk,cfl */
> > 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index d5ff0b9f999f..45be01ce8e68 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -58,24 +58,23 @@
> >
> > static void gen9_init_clock_gating(struct drm_i915_private *dev_priv)
> > {
> >+	if (HAS_LLC(dev_priv)) {
> >+		/*
> >+		 * WaCompressedResourceDisplayNewHashMode:skl,kbl
> >+		 * Display WA#0390: skl,kbl
> >+		 *
> >+		 * Must match Sampler, Pixel Back End, and Media. See
> >+		 * WaCompressedResourceSamplerPbeMediaNewHashMode.
> >+		 */
> >+		I915_WRITE(CHICKEN_PAR1_1,
> >+			   I915_READ(CHICKEN_PAR1_1) |
> >+			   SKL_DE_COMPRESSED_HASH_MODE);
> >+	}
> >+
> > 	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */
> > 	I915_WRITE(CHICKEN_PAR1_1,
> > 		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> >
> >-	/*
> >-	 * Display WA#0390: skl,bxt,kbl,glk
> >-	 *
> >-	 * Must match Sampler, Pixel Back End, and Media
> >-	 * (0xE194 bit 8, 0x7014 bit 13, 0x4DDC bits 27 and 31).
> >-	 *
> >-	 * Including bits outside the page in the hash would
> >-	 * require 2 (or 4?) MiB alignment of resources. Just
> >-	 * assume the defaul hashing mode which only uses bits
> >-	 * within the page.
> >-	 */
> >-	I915_WRITE(CHICKEN_PAR1_1,
> >-		   I915_READ(CHICKEN_PAR1_1) & ~SKL_RC_HASH_OUTSIDE);
> >-
> > 	I915_WRITE(GEN8_CONFIG0,
> > 		   I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES);
> >
> >-- 
> >2.13.0
> >
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/12] drm: Fix modifiers_property kernel doc

[Daniel Vetter]

On Thu, Aug 24, 2017 at 10:10:58PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The member is called 'modifiers_property' instead of 'modifiers'. Adjust
> the kernel docs to match.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pls push to drm-misc-next (imo no need to for this to land in 4.14).
-Daniel

> ---
>  include/drm/drm_mode_config.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1b37368416c8..6040c4b73e6d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -758,7 +758,7 @@ struct drm_mode_config {
>  	bool allow_fb_modifiers;
>  
>  	/**
> -	 * @modifiers: Plane property to list support modifier/format
> +	 * @modifiers_property: Plane property to list support modifier/format
>  	 * combination.
>  	 */
>  	struct drm_property *modifiers_property;
> -- 
> 2.13.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo

[Daniel Vetter]

On Thu, Aug 24, 2017 at 10:10:59PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we only check that the plane supports the pixel format of the
> fb we're about to feed to it. Extend it to check also the modifier, and
> more specifically that the combination of the format and modifier is
> supported.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I think Daniel Stone is on the hook to augment kms_ccs to properly test
this all in igt. Would be nice to add the corresponding Testcase: lines
when that's done.

With the test coverage gap addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  drivers/gpu/drm/drm_atomic.c        |  8 +++++---
>  drivers/gpu/drm/drm_crtc.c          |  8 +++++---
>  drivers/gpu/drm/drm_crtc_internal.h |  4 ++--
>  drivers/gpu/drm/drm_plane.c         | 31 +++++++++++++++++++++++++------
>  4 files changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 2fd383d7253a..51cd05a7360b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -884,12 +884,14 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>  	}
>  
>  	/* Check whether this plane supports the fb pixel format. */
> -	ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
> +	ret = drm_plane_check_pixel_format(plane, state->fb->format->format,
> +					   state->fb->modifier);
>  	if (ret) {
>  		struct drm_format_name_buf format_name;
> -		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> +		DRM_DEBUG_ATOMIC("Invalid pixel format %s, modifier 0x%llx\n",
>  		                 drm_get_format_name(state->fb->format->format,
> -		                                     &format_name));
> +		                                     &format_name),
> +				 state->fb->modifier);
>  		return ret;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5af25ce5bf7c..dd54deb75c0d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -625,12 +625,14 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  		 */
>  		if (!crtc->primary->format_default) {
>  			ret = drm_plane_check_pixel_format(crtc->primary,
> -							   fb->format->format);
> +							   fb->format->format,
> +							   fb->modifier);
>  			if (ret) {
>  				struct drm_format_name_buf format_name;
> -				DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +				DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
>  				              drm_get_format_name(fb->format->format,
> -				                                  &format_name));
> +				                                  &format_name),
> +					      fb->modifier);
>  				goto out;
>  			}
>  		}
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index a43582076b20..81865841b656 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -194,8 +194,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  /* drm_plane.c */
>  int drm_plane_register_all(struct drm_device *dev);
>  void drm_plane_unregister_all(struct drm_device *dev);
> -int drm_plane_check_pixel_format(const struct drm_plane *plane,
> -				 u32 format);
> +int drm_plane_check_pixel_format(struct drm_plane *plane,
> +				 u32 format, u64 modifier);
>  
>  /* drm_bridge.c */
>  void drm_bridge_detach(struct drm_bridge *bridge);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 7a00351d5b5d..c63a81e32e23 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -555,16 +555,33 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  	return 0;
>  }
>  
> -int drm_plane_check_pixel_format(const struct drm_plane *plane, u32 format)
> +int drm_plane_check_pixel_format(struct drm_plane *plane,
> +				 u32 format, u64 modifier)
>  {
>  	unsigned int i;
>  
>  	for (i = 0; i < plane->format_count; i++) {
>  		if (format == plane->format_types[i])
> -			return 0;
> +			break;
> +	}
> +	if (i == plane->format_count)
> +		return -EINVAL;
> +
> +	if (!plane->modifier_count)
> +		return 0;
> +
> +	for (i = 0; i < plane->modifier_count; i++) {
> +		if (modifier == plane->modifiers[i])
> +			break;
>  	}
> +	if (i == plane->modifier_count)
> +		return -EINVAL;
>  
> -	return -EINVAL;
> +	if (plane->funcs->format_mod_supported &&
> +	    !plane->funcs->format_mod_supported(plane, format, modifier))
> +		return -EINVAL;
> +
> +	return 0;
>  }
>  
>  /*
> @@ -609,12 +626,14 @@ static int __setplane_internal(struct drm_plane *plane,
>  	}
>  
>  	/* Check whether this plane supports the fb pixel format. */
> -	ret = drm_plane_check_pixel_format(plane, fb->format->format);
> +	ret = drm_plane_check_pixel_format(plane, fb->format->format,
> +					   fb->modifier);
>  	if (ret) {
>  		struct drm_format_name_buf format_name;
> -		DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +		DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
>  		              drm_get_format_name(fb->format->format,
> -		                                  &format_name));
> +		                                  &format_name),
> +			      fb->modifier);
>  		goto out;
>  	}
>  
> -- 
> 2.13.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/12] drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites

[Daniel Stone]

Hi,

On 25 August 2017 at 12:34, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Aug 25, 2017 at 10:40:28AM +0100, Daniel Stone wrote:
>> On 24 August 2017 at 20:10,  <ville.syrjala@linux.intel.com> wrote:
>> > Y/Yf somehow dropped out from the SKL+ sprite modifier list. Add them
>> > in.
>>
>> There's no 'somehow':
>> https://lists.freedesktop.org/archives/intel-gfx/2017-August/134932.html
>>
>> I would prefer to not see this pushed whilst it doesn't actually work.
>
> Works fine here. Well, I should say it works just as well as it does for
> the primary plane. There are no plane specific checks in the wm/ddb code
> IIRC so if something is broken for sprites then it's most likely equally
> broken for the primary plane.

How did you test it?

The failure mode I observed was that the primary plane had a giant
allocation, having previously had plain Y-tiling or Y-CCS enabled.
After switching the primary to linear or X-tiled, trying to configure
a 256x256 sprite plane with either Y-tiled or CCS failed, as it only
had a DDB allocation of 31 blocks, where it needed 33. So it's not
that there are plane-specific checks rejecting anything, it's that the
allocations never got drawn up to give the sprite plane enough room to
do anything other than X-tiled.

I saw this on both SKL (3200x1800 or 2560x1440) and APL (1920x1080).

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

Re: [PATCH 05/12] drm/i915: Nuke a pointless unreachable()

[Emil Velikov]

On 25 August 2017 at 05:40, Ben Widawsky <ben@bwidawsk.net> wrote:
> This was specifically requested by Emil.
>
Not quite, a gist is below.

Regardless, I do not want inspire lengthy discussions over a trivial suggestion.
Please proceed as you gents prefer.

Thanks
Emil

a)Original code

if X
   return A;
else if Y
   return B;
else
   return C;

return false;

b)My suggestion

if X
   return A;
if Y
   return B;

return C;

c)Ben's counter suggestion

if X
   return A;
else if Y
   return B;
else
   return C;

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

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Daniel Vetter]

On Thu, Aug 24, 2017 at 10:10:48PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Looks like we ended up with a stale version of my CCS code in dinq.
> This series contains the remainder in smaller chunks. I also ended
> up adding a bunch of extra cleanup etc. on top.
> 
> The most important thing we need to get in is the change
> to the fb->offsets[] interpretation since that's ABI territory.
> The hash mode apparently doesn't require the nasty virtual address
> alignment tricks so that shouldn't have any ABI issues after all.
> 
> Entire series available here:
> git://github.com/vsyrjala/linux.git ccs_fixes
> 
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>

Which of these do we need to cherry-pick over to -next-fixes? There's no
annotations about that. If the answer is "most" I'm leaning towards
disabling CCS for 4.14, minimal set would be ideal (and first in the patch
series).

Thanks, Daniel
> 
> Ville Syrjälä (12):
>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
>     offset
>   drm/i915: Skip fence alignemnt check for the CCS plane
>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
>     CCS
>   drm/i915: Add a comment exlaining CCS hsub/vsub
>   drm/i915: Nuke a pointless unreachable()
>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
>   drm/i915: Clean up the sprite modifier checks
>   drm/i915: Add CCS capability for sprites
>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
>   drm: Fix modifiers_property kernel doc
>   drm: Check that the plane supports the request format+modifier combo
>   drm/i915: Remove the pipe/plane ID checks from
>     skl_check_ccs_aux_surface()
> 
>  drivers/gpu/drm/drm_atomic.c           |   8 +-
>  drivers/gpu/drm/drm_crtc.c             |   8 +-
>  drivers/gpu/drm/drm_crtc_internal.h    |   4 +-
>  drivers/gpu/drm/drm_plane.c            |  31 +++++--
>  drivers/gpu/drm/i915/i915_reg.h        |   8 +-
>  drivers/gpu/drm/i915/intel_display.c   | 145 +++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h       |   2 +
>  drivers/gpu/drm/i915/intel_engine_cs.c |  13 +++
>  drivers/gpu/drm/i915/intel_pm.c        |  27 +++---
>  drivers/gpu/drm/i915/intel_sprite.c    | 102 +++++++++++++++--------
>  include/drm/drm_mode_config.h          |   2 +-
>  11 files changed, 217 insertions(+), 133 deletions(-)
> 
> -- 
> 2.13.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Daniel Stone]

Hi Daniel,

On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
> Which of these do we need to cherry-pick over to -next-fixes? There's no
> annotations about that. If the answer is "most" I'm leaning towards
> disabling CCS for 4.14, minimal set would be ideal (and first in the patch
> series).

My opinion below; tl;dr is that I don't think most of them are
super-critical. Ville obviously has a far stronger opinion than me on
the shape of the code, so I'm fine with this series, which seems to
mostly be a merge back of the delta between whatever Ville's latest
branch was, and whatever the last patchset Ben sent out was.

>> Ville Syrjälä (12):
>>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
>>     offset

This should land into -fixes. I trust Ville that it has no UABI
impact, but seems like something to be very consistent on.

>>   drm/i915: Skip fence alignemnt check for the CCS plane

Not sure if this is -fixes material really, just a cleanup?

>>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
>>     CCS

Not -fixes, performance optimisation.

>>   drm/i915: Add a comment exlaining CCS hsub/vsub

Seems harmless to land to -fixes.

>>   drm/i915: Nuke a pointless unreachable()

Ditto.

>>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites

Per my previous reply, NAK to landing at all, since DDB/WM allocation
seems too broken for it to work.

>>   drm/i915: Clean up the sprite modifier checks

Fine with this, but doesn't seem like -fixes material.

>>   drm/i915: Add CCS capability for sprites

NAK, same reason as Y/Yf.

>>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"

Again doesn't seem like -fixes necessarily?

>>   drm: Fix modifiers_property kernel doc

Good for -fixes.

>>   drm: Check that the plane supports the request format+modifier combo

Good for core (not Intel) -fixes.

>>   drm/i915: Remove the pipe/plane ID checks from
>>     skl_check_ccs_aux_surface()

Seems fine but probably not -fixes material; land in Intel after a merge?

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

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Ville Syrjälä]

On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
> Hi Daniel,
> 
> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
> > Which of these do we need to cherry-pick over to -next-fixes? There's no
> > annotations about that. If the answer is "most" I'm leaning towards
> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
> > series).
> 
> My opinion below; tl;dr is that I don't think most of them are
> super-critical. Ville obviously has a far stronger opinion than me on
> the shape of the code, so I'm fine with this series, which seems to
> mostly be a merge back of the delta between whatever Ville's latest
> branch was, and whatever the last patchset Ben sent out was.
> 
> >> Ville Syrjälä (12):
> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
> >>     offset
> 
> This should land into -fixes. I trust Ville that it has no UABI
> impact, but seems like something to be very consistent on.

It does change the uabi. That's the whole point. What was merged doesn't
agree with what userspace wants. So this we want in definitely so that
we don't end up exposing the wrong uabi in any released kernel.

> 
> >>   drm/i915: Skip fence alignemnt check for the CCS plane
> 
> Not sure if this is -fixes material really, just a cleanup?

It makes the kernel less likely to reject the fb entirely. So
without this userspace has to be rather careful where it places
the aux surface. I would include this as well.

> 
> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
> >>     CCS
> 
> Not -fixes, performance optimisation.

We hope. It does change the layout of the compressed data though so if
our testcases try to generate compressed data with the CPU it'll not go
well if the test assumes the wrong hash mode. I would include this as
well so that we don't end up in any kind of a mess later when we try to
change it.

So the patches were more or less sorted in priority order, and we want
at least 01,02 and maybe 03.

> 
> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
> 
> Seems harmless to land to -fixes.
> 
> >>   drm/i915: Nuke a pointless unreachable()
> 
> Ditto.
> 
> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
> 
> Per my previous reply, NAK to landing at all, since DDB/WM allocation
> seems too broken for it to work.
> 
> >>   drm/i915: Clean up the sprite modifier checks
> 
> Fine with this, but doesn't seem like -fixes material.
> 
> >>   drm/i915: Add CCS capability for sprites
> 
> NAK, same reason as Y/Yf.
> 
> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
> 
> Again doesn't seem like -fixes necessarily?
> 
> >>   drm: Fix modifiers_property kernel doc
> 
> Good for -fixes.
> 
> >>   drm: Check that the plane supports the request format+modifier combo
> 
> Good for core (not Intel) -fixes.
> 
> >>   drm/i915: Remove the pipe/plane ID checks from
> >>     skl_check_ccs_aux_surface()
> 
> Seems fine but probably not -fixes material; land in Intel after a merge?
> 
> Cheers,
> Daniel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Jani Nikula]

On Mon, 28 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
>> Hi Daniel,
>> 
>> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > Which of these do we need to cherry-pick over to -next-fixes? There's no
>> > annotations about that. If the answer is "most" I'm leaning towards
>> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
>> > series).
>> 
>> My opinion below; tl;dr is that I don't think most of them are
>> super-critical. Ville obviously has a far stronger opinion than me on
>> the shape of the code, so I'm fine with this series, which seems to
>> mostly be a merge back of the delta between whatever Ville's latest
>> branch was, and whatever the last patchset Ben sent out was.
>> 
>> >> Ville Syrjälä (12):
>> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
>> >>     offset
>> 
>> This should land into -fixes. I trust Ville that it has no UABI
>> impact, but seems like something to be very consistent on.
>
> It does change the uabi. That's the whole point. What was merged doesn't
> agree with what userspace wants. So this we want in definitely so that
> we don't end up exposing the wrong uabi in any released kernel.
>
>> 
>> >>   drm/i915: Skip fence alignemnt check for the CCS plane
>> 
>> Not sure if this is -fixes material really, just a cleanup?
>
> It makes the kernel less likely to reject the fb entirely. So
> without this userspace has to be rather careful where it places
> the aux surface. I would include this as well.
>
>> 
>> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
>> >>     CCS
>> 
>> Not -fixes, performance optimisation.
>
> We hope. It does change the layout of the compressed data though so if
> our testcases try to generate compressed data with the CPU it'll not go
> well if the test assumes the wrong hash mode. I would include this as
> well so that we don't end up in any kind of a mess later when we try to
> change it.
>
> So the patches were more or less sorted in priority order, and we want
> at least 01,02 and maybe 03.

When you decide what to apply, please *please* add the appropriate
Fixes: tags for the ones you want to show up in v4.14.

BR,
Jani.


>
>> 
>> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
>> 
>> Seems harmless to land to -fixes.
>> 
>> >>   drm/i915: Nuke a pointless unreachable()
>> 
>> Ditto.
>> 
>> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
>> 
>> Per my previous reply, NAK to landing at all, since DDB/WM allocation
>> seems too broken for it to work.
>> 
>> >>   drm/i915: Clean up the sprite modifier checks
>> 
>> Fine with this, but doesn't seem like -fixes material.
>> 
>> >>   drm/i915: Add CCS capability for sprites
>> 
>> NAK, same reason as Y/Yf.
>> 
>> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
>> 
>> Again doesn't seem like -fixes necessarily?
>> 
>> >>   drm: Fix modifiers_property kernel doc
>> 
>> Good for -fixes.
>> 
>> >>   drm: Check that the plane supports the request format+modifier combo
>> 
>> Good for core (not Intel) -fixes.
>> 
>> >>   drm/i915: Remove the pipe/plane ID checks from
>> >>     skl_check_ccs_aux_surface()
>> 
>> Seems fine but probably not -fixes material; land in Intel after a merge?
>> 
>> Cheers,
>> Daniel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Ville Syrjälä]

On Wed, Aug 30, 2017 at 11:31:16AM +0300, Jani Nikula wrote:
> On Mon, 28 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
> >> Hi Daniel,
> >> 
> >> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> > Which of these do we need to cherry-pick over to -next-fixes? There's no
> >> > annotations about that. If the answer is "most" I'm leaning towards
> >> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
> >> > series).
> >> 
> >> My opinion below; tl;dr is that I don't think most of them are
> >> super-critical. Ville obviously has a far stronger opinion than me on
> >> the shape of the code, so I'm fine with this series, which seems to
> >> mostly be a merge back of the delta between whatever Ville's latest
> >> branch was, and whatever the last patchset Ben sent out was.
> >> 
> >> >> Ville Syrjälä (12):
> >> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
> >> >>     offset
> >> 
> >> This should land into -fixes. I trust Ville that it has no UABI
> >> impact, but seems like something to be very consistent on.
> >
> > It does change the uabi. That's the whole point. What was merged doesn't
> > agree with what userspace wants. So this we want in definitely so that
> > we don't end up exposing the wrong uabi in any released kernel.
> >
> >> 
> >> >>   drm/i915: Skip fence alignemnt check for the CCS plane
> >> 
> >> Not sure if this is -fixes material really, just a cleanup?
> >
> > It makes the kernel less likely to reject the fb entirely. So
> > without this userspace has to be rather careful where it places
> > the aux surface. I would include this as well.
> >
> >> 
> >> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
> >> >>     CCS
> >> 
> >> Not -fixes, performance optimisation.
> >
> > We hope. It does change the layout of the compressed data though so if
> > our testcases try to generate compressed data with the CPU it'll not go
> > well if the test assumes the wrong hash mode. I would include this as
> > well so that we don't end up in any kind of a mess later when we try to
> > change it.
> >
> > So the patches were more or less sorted in priority order, and we want
> > at least 01,02 and maybe 03.
> 
> When you decide what to apply, please *please* add the appropriate
> Fixes: tags for the ones you want to show up in v4.14.

I just pushed 01 and 02 to dinq with the approriage Fixes: tags.
I'd still prefer to get 03 in as well, but that would need an
r-b/ack.

> 
> BR,
> Jani.
> 
> 
> >
> >> 
> >> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
> >> 
> >> Seems harmless to land to -fixes.
> >> 
> >> >>   drm/i915: Nuke a pointless unreachable()
> >> 
> >> Ditto.
> >> 
> >> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
> >> 
> >> Per my previous reply, NAK to landing at all, since DDB/WM allocation
> >> seems too broken for it to work.
> >> 
> >> >>   drm/i915: Clean up the sprite modifier checks
> >> 
> >> Fine with this, but doesn't seem like -fixes material.
> >> 
> >> >>   drm/i915: Add CCS capability for sprites
> >> 
> >> NAK, same reason as Y/Yf.
> >> 
> >> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
> >> 
> >> Again doesn't seem like -fixes necessarily?
> >> 
> >> >>   drm: Fix modifiers_property kernel doc
> >> 
> >> Good for -fixes.
> >> 
> >> >>   drm: Check that the plane supports the request format+modifier combo
> >> 
> >> Good for core (not Intel) -fixes.
> >> 
> >> >>   drm/i915: Remove the pipe/plane ID checks from
> >> >>     skl_check_ccs_aux_surface()
> >> 
> >> Seems fine but probably not -fixes material; land in Intel after a merge?
> >> 
> >> Cheers,
> >> Daniel
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Rodrigo Vivi]

Hi Ville,

On Wed, Aug 30, 2017 at 10:09 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Aug 30, 2017 at 11:31:16AM +0300, Jani Nikula wrote:
>> On Mon, 28 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
>> >> Hi Daniel,
>> >>
>> >> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> > Which of these do we need to cherry-pick over to -next-fixes? There's no
>> >> > annotations about that. If the answer is "most" I'm leaning towards
>> >> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
>> >> > series).
>> >>
>> >> My opinion below; tl;dr is that I don't think most of them are
>> >> super-critical. Ville obviously has a far stronger opinion than me on
>> >> the shape of the code, so I'm fine with this series, which seems to
>> >> mostly be a merge back of the delta between whatever Ville's latest
>> >> branch was, and whatever the last patchset Ben sent out was.
>> >>
>> >> >> Ville Syrjälä (12):
>> >> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
>> >> >>     offset
>> >>
>> >> This should land into -fixes. I trust Ville that it has no UABI
>> >> impact, but seems like something to be very consistent on.
>> >
>> > It does change the uabi. That's the whole point. What was merged doesn't
>> > agree with what userspace wants. So this we want in definitely so that
>> > we don't end up exposing the wrong uabi in any released kernel.
>> >
>> >>
>> >> >>   drm/i915: Skip fence alignemnt check for the CCS plane
>> >>
>> >> Not sure if this is -fixes material really, just a cleanup?
>> >
>> > It makes the kernel less likely to reject the fb entirely. So
>> > without this userspace has to be rather careful where it places
>> > the aux surface. I would include this as well.
>> >
>> >>
>> >> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
>> >> >>     CCS
>> >>
>> >> Not -fixes, performance optimisation.
>> >
>> > We hope. It does change the layout of the compressed data though so if
>> > our testcases try to generate compressed data with the CPU it'll not go
>> > well if the test assumes the wrong hash mode. I would include this as
>> > well so that we don't end up in any kind of a mess later when we try to
>> > change it.
>> >
>> > So the patches were more or less sorted in priority order, and we want
>> > at least 01,02 and maybe 03.
>>
>> When you decide what to apply, please *please* add the appropriate
>> Fixes: tags for the ones you want to show up in v4.14.
>
> I just pushed 01 and 02 to dinq with the approriage Fixes: tags.
> I'd still prefer to get 03 in as well, but that would need an
> r-b/ack.
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> >>
>> >> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
>> >>
>> >> Seems harmless to land to -fixes.
>> >>
>> >> >>   drm/i915: Nuke a pointless unreachable()
>> >>
>> >> Ditto.
>> >>
>> >> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
>> >>
>> >> Per my previous reply, NAK to landing at all, since DDB/WM allocation
>> >> seems too broken for it to work.
>> >>
>> >> >>   drm/i915: Clean up the sprite modifier checks
>> >>
>> >> Fine with this, but doesn't seem like -fixes material.
>> >>
>> >> >>   drm/i915: Add CCS capability for sprites
>> >>
>> >> NAK, same reason as Y/Yf.
>> >>
>> >> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
>> >>
>> >> Again doesn't seem like -fixes necessarily?
>> >>
>> >> >>   drm: Fix modifiers_property kernel doc
>> >>
>> >> Good for -fixes.
>> >>
>> >> >>   drm: Check that the plane supports the request format+modifier combo
>> >>
>> >> Good for core (not Intel) -fixes.
>> >>
>> >> >>   drm/i915: Remove the pipe/plane ID checks from
>> >> >>     skl_check_ccs_aux_surface()
>> >>
>> >> Seems fine but probably not -fixes material; land in Intel after a merge?
>> >>
>> >> Cheers,
>> >> Daniel

Should I wait any more of this for drm-intel-next-fixes?

Otherwise I will move with the pull request.

Thanks,
Rodrigo.

>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Ville Syrjälä]

On Thu, Aug 31, 2017 at 05:05:01PM -0700, Rodrigo Vivi wrote:
> Hi Ville,
> 
> On Wed, Aug 30, 2017 at 10:09 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Aug 30, 2017 at 11:31:16AM +0300, Jani Nikula wrote:
> >> On Mon, 28 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> > On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
> >> >> Hi Daniel,
> >> >>
> >> >> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> >> > Which of these do we need to cherry-pick over to -next-fixes? There's no
> >> >> > annotations about that. If the answer is "most" I'm leaning towards
> >> >> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
> >> >> > series).
> >> >>
> >> >> My opinion below; tl;dr is that I don't think most of them are
> >> >> super-critical. Ville obviously has a far stronger opinion than me on
> >> >> the shape of the code, so I'm fine with this series, which seems to
> >> >> mostly be a merge back of the delta between whatever Ville's latest
> >> >> branch was, and whatever the last patchset Ben sent out was.
> >> >>
> >> >> >> Ville Syrjälä (12):
> >> >> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
> >> >> >>     offset
> >> >>
> >> >> This should land into -fixes. I trust Ville that it has no UABI
> >> >> impact, but seems like something to be very consistent on.
> >> >
> >> > It does change the uabi. That's the whole point. What was merged doesn't
> >> > agree with what userspace wants. So this we want in definitely so that
> >> > we don't end up exposing the wrong uabi in any released kernel.
> >> >
> >> >>
> >> >> >>   drm/i915: Skip fence alignemnt check for the CCS plane
> >> >>
> >> >> Not sure if this is -fixes material really, just a cleanup?
> >> >
> >> > It makes the kernel less likely to reject the fb entirely. So
> >> > without this userspace has to be rather careful where it places
> >> > the aux surface. I would include this as well.
> >> >
> >> >>
> >> >> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
> >> >> >>     CCS
> >> >>
> >> >> Not -fixes, performance optimisation.
> >> >
> >> > We hope. It does change the layout of the compressed data though so if
> >> > our testcases try to generate compressed data with the CPU it'll not go
> >> > well if the test assumes the wrong hash mode. I would include this as
> >> > well so that we don't end up in any kind of a mess later when we try to
> >> > change it.
> >> >
> >> > So the patches were more or less sorted in priority order, and we want
> >> > at least 01,02 and maybe 03.
> >>
> >> When you decide what to apply, please *please* add the appropriate
> >> Fixes: tags for the ones you want to show up in v4.14.
> >
> > I just pushed 01 and 02 to dinq with the approriage Fixes: tags.
> > I'd still prefer to get 03 in as well, but that would need an
> > r-b/ack.
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >
> >> >>
> >> >> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
> >> >>
> >> >> Seems harmless to land to -fixes.
> >> >>
> >> >> >>   drm/i915: Nuke a pointless unreachable()
> >> >>
> >> >> Ditto.
> >> >>
> >> >> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
> >> >>
> >> >> Per my previous reply, NAK to landing at all, since DDB/WM allocation
> >> >> seems too broken for it to work.
> >> >>
> >> >> >>   drm/i915: Clean up the sprite modifier checks
> >> >>
> >> >> Fine with this, but doesn't seem like -fixes material.
> >> >>
> >> >> >>   drm/i915: Add CCS capability for sprites
> >> >>
> >> >> NAK, same reason as Y/Yf.
> >> >>
> >> >> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
> >> >>
> >> >> Again doesn't seem like -fixes necessarily?
> >> >>
> >> >> >>   drm: Fix modifiers_property kernel doc
> >> >>
> >> >> Good for -fixes.
> >> >>
> >> >> >>   drm: Check that the plane supports the request format+modifier combo
> >> >>
> >> >> Good for core (not Intel) -fixes.
> >> >>
> >> >> >>   drm/i915: Remove the pipe/plane ID checks from
> >> >> >>     skl_check_ccs_aux_surface()
> >> >>
> >> >> Seems fine but probably not -fixes material; land in Intel after a merge?
> >> >>
> >> >> Cheers,
> >> >> Daniel
> 
> Should I wait any more of this for drm-intel-next-fixes?
> 
> Otherwise I will move with the pull request.

Go ahead with the pull request. We should be able to live with just the
first two patches for now.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/12] drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS

[Ben Widawsky]

On 17-08-24 22:10:51, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Use the LLC/eLLC hotspot avoidance mode for CCS on LLC machines. This is
>reported to give better performance.
>
>Testing has indicated that we don't need to enforce any massive 2 or 4
>MiB alignment for all compressed resources even though there are still
>plenty of stale comments in the spec suggesting that we do.
>
>We do need to make sure every hardware unit that deals with the
>compressed data uses the same hash mode.
>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: Jason Ekstrand <jason@jlekstrand.net>
>Cc: Daniel Stone <daniels@collabora.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

[snip]

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/12] drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for CCS

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 04:29:05PM -0700, Ben Widawsky wrote:
> On 17-08-24 22:10:51, Ville Syrjälä wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Use the LLC/eLLC hotspot avoidance mode for CCS on LLC machines. This is
> >reported to give better performance.
> >
> >Testing has indicated that we don't need to enforce any massive 2 or 4
> >MiB alignment for all compressed resources even though there are still
> >plenty of stale comments in the spec suggesting that we do.
> >
> >We do need to make sure every hardware unit that deals with the
> >compressed data uses the same hash mode.
> >
> >Cc: Ben Widawsky <ben@bwidawsk.net>
> >Cc: Jason Ekstrand <jason@jlekstrand.net>
> >Cc: Daniel Stone <daniels@collabora.com>
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> [snip]
> 
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Thanks. Patch pushed to dinq.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] drm/i915: Fix up the CCS code

[Kristian Høgsberg]

On Fri, Sep 1, 2017 at 6:46 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Aug 31, 2017 at 05:05:01PM -0700, Rodrigo Vivi wrote:
>> Hi Ville,
>>
>> On Wed, Aug 30, 2017 at 10:09 AM, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>> > On Wed, Aug 30, 2017 at 11:31:16AM +0300, Jani Nikula wrote:
>> >> On Mon, 28 Aug 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> >> > On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
>> >> >> Hi Daniel,
>> >> >>
>> >> >> On 25 August 2017 at 18:17, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> >> > Which of these do we need to cherry-pick over to -next-fixes? There's no
>> >> >> > annotations about that. If the answer is "most" I'm leaning towards
>> >> >> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
>> >> >> > series).
>> >> >>
>> >> >> My opinion below; tl;dr is that I don't think most of them are
>> >> >> super-critical. Ville obviously has a far stronger opinion than me on
>> >> >> the shape of the code, so I'm fine with this series, which seems to
>> >> >> mostly be a merge back of the delta between whatever Ville's latest
>> >> >> branch was, and whatever the last patchset Ben sent out was.
>> >> >>
>> >> >> >> Ville Syrjälä (12):
>> >> >> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
>> >> >> >>     offset
>> >> >>
>> >> >> This should land into -fixes. I trust Ville that it has no UABI
>> >> >> impact, but seems like something to be very consistent on.
>> >> >
>> >> > It does change the uabi. That's the whole point. What was merged doesn't
>> >> > agree with what userspace wants. So this we want in definitely so that
>> >> > we don't end up exposing the wrong uabi in any released kernel.
>> >> >
>> >> >>
>> >> >> >>   drm/i915: Skip fence alignemnt check for the CCS plane
>> >> >>
>> >> >> Not sure if this is -fixes material really, just a cleanup?
>> >> >
>> >> > It makes the kernel less likely to reject the fb entirely. So
>> >> > without this userspace has to be rather careful where it places
>> >> > the aux surface. I would include this as well.
>> >> >
>> >> >>
>> >> >> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
>> >> >> >>     CCS
>> >> >>
>> >> >> Not -fixes, performance optimisation.
>> >> >
>> >> > We hope. It does change the layout of the compressed data though so if
>> >> > our testcases try to generate compressed data with the CPU it'll not go
>> >> > well if the test assumes the wrong hash mode. I would include this as
>> >> > well so that we don't end up in any kind of a mess later when we try to
>> >> > change it.
>> >> >
>> >> > So the patches were more or less sorted in priority order, and we want
>> >> > at least 01,02 and maybe 03.
>> >>
>> >> When you decide what to apply, please *please* add the appropriate
>> >> Fixes: tags for the ones you want to show up in v4.14.
>> >
>> > I just pushed 01 and 02 to dinq with the approriage Fixes: tags.
>> > I'd still prefer to get 03 in as well, but that would need an
>> > r-b/ack.
>> >
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >> >
>> >> >>
>> >> >> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
>> >> >>
>> >> >> Seems harmless to land to -fixes.
>> >> >>
>> >> >> >>   drm/i915: Nuke a pointless unreachable()
>> >> >>
>> >> >> Ditto.
>> >> >>
>> >> >> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
>> >> >>
>> >> >> Per my previous reply, NAK to landing at all, since DDB/WM allocation
>> >> >> seems too broken for it to work.
>> >> >>
>> >> >> >>   drm/i915: Clean up the sprite modifier checks
>> >> >>
>> >> >> Fine with this, but doesn't seem like -fixes material.
>> >> >>
>> >> >> >>   drm/i915: Add CCS capability for sprites
>> >> >>
>> >> >> NAK, same reason as Y/Yf.

What's the status for CCS_E on the overlay plane?

thanks,
Kristian

>> >> >>
>> >> >> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
>> >> >>
>> >> >> Again doesn't seem like -fixes necessarily?
>> >> >>
>> >> >> >>   drm: Fix modifiers_property kernel doc
>> >> >>
>> >> >> Good for -fixes.
>> >> >>
>> >> >> >>   drm: Check that the plane supports the request format+modifier combo
>> >> >>
>> >> >> Good for core (not Intel) -fixes.
>> >> >>
>> >> >> >>   drm/i915: Remove the pipe/plane ID checks from
>> >> >> >>     skl_check_ccs_aux_surface()
>> >> >>
>> >> >> Seems fine but probably not -fixes material; land in Intel after a merge?
>> >> >>
>> >> >> Cheers,
>> >> >> Daniel
>>
>> Should I wait any more of this for drm-intel-next-fixes?
>>
>> Otherwise I will move with the pull request.
>
> Go ahead with the pull request. We should be able to live with just the
> first two patches for now.
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Mika Kahola]

On Thu, 2017-08-24 at 22:10 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Allow sprites to scan out compressed framebuffers.
> 
> Since different platforms have a different set of planes that
> support CCS let's add a small helper to determine whether a
> specific plane supports CCS or not. Currently that information
> is spread around in many places, and not all the pieces of
> code even agree with each other.
> 
> In addition to allowing sprites to scan out compressed fbs,
> the other fix here is that we stop rejecting them on pipe C
> on CNL.
Unfortunately, this patch didn't apply cleanly on top of the latest
drm-tip. Overall, the patch looks good to me. 

> 
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 24 ++++--------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_sprite.c  | 49
> +++++++++++++++++++++++++++---------
>  3 files changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ba351cbef93b..d52a5e01aaf9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3120,6 +3120,7 @@ static int skl_check_nv12_aux_surface(struct
> intel_plane_state *plane_state)
>  static int skl_check_ccs_aux_surface(struct intel_plane_state
> *plane_state)
>  {
>  	struct intel_plane *plane = to_intel_plane(plane_state-
> >base.plane);
> +	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
Seems that this addition is not really needed.

>  	struct intel_crtc *crtc = to_intel_crtc(plane_state-
> >base.crtc);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	int src_x = plane_state->base.src.x1 >> 16;
> @@ -3130,17 +3131,8 @@ static int skl_check_ccs_aux_surface(struct
> intel_plane_state *plane_state)
>  	int y = src_y / vsub;
>  	u32 offset;
>  
> -	switch (plane->id) {
> -	case PLANE_PRIMARY:
> -	case PLANE_SPRITE0:
> -		break;
> -	default:
> -		DRM_DEBUG_KMS("RC support only on plane 1 and 2\n");
> -		return -EINVAL;
> -	}
> -
> -	if (crtc->pipe == PIPE_C) {
> -		DRM_DEBUG_KMS("No RC support on pipe C\n");
> +	if (!skl_plane_has_ccs(dev_priv, crtc->pipe, plane->id)) {
> +		DRM_DEBUG_KMS("No RC support on %s\n", plane-
> >base.name);
>  		return -EINVAL;
>  	}
>  
> @@ -13279,17 +13271,11 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
>  
> -	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_primary_formats = skl_primary_formats;
>  		num_formats = ARRAY_SIZE(skl_primary_formats);
> -		modifiers = skl_format_modifiers_ccs;
>  
> -		primary->update_plane =
> skylake_update_primary_plane;
> -		primary->disable_plane =
> skylake_disable_primary_plane;
> -	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> -		if (pipe < PIPE_C)
> +		if (skl_plane_has_ccs(dev_priv, pipe,
> PLANE_PRIMARY))
>  			modifiers = skl_format_modifiers_ccs;
>  		else
>  			modifiers = skl_format_modifiers_noccs;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f60995fe455c..b2bec0b2e582 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1890,6 +1890,8 @@ int intel_sprite_set_colorkey(struct drm_device
> *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_pipe_update_start(struct intel_crtc *crtc);
>  void intel_pipe_update_end(struct intel_crtc *crtc);
> +bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> +		       enum pipe pipe, enum plane_id plane_id);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 7610f84f3525..345873be14c3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1079,7 +1079,17 @@ static uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> -static const uint64_t skl_plane_format_modifiers[] = {
> +static const uint64_t skl_plane_format_modifiers_noccs[] = {
> +	I915_FORMAT_MOD_Yf_TILED,
> +	I915_FORMAT_MOD_Y_TILED,
> +	I915_FORMAT_MOD_X_TILED,
> +	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_INVALID
> +};
> +
> +static const uint64_t skl_plane_format_modifiers_ccs[] = {
> +	I915_FORMAT_MOD_Yf_TILED_CCS,
> +	I915_FORMAT_MOD_Y_TILED_CCS,
>  	I915_FORMAT_MOD_Yf_TILED,
>  	I915_FORMAT_MOD_Y_TILED,
>  	I915_FORMAT_MOD_X_TILED,
> @@ -1152,6 +1162,10 @@ static bool skl_mod_supported(uint32_t format,
> uint64_t modifier)
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_ARGB8888:
>  	case DRM_FORMAT_ABGR8888:
> +		if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> +		    modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> +			return true;
> +		/* fall through */
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
> @@ -1207,6 +1221,23 @@ static const struct drm_plane_funcs
> intel_sprite_plane_funcs = {
>          .format_mod_supported =
> intel_sprite_plane_format_mod_supported,
>  };
>  
> +bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> +		       enum pipe pipe, enum plane_id plane_id)
> +{
> +	if (plane_id == PLANE_CURSOR)
> +		return false;
> +
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		return true;
> +
> +	if (IS_GEMINILAKE(dev_priv))
> +		return pipe != PIPE_C;
> +
> +	return pipe != PIPE_C &&
> +		(plane_id == PLANE_PRIMARY ||
> +		 plane_id == PLANE_SPRITE0);
> +}
> +
>  struct intel_plane *
>  intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  			  enum pipe pipe, int plane)
> @@ -1233,7 +1264,7 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
>  	}
>  	intel_plane->base.state = &state->base;
>  
> -	if (INTEL_GEN(dev_priv) >= 10) {
> +	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_plane->can_scale = true;
>  		state->scaler_id = -1;
>  
> @@ -1242,17 +1273,11 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
>  
>  		plane_formats = skl_plane_formats;
>  		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> -		modifiers = skl_plane_format_modifiers;
> -	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_plane->can_scale = true;
> -		state->scaler_id = -1;
> -
> -		intel_plane->update_plane = skl_update_plane;
> -		intel_plane->disable_plane = skl_disable_plane;
>  
> -		plane_formats = skl_plane_formats;
> -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> -		modifiers = skl_plane_format_modifiers;
> +		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0
> + plane))
> +			modifiers = skl_plane_format_modifiers_ccs;
> +		else
> +			modifiers =
> skl_plane_format_modifiers_noccs;
>  	} else if (IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv)) {
>  		intel_plane->can_scale = false;
>  		intel_plane->max_downscale = 1;
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Daniel Stone]

Hi Mika,

On 11 December 2017 at 11:11, Mika Kahola <mika.kahola@intel.com> wrote:
> On Thu, 2017-08-24 at 22:10 +0300, ville.syrjala@linux.intel.com wrote:
>> Allow sprites to scan out compressed framebuffers.
>>
>> Since different platforms have a different set of planes that
>> support CCS let's add a small helper to determine whether a
>> specific plane supports CCS or not. Currently that information
>> is spread around in many places, and not all the pieces of
>> code even agree with each other.
>>
>> In addition to allowing sprites to scan out compressed fbs,
>> the other fix here is that we stop rejecting them on pipe C
>> on CNL.
> Unfortunately, this patch didn't apply cleanly on top of the latest
> drm-tip. Overall, the patch looks good to me.

Did you manage to test this? When I tried, the DDB/watermark
allocation was too conservative for sprites, and never allowed enough
blocks to be able to use anything but linear or X-tiled on sprite
planes. The allocation was almost entirely taken up by the primary
plane, even if that itself was later set to linear or X-tiled.

I observed this on SKL with both 3200x1800 and 2560x1440 output sizes
(and primary plane sizes; no scaling), as well as APL with 1920x1080.

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

Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Mika Kahola]

On Mon, 2017-12-11 at 12:00 +0000, Daniel Stone wrote:
> Hi Mika,
> 
> On 11 December 2017 at 11:11, Mika Kahola <mika.kahola@intel.com>
> wrote:
> > 
> > On Thu, 2017-08-24 at 22:10 +0300, ville.syrjala@linux.intel.com
> > wrote:
> > > 
> > > Allow sprites to scan out compressed framebuffers.
> > > 
> > > Since different platforms have a different set of planes that
> > > support CCS let's add a small helper to determine whether a
> > > specific plane supports CCS or not. Currently that information
> > > is spread around in many places, and not all the pieces of
> > > code even agree with each other.
> > > 
> > > In addition to allowing sprites to scan out compressed fbs,
> > > the other fix here is that we stop rejecting them on pipe C
> > > on CNL.
> > Unfortunately, this patch didn't apply cleanly on top of the latest
> > drm-tip. Overall, the patch looks good to me.
> Did you manage to test this? When I tried, the DDB/watermark
> allocation was too conservative for sprites, and never allowed enough
> blocks to be able to use anything but linear or X-tiled on sprite
> planes. The allocation was almost entirely taken up by the primary
> plane, even if that itself was later set to linear or X-tiled.
I tried to test this but unfortunately this didn't apply on top of drm-
tip. The whole series dates back to August so a lot have changed since
then.

I would need this patch for my CNL work so I would like to see this
merged.
 
> 
> I observed this on SKL with both 3200x1800 and 2560x1440 output sizes
> (and primary plane sizes; no scaling), as well as APL with 1920x1080.
> 
> Cheers,
> Daniel
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Daniel Stone]

Hi,

On 11 December 2017 at 12:08, Mika Kahola <mika.kahola@intel.com> wrote:
> On Mon, 2017-12-11 at 12:00 +0000, Daniel Stone wrote:
>> Did you manage to test this? When I tried, the DDB/watermark
>> allocation was too conservative for sprites, and never allowed enough
>> blocks to be able to use anything but linear or X-tiled on sprite
>> planes. The allocation was almost entirely taken up by the primary
>> plane, even if that itself was later set to linear or X-tiled.
>
> I tried to test this but unfortunately this didn't apply on top of drm-
> tip. The whole series dates back to August so a lot have changed since
> then.
>
> I would need this patch for my CNL work so I would like to see this
> merged.

I would like to see this merged, but I would also like to see it ever
working first. :) When I tried it at the time, even a 256x256 plane
could not use Y-tiling or CCS, due to DDB exhaustion.

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

Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Ville Syrjälä]

On Mon, Dec 11, 2017 at 12:33:47PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 11 December 2017 at 12:08, Mika Kahola <mika.kahola@intel.com> wrote:
> > On Mon, 2017-12-11 at 12:00 +0000, Daniel Stone wrote:
> >> Did you manage to test this? When I tried, the DDB/watermark
> >> allocation was too conservative for sprites, and never allowed enough
> >> blocks to be able to use anything but linear or X-tiled on sprite
> >> planes. The allocation was almost entirely taken up by the primary
> >> plane, even if that itself was later set to linear or X-tiled.
> >
> > I tried to test this but unfortunately this didn't apply on top of drm-
> > tip. The whole series dates back to August so a lot have changed since
> > then.
> >
> > I would need this patch for my CNL work so I would like to see this
> > merged.
> 
> I would like to see this merged, but I would also like to see it ever
> working first. :) When I tried it at the time, even a 256x256 plane
> could not use Y-tiling or CCS, due to DDB exhaustion.

It worked just fine when I tried it a few months ago.

22:45 < vsyrjala> my skl seems perfectly happy here with two 2560x1080
planes at same time scanning out yccs
22:47 < danvet> I think daniels had a huge screen or something like that
22:47 < vsyrjala> let me try my 4k
22:54 < vsyrjala> ok, so now i hit a wm limit
23:01 < vsyrjala> so two 3k x 2x planes both scanning out yccs works
23:02 < vsyrjala> 3.5k x 2k fails

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx