[Intel-gfx] [PATCH v3 1/8] drm/i915: Add a new "remapped" gtt_view

Daniel Vetter daniel at ffwll.ch
Wed Jan 30 19:06:08 UTC 2019


On Fri, Jan 18, 2019 at 05:27:13PM +0200, 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.
> 
> 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 24d6d4ce14ef..b7c6b06cf2f3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -198,6 +198,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 310d9e1e1620..4cb0c9b756d4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2855,6 +2855,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 b359390ba22c..1e7c95d0fea1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5660,16 +5660,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 9081e3bc5a59..64e278c9479b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3612,6 +3612,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;

Not sure trying to keep the compacted sg list is worth all the code, but
can't hurt really either. lgtm, and I trust Chris to have fully reviewed
test coverage for all the corner cases :-)

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


> +	}
> +
> +	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)
> @@ -3690,6 +3773,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 9229b03d629b..7892e5467c6a 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 b087ed285cc1..6e5ce084b244 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1947,6 +1947,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 e5a436c33307..d025821a71d5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1604,6 +1604,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.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list