[Intel-gfx] [PATCH v2 1/6] drm/i915: Add a new "remapped" gtt_view
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Jan 15 13:43:41 UTC 2019
On Mon, Jan 14, 2019 at 09:55:10AM +0000, Tvrtko Ursulin wrote:
>
> On 11/01/2019 19:46, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at 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.
>
> I think I asked this before, so sorry if you also answered it - what
> about rotated setups which go over display engine stride limits?
We already have the code to do the rotated mapping.
>
> > 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)
> > v3: Use i915_gem_object_get_dma_address() (Chris)
> > Trim the sg (Tvrtko)
> > v4: Actually trim this time. Limit the max length
> > to one row of pages to keep things simple
> >
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++
> > drivers/gpu/drm/i915/i915_drv.h | 4 ++
> > drivers/gpu/drm/i915/i915_gem.c | 17 ++++-
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 88 +++++++++++++++++++++++
> > 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 +-
> > 10 files changed, 163 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index d460ef522d9c..cab7771799ad 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_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5df26ccda8a4..cef76cf343c4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2845,6 +2845,10 @@ i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj,
> > unsigned int n);
> >
> > dma_addr_t
> > +i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj,
> > + unsigned long n,
> > + unsigned int *len);
> > +dma_addr_t
> > i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj,
> > unsigned long n);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index ea85da393662..f65d81f4f3d7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -6041,16 +6041,29 @@ i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj,
> > }
> >
> > dma_addr_t
> > -i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj,
> > - unsigned long n)
> > +i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj,
> > + unsigned long n,
> > + unsigned int *len)
> > {
> > struct scatterlist *sg;
> > unsigned int offset;
> >
> > sg = i915_gem_object_get_sg(obj, n, &offset);
> > +
> > + if (len)
> > + *len = sg_dma_len(sg) - (offset << PAGE_SHIFT);
> > +
> > return sg_dma_address(sg) + (offset << PAGE_SHIFT);
> > }
> >
> > +dma_addr_t
> > +i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj,
> > + unsigned long n)
> > +{
> > + return i915_gem_object_get_dma_address_len(obj, n, NULL);
> > +}
> > +
> > +
> > int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
> > {
> > struct sg_table *pages;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index a8807fbed0aa..be5dbbb1b6ff 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -3605,6 +3605,89 @@ intel_rotate_pages(struct intel_rotation_info *rot_info,
> > return ERR_PTR(ret);
> > }
> >
> > +static struct scatterlist *
> > +remap_pages(struct drm_i915_gem_object *obj, unsigned int offset,
> > + unsigned int width, unsigned int height,
> > + unsigned int stride,
> > + struct sg_table *st, struct scatterlist *sg)
> > +{
> > + unsigned int row;
> > +
> > + for (row = 0; row < height; row++) {
> > + unsigned int left = width * I915_GTT_PAGE_SIZE;
> > +
> > + while (left) {
> > + dma_addr_t addr;
> > + unsigned int length;
> > +
> > + /* 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.
> > + */
> > +
> > + addr = i915_gem_object_get_dma_address_len(obj, offset, &length);
> > +
> > + length = min(left, length);
> > +
> > + st->nents++;
> > +
> > + sg_set_page(sg, NULL, length, 0);
> > + sg_dma_address(sg) = addr;
> > + sg_dma_len(sg) = length;
> > + sg = sg_next(sg);
> > +
> > + offset += length / I915_GTT_PAGE_SIZE;
> > + left -= length;
> > + }
> > +
> > + offset += stride - width;
> > + }
> > +
> > + return sg;
> > +}
> > +
> > +static noinline struct sg_table *
> > +intel_remap_pages(struct intel_remapped_info *rem_info,
> > + struct drm_i915_gem_object *obj)
> > +{
> > + unsigned int size = intel_remapped_info_size(rem_info);
> > + struct sg_table *st;
> > + struct scatterlist *sg;
> > + int ret = -ENOMEM;
> > + int i;
> > +
> > + /* 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;
> > +
> > + st->nents = 0;
> > + sg = st->sgl;
> > +
> > + for (i = 0 ; i < ARRAY_SIZE(rem_info->plane); i++) {
> > + sg = remap_pages(obj, rem_info->plane[i].offset,
> > + rem_info->plane[i].width, rem_info->plane[i].height,
> > + rem_info->plane[i].stride, st, sg);
> > + }
> > +
> > + i915_sg_trim(st);
> > +
> > + return st;
> > +
> > +err_sg_alloc:
> > + kfree(st);
> > +err_st_alloc:
> > +
> > + 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)
> > @@ -3683,6 +3766,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 e2360f16427a..8f26b4292084 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -162,11 +162,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 {
> > @@ -178,12 +185,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.
> > @@ -192,6 +207,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;
> > }
> > @@ -203,6 +219,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 5b4d78cdb4ca..9a039c36dc0c 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 5dc0de89c49e..c17a1723a589 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1942,6 +1942,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 3b051fdd0fce..7b9b76d5895e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1601,6 +1601,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[] = {
>
> Why this?
Just a new name to better reflect the widened scope of the struct.
>
> > { .width = 1, .height = 1, .stride = 1 },
> > { .width = 2, .height = 2, .stride = 2 },
> > { .width = 4, .height = 4, .stride = 4 },
> >
>
> Regards,
>
> Tvrtko
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list