Re: [PATCH 3/4] drm/i915: Support NV12 in rotated GGTT mapping

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Sep 21, 2015 at 10:45:34AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Just adding the rotated UV plane at the end of the rotated Y plane.
> 
> v2: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c  | 37 ++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
>  3 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 59c934fb9230..2df9d16dcefd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3272,10 +3272,13 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  {
>  	struct intel_rotation_info *rot_info = &ggtt_view->rotation_info;
>  	unsigned int size_pages = rot_info->size >> PAGE_SHIFT;
> +	unsigned int size_pages_uv;
>  	struct sg_page_iter sg_iter;
>  	unsigned long i;
>  	dma_addr_t *page_addr_list;
>  	struct sg_table *st;
> +	unsigned int uv_start_page;
> +	struct scatterlist *sg;
>  	int ret = -ENOMEM;
>  
>  	/* Allocate a temporary list of source pages for random access. */
> @@ -3284,12 +3287,18 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  	if (!page_addr_list)
>  		return ERR_PTR(ret);
>  
> +	/* Account for UV plane with NV12. */
> +	if (rot_info->pixel_format == DRM_FORMAT_NV12)
> +		size_pages_uv = rot_info->size_uv >> PAGE_SHIFT;
> +	else
> +		size_pages_uv = 0;
> +
>  	/* Allocate target SG list. */
>  	st = kmalloc(sizeof(*st), GFP_KERNEL);
>  	if (!st)
>  		goto err_st_alloc;
>  
> -	ret = sg_alloc_table(st, size_pages, GFP_KERNEL);
> +	ret = sg_alloc_table(st, size_pages + size_pages_uv, GFP_KERNEL);
>  	if (ret)
>  		goto err_sg_alloc;
>  
> @@ -3301,15 +3310,30 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  	}
>  
>  	/* Rotate the pages. */
> -	rotate_pages(page_addr_list, 0,
> +	sg = rotate_pages(page_addr_list, 0,
>  		     rot_info->width_pages, rot_info->height_pages,
>  		     st, NULL);
>  
> +	/* Append the UV plane if NV12. */
> +	if (rot_info->pixel_format == DRM_FORMAT_NV12) {
> +		uv_start_page = size_pages;
> +
> +		/* Check for tile-row un-alignment. */
> +		if (offset_in_page(rot_info->uv_offset))
> +			uv_start_page--;
> +
> +		rotate_pages(page_addr_list, uv_start_page,
> +			     rot_info->width_pages_uv,
> +			     rot_info->height_pages_uv,
> +			     st, sg);
> +	}
> +
>  	DRM_DEBUG_KMS(
> -		      "Created rotated page mapping for object size %zu (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages).\n",
> +		      "Created rotated page mapping for object size %zu (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages (%u plane 0)).\n",
>  		      obj->base.size, rot_info->pitch, rot_info->height,
>  		      rot_info->pixel_format, rot_info->width_pages,
> -		      rot_info->height_pages, size_pages);
> +		      rot_info->height_pages, size_pages + size_pages_uv,
> +		      size_pages);
>  
>  	drm_free_large(page_addr_list);
>  
> @@ -3321,10 +3345,11 @@ err_st_alloc:
>  	drm_free_large(page_addr_list);
>  
>  	DRM_DEBUG_KMS(
> -		      "Failed to create rotated mapping for object size %zu! (%d) (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages)\n",
> +		      "Failed to create rotated mapping for object size %zu! (%d) (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages (%u plane 0))\n",
>  		      obj->base.size, ret, rot_info->pitch, rot_info->height,
>  		      rot_info->pixel_format, rot_info->width_pages,
> -		      rot_info->height_pages, size_pages);
> +		      rot_info->height_pages, size_pages + size_pages_uv,
> +		      size_pages);
>  	return ERR_PTR(ret);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 82750073d5b3..197183d5c543 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -138,10 +138,13 @@ enum i915_ggtt_view_type {
>  struct intel_rotation_info {
>  	unsigned int height;
>  	unsigned int pitch;
> +	unsigned int uv_offset;
>  	uint32_t pixel_format;
>  	uint64_t fb_modifier;
>  	unsigned int width_pages, height_pages;
>  	uint64_t size;
> +	unsigned int width_pages_uv, height_pages_uv;
> +	uint64_t size_uv;
>  };
>  
>  struct i915_ggtt_view {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e19b8e699c00..2db7cc42539c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2263,6 +2263,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
>  	info->height = fb->height;
>  	info->pixel_format = fb->pixel_format;
>  	info->pitch = fb->pitches[0];
> +	info->uv_offset = fb->offsets[1];

OK, so this already came up a bit when I was looking at Chandra's stuff,
but we really have to define what fb->offsets[] means now.

On one hand, it would be very logical for it to be a raw byte offset
into the object where the fb starts. But on the other hand that perhaps
makes it a bit more difficult for userspace to compute offsets[1] for
the CbCr plane. Also we have fences to consider, and currently we even
require that the fence stride matches the fb stride, even though that
may not really be necessary. So maybe it should be the linear offset
instead.

So which way should we go?


>  	info->fb_modifier = fb->modifier[0];
>  
>  	tile_height = intel_tile_height(fb->dev, fb->pixel_format,
> @@ -2272,6 +2273,17 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
>  	info->height_pages = DIV_ROUND_UP(fb->height, tile_height);
>  	info->size = info->width_pages * info->height_pages * PAGE_SIZE;
>  
> +	if (info->pixel_format == DRM_FORMAT_NV12) {
> +		tile_height = intel_tile_height(fb->dev, fb->pixel_format,
> +						fb->modifier[0], 1);
> +		tile_pitch = PAGE_SIZE / tile_height;
> +		info->width_pages_uv = DIV_ROUND_UP(fb->pitches[0], tile_pitch);
> +		info->height_pages_uv = DIV_ROUND_UP(fb->height / 2,
> +						     tile_height);
> +		info->size_uv = info->width_pages_uv * info->height_pages_uv *
> +				PAGE_SIZE;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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