[PATCH v2 0/5] drm/i915: drm/i915: GTT remapping for display

[PATCH v2 0/5] drm/i915: drm/i915: GTT remapping for display

[Ville Syrjala]

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

Lots of prep stuff for the GTT remapping already landed, so now
we're left with just the real meat: the remapped vma and actually
constructing it.

I spotted that I kinda busted up the skl+ stride_mult stuff for
linear buffers in the BIOS fb takeover path so I included a fix
for that as a separate patch.

Ville Syrjälä (5):
  drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()
  drm/i915: Add a new "remapped" gtt_view
  drm/i915: Overcome display engine stride limits via GTT remapping
  drm/i915: Bump gen4+ fb stride limit to 256KiB
  drm/i915: Bump gen4+ fb size limits to 32kx32k

 drivers/gpu/drm/i915/i915_debugfs.c       |  12 +
 drivers/gpu/drm/i915/i915_gem_gtt.c       |  91 +++++++
 drivers/gpu/drm/i915/i915_gem_gtt.h       |  25 +-
 drivers/gpu/drm/i915/i915_vma.c           |   6 +-
 drivers/gpu/drm/i915/i915_vma.h           |   3 +
 drivers/gpu/drm/i915/intel_display.c      | 428 ++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h          |   1 +
 drivers/gpu/drm/i915/selftests/i915_vma.c |   6 +-
 8 files changed, 480 insertions(+), 92 deletions(-)

-- 
2.16.4

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

[PATCH v2 1/5] drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()

[Ville Syrjala]

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

In the future framebuffer stride alignment requirements won't exactly
match the units in which skl+ plane stride is specified. So extract
the code for the skl+ stuff into a separate helper.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 947301312ea3..3c9151a28103 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3503,6 +3503,21 @@ static void skl_detach_scalers(struct intel_crtc *intel_crtc)
 	}
 }
 
+static unsigned int skl_plane_stride_mult(const struct drm_framebuffer *fb,
+					  int color_plane, unsigned int rotation)
+{
+	/*
+	 * The stride is either expressed as a multiple of 64 bytes chunks for
+	 * linear buffers or in number of tiles for tiled buffers.
+	 */
+	if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
+		return 64;
+	else if (drm_rotation_90_or_270(rotation))
+		return intel_tile_height(fb, color_plane);
+	else
+		return intel_tile_width_bytes(fb, color_plane);
+}
+
 u32 skl_plane_stride(const struct intel_plane_state *plane_state,
 		     int color_plane)
 {
@@ -3513,16 +3528,7 @@ u32 skl_plane_stride(const struct intel_plane_state *plane_state,
 	if (color_plane >= fb->format->num_planes)
 		return 0;
 
-	/*
-	 * The stride is either expressed as a multiple of 64 bytes chunks for
-	 * linear buffers or in number of tiles for tiled buffers.
-	 */
-	if (drm_rotation_90_or_270(rotation))
-		stride /= intel_tile_height(fb, color_plane);
-	else
-		stride /= intel_fb_stride_alignment(fb, color_plane);
-
-	return stride;
+	return stride / skl_plane_stride_mult(fb, color_plane, rotation);
 }
 
 static u32 skl_plane_ctl_format(uint32_t pixel_format)
@@ -8875,7 +8881,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->width = ((val >> 0) & 0x1fff) + 1;
 
 	val = I915_READ(PLANE_STRIDE(pipe, plane_id));
-	stride_mult = intel_fb_stride_alignment(fb, 0);
+	stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
 
 	aligned_height = intel_fb_align_height(fb, 0, fb->height);
-- 
2.16.4

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

[PATCH v2 2/5] drm/i915: Add a new "remapped" gtt_view

[Ville Syrjala]

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

To overcome display engine stride limits we'll want to remap the
pages in the GTT. To that end we need a new gtt_view type which
is just like the "rotated" type except not rotated.

v2: Use intel_remapped_plane_info base type
    s/unused/unused_mbz/ (Chris)
    Separate BUILD_BUG_ON()s (Chris)
    Use I915_GTT_PAGE_SIZE (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c       | 12 ++++
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 91 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 25 +++++++--
 drivers/gpu/drm/i915/i915_vma.c           |  6 +-
 drivers/gpu/drm/i915/i915_vma.h           |  3 +
 drivers/gpu/drm/i915/intel_display.c      | 11 ++++
 drivers/gpu/drm/i915/intel_drv.h          |  1 +
 drivers/gpu/drm/i915/selftests/i915_vma.c |  6 +-
 8 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b4744a68cd88..edb9f6f3c0cf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -196,6 +196,18 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 					   vma->ggtt_view.rotated.plane[1].offset);
 				break;
 
+			case I915_GGTT_VIEW_REMAPPED:
+				seq_printf(m, ", remapped [(%ux%u, stride=%u, offset=%u), (%ux%u, stride=%u, offset=%u)]",
+					   vma->ggtt_view.remapped.plane[0].width,
+					   vma->ggtt_view.remapped.plane[0].height,
+					   vma->ggtt_view.remapped.plane[0].stride,
+					   vma->ggtt_view.remapped.plane[0].offset,
+					   vma->ggtt_view.remapped.plane[1].width,
+					   vma->ggtt_view.remapped.plane[1].height,
+					   vma->ggtt_view.remapped.plane[1].stride,
+					   vma->ggtt_view.remapped.plane[1].offset);
+				break;
+
 			default:
 				MISSING_CASE(vma->ggtt_view.type);
 				break;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8be1acd097db..d805f49647ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3798,6 +3798,92 @@ intel_rotate_pages(struct intel_rotation_info *rot_info,
 	return ERR_PTR(ret);
 }
 
+static struct scatterlist *
+remap_pages(const dma_addr_t *in, unsigned int offset,
+	    unsigned int width, unsigned int height,
+	    unsigned int stride,
+	    struct sg_table *st, struct scatterlist *sg)
+{
+	unsigned int column, row;
+
+	for (row = 0; row < height; row++) {
+		for (column = 0; column < width; column++) {
+			st->nents++;
+			/* We don't need the pages, but need to initialize
+			 * the entries so the sg list can be happily traversed.
+			 * The only thing we need are DMA addresses.
+			 */
+			sg_set_page(sg, NULL, I915_GTT_PAGE_SIZE, 0);
+			sg_dma_address(sg) = in[offset + column];
+			sg_dma_len(sg) = I915_GTT_PAGE_SIZE;
+			sg = sg_next(sg);
+		}
+		offset += stride;
+	}
+
+	return sg;
+}
+
+static noinline struct sg_table *
+intel_remap_pages(struct intel_remapped_info *rem_info,
+		  struct drm_i915_gem_object *obj)
+{
+	const unsigned long n_pages = obj->base.size / I915_GTT_PAGE_SIZE;
+	unsigned int size = intel_remapped_info_size(rem_info);
+	struct sgt_iter sgt_iter;
+	dma_addr_t dma_addr;
+	unsigned long i;
+	dma_addr_t *page_addr_list;
+	struct sg_table *st;
+	struct scatterlist *sg;
+	int ret = -ENOMEM;
+
+	/* Allocate a temporary list of source pages for random access. */
+	page_addr_list = kvmalloc_array(n_pages,
+					sizeof(dma_addr_t),
+					GFP_KERNEL);
+	if (!page_addr_list)
+		return ERR_PTR(ret);
+
+	/* Allocate target SG list. */
+	st = kmalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		goto err_st_alloc;
+
+	ret = sg_alloc_table(st, size, GFP_KERNEL);
+	if (ret)
+		goto err_sg_alloc;
+
+	/* Populate source page list from the object. */
+	i = 0;
+	for_each_sgt_dma(dma_addr, sgt_iter, obj->mm.pages)
+		page_addr_list[i++] = dma_addr;
+
+	GEM_BUG_ON(i != n_pages);
+	st->nents = 0;
+	sg = st->sgl;
+
+	for (i = 0 ; i < ARRAY_SIZE(rem_info->plane); i++) {
+		sg = remap_pages(page_addr_list, rem_info->plane[i].offset,
+				 rem_info->plane[i].width, rem_info->plane[i].height,
+				 rem_info->plane[i].stride, st, sg);
+	}
+
+	kvfree(page_addr_list);
+
+	return st;
+
+err_sg_alloc:
+	kfree(st);
+err_st_alloc:
+	kvfree(page_addr_list);
+
+	DRM_DEBUG_DRIVER("Failed to create remapped mapping for object size %zu! (%ux%u tiles, %u pages)\n",
+			 obj->base.size, rem_info->plane[0].width, rem_info->plane[0].height, size);
+
+	return ERR_PTR(ret);
+}
+
 static noinline struct sg_table *
 intel_partial_pages(const struct i915_ggtt_view *view,
 		    struct drm_i915_gem_object *obj)
@@ -3874,6 +3960,11 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
 			intel_rotate_pages(&vma->ggtt_view.rotated, vma->obj);
 		break;
 
+	case I915_GGTT_VIEW_REMAPPED:
+		vma->pages =
+			intel_remap_pages(&vma->ggtt_view.remapped, vma->obj);
+		break;
+
 	case I915_GGTT_VIEW_PARTIAL:
 		vma->pages = intel_partial_pages(&vma->ggtt_view, vma->obj);
 		break;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 7e2af5f4f39b..69a22f57e6ca 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -160,11 +160,18 @@ typedef u64 gen8_ppgtt_pml4e_t;
 
 struct sg_table;
 
+struct intel_remapped_plane_info {
+	/* in gtt pages */
+	unsigned int width, height, stride, offset;
+} __packed;
+
+struct intel_remapped_info {
+	struct intel_remapped_plane_info plane[2];
+	unsigned int unused_mbz;
+} __packed;
+
 struct intel_rotation_info {
-	struct intel_rotation_plane_info {
-		/* tiles */
-		unsigned int width, height, stride, offset;
-	} plane[2];
+	struct intel_remapped_plane_info plane[2];
 } __packed;
 
 struct intel_partial_info {
@@ -176,12 +183,20 @@ enum i915_ggtt_view_type {
 	I915_GGTT_VIEW_NORMAL = 0,
 	I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
 	I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
+	I915_GGTT_VIEW_REMAPPED = sizeof(struct intel_remapped_info),
 };
 
 static inline void assert_i915_gem_gtt_types(void)
 {
 	BUILD_BUG_ON(sizeof(struct intel_rotation_info) != 8*sizeof(unsigned int));
 	BUILD_BUG_ON(sizeof(struct intel_partial_info) != sizeof(u64) + sizeof(unsigned int));
+	BUILD_BUG_ON(sizeof(struct intel_remapped_info) != 9*sizeof(unsigned int));
+
+	/* Check that rotation/remapped shares offsets for simplicity */
+	BUILD_BUG_ON(offsetof(struct intel_remapped_info, plane[0]) !=
+		     offsetof(struct intel_rotation_info, plane[0]));
+	BUILD_BUG_ON(offsetofend(struct intel_remapped_info, plane[1]) !=
+		     offsetofend(struct intel_rotation_info, plane[1]));
 
 	/* As we encode the size of each branch inside the union into its type,
 	 * we have to be careful that each branch has a unique size.
@@ -190,6 +205,7 @@ static inline void assert_i915_gem_gtt_types(void)
 	case I915_GGTT_VIEW_NORMAL:
 	case I915_GGTT_VIEW_PARTIAL:
 	case I915_GGTT_VIEW_ROTATED:
+	case I915_GGTT_VIEW_REMAPPED:
 		/* gcc complains if these are identical cases */
 		break;
 	}
@@ -201,6 +217,7 @@ struct i915_ggtt_view {
 		/* Members need to contain no holes/padding */
 		struct intel_partial_info partial;
 		struct intel_rotation_info rotated;
+		struct intel_remapped_info remapped;
 	};
 };
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 31efc971a3a8..1793c702ab94 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -164,6 +164,9 @@ vma_create(struct drm_i915_gem_object *obj,
 		} else if (view->type == I915_GGTT_VIEW_ROTATED) {
 			vma->size = intel_rotation_info_size(&view->rotated);
 			vma->size <<= PAGE_SHIFT;
+		} else if (view->type == I915_GGTT_VIEW_REMAPPED) {
+			vma->size = intel_remapped_info_size(&view->remapped);
+			vma->size <<= PAGE_SHIFT;
 		}
 	}
 
@@ -464,7 +467,8 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
 	 * Explicitly disable for rotated VMA since the display does not
 	 * need the fence and the VMA is not accessible to other users.
 	 */
-	if (vma->ggtt_view.type == I915_GGTT_VIEW_ROTATED)
+	if (vma->ggtt_view.type == I915_GGTT_VIEW_ROTATED ||
+	    vma->ggtt_view.type == I915_GGTT_VIEW_REMAPPED)
 		return;
 
 	fenceable = (vma->node.size >= vma->fence_size &&
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4f7c1c7599f4..64cf029c028a 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -265,8 +265,11 @@ i915_vma_compare(struct i915_vma *vma,
 	 */
 	BUILD_BUG_ON(I915_GGTT_VIEW_NORMAL >= I915_GGTT_VIEW_PARTIAL);
 	BUILD_BUG_ON(I915_GGTT_VIEW_PARTIAL >= I915_GGTT_VIEW_ROTATED);
+	BUILD_BUG_ON(I915_GGTT_VIEW_ROTATED >= I915_GGTT_VIEW_REMAPPED);
 	BUILD_BUG_ON(offsetof(typeof(*view), rotated) !=
 		     offsetof(typeof(*view), partial));
+	BUILD_BUG_ON(offsetof(typeof(*view), rotated) !=
+		     offsetof(typeof(*view), remapped));
 	return memcmp(&vma->ggtt_view.partial, &view->partial, view->type);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3c9151a28103..21aa0c0321b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2006,6 +2006,17 @@ unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info
 	return size;
 }
 
+unsigned int intel_remapped_info_size(const struct intel_remapped_info *rem_info)
+{
+	unsigned int size = 0;
+	int i;
+
+	for (i = 0 ; i < ARRAY_SIZE(rem_info->plane); i++)
+		size += rem_info->plane[i].width * rem_info->plane[i].height;
+
+	return size;
+}
+
 static void
 intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
 			const struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..4f3216dd0fdf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1503,6 +1503,7 @@ unsigned int intel_fb_xy_to_linear(int x, int y,
 void intel_add_fb_offsets(int *x, int *y,
 			  const struct intel_plane_state *state, int plane);
 unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
+unsigned int intel_remapped_info_size(const struct intel_remapped_info *rem_info);
 bool intel_has_pending_fb_unpin(struct drm_i915_private *dev_priv);
 void intel_mark_busy(struct drm_i915_private *dev_priv);
 void intel_mark_idle(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index ffa74290e054..4fc49c27f13c 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -395,8 +395,8 @@ assert_rotated(struct drm_i915_gem_object *obj,
 	return sg;
 }
 
-static unsigned int rotated_size(const struct intel_rotation_plane_info *a,
-				 const struct intel_rotation_plane_info *b)
+static unsigned int rotated_size(const struct intel_remapped_plane_info *a,
+				 const struct intel_remapped_plane_info *b)
 {
 	return a->width * a->height + b->width * b->height;
 }
@@ -406,7 +406,7 @@ static int igt_vma_rotate(void *arg)
 	struct drm_i915_private *i915 = arg;
 	struct i915_address_space *vm = &i915->ggtt.vm;
 	struct drm_i915_gem_object *obj;
-	const struct intel_rotation_plane_info planes[] = {
+	const struct intel_remapped_plane_info planes[] = {
 		{ .width = 1, .height = 1, .stride = 1 },
 		{ .width = 2, .height = 2, .stride = 2 },
 		{ .width = 4, .height = 4, .stride = 4 },
-- 
2.16.4

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

[PATCH v2 3/5] drm/i915: Overcome display engine stride limits via GTT remapping

[Ville Syrjala]

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

The display engine stride limits are getting in our way. On SKL+
we are limited to 8k pixels, which is easily exceeded with three
4k displays. To overcome this limitation we can remap the pages
in the GTT to provide the display engine with a view of memory
with a smaller stride.

The code is mostly already there as We already play tricks with
the plane surface address and x/y offsets.

A few caveats apply:
* linear buffers need the fb stride to be page aligned, as
  otherwise the remapped lines wouldn't start at the same
  spot
* compressed buffers can't be remapped due to the new
  ccs hash mode causing the virtual address of the pages
  to affect the interpretation of the compressed data. IIRC
  the old hash was limited to the low 12 bits so if we were
  using that mode we could remap. As it stands we just refuse
  to remapp with compressed fbs.
* no remapping gen2/3 as we'd need a fence for the remapped
  vma, which we currently don't have. Need to deal with the
  fence POT requirements, and do something about the gen2
  gtt page size vs tile size difference

v2: Reabase due to is_ccs_modifier()
    Fix up the skl+ stride_mult mess
    memset() the gtt_view because otherwise we could leave
    junk in plane[1] when going from 2 plane to 1 plane format

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 21aa0c0321b6..93b0de594c5d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1924,7 +1924,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
 
 	switch (fb->modifier) {
 	case DRM_FORMAT_MOD_LINEAR:
-		return cpp;
+		return intel_tile_size(dev_priv);
 	case I915_FORMAT_MOD_X_TILED:
 		if (IS_GEN2(dev_priv))
 			return 128;
@@ -1967,11 +1967,8 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
 static unsigned int
 intel_tile_height(const struct drm_framebuffer *fb, int color_plane)
 {
-	if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
-		return 1;
-	else
-		return intel_tile_size(to_i915(fb->dev)) /
-			intel_tile_width_bytes(fb, color_plane);
+	return intel_tile_size(to_i915(fb->dev)) /
+		intel_tile_width_bytes(fb, color_plane);
 }
 
 /* Return the tile dimensions in pixel units */
@@ -2226,16 +2223,8 @@ void intel_add_fb_offsets(int *x, int *y,
 			  int color_plane)
 
 {
-	const struct intel_framebuffer *intel_fb = to_intel_framebuffer(state->base.fb);
-	unsigned int rotation = state->base.rotation;
-
-	if (drm_rotation_90_or_270(rotation)) {
-		*x += intel_fb->rotated[color_plane].x;
-		*y += intel_fb->rotated[color_plane].y;
-	} else {
-		*x += intel_fb->normal[color_plane].x;
-		*y += intel_fb->normal[color_plane].y;
-	}
+	*x += state->color_plane[color_plane].x;
+	*y += state->color_plane[color_plane].y;
 }
 
 static u32 intel_adjust_tile_offset(int *x, int *y,
@@ -2495,6 +2484,105 @@ bool is_ccs_modifier(u64 modifier)
 	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
 }
 
+static
+u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
+			      u32 pixel_format, u64 modifier)
+{
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
+
+	/*
+	 * We assume the primary plane for pipe A has
+	 * the highest stride limits of them all.
+	 */
+	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
+	plane = to_intel_plane(crtc->base.primary);
+
+	return plane->max_stride(plane, pixel_format, modifier,
+				 DRM_MODE_ROTATE_0);
+}
+
+static
+u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
+			u32 pixel_format, u64 modifier)
+{
+	return intel_plane_fb_max_stride(dev_priv, pixel_format, modifier);
+}
+
+static u32
+intel_fb_stride_alignment(const struct drm_framebuffer *fb, int color_plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(fb->dev);
+
+	if (fb->modifier == DRM_FORMAT_MOD_LINEAR) {
+		u32 max_stride = intel_plane_fb_max_stride(dev_priv,
+							   fb->format->format,
+							   fb->modifier);
+
+		/*
+		 * To make remapping with linear generally feasible
+		 * we need the stride to be page aligned.
+		 */
+		if (fb->pitches[color_plane] > max_stride)
+			return intel_tile_size(dev_priv);
+		else
+			return 64;
+	} else {
+		return intel_tile_width_bytes(fb, color_plane);
+	}
+}
+
+static int
+intel_plane_check_stride(const struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int rotation = plane_state->base.rotation;
+	u32 stride, max_stride;
+
+	/* FIXME other color planes? */
+	stride = plane_state->color_plane[0].stride;
+	max_stride = plane->max_stride(plane, fb->format->format,
+				       fb->modifier, rotation);
+
+	if (stride > max_stride) {
+		DRM_DEBUG_KMS("[FB:%d] stride (%d) exceeds [PLANE:%d:%s] max stride (%d)\n",
+			      fb->base.id, stride,
+			      plane->base.base.id, plane->base.name, max_stride);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static bool intel_plane_needs_remap(const 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);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int rotation = plane_state->base.rotation;
+	u32 stride, max_stride;
+
+	/* We don't want to deal with remapping with cursors */
+	if (plane->id == PLANE_CURSOR)
+		return false;
+
+	/* No fence for the remapped vma */
+	if (INTEL_GEN(dev_priv) < 4)
+		return false;
+
+	/* New CCS hash mode makes remapping impossible */
+	if (is_ccs_modifier(fb->modifier))
+		return false;
+
+	/* FIXME other color planes? */
+	stride = intel_fb_pitch(fb, 0, rotation);
+	max_stride = plane->max_stride(plane, fb->format->format,
+				       fb->modifier, rotation);
+
+	return stride > max_stride;
+}
+
 static int
 intel_fill_fb_info(struct drm_i915_private *dev_priv,
 		   struct drm_framebuffer *fb)
@@ -2660,6 +2748,182 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
 	return 0;
 }
 
+static void
+intel_plane_remap_gtt(struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+	struct intel_rotation_info *info = &plane_state->view.rotated;
+	unsigned int rotation = plane_state->base.rotation;
+	int i, num_planes = fb->format->num_planes;
+	unsigned int tile_size = intel_tile_size(dev_priv);
+	unsigned int tile_width, tile_height;
+	unsigned int aligned_x, aligned_y;
+	unsigned int aligned_w, aligned_h;
+	unsigned int src_x, src_y;
+	unsigned int src_w, src_h;
+	unsigned int x, y;
+	u32 gtt_offset = 0;
+
+	memset(&plane_state->view, 0, sizeof(plane_state->view));
+	plane_state->view.type = drm_rotation_90_or_270(rotation) ?
+		I915_GGTT_VIEW_ROTATED : I915_GGTT_VIEW_REMAPPED;
+
+	src_x = plane_state->base.src.x1 >> 16;
+	src_y = plane_state->base.src.y1 >> 16;
+	src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	src_h = drm_rect_height(&plane_state->base.src) >> 16;
+
+	WARN_ON(is_ccs_modifier(fb->modifier));
+
+	/* Align our viewport start to tile boundary */
+	intel_tile_dims(fb, 0, &tile_width, &tile_height);
+
+	x = src_x & (tile_width - 1);
+	y = src_y & (tile_height - 1);
+
+	aligned_x = src_x - x;
+	aligned_y = src_y - y;
+
+	aligned_w = x + src_w;
+	aligned_h = y + src_h;
+
+	/* Make src coordinates relative to the aligned viewport */
+	drm_rect_translate(&plane_state->base.src,
+			   -(aligned_x << 16), -(aligned_y << 16));
+
+	/* Rotate src coordinates to match rotated GTT view */
+	if (drm_rotation_90_or_270(rotation))
+		drm_rect_rotate(&plane_state->base.src,
+				aligned_w << 16, aligned_h << 16,
+				DRM_MODE_ROTATE_270);
+
+	for (i = 0; i < num_planes; i++) {
+		unsigned int hsub = i ? fb->format->hsub : 1;
+		unsigned int vsub = i ? fb->format->vsub : 1;
+		unsigned int cpp = fb->format->cpp[i];
+		unsigned int width, height;
+		unsigned int pitch_tiles;
+		unsigned int x, y;
+		u32 offset;
+
+		intel_tile_dims(fb, i, &tile_width, &tile_height);
+
+		x = aligned_x / hsub;
+		y = aligned_y / vsub;
+		width = aligned_w / hsub;
+		height = aligned_h / vsub;
+
+		/*
+		 * First pixel of the aligned src viewport
+		 * from the start of the normal gtt mapping.
+		 */
+		x += intel_fb->normal[i].x;
+		y += intel_fb->normal[i].y;
+
+		offset = intel_compute_aligned_offset(dev_priv, &x, &y,
+						      fb, i, fb->pitches[i],
+						      DRM_MODE_ROTATE_0, tile_size);
+		offset /= tile_size;
+
+		info->plane[i].offset = offset;
+		info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i],
+						     tile_width * cpp);
+		info->plane[i].width = DIV_ROUND_UP(x + width, tile_width);
+		info->plane[i].height = DIV_ROUND_UP(y + height, tile_height);
+
+		if (drm_rotation_90_or_270(rotation)) {
+			struct drm_rect r;
+
+			/* rotate the x/y offsets to match the GTT view */
+			r.x1 = x;
+			r.y1 = y;
+			r.x2 = x + width;
+			r.y2 = y + height;
+			drm_rect_rotate(&r,
+					info->plane[i].width * tile_width,
+					info->plane[i].height * tile_height,
+					DRM_MODE_ROTATE_270);
+			x = r.x1;
+			y = r.y1;
+
+			pitch_tiles = info->plane[i].height;
+			plane_state->color_plane[i].stride = pitch_tiles * tile_height;
+
+			/* rotate the tile dimensions to match the GTT view */
+			swap(tile_width, tile_height);
+		} else {
+			pitch_tiles = info->plane[i].width;
+			plane_state->color_plane[i].stride = pitch_tiles * tile_width * cpp;
+		}
+
+		/*
+		 * 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 * tile_size, 0);
+
+		gtt_offset += info->plane[i].width * info->plane[i].height;
+
+		plane_state->color_plane[i].offset = 0;
+		plane_state->color_plane[i].x = x;
+		plane_state->color_plane[i].y = y;
+	}
+}
+
+static int
+intel_plane_compute_gtt(struct intel_plane_state *plane_state)
+{
+	const struct intel_framebuffer *fb =
+		to_intel_framebuffer(plane_state->base.fb);
+	unsigned int rotation = plane_state->base.rotation;
+	int i, num_planes = fb->base.format->num_planes;
+	int ret;
+
+	if (intel_plane_needs_remap(plane_state)) {
+		intel_plane_remap_gtt(plane_state);
+
+		/* Remapping should take care of this always */
+		ret = intel_plane_check_stride(plane_state);
+		if (WARN_ON(ret))
+			return ret;
+
+		return 0;
+	}
+
+	intel_fill_fb_ggtt_view(&plane_state->view, &fb->base, rotation);
+
+	for (i = 0; i < num_planes; i++) {
+		plane_state->color_plane[i].stride = intel_fb_pitch(&fb->base, i, rotation);
+		plane_state->color_plane[i].offset = 0;
+
+		if (drm_rotation_90_or_270(rotation)) {
+			plane_state->color_plane[i].x = fb->rotated[i].x;
+			plane_state->color_plane[i].y = fb->rotated[i].y;
+		} else {
+			plane_state->color_plane[i].x = fb->normal[i].x;
+			plane_state->color_plane[i].y = fb->normal[i].y;
+		}
+	}
+
+	/* Rotate src coordinates to match rotated GTT view */
+	if (drm_rotation_90_or_270(rotation))
+		drm_rect_rotate(&plane_state->base.src,
+				fb->base.width << 16, fb->base.height << 16,
+				DRM_MODE_ROTATE_270);
+
+	ret = intel_plane_check_stride(plane_state);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int i9xx_format_to_fourcc(int format)
 {
 	switch (format) {
@@ -3155,21 +3419,11 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 int skl_check_plane_surface(struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
 	int ret;
 
-	intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
-	plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
-	plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1, rotation);
-
-	if (!plane_state->base.visible)
-		return 0;
-
-	/* Rotate src coordinates to match rotated GTT view */
-	if (drm_rotation_90_or_270(rotation))
-		drm_rect_rotate(&plane_state->base.src,
-				fb->width << 16, fb->height << 16,
-				DRM_MODE_ROTATE_270);
+	ret = intel_plane_compute_gtt(plane_state);
+	if (ret)
+		return ret;
 
 	/*
 	 * Handle the AUX surface first since
@@ -3292,14 +3546,16 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
-	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
-	int src_x = plane_state->base.src.x1 >> 16;
-	int src_y = plane_state->base.src.y1 >> 16;
+	int src_x, src_y;
 	u32 offset;
+	int ret;
 
-	intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
-	plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
+	ret = intel_plane_compute_gtt(plane_state);
+	if (ret)
+		return ret;
+
+	src_x = plane_state->base.src.x1 >> 16;
+	src_y = plane_state->base.src.y1 >> 16;
 
 	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
 
@@ -3311,6 +3567,7 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 
 	/* HSW/BDW do this automagically in hardware */
 	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
+		unsigned int rotation = plane_state->base.rotation;
 		int src_w = drm_rect_width(&plane_state->base.src) >> 16;
 		int src_h = drm_rect_height(&plane_state->base.src) >> 16;
 
@@ -3478,15 +3735,6 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *plane,
 	return ret;
 }
 
-static u32
-intel_fb_stride_alignment(const struct drm_framebuffer *fb, int color_plane)
-{
-	if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
-		return 64;
-	else
-		return intel_tile_width_bytes(fb, color_plane);
-}
-
 static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -9698,13 +9946,13 @@ static bool intel_cursor_size_ok(const struct intel_plane_state *plane_state)
 
 static int intel_cursor_check_surface(struct intel_plane_state *plane_state)
 {
-	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
 	int src_x, src_y;
 	u32 offset;
+	int ret;
 
-	intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
-	plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
+	ret = intel_plane_compute_gtt(plane_state);
+	if (ret)
+		return ret;
 
 	src_x = plane_state->base.src_x >> 16;
 	src_y = plane_state->base.src_y >> 16;
@@ -14431,24 +14679,6 @@ static const struct drm_framebuffer_funcs intel_fb_funcs = {
 	.dirty = intel_user_framebuffer_dirty,
 };
 
-static
-u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
-			 uint64_t fb_modifier, uint32_t pixel_format)
-{
-	struct intel_crtc *crtc;
-	struct intel_plane *plane;
-
-	/*
-	 * We assume the primary plane for pipe A has
-	 * the highest stride limits of them all.
-	 */
-	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
-	plane = to_intel_plane(crtc->base.primary);
-
-	return plane->max_stride(plane, pixel_format, fb_modifier,
-				 DRM_MODE_ROTATE_0);
-}
-
 static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 				  struct drm_i915_gem_object *obj,
 				  struct drm_mode_fb_cmd2 *mode_cmd)
@@ -14456,7 +14686,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
 	struct drm_framebuffer *fb = &intel_fb->base;
 	struct drm_format_name_buf format_name;
-	u32 pitch_limit;
+	u32 max_stride;
 	unsigned int tiling, stride;
 	int ret = -EINVAL;
 	int i;
@@ -14527,13 +14757,13 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 		goto err;
 	}
 
-	pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->modifier[0],
-					   mode_cmd->pixel_format);
-	if (mode_cmd->pitches[0] > pitch_limit) {
+	max_stride = intel_fb_max_stride(dev_priv, mode_cmd->modifier[0],
+					 mode_cmd->pixel_format);
+	if (mode_cmd->pitches[0] > max_stride) {
 		DRM_DEBUG_KMS("%s pitch (%u) must be at most %d\n",
 			      mode_cmd->modifier[0] != DRM_FORMAT_MOD_LINEAR ?
 			      "tiled" : "linear",
-			      mode_cmd->pitches[0], pitch_limit);
+			      mode_cmd->pitches[0], max_stride);
 		goto err;
 	}
 
-- 
2.16.4

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

[PATCH v2 4/5] drm/i915: Bump gen4+ fb stride limit to 256KiB

[Ville Syrjala]

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

With gtt remapping plugged in we can simply raise the stride
limit on gen4+. Let's just arbitraily pick 256 KiB as the limit.

No remapping CCS because the virtual address of each page actually
matters due to the new hash mode
(WaCompressedResourceDisplayNewHashMode:skl,kbl etc.), and no remapping
on gen2/3 due to lack of fence on the remapped vma.

v2: Rebase due to is_ccs_modifier()

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 93b0de594c5d..346572cf734a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2506,6 +2506,19 @@ static
 u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
 			u32 pixel_format, u64 modifier)
 {
+	/*
+	 * Arbitrary limit for gen4+. We can deal with any page
+	 * aligned stride via GTT remapping. Gen2/3 need a fence
+	 * for tiled scanout which the remapped vma won't have,
+	 * so we don't allow remapping on those platforms.
+	 *
+	 * Also the new hash mode we use for CCS isn't compatible
+	 * with remapping as the virtual address of the pages
+	 * affects the compressed data.
+	 */
+	if (INTEL_GEN(dev_priv) >= 4 && !is_ccs_modifier(modifier))
+		return 256*1024;
+
 	return intel_plane_fb_max_stride(dev_priv, pixel_format, modifier);
 }
 
