[PATCH 1/4] drm/i915: Support planar formats in tile height calculations

[PATCH 1/4] drm/i915: Support planar formats in tile height calculations

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This will be needed for NV12 support.

v2: Rebase.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 43b0f17ad1fa..03e550874aec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2224,7 +2224,7 @@ static bool need_vtd_wa(struct drm_device *dev)
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
-		  uint64_t fb_format_modifier)
+		  uint64_t fb_format_modifier, unsigned int plane)
 {
 	unsigned int tile_height;
 	uint32_t pixel_bytes;
@@ -2240,7 +2240,7 @@ intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
 		tile_height = 32;
 		break;
 	case I915_FORMAT_MOD_Yf_TILED:
-		pixel_bytes = drm_format_plane_cpp(pixel_format, 0);
+		pixel_bytes = drm_format_plane_cpp(pixel_format, plane);
 		switch (pixel_bytes) {
 		default:
 		case 1:
@@ -2274,7 +2274,7 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height,
 		      uint32_t pixel_format, uint64_t fb_format_modifier)
 {
 	return ALIGN(height, intel_tile_height(dev, pixel_format,
-					       fb_format_modifier));
+					       fb_format_modifier, 0));
 }
 
 static int
@@ -2300,7 +2300,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
 	info->fb_modifier = fb->modifier[0];
 
 	tile_height = intel_tile_height(fb->dev, fb->pixel_format,
-					fb->modifier[0]);
+					fb->modifier[0], 0);
 	tile_pitch = PAGE_SIZE / tile_height;
 	info->width_pages = DIV_ROUND_UP(fb->pitches[0], tile_pitch);
 	info->height_pages = DIV_ROUND_UP(fb->height, tile_height);
@@ -3094,7 +3094,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride = Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->pixel_format,
-						fb->modifier[0]);
+						fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
 		x_offset = stride * tile_height - y - src_h;
 		y_offset = x;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 320c9e6bd848..429d05406a5c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1061,7 +1061,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
-		  uint64_t fb_format_modifier);
+		  uint64_t fb_format_modifier, unsigned int plane);
 
 static inline bool
 intel_rotation_90_or_270(unsigned int rotation)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9d8af2f8a875..f0bfe1647967 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -228,7 +228,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride: Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->pixel_format,
-						fb->modifier[0]);
+						fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
 		plane_size = (src_w << 16) | src_h;
 		x_offset = stride * tile_height - y - (src_h + 1);
-- 
2.4.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/4] NV12 rotation GTT handling prep work

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

GTT page remapping logic for 90/270 rotation needs some
extensions to support NV12 90/270 rotation work which is
currently underway.

Main thing is really to support building of the rotated
page mapping from two planes instead of one, and adding
appropriate calculations for the half-height UV plane
geometry.

Those are stored in the existing rotation info data
associated with the rotated view and are also used to
return the appropriate plane start address when queried
from the display code.

Tvrtko Ursulin (4):
  drm/i915: Support planar formats in tile height calculations
  drm/i915: Support appending to the rotated pages mapping
  drm/i915: Support NV12 in rotated GGTT mapping
  drm/i915: Enable querying offset of UV plane with
    intel_plane_obj_offset

 drivers/gpu/drm/i915/i915_gem_gtt.c  | 58 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  4 +++
 drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  6 ++--
 drivers/gpu/drm/i915/intel_sprite.c  |  4 +--
 5 files changed, 93 insertions(+), 27 deletions(-)

-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/4] drm/i915: Support planar formats in tile height calculations

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This will be needed for NV12 support.

