diff mbox series

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

Message ID 20180913200140.14073-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: drm/i915: GTT remapping for display | expand

Commit Message

Ville Syrjälä Sept. 13, 2018, 8:01 p.m. UTC
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(-)

Comments

Chris Wilson Sept. 13, 2018, 8:19 p.m. UTC | #1
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
Ville Syrjälä Sept. 14, 2018, 12:58 p.m. UTC | #2
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.
Chris Wilson Sept. 14, 2018, 1 p.m. UTC | #3
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
diff mbox series

Patch

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 },