-- 
2.16.4

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

[PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Ville Syrjala]

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

With gtt remapping in place we can use arbitraily large framebuffers.
Let's bump the limits as high as we can (32k-1). Going beyond that
would require switching our s16.16 src coordinate representation to
something with more spare bits.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 346572cf734a..0ee6255cd040 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
 		dev->mode_config.max_width = 4096;
 		dev->mode_config.max_height = 4096;
 	} else {
-		dev->mode_config.max_width = 8192;
-		dev->mode_config.max_height = 8192;
+		dev->mode_config.max_width = 32767;
+		dev->mode_config.max_height = 32767;
 	}
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
-- 
2.16.4

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

Re: [PATCH v2 3/5] drm/i915: Overcome display engine stride limits via GTT remapping

[Chris Wilson]

Quoting Ville Syrjala (2018-09-13 21:01:38)
> +static void
> +intel_plane_remap_gtt(struct intel_plane_state *plane_state)
> +{
> +       struct drm_i915_private *dev_priv =
> +               to_i915(plane_state->base.plane->dev);
> +       struct drm_framebuffer *fb = plane_state->base.fb;
> +       struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +       struct intel_rotation_info *info = &plane_state->view.rotated;
> +       unsigned int rotation = plane_state->base.rotation;
> +       int i, num_planes = fb->format->num_planes;
> +       unsigned int tile_size = intel_tile_size(dev_priv);
> +       unsigned int tile_width, tile_height;
> +       unsigned int aligned_x, aligned_y;
> +       unsigned int aligned_w, aligned_h;
> +       unsigned int src_x, src_y;
> +       unsigned int src_w, src_h;
> +       unsigned int x, y;
> +       u32 gtt_offset = 0;
> +
> +       memset(&plane_state->view, 0, sizeof(plane_state->view));
> +       plane_state->view.type = drm_rotation_90_or_270(rotation) ?
> +               I915_GGTT_VIEW_ROTATED : I915_GGTT_VIEW_REMAPPED;
> +
> +       src_x = plane_state->base.src.x1 >> 16;
> +       src_y = plane_state->base.src.y1 >> 16;
> +       src_w = drm_rect_width(&plane_state->base.src) >> 16;
> +       src_h = drm_rect_height(&plane_state->base.src) >> 16;

Just doing a quick exercise to see if we might overflow later.

src_w/src_h are 15 bits (int 31 bits >> 16). So size is 30 bits and
total size of planes[2], 31 bits. (I'm assuming we go in and out of
pages.)

Did I miss anything? Should I be asking "what about the overflow?" :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Add a new "remapped" gtt_view

[Chris Wilson]

Quoting Ville Syrjala (2018-09-13 21:01:37)
> diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
> index ffa74290e054..4fc49c27f13c 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> @@ -395,8 +395,8 @@ assert_rotated(struct drm_i915_gem_object *obj,
>         return sg;
>  }
>  
> -static unsigned int rotated_size(const struct intel_rotation_plane_info *a,
> -                                const struct intel_rotation_plane_info *b)
> +static unsigned int rotated_size(const struct intel_remapped_plane_info *a,
> +                                const struct intel_remapped_plane_info *b)
>  {
>         return a->width * a->height + b->width * b->height;
>  }
> @@ -406,7 +406,7 @@ static int igt_vma_rotate(void *arg)
>         struct drm_i915_private *i915 = arg;
>         struct i915_address_space *vm = &i915->ggtt.vm;
>         struct drm_i915_gem_object *obj;
> -       const struct intel_rotation_plane_info planes[] = {
> +       const struct intel_remapped_plane_info planes[] = {
>                 { .width = 1, .height = 1, .stride = 1 },
>                 { .width = 2, .height = 2, .stride = 2 },
>                 { .width = 4, .height = 4, .stride = 4 },

Could we prove our remapping vma works by doing an i915_vma_pin_iomap()
and checking that a write into each page ends up in the correct address?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/5] drm/i915: Bump gen4+ fb stride limit to 256KiB

[Chris Wilson]

Quoting Ville Syrjala (2018-09-13 21:01:39)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> With gtt remapping plugged in we can simply raise the stride
> limit on gen4+. Let's just arbitraily pick 256 KiB as the limit.
> 
> No remapping CCS because the virtual address of each page actually
> matters due to the new hash mode
> (WaCompressedResourceDisplayNewHashMode:skl,kbl etc.), and no remapping
> on gen2/3 due to lack of fence on the remapped vma.
> 
> v2: Rebase due to is_ccs_modifier()
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 93b0de594c5d..346572cf734a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2506,6 +2506,19 @@ static
>  u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
>                         u32 pixel_format, u64 modifier)
>  {
> +       /*
> +        * Arbitrary limit for gen4+. We can deal with any page
> +        * aligned stride via GTT remapping. Gen2/3 need a fence
> +        * for tiled scanout which the remapped vma won't have,
> +        * so we don't allow remapping on those platforms.
> +        *
> +        * Also the new hash mode we use for CCS isn't compatible
> +        * with remapping as the virtual address of the pages
> +        * affects the compressed data.
> +        */
> +       if (INTEL_GEN(dev_priv) >= 4 && !is_ccs_modifier(modifier))
> +               return 256*1024;

If arbitrary, why 256k? Will we not go beyond 8 bytes per pixel in the
future? If you used U32_MAX then we will just reject v.large strides on
other grounds (too large for GTT, stride/size overflow).

Hmm, or is the 256k to keep the overflow checks simpler?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: drm/i915: GTT remapping for display

[Patchwork]

== Series Details ==

Series: drm/i915: drm/i915: GTT remapping for display
URL   : https://patchwork.freedesktop.org/series/49663/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b24877977210 drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()
7a895a3a3d0d drm/i915: Add a new "remapped" gtt_view
-:190: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#190: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:193:
+	BUILD_BUG_ON(sizeof(struct intel_remapped_info) != 9*sizeof(unsigned int));
 	                                                    ^

total: 0 errors, 0 warnings, 1 checks, 248 lines checked
018b3a571a49 drm/i915: Overcome display engine stride limits via GTT remapping
0a5360d29ad3 drm/i915: Bump gen4+ fb stride limit to 256KiB
-:40: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#40: FILE: drivers/gpu/drm/i915/intel_display.c:2520:
+		return 256*1024;
 		          ^

total: 0 errors, 0 warnings, 1 checks, 19 lines checked
ddda39100946 drm/i915: Bump gen4+ fb size limits to 32kx32k

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

✗ Fi.CI.SPARSE: warning for drm/i915: drm/i915: GTT remapping for display

[Patchwork]

== Series Details ==

Series: drm/i915: drm/i915: GTT remapping for display
URL   : https://patchwork.freedesktop.org/series/49663/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()
Okay!

Commit: drm/i915: Add a new "remapped" gtt_view
+./include/linux/mm.h:592:13: error: not a function <noident>

Commit: drm/i915: Overcome display engine stride limits via GTT remapping
Okay!

Commit: drm/i915: Bump gen4+ fb stride limit to 256KiB
Okay!

Commit: drm/i915: Bump gen4+ fb size limits to 32kx32k
Okay!

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

✗ Fi.CI.BAT: failure for drm/i915: drm/i915: GTT remapping for display

[Patchwork]

== Series Details ==

Series: drm/i915: drm/i915: GTT remapping for display
URL   : https://patchwork.freedesktop.org/series/49663/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4819 -> Patchwork_10178 =

== Summary - FAILURE ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        PASS -> DMESG-FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           PASS -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-skl-6700k2:      FAIL (fdo#103191) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-whl-u:           FAIL (fdo#107336) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


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

  Additional (1): fi-icl-u 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4819 -> Patchwork_10178

  CI_DRM_4819: 974889a04616f44de2f342dfdc9753b6dad8de88 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10178: ddda39100946d5f3122523878b5e70b975ea0a91 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ddda39100946 drm/i915: Bump gen4+ fb size limits to 32kx32k
0a5360d29ad3 drm/i915: Bump gen4+ fb stride limit to 256KiB
018b3a571a49 drm/i915: Overcome display engine stride limits via GTT remapping
7a895a3a3d0d drm/i915: Add a new "remapped" gtt_view
b24877977210 drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()

== Logs ==

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

✓ Fi.CI.BAT: success for drm/i915: drm/i915: GTT remapping for display

[Patchwork]

== Series Details ==

Series: drm/i915: drm/i915: GTT remapping for display
URL   : https://patchwork.freedesktop.org/series/49663/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4822 -> Patchwork_10185 =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rpm@module-reload:
      fi-hsw-4770r:       SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@prime_vgem@basic-fence-flip:
      fi-cfl-8700k:       PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload-inject:
      fi-hsw-4770r:       DMESG-WARN (fdo#107924, fdo#107425) -> PASS

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        DMESG-FAIL -> PASS

    igt@kms_flip@basic-flip-vs-dpms:
      fi-hsw-4770r:       DMESG-WARN (fdo#105602) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-whl-u:           FAIL (fdo#107336) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425
  fdo#107924 https://bugs.freedesktop.org/show_bug.cgi?id=107924


== Participating hosts (49 -> 42) ==

  Missing    (7): fi-cnl-u fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-gdg-551 


== Build changes ==

    * Linux: CI_DRM_4822 -> Patchwork_10185

  CI_DRM_4822: e3090c9e82d61aa837ccae2ba224e5a0b9b89d3f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10185: ae819108209073320b7b6ded2b17590b531c9363 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ae8191082090 drm/i915: Bump gen4+ fb size limits to 32kx32k
a3d621e610d5 drm/i915: Bump gen4+ fb stride limit to 256KiB
325e25993383 drm/i915: Overcome display engine stride limits via GTT remapping
c811b32f3b81 drm/i915: Add a new "remapped" gtt_view
14f8fc46f959 drm/i915: Decouple SKL stride units from intel_fb_stride_alignment()

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: drm/i915: GTT remapping for display

[Patchwork]

== Series Details ==

Series: drm/i915: drm/i915: GTT remapping for display
URL   : https://patchwork.freedesktop.org/series/49663/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4822_full -> Patchwork_10185_full =

== Summary - WARNING ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_flush@basic-wb-ro-default:
      shard-apl:          PASS -> ( 2 PASS ) +50

    igt@gem_pwrite_pread@snooped-pwrite-blt-cpu_mmap-correctness:
      shard-snb:          PASS -> ( 2 PASS ) +37

    igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
      shard-snb:          SKIP -> ( 2 SKIP ) +38

    igt@kms_chv_cursor_fail@pipe-b-256x256-top-edge:
      shard-hsw:          PASS -> ( 2 PASS ) +106

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
      shard-apl:          SKIP -> ( 2 SKIP ) +24

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
      shard-glk:          SKIP -> ( 2 SKIP ) +300

    igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
      shard-glk:          PASS -> ( 2 PASS ) +774

    igt@perf_pmu@busy-accuracy-50-rcs0:
      shard-hsw:          SKIP -> ( 2 SKIP ) +47

    igt@prime_mmap_coherency@read:
      shard-kbl:          PASS -> ( 2 PASS ) +55

    igt@prime_nv_pcopy@test3_4:
      shard-kbl:          SKIP -> ( 2 SKIP ) +20

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927, fdo#106886)

    igt@gem_userptr_blits@readonly-unsync:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133) +1

    igt@kms_vblank@pipe-c-ts-continuation-suspend:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-kbl:          FAIL (fdo#106680) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363, fdo#102887) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt:
      shard-glk:          DMESG-FAIL (fdo#106538) -> PASS +1

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

    igt@kms_vblank@pipe-a-query-forked-hang:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS +2

    
    ==== Warnings ====

    igt@gem_exec_schedule@pi-ringfull-blt:
      shard-glk:          FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158) +1

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-glk:          FAIL (fdo#106641) -> ( 2 FAIL ) (fdo#106641)

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

    igt@kms_sysfs_edid_timing:
      shard-apl:          FAIL (fdo#100047) -> ( 2 FAIL ) (fdo#100047)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4822 -> Patchwork_10185

  CI_DRM_4822: e3090c9e82d61aa837ccae2ba224e5a0b9b89d3f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10185: ae819108209073320b7b6ded2b17590b531c9363 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH v2 4/5] drm/i915: Bump gen4+ fb stride limit to 256KiB

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 09:27:25PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-09-13 21:01:39)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > With gtt remapping plugged in we can simply raise the stride
> > limit on gen4+. Let's just arbitraily pick 256 KiB as the limit.
> > 
> > No remapping CCS because the virtual address of each page actually
> > matters due to the new hash mode
> > (WaCompressedResourceDisplayNewHashMode:skl,kbl etc.), and no remapping
> > on gen2/3 due to lack of fence on the remapped vma.
> > 
> > v2: Rebase due to is_ccs_modifier()
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 93b0de594c5d..346572cf734a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2506,6 +2506,19 @@ static
> >  u32 intel_fb_max_stride(struct drm_i915_private *dev_priv,
> >                         u32 pixel_format, u64 modifier)
> >  {
> > +       /*
> > +        * Arbitrary limit for gen4+. We can deal with any page
> > +        * aligned stride via GTT remapping. Gen2/3 need a fence
> > +        * for tiled scanout which the remapped vma won't have,
> > +        * so we don't allow remapping on those platforms.
> > +        *
> > +        * Also the new hash mode we use for CCS isn't compatible
> > +        * with remapping as the virtual address of the pages
> > +        * affects the compressed data.
> > +        */
> > +       if (INTEL_GEN(dev_priv) >= 4 && !is_ccs_modifier(modifier))
> > +               return 256*1024;
> 
> If arbitrary, why 256k? Will we not go beyond 8 bytes per pixel in the
> future?

I'm not aware of 32bpc formats being a thing for the display. 16bpc is
a thing for sure and that was on factor I used to pick the 256 KiB.
Another one was the max fence stride we support is 256KiB. While that's
not necessarily important these days I figured at least it's a limit
we already have elsewhere so should hopefully avoid some unknown
overflows :)

> If you used U32_MAX then we will just reject v.large strides on
> other grounds (too large for GTT, stride/size overflow).
> 
> Hmm, or is the 256k to keep the overflow checks simpler?

I was trying to keep it reasonably low to avoid introducing more
overflow possibilities. But I think I will have to review more of
the existing code against those even with the 256KiB limit. Didn't
really think through the tile/aligned offset calculations yet to
see if they are in danger now.

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

Re: [PATCH v2 2/5] drm/i915: Add a new "remapped" gtt_view

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 09:19:00PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-09-13 21:01:37)
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > index ffa74290e054..4fc49c27f13c 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > @@ -395,8 +395,8 @@ assert_rotated(struct drm_i915_gem_object *obj,
> >         return sg;
> >  }
> >  
> > -static unsigned int rotated_size(const struct intel_rotation_plane_info *a,
> > -                                const struct intel_rotation_plane_info *b)
> > +static unsigned int rotated_size(const struct intel_remapped_plane_info *a,
> > +                                const struct intel_remapped_plane_info *b)
> >  {
> >         return a->width * a->height + b->width * b->height;
> >  }
> > @@ -406,7 +406,7 @@ static int igt_vma_rotate(void *arg)
> >         struct drm_i915_private *i915 = arg;
> >         struct i915_address_space *vm = &i915->ggtt.vm;
> >         struct drm_i915_gem_object *obj;
> > -       const struct intel_rotation_plane_info planes[] = {
> > +       const struct intel_remapped_plane_info planes[] = {
> >                 { .width = 1, .height = 1, .stride = 1 },
> >                 { .width = 2, .height = 2, .stride = 2 },
> >                 { .width = 4, .height = 4, .stride = 4 },
> 
> Could we prove our remapping vma works by doing an i915_vma_pin_iomap()
> and checking that a write into each page ends up in the correct address?

So write through a fence and check it lands in the right place?
Seems doable. Though we'd have to allow a fence for these vmas,
which I guess isn't really a problem.

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

Re: [PATCH v2 2/5] drm/i915: Add a new "remapped" gtt_view

[Chris Wilson]

Quoting Ville Syrjälä (2018-09-14 13:58:15)
> On Thu, Sep 13, 2018 at 09:19:00PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-09-13 21:01:37)
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > > index ffa74290e054..4fc49c27f13c 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> > > @@ -395,8 +395,8 @@ assert_rotated(struct drm_i915_gem_object *obj,
> > >         return sg;
> > >  }
> > >  
> > > -static unsigned int rotated_size(const struct intel_rotation_plane_info *a,
> > > -                                const struct intel_rotation_plane_info *b)
> > > +static unsigned int rotated_size(const struct intel_remapped_plane_info *a,
> > > +                                const struct intel_remapped_plane_info *b)
> > >  {
> > >         return a->width * a->height + b->width * b->height;
> > >  }
> > > @@ -406,7 +406,7 @@ static int igt_vma_rotate(void *arg)
> > >         struct drm_i915_private *i915 = arg;
> > >         struct i915_address_space *vm = &i915->ggtt.vm;
> > >         struct drm_i915_gem_object *obj;
> > > -       const struct intel_rotation_plane_info planes[] = {
> > > +       const struct intel_remapped_plane_info planes[] = {
> > >                 { .width = 1, .height = 1, .stride = 1 },
> > >                 { .width = 2, .height = 2, .stride = 2 },
> > >                 { .width = 4, .height = 4, .stride = 4 },
> > 
> > Could we prove our remapping vma works by doing an i915_vma_pin_iomap()
> > and checking that a write into each page ends up in the correct address?
> 
> So write through a fence and check it lands in the right place?
> Seems doable. Though we'd have to allow a fence for these vmas,
> which I guess isn't really a problem.

We don't need the fence for a linear view. For the short term, I'd
PIN_MAPPABLE and bludgeon can-map (although that's all that would
required for us to get a fence...) so that i915_vma_pin_iomap works.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> With gtt remapping in place we can use arbitraily large framebuffers.
> Let's bump the limits as high as we can (32k-1). Going beyond that
> would require switching our s16.16 src coordinate representation to
> something with more spare bits.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 346572cf734a..0ee6255cd040 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
>  		dev->mode_config.max_width = 4096;
>  		dev->mode_config.max_height = 4096;
>  	} else {
> -		dev->mode_config.max_width = 8192;
> -		dev->mode_config.max_height = 8192;
> +		dev->mode_config.max_width = 32767;
> +		dev->mode_config.max_height = 32767;

It appears that neither Mesa nor glamor will check whether window system
buffers exceed the capabilities of the 3D engine. So trying to use a >16k
trips an assert when genxml tries to pack the surface_state.

So looks like we'll need to limit this to 16k for gen7+, and leave it
at 8k for gen4+. If userspace gets smarter later on we could add a new
client cap to expose higher limits.

If I'm reading the spec correctly gen4+ render engine has a stride
limit of 512KiB and gen7+ has 256KiB. So my choice of 256KiB seems
good enough for both.

>  	}
>  
>  	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
> -- 
> 2.16.4

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

Re: [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Chris Wilson]

Quoting Ville Syrjälä (2018-09-19 17:59:51)
> On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > With gtt remapping in place we can use arbitraily large framebuffers.
> > Let's bump the limits as high as we can (32k-1). Going beyond that
> > would require switching our s16.16 src coordinate representation to
> > something with more spare bits.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 346572cf734a..0ee6255cd040 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> >               dev->mode_config.max_width = 4096;
> >               dev->mode_config.max_height = 4096;
> >       } else {
> > -             dev->mode_config.max_width = 8192;
> > -             dev->mode_config.max_height = 8192;
> > +             dev->mode_config.max_width = 32767;
> > +             dev->mode_config.max_height = 32767;
> 
> It appears that neither Mesa nor glamor will check whether window system
> buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> trips an assert when genxml tries to pack the surface_state.
> 
> So looks like we'll need to limit this to 16k for gen7+, and leave it
> at 8k for gen4+. If userspace gets smarter later on we could add a new
> client cap to expose higher limits.

At which point, the client can just ignore this field and just use
rejection criteria from addfb2 and/or setcrtc (or the atomic variant).

Or we can just keep this field as meaning the maximum size of a single
CRTC and just ignore it entirely in -modesetting for fb size as we do
elsewhere.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Ville Syrjälä]

On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > would require switching our s16.16 src coordinate representation to
> > > something with more spare bits.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 346572cf734a..0ee6255cd040 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > >               dev->mode_config.max_width = 4096;
> > >               dev->mode_config.max_height = 4096;
> > >       } else {
> > > -             dev->mode_config.max_width = 8192;
> > > -             dev->mode_config.max_height = 8192;
> > > +             dev->mode_config.max_width = 32767;
> > > +             dev->mode_config.max_height = 32767;
> > 
> > It appears that neither Mesa nor glamor will check whether window system
> > buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> > trips an assert when genxml tries to pack the surface_state.
> > 
> > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > client cap to expose higher limits.
> 
> At which point, the client can just ignore this field and just use
> rejection criteria from addfb2 and/or setcrtc (or the atomic variant).

I suppose. Though probing the max size using addfb might be a bit
tedious. That's assuming the client wants to report the max in some
way, as X does.

> 
> Or we can just keep this field as meaning the maximum size of a single
> CRTC and just ignore it entirely in -modesetting for fb size as we do
> elsewhere.

Would require changing the core addfb code to ignore these
limits for i915 but keep chekcing them for the other drivers.
So a bit of work, and I'm not really sure what the actual
benefit for i915 would be.

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

Re: [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Chris Wilson]

Quoting Ville Syrjälä (2018-09-20 20:41:30)
> On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > > would require switching our s16.16 src coordinate representation to
> > > > something with more spare bits.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 346572cf734a..0ee6255cd040 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > > >               dev->mode_config.max_width = 4096;
> > > >               dev->mode_config.max_height = 4096;
> > > >       } else {
> > > > -             dev->mode_config.max_width = 8192;
> > > > -             dev->mode_config.max_height = 8192;
> > > > +             dev->mode_config.max_width = 32767;
> > > > +             dev->mode_config.max_height = 32767;
> > > 
> > > It appears that neither Mesa nor glamor will check whether window system
> > > buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> > > trips an assert when genxml tries to pack the surface_state.
> > > 
> > > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > > client cap to expose higher limits.
> > 
> > At which point, the client can just ignore this field and just use
> > rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
> 
> I suppose. Though probing the max size using addfb might be a bit
> tedious. That's assuming the client wants to report the max in some
> way, as X does.
> 
> > 
> > Or we can just keep this field as meaning the maximum size of a single
> > CRTC and just ignore it entirely in -modesetting for fb size as we do
> > elsewhere.
> 
> Would require changing the core addfb code to ignore these
> limits for i915 but keep chekcing them for the other drivers.
> So a bit of work, and I'm not really sure what the actual
> benefit for i915 would be.

Why is the core addfb using these fields? Since when did they *stop*
being per-CRTC limits?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k

[Ville Syrjälä]

On Thu, Sep 20, 2018 at 08:46:42PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-20 20:41:30)
> > On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > > > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > > > would require switching our s16.16 src coordinate representation to
> > > > > something with more spare bits.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index 346572cf734a..0ee6255cd040 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > > > >               dev->mode_config.max_width = 4096;
> > > > >               dev->mode_config.max_height = 4096;
> > > > >       } else {
> > > > > -             dev->mode_config.max_width = 8192;
> > > > > -             dev->mode_config.max_height = 8192;
> > > > > +             dev->mode_config.max_width = 32767;
> > > > > +             dev->mode_config.max_height = 32767;
> > > > 
> > > > It appears that neither Mesa nor glamor will check whether window system
> > > > buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> > > > trips an assert when genxml tries to pack the surface_state.
> > > > 
> > > > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > > > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > > > client cap to expose higher limits.
> > > 
> > > At which point, the client can just ignore this field and just use
> > > rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
> > 
> > I suppose. Though probing the max size using addfb might be a bit
> > tedious. That's assuming the client wants to report the max in some
> > way, as X does.
> > 
> > > 
> > > Or we can just keep this field as meaning the maximum size of a single
> > > CRTC and just ignore it entirely in -modesetting for fb size as we do
> > > elsewhere.
> > 
> > Would require changing the core addfb code to ignore these
> > limits for i915 but keep chekcing them for the other drivers.
> > So a bit of work, and I'm not really sure what the actual
> > benefit for i915 would be.
> 
> Why is the core addfb using these fields? Since when did they *stop*
> being per-CRTC limits?

Looks like addfb has been using them since the beginning:

commit f453ba0460742ad027ae0c4c7d61e62817b3e7ef
Author: Dave Airlie <airlied@redhat.com>
Date:   Fri Nov 7 14:05:41 2008 -0800

    DRM: add mode setting support

And looks like they never matched the per-crtc limits for i915 either.
Until HSW hdisplay limit was 4k but max_width was already set to 8k
in:

commit 79e539453b34e35f39299a899d263b0a1f1670bd
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri Nov 7 14:24:08 2008 -0800

    DRM: i915: add mode setting support

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