v2: Rebase.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc0086748b71..e19b8e699c00 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2190,7 +2190,7 @@ static bool need_vtd_wa(struct drm_device *dev)
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
-		  uint64_t fb_format_modifier)
+		  uint64_t fb_format_modifier, unsigned int plane)
 {
 	unsigned int tile_height;
 	uint32_t pixel_bytes;
@@ -2206,7 +2206,7 @@ intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
 		tile_height = 32;
 		break;
 	case I915_FORMAT_MOD_Yf_TILED:
-		pixel_bytes = drm_format_plane_cpp(pixel_format, 0);
+		pixel_bytes = drm_format_plane_cpp(pixel_format, plane);
 		switch (pixel_bytes) {
 		default:
 		case 1:
@@ -2240,7 +2240,7 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height,
 		      uint32_t pixel_format, uint64_t fb_format_modifier)
 {
 	return ALIGN(height, intel_tile_height(dev, pixel_format,
-					       fb_format_modifier));
+					       fb_format_modifier, 0));
 }
 
 static int
@@ -2266,7 +2266,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
 	info->fb_modifier = fb->modifier[0];
 
 	tile_height = intel_tile_height(fb->dev, fb->pixel_format,
-					fb->modifier[0]);
+					fb->modifier[0], 0);
 	tile_pitch = PAGE_SIZE / tile_height;
 	info->width_pages = DIV_ROUND_UP(fb->pitches[0], tile_pitch);
 	info->height_pages = DIV_ROUND_UP(fb->height, tile_height);
@@ -3069,7 +3069,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride = Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->pixel_format,
-						fb->modifier[0]);
+						fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
 		x_offset = stride * tile_height - y - src_h;
 		y_offset = x;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1df6ebf50146..94dab9bd8ebd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1085,7 +1085,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
-		  uint64_t fb_format_modifier);
+		  uint64_t fb_format_modifier, unsigned int plane);
 
 static inline bool
 intel_rotation_90_or_270(unsigned int rotation)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 79f7cc247ab7..4372fa0b1ec5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -240,7 +240,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride: Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->pixel_format,
-						fb->modifier[0]);
+						fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
 		plane_size = (src_w << 16) | src_h;
 		x_offset = stride * tile_height - y - (src_h + 1);
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: Support appending to the rotated pages mapping

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

By providing a start offset into the source array of pages, and returning the
end position in the scatter-gather table, we will be able to append the UV
plane to the rotated mapping in later patches.

v2: Rebase.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 87862813cfde..59c934fb9230 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3234,15 +3234,18 @@ i915_gem_obj_lookup_or_create_ggtt_vma(struct drm_i915_gem_object *obj,
 
 }
 
-static void
-rotate_pages(dma_addr_t *in, unsigned int width, unsigned int height,
-	     struct sg_table *st)
+static struct scatterlist *
+rotate_pages(dma_addr_t *in, unsigned int offset,
+	     unsigned int width, unsigned int height,
+	     struct sg_table *st, struct scatterlist *sg)
 {
 	unsigned int column, row;
 	unsigned int src_idx;
-	struct scatterlist *sg = st->sgl;
 
-	st->nents = 0;
+	if (!sg) {
+		st->nents = 0;
+		sg = st->sgl;
+	}
 
 	for (column = 0; column < width; column++) {
 		src_idx = width * (height - 1) + column;
@@ -3253,12 +3256,14 @@ rotate_pages(dma_addr_t *in, unsigned int width, unsigned int height,
 			 * The only thing we need are DMA addresses.
 			 */
 			sg_set_page(sg, NULL, PAGE_SIZE, 0);
-			sg_dma_address(sg) = in[src_idx];
+			sg_dma_address(sg) = in[offset + src_idx];
 			sg_dma_len(sg) = PAGE_SIZE;
 			sg = sg_next(sg);
 			src_idx -= width;
 		}
 	}
+
+	return sg;
 }
 
 static struct sg_table *
@@ -3296,9 +3301,9 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	}
 
 	/* Rotate the pages. */
-	rotate_pages(page_addr_list,
+	rotate_pages(page_addr_list, 0,
 		     rot_info->width_pages, rot_info->height_pages,
-		     st);
+		     st, NULL);
 
 	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",
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Tvrtko Ursulin]

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];
 	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

[PATCH 4/4] drm/i915: Enable querying offset of UV plane with intel_plane_obj_offset

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

v2: Rebase.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2df9d16dcefd..4023c86fabf9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3322,6 +3322,8 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 		if (offset_in_page(rot_info->uv_offset))
 			uv_start_page--;
 
+		rot_info->uv_start_page = uv_start_page;
+
 		rotate_pages(page_addr_list, uv_start_page,
 			     rot_info->width_pages_uv,
 			     rot_info->height_pages_uv,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 197183d5c543..430cc283d3c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -145,6 +145,7 @@ struct intel_rotation_info {
 	uint64_t size;
 	unsigned int width_pages_uv, height_pages_uv;
 	uint64_t size_uv;
+	unsigned int uv_start_page;
 };
 
 struct i915_ggtt_view {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2db7cc42539c..ebfd34beb841 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2891,14 +2891,29 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
 }
 
 unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-				     struct drm_i915_gem_object *obj)
+				     struct drm_i915_gem_object *obj,
+				     unsigned int plane)
 {
 	const struct i915_ggtt_view *view = &i915_ggtt_view_normal;
+	struct i915_vma *vma;
+	unsigned char *offset;
 
 	if (intel_rotation_90_or_270(intel_plane->base.state->rotation))
 		view = &i915_ggtt_view_rotated;
 
-	return i915_gem_obj_ggtt_offset_view(obj, view);
+	vma = i915_gem_obj_to_ggtt_view(obj, view);
+	if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
+		view->type))
+		return -1;
+
+	offset = (unsigned char *)vma->node.start;
+
+	if (plane == 1) {
+		offset += vma->ggtt_view.rotation_info.uv_start_page *
+			  PAGE_SIZE;
+	}
+
+	return (unsigned long)offset;
 }
 
 static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
@@ -3054,7 +3069,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
-	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
+	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
 
 	/*
 	 * FIXME: intel_plane_state->src, dst aren't set when transitional
@@ -11414,8 +11429,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (ret)
 		goto cleanup_pending;
 
-	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary), obj)
-						  + intel_crtc->dspaddr_offset;
+	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
+						  obj, 0);
+	work->gtt_offset += intel_crtc->dspaddr_offset;
 
 	if (mmio_flip) {
 		ret = intel_queue_mmio_flip(dev, crtc, fb, obj, ring,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 94dab9bd8ebd..13c64c5ec22b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1166,7 +1166,9 @@ int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
 
 unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-				     struct drm_i915_gem_object *obj);
+				     struct drm_i915_gem_object *obj,
+				     unsigned int plane);
+
 u32 skl_plane_ctl_format(uint32_t pixel_format);
 u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
 u32 skl_plane_ctl_rotation(unsigned int rotation);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4372fa0b1ec5..4349fde4b72c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -235,7 +235,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	else if (key->flags & I915_SET_COLORKEY_SOURCE)
 		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
 
-	surf_addr = intel_plane_obj_offset(intel_plane, obj);
+	surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
 
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride: Surface height in tiles */
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Support planar formats in tile height calculations

[Joonas Lahtinen]

