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@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx