[PATCH 1/3] drm/i915: Remove mostly unused variable in intel_rotate_fb_obj_pages

[PATCH 1/3] drm/i915: Remove mostly unused variable in intel_rotate_fb_obj_pages

[Tvrtko Ursulin]

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

It is only used in logging and it doesn't need to exist on its own.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a8c33f7..c1c16b0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2702,7 +2702,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 {
 	struct drm_device *dev = obj->base.dev;
 	struct intel_rotation_info *rot_info = &ggtt_view->rotation_info;
-	unsigned long size, pages, rot_pages;
+	unsigned long pages, rot_pages;
 	struct sg_page_iter sg_iter;
 	unsigned long i;
 	dma_addr_t *page_addr_list;
@@ -2720,7 +2720,6 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	width_pages = DIV_ROUND_UP(rot_info->pitch, tile_pitch);
 	height_pages = DIV_ROUND_UP(rot_info->height, tile_height);
 	rot_pages = width_pages * height_pages;
-	size = rot_pages * PAGE_SIZE;
 
 	/* Allocate a temporary list of source pages for random access. */
 	page_addr_list = drm_malloc_ab(pages, sizeof(dma_addr_t));
@@ -2748,7 +2747,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 
 	DRM_DEBUG_KMS(
 		      "Created rotated page mapping for object size %lu (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %lu pages).\n",
-		      size, rot_info->pitch, rot_info->height,
+		      obj->base.size, rot_info->pitch, rot_info->height,
 		      rot_info->pixel_format, width_pages, height_pages,
 		      rot_pages);
 
@@ -2763,7 +2762,7 @@ err_st_alloc:
 
 	DRM_DEBUG_KMS(
 		      "Failed to create rotated mapping for object size %lu! (%d) (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %lu pages)\n",
-		      size, ret, rot_info->pitch, rot_info->height,
+		      obj->base.size, ret, rot_info->pitch, rot_info->height,
 		      rot_info->pixel_format, width_pages, height_pages,
 		      rot_pages);
 	return ERR_PTR(ret);
-- 
2.4.2

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

[PATCH 2/3] drm/i915: Move rotated geometry calculations into the fill helper

[Tvrtko Ursulin]

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

This way data is available as soon as the view is passed into the call chain.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
This will be needed in the following patch which fixes i915_ggtt_view_size.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c  | 35 ++++++++++++-----------------------
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  2 ++
 drivers/gpu/drm/i915/intel_display.c |  8 ++++++++
 3 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c1c16b0..43472c4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2700,29 +2700,16 @@ static struct sg_table *
 intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 			  struct drm_i915_gem_object *obj)
 {
-	struct drm_device *dev = obj->base.dev;
 	struct intel_rotation_info *rot_info = &ggtt_view->rotation_info;
-	unsigned long pages, rot_pages;
 	struct sg_page_iter sg_iter;
 	unsigned long i;
 	dma_addr_t *page_addr_list;
 	struct sg_table *st;
-	unsigned int tile_pitch, tile_height;
-	unsigned int width_pages, height_pages;
 	int ret = -ENOMEM;
 
-	pages = obj->base.size / PAGE_SIZE;
-
-	/* Calculate tiling geometry. */
-	tile_height = intel_tile_height(dev, rot_info->pixel_format,
-					rot_info->fb_modifier);
-	tile_pitch = PAGE_SIZE / tile_height;
-	width_pages = DIV_ROUND_UP(rot_info->pitch, tile_pitch);
-	height_pages = DIV_ROUND_UP(rot_info->height, tile_height);
-	rot_pages = width_pages * height_pages;
-
 	/* Allocate a temporary list of source pages for random access. */
-	page_addr_list = drm_malloc_ab(pages, sizeof(dma_addr_t));
+	page_addr_list = drm_malloc_ab(obj->base.size / PAGE_SIZE,
+				       sizeof(dma_addr_t));
 	if (!page_addr_list)
 		return ERR_PTR(ret);
 
@@ -2731,7 +2718,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	if (!st)
 		goto err_st_alloc;
 
-	ret = sg_alloc_table(st, rot_pages, GFP_KERNEL);
+	ret = sg_alloc_table(st, rot_info->pages, GFP_KERNEL);
 	if (ret)
 		goto err_sg_alloc;
 
@@ -2743,13 +2730,15 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	}
 
 	/* Rotate the pages. */
-	rotate_pages(page_addr_list, width_pages, height_pages, st);
+	rotate_pages(page_addr_list,
+		     rot_info->width_pages, rot_info->height_pages,
+		     st);
 
 	DRM_DEBUG_KMS(
-		      "Created rotated page mapping for object size %lu (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %lu pages).\n",
+		      "Created rotated page mapping for object size %lu (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages).\n",
 		      obj->base.size, rot_info->pitch, rot_info->height,
-		      rot_info->pixel_format, width_pages, height_pages,
-		      rot_pages);
+		      rot_info->pixel_format, rot_info->width_pages,
+		      rot_info->height_pages, rot_info->pages);
 
 	drm_free_large(page_addr_list);
 
@@ -2761,10 +2750,10 @@ err_st_alloc:
 	drm_free_large(page_addr_list);
 
 	DRM_DEBUG_KMS(
-		      "Failed to create rotated mapping for object size %lu! (%d) (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %lu pages)\n",
+		      "Failed to create rotated mapping for object size %lu! (%d) (pitch=%u, height=%u, pixel_format=0x%x, %ux%u tiles, %u pages)\n",
 		      obj->base.size, ret, rot_info->pitch, rot_info->height,
-		      rot_info->pixel_format, width_pages, height_pages,
-		      rot_pages);
+		      rot_info->pixel_format, rot_info->width_pages,
+		      rot_info->height_pages, rot_info->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 735f119..33b9580 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -126,6 +126,8 @@ struct intel_rotation_info {
 	unsigned int pitch;
 	uint32_t pixel_format;
 	uint64_t fb_modifier;
+	unsigned int width_pages, height_pages;
+	unsigned int pages;
 };
 
 struct i915_ggtt_view {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1bc217a..d86406c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2268,6 +2268,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
 			const struct drm_plane_state *plane_state)
 {
 	struct intel_rotation_info *info = &view->rotation_info;
+	unsigned int tile_height, tile_pitch;
 
 	*view = i915_ggtt_view_normal;
 
@@ -2284,6 +2285,13 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
 	info->pitch = fb->pitches[0];
 	info->fb_modifier = fb->modifier[0];
 
+	tile_height = intel_tile_height(fb->dev, fb->pixel_format,
+					fb->modifier[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);
+	info->pages = info->width_pages * info->height_pages;
+
 	return 0;
 }
 
-- 
2.4.2

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

[PATCH 3/3] drm/i915: Return correct size for rotated views

[Tvrtko Ursulin]

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

Currently object size is returned for the rotated VMA size which can be
bigger than the rotated view itself. Since the binding code pads all
excess size with scratch pages the only minor issue with this is wasting
some GGTT space, but still feels nicer to fix and report the real size.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 43472c4..8e7cd22 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2898,9 +2898,10 @@ size_t
 i915_ggtt_view_size(struct drm_i915_gem_object *obj,
 		    const struct i915_ggtt_view *view)
 {
-	if (view->type == I915_GGTT_VIEW_NORMAL ||
-	    view->type == I915_GGTT_VIEW_ROTATED) {
+	if (view->type == I915_GGTT_VIEW_NORMAL) {
 		return obj->base.size;
+	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
+		return view->rotation_info.pages << PAGE_SHIFT;
 	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
 		return view->params.partial.size << PAGE_SHIFT;
 	} else {
-- 
2.4.2

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

Re: [PATCH 3/3] drm/i915: Return correct size for rotated views

[Chris Wilson]

On Tue, Jun 23, 2015 at 11:04:42AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Currently object size is returned for the rotated VMA size which can be
> bigger than the rotated view itself. Since the binding code pads all
> excess size with scratch pages the only minor issue with this is wasting
> some GGTT space, but still feels nicer to fix and report the real size.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 43472c4..8e7cd22 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2898,9 +2898,10 @@ size_t
>  i915_ggtt_view_size(struct drm_i915_gem_object *obj,
>  		    const struct i915_ggtt_view *view)
>  {
> -	if (view->type == I915_GGTT_VIEW_NORMAL ||
> -	    view->type == I915_GGTT_VIEW_ROTATED) {
> +	if (view->type == I915_GGTT_VIEW_NORMAL) {
>  		return obj->base.size;
> +	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
> +		return view->rotation_info.pages << PAGE_SHIFT;

Time to change that variable name. pages is already used in both the vma
and obj to represent the struct sg_table, not the count.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Return correct size for rotated views

[Tvrtko Ursulin]

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

Currently object size is returned for the rotated VMA size which can be
bigger than the rotated view itself. Since the binding code pads all
excess size with scratch pages the only minor issue with this is wasting
some GGTT space, but still feels nicer to fix and report the real size.

v2: Rebase for tracking size in bytes instead of pages.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9777ef4..1a54565 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2899,9 +2899,10 @@ size_t
 i915_ggtt_view_size(struct drm_i915_gem_object *obj,
 		    const struct i915_ggtt_view *view)
 {
-	if (view->type == I915_GGTT_VIEW_NORMAL ||
-	    view->type == I915_GGTT_VIEW_ROTATED) {
+	if (view->type == I915_GGTT_VIEW_NORMAL) {
 		return obj->base.size;
+	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
+		return view->rotation_info.size;
 	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
 		return view->params.partial.size << PAGE_SHIFT;
 	} else {
-- 
2.4.2

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

Re: [PATCH 3/3] drm/i915: Return correct size for rotated views

[Tvrtko Ursulin]

On 06/23/2015 11:29 AM, Chris Wilson wrote:
> On Tue, Jun 23, 2015 at 11:04:42AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Currently object size is returned for the rotated VMA size which can be
>> bigger than the rotated view itself. Since the binding code pads all
>> excess size with scratch pages the only minor issue with this is wasting
>> some GGTT space, but still feels nicer to fix and report the real size.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 43472c4..8e7cd22 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -2898,9 +2898,10 @@ size_t
>>   i915_ggtt_view_size(struct drm_i915_gem_object *obj,
>>   		    const struct i915_ggtt_view *view)
>>   {
>> -	if (view->type == I915_GGTT_VIEW_NORMAL ||
>> -	    view->type == I915_GGTT_VIEW_ROTATED) {
>> +	if (view->type == I915_GGTT_VIEW_NORMAL) {
>>   		return obj->base.size;
>> +	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
>> +		return view->rotation_info.pages << PAGE_SHIFT;
>
> Time to change that variable name. pages is already used in both the vma
> and obj to represent the struct sg_table, not the count.

Changed to "size_t size" as per IRC discussion and resent the series. 
Thought that will be cleaner since two of of three patches are now v2.

Regards,

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

Re: [PATCH 3/3] drm/i915: Return correct size for rotated views

[Joonas Lahtinen]

On ti, 2015-06-23 at 12:57 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Currently object size is returned for the rotated VMA size which can be
> bigger than the rotated view itself. Since the binding code pads all
> excess size with scratch pages the only minor issue with this is wasting
> some GGTT space, but still feels nicer to fix and report the real size.
> 
> v2: Rebase for tracking size in bytes instead of pages.
> 

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

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9777ef4..1a54565 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2899,9 +2899,10 @@ size_t
>  i915_ggtt_view_size(struct drm_i915_gem_object *obj,
>  		    const struct i915_ggtt_view *view)
>  {
> -	if (view->type == I915_GGTT_VIEW_NORMAL ||
> -	    view->type == I915_GGTT_VIEW_ROTATED) {
> +	if (view->type == I915_GGTT_VIEW_NORMAL) {
>  		return obj->base.size;
> +	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
> +		return view->rotation_info.size;
>  	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
>  		return view->params.partial.size << PAGE_SHIFT;
>  	} else {


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

Re: [PATCH 3/3] drm/i915: Return correct size for rotated views

[Daniel Vetter]

On Wed, Jun 24, 2015 at 12:04:58PM +0300, Joonas Lahtinen wrote:
> On ti, 2015-06-23 at 12:57 +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Currently object size is returned for the rotated VMA size which can be
> > bigger than the rotated view itself. Since the binding code pads all
> > excess size with scratch pages the only minor issue with this is wasting
> > some GGTT space, but still feels nicer to fix and report the real size.
> > 
> > v2: Rebase for tracking size in bytes instead of pages.
> > 
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

All three merged to dinq, thanks.
-Daniel

> 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 9777ef4..1a54565 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2899,9 +2899,10 @@ size_t
> >  i915_ggtt_view_size(struct drm_i915_gem_object *obj,
> >  		    const struct i915_ggtt_view *view)
> >  {
> > -	if (view->type == I915_GGTT_VIEW_NORMAL ||
> > -	    view->type == I915_GGTT_VIEW_ROTATED) {
> > +	if (view->type == I915_GGTT_VIEW_NORMAL) {
> >  		return obj->base.size;
> > +	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
> > +		return view->rotation_info.size;
> >  	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
> >  		return view->params.partial.size << PAGE_SHIFT;
> >  	} else {
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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