On ma, 2015-09-21 at 10:45 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This will be needed for NV12 support.
> 
> v2: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fc0086748b71..e19b8e699c00 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2190,7 +2190,7 @@ static bool need_vtd_wa(struct drm_device *dev)
>  
>  unsigned int
>  intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
> -		  uint64_t fb_format_modifier)
> +		  uint64_t fb_format_modifier, unsigned int plane)
>  {
>  	unsigned int tile_height;
>  	uint32_t pixel_bytes;
> @@ -2206,7 +2206,7 @@ intel_tile_height(struct drm_device *dev,
> uint32_t pixel_format,
>  		tile_height = 32;
>  		break;
>  	case I915_FORMAT_MOD_Yf_TILED:
> -		pixel_bytes = drm_format_plane_cpp(pixel_format, 0);
> +		pixel_bytes = drm_format_plane_cpp(pixel_format,
> plane);
>  		switch (pixel_bytes) {
>  		default:
>  		case 1:
> @@ -2240,7 +2240,7 @@ intel_fb_align_height(struct drm_device *dev,
> unsigned int height,
>  		      uint32_t pixel_format, uint64_t
> fb_format_modifier)
>  {
>  	return ALIGN(height, intel_tile_height(dev, pixel_format,
> -					       fb_format_modifier));
> +					       fb_format_modifier,
> 0));
>  }
>  
>  static int
> @@ -2266,7 +2266,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view
> *view, struct drm_framebuffer *fb,
>  	info->fb_modifier = fb->modifier[0];
>  
>  	tile_height = intel_tile_height(fb->dev, fb->pixel_format,
> -					fb->modifier[0]);
> +					fb->modifier[0], 0);
>  	tile_pitch = PAGE_SIZE / tile_height;
>  	info->width_pages = DIV_ROUND_UP(fb->pitches[0],
> tile_pitch);
>  	info->height_pages = DIV_ROUND_UP(fb->height, tile_height);
> @@ -3069,7 +3069,7 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb
> ->pixel_format,
> -						fb->modifier[0]);
> +						fb->modifier[0], 0);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
>  		x_offset = stride * tile_height - y - src_h;
>  		y_offset = x;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1df6ebf50146..94dab9bd8ebd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1085,7 +1085,7 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>  
>  unsigned int
>  intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
> -		  uint64_t fb_format_modifier);
> +		  uint64_t fb_format_modifier, unsigned int plane);
>  
>  static inline bool
>  intel_rotation_90_or_270(unsigned int rotation)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 79f7cc247ab7..4372fa0b1ec5 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -240,7 +240,7 @@ skl_update_plane(struct drm_plane *drm_plane,
> struct drm_crtc *crtc,
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride: Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb
> ->pixel_format,
> -						fb->modifier[0]);
> +						fb->modifier[0], 0);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
>  		plane_size = (src_w << 16) | src_h;
>  		x_offset = stride * tile_height - y - (src_h + 1);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Support appending to the rotated pages mapping

[Joonas Lahtinen]

Hi,

On ma, 2015-09-21 at 10:45 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> By providing a start offset into the source array of pages, and
> returning the
> end position in the scatter-gather table, we will be able to append
> the UV
> plane to the rotated mapping in later patches.
> 
> v2: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 87862813cfde..59c934fb9230 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3234,15 +3234,18 @@ i915_gem_obj_lookup_or_create_ggtt_vma(struct
> drm_i915_gem_object *obj,
>  
>  }
>  
> -static void
> -rotate_pages(dma_addr_t *in, unsigned int width, unsigned int
> height,
> -	     struct sg_table *st)
> +static struct scatterlist *
> +rotate_pages(dma_addr_t *in, unsigned int offset,
> +	     unsigned int width, unsigned int height,
> +	     struct sg_table *st, struct scatterlist *sg)
>  {
>  	unsigned int column, row;
>  	unsigned int src_idx;
> -	struct scatterlist *sg = st->sgl;
>  
> -	st->nents = 0;
> +	if (!sg) {
> +		st->nents = 0;
> +		sg = st->sgl;
> +	}
>  
>  	for (column = 0; column < width; column++) {
>  		src_idx = width * (height - 1) + column;
> @@ -3253,12 +3256,14 @@ rotate_pages(dma_addr_t *in, unsigned int
> width, unsigned int height,
>  			 * The only thing we need are DMA addresses.
>  			 */
>  			sg_set_page(sg, NULL, PAGE_SIZE, 0);
> -			sg_dma_address(sg) = in[src_idx];
> +			sg_dma_address(sg) = in[offset + src_idx];
>  			sg_dma_len(sg) = PAGE_SIZE;
>  			sg = sg_next(sg);
>  			src_idx -= width;
>  		}
>  	}
> +
> +	return sg;
>  }
>  
>  static struct sg_table *
> @@ -3296,9 +3301,9 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view
> *ggtt_view,
>  	}
>  
>  	/* Rotate the pages. */
> -	rotate_pages(page_addr_list,
> +	rotate_pages(page_addr_list, 0,
>  		     rot_info->width_pages, rot_info->height_pages,
> -		     st);
> +		     st, NULL);
>  
>  	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",
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Joonas Lahtinen]

On ma, 2015-09-21 at 10:45 +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>
> 

One comment below, otherwise.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.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;

Could be initialized to zero here already as majority of the time it'll
be unchanged.

>  > 	> 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];
>  > 	> 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;
>  }
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Enable querying offset of UV plane with intel_plane_obj_offset

[Joonas Lahtinen]

On ma, 2015-09-21 at 10:45 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> v2: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 ++
>  drivers/gpu/drm/i915/i915_gem_gtt.h  |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +++-
>  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>  5 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 2df9d16dcefd..4023c86fabf9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3322,6 +3322,8 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view
> *ggtt_view,
>  		if (offset_in_page(rot_info->uv_offset))
>  			uv_start_page--;
>  
> +		rot_info->uv_start_page = uv_start_page;
> +
>  		rotate_pages(page_addr_list, uv_start_page,
>  			     rot_info->width_pages_uv,
>  			     rot_info->height_pages_uv,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 197183d5c543..430cc283d3c9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -145,6 +145,7 @@ struct intel_rotation_info {
>  	uint64_t size;
>  	unsigned int width_pages_uv, height_pages_uv;
>  	uint64_t size_uv;
> +	unsigned int uv_start_page;
>  };
>  
>  struct i915_ggtt_view {
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2db7cc42539c..ebfd34beb841 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2891,14 +2891,29 @@ u32 intel_fb_stride_alignment(struct
> drm_device *dev, uint64_t fb_modifier,
>  }
>  
>  unsigned long intel_plane_obj_offset(struct intel_plane
> *intel_plane,
> -				     struct drm_i915_gem_object
> *obj)
> +				     struct drm_i915_gem_object
> *obj,
> +				     unsigned int plane)
>  {
>  	const struct i915_ggtt_view *view = &i915_ggtt_view_normal;
> +	struct i915_vma *vma;
> +	unsigned char *offset;
>  
>  	if (intel_rotation_90_or_270(intel_plane->base.state
> ->rotation))
>  		view = &i915_ggtt_view_rotated;
>  
> -	return i915_gem_obj_ggtt_offset_view(obj, view);
> +	vma = i915_gem_obj_to_ggtt_view(obj, view);
> +	if (WARN(!vma, "ggtt vma for display object not found!
> (view=%u)\n",
> +		view->type))
> +		return -1;
> +
> +	offset = (unsigned char *)vma->node.start;
> +
> +	if (plane == 1) {
> +		offset += vma->ggtt_view.rotation_info.uv_start_page
> *
> +			  PAGE_SIZE;
> +	}
> +
> +	return (unsigned long)offset;
>  }
>  
>  static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
> @@ -3054,7 +3069,7 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
>  	obj = intel_fb_obj(fb);
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
> -	surf_addr = intel_plane_obj_offset(to_intel_plane(plane),
> obj);
> +	surf_addr = intel_plane_obj_offset(to_intel_plane(plane),
> obj, 0);
>  
>  	/*
>  	 * FIXME: intel_plane_state->src, dst aren't set when
> transitional
> @@ -11414,8 +11429,9 @@ static int intel_crtc_page_flip(struct
> drm_crtc *crtc,
>  	if (ret)
>  		goto cleanup_pending;
>  
> -	work->gtt_offset =
> intel_plane_obj_offset(to_intel_plane(primary), obj)
> -						  + intel_crtc
> ->dspaddr_offset;
> +	work->gtt_offset =
> intel_plane_obj_offset(to_intel_plane(primary),
> +						  obj, 0);
> +	work->gtt_offset += intel_crtc->dspaddr_offset;
>  
>  	if (mmio_flip) {
>  		ret = intel_queue_mmio_flip(dev, crtc, fb, obj,
> ring,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 94dab9bd8ebd..13c64c5ec22b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1166,7 +1166,9 @@ int skl_update_scaler_crtc(struct
> intel_crtc_state *crtc_state);
>  int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state
> *crtc_state);
>  
>  unsigned long intel_plane_obj_offset(struct intel_plane
> *intel_plane,
> -				     struct drm_i915_gem_object
> *obj);
> +				     struct drm_i915_gem_object
> *obj,
> +				     unsigned int plane);
> +
>  u32 skl_plane_ctl_format(uint32_t pixel_format);
>  u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
>  u32 skl_plane_ctl_rotation(unsigned int rotation);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 4372fa0b1ec5..4349fde4b72c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -235,7 +235,7 @@ skl_update_plane(struct drm_plane *drm_plane,
> struct drm_crtc *crtc,
>  	else if (key->flags & I915_SET_COLORKEY_SOURCE)
>  		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
>  
> -	surf_addr = intel_plane_obj_offset(intel_plane, obj);
> +	surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
>  
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride: Surface height in tiles */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] NV12 rotation GTT handling prep work

[Ville Syrjälä]

On Mon, Sep 21, 2015 at 10:45:31AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> GTT page remapping logic for 90/270 rotation needs some
> extensions to support NV12 90/270 rotation work which is
> currently underway.
> 
> Main thing is really to support building of the rotated
> page mapping from two planes instead of one, and adding
> appropriate calculations for the half-height UV plane
> geometry.
> 
> Those are stored in the existing rotation info data
> associated with the rotated view and are also used to
> return the appropriate plane start address when queried
> from the display code.

BTW I started to hatch something to uswe
intel_gen4_compute_page_offset() on SKL+, and also start to handle fb
offsets[0] somehow. I didn't get too far yet, but I figured I'll post a
reference here in case people are intersted in this sort of stuff:

git://github.com/vsyrjala/linux.git tile_size

> 
> Tvrtko Ursulin (4):
>   drm/i915: Support planar formats in tile height calculations
>   drm/i915: Support appending to the rotated pages mapping
>   drm/i915: Support NV12 in rotated GGTT mapping
>   drm/i915: Enable querying offset of UV plane with
>     intel_plane_obj_offset
> 
>  drivers/gpu/drm/i915/i915_gem_gtt.c  | 58 ++++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/i915_gem_gtt.h  |  4 +++
>  drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 ++--
>  drivers/gpu/drm/i915/intel_sprite.c  |  4 +--
>  5 files changed, 93 insertions(+), 27 deletions(-)
> 
> -- 
> 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

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

[Daniel Vetter]

On Mon, Sep 21, 2015 at 02:14:47PM +0300, Joonas Lahtinen wrote:
> On ma, 2015-09-21 at 10:45 +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>
> > 
> 
> One comment below, otherwise.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.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;
> 
> Could be initialized to zero here already as majority of the time it'll
> be unchanged.

Personally (and this is really pretty clearly in bikeshed territory) I
prefer it like Tvrtko has done since if you ever rework the code for new
platform support and forget one case gcc will complain about potentially
uninitialized variable. But if you initialize it at first that will paper
over such a bug. And gcc will optimize this anyway for you (besides that
generally performance in modeset code just doesn't really matter all that
much).

And something funny is going on with your mail replies, you insert piles
of spurious > even though it's just one reply level ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

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

[Tvrtko Ursulin]

On 09/24/2015 05:35 PM, Ville Syrjälä wrote:
> 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?

No idea, I'm afraid I don't have the wide enough knowledge on this one 
to contribute hugely.

Why would it be difficult for userspace to compute a byte offset to the 
UV plane though? Presumably it had to know it already to have rendered 
anything - otherwise why and how would it be giving this frame buffer to 
the kernel?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Fri, Sep 25, 2015 at 10:44:44AM +0100, Tvrtko Ursulin wrote:
> 
> On 09/24/2015 05:35 PM, Ville Syrjälä wrote:
> > 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?
> 
> No idea, I'm afraid I don't have the wide enough knowledge on this one 
> to contribute hugely.
> 
> Why would it be difficult for userspace to compute a byte offset to the 
> UV plane though? Presumably it had to know it already to have rendered 
> anything - otherwise why and how would it be giving this frame buffer to 
> the kernel?

Well, it's not difficult per say, but it would have to know the tile
dimensions to do it. When using a linear offset, the tile dimensions are
mostly meaningless, although some of the hw restrictions (stride must be
tile aligned, and CbCr plane must start at somewhat aligned offset
at least sometimes) does blow a hole to that theory a bit.

I suppose it's not really a huge deal which way we go, we just have to
pick one and stick with it, and document it.

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

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

[Daniel Vetter]

On Fri, Sep 25, 2015 at 02:29:59PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 25, 2015 at 10:44:44AM +0100, Tvrtko Ursulin wrote:
> > 
> > On 09/24/2015 05:35 PM, Ville Syrjälä wrote:
> > > 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?
> > 
> > No idea, I'm afraid I don't have the wide enough knowledge on this one 
> > to contribute hugely.
> > 
> > Why would it be difficult for userspace to compute a byte offset to the 
> > UV plane though? Presumably it had to know it already to have rendered 
> > anything - otherwise why and how would it be giving this frame buffer to 
> > the kernel?
> 
> Well, it's not difficult per say, but it would have to know the tile
> dimensions to do it. When using a linear offset, the tile dimensions are
> mostly meaningless, although some of the hw restrictions (stride must be
> tile aligned, and CbCr plane must start at somewhat aligned offset
> at least sometimes) does blow a hole to that theory a bit.
> 
> I suppose it's not really a huge deal which way we go, we just have to
> pick one and stick with it, and document it.

Hm I kinda assumed that for tiled layouts plane offsets must be
tile-aligned for everyone. Since tiling is now a separate part with the fb
modifiers I'd go with using it as the linear offset. But definitely needs
to be documented with a kerneldoc pathc somewhere ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Mon, Sep 28, 2015 at 10:37:24AM +0200, Daniel Vetter wrote:
> On Fri, Sep 25, 2015 at 02:29:59PM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 25, 2015 at 10:44:44AM +0100, Tvrtko Ursulin wrote:
> > > 
> > > On 09/24/2015 05:35 PM, Ville Syrjälä wrote:
> > > > 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?
> > > 
> > > No idea, I'm afraid I don't have the wide enough knowledge on this one 
> > > to contribute hugely.
> > > 
> > > Why would it be difficult for userspace to compute a byte offset to the 
> > > UV plane though? Presumably it had to know it already to have rendered 
> > > anything - otherwise why and how would it be giving this frame buffer to 
> > > the kernel?
> > 
> > Well, it's not difficult per say, but it would have to know the tile
> > dimensions to do it. When using a linear offset, the tile dimensions are
> > mostly meaningless, although some of the hw restrictions (stride must be
> > tile aligned, and CbCr plane must start at somewhat aligned offset
> > at least sometimes) does blow a hole to that theory a bit.
> > 
> > I suppose it's not really a huge deal which way we go, we just have to
> > pick one and stick with it, and document it.
> 
> Hm I kinda assumed that for tiled layouts plane offsets must be
> tile-aligned for everyone.

Nah, we should be able to have it just (macro)pixel aligned. I have
it as stride aligned in my tile_size branch just because I didn't want
to think about the extra x offset initially. But I suppose the extra x
offset shouldn't really cause any issues. I also didn't (yet) properly
deal with the fact that the rotated view wouldn't necessarily cover
the entire object.

> Since tiling is now a separate part with the fb
> modifiers I'd go with using it as the linear offset. But definitely needs
> to be documented with a kerneldoc pathc somewhere ...
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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