[PATCH 1/2] drm/i915: Drop the unbound page cache upon idling

[PATCH 1/2] drm/i915: Drop the unbound page cache upon idling

[Chris Wilson]

When the GPU is idle, we can take a breather and release our hold on our
many caches. One such cache is that we keep objects pinned in memory
even when they are no longer accessible by the GPU, that cache is held
until the system comes under memory pressure. As we are idle, we can be
reasonably confident that the pages will not be used again in the near
future, so let the system know it can recover the memory for its own
purposes.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a7979b74ce21..8a9efc50e9d9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3373,6 +3373,9 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	intel_engines_park(dev_priv);
 	i915_gem_timelines_mark_idle(dev_priv);
 
+	/* Discard all currently unused caching of obj->mm.pages */
+	i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
+
 	GEM_BUG_ON(!dev_priv->gt.awake);
 	dev_priv->gt.awake = false;
 	rearm_hangcheck = false;
-- 
2.15.0

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

[PATCH 2/2] drm/i915: Remove temporary allocation of dma addresses when rotating

[Chris Wilson]

The object already stores (computed on the fly) the index to dma address
so use it instead of reallocating a large temporary array every time we
bind a rotated framebuffer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 77 ++++++++++++-------------------------
 1 file changed, 25 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 3c3a699436c9..d8ab09a8b350 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3665,27 +3665,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 }
 
 static struct scatterlist *
-rotate_pages(const dma_addr_t *in, unsigned int offset,
-	     unsigned int width, unsigned int height,
-	     unsigned int stride,
+rotate_pages(struct drm_i915_gem_object *obj,
+	     const struct intel_rotation_plane_info *p,
 	     struct sg_table *st, struct scatterlist *sg)
 {
 	unsigned int column, row;
-	unsigned int src_idx;
 
-	for (column = 0; column < width; column++) {
-		src_idx = stride * (height - 1) + column;
-		for (row = 0; row < height; row++) {
-			st->nents++;
+	for (column = 0; column < p->width; column++) {
+		unsigned long src_idx =
+			p->stride * (p->height - 1) + column + p->offset;
+		for (row = 0; row < p->height; row++) {
+			struct scatterlist *src;
+			unsigned int n;
+
+			src = i915_gem_object_get_sg(obj, src_idx, &n);
+			src_idx -= p->stride;
+
 			/* We don't need the pages, but need to initialize
 			 * the entries so the sg list can be happily traversed.
 			 * The only thing we need are DMA addresses.
 			 */
 			sg_set_page(sg, NULL, PAGE_SIZE, 0);
-			sg_dma_address(sg) = in[offset + src_idx];
+			sg_dma_address(sg) = sg_dma_address(src) + n*PAGE_SIZE;
 			sg_dma_len(sg) = PAGE_SIZE;
-			sg = sg_next(sg);
-			src_idx -= stride;
+			sg = __sg_next(sg);
+
+			st->nents++;
 		}
 	}
 
@@ -3696,62 +3701,30 @@ static noinline struct sg_table *
 intel_rotate_pages(struct intel_rotation_info *rot_info,
 		   struct drm_i915_gem_object *obj)
 {
-	const unsigned long n_pages = obj->base.size / PAGE_SIZE;
-	unsigned int size = intel_rotation_info_size(rot_info);
-	struct sgt_iter sgt_iter;
-	dma_addr_t dma_addr;
-	unsigned long i;
-	dma_addr_t *page_addr_list;
-	struct sg_table *st;
+	const unsigned int size = intel_rotation_info_size(rot_info);
 	struct scatterlist *sg;
+	struct sg_table *st;
+	unsigned long i;
 	int ret = -ENOMEM;
 
-	/* Allocate a temporary list of source pages for random access. */
-	page_addr_list = kvmalloc_array(n_pages,
-					sizeof(dma_addr_t),
-					GFP_KERNEL);
-	if (!page_addr_list)
-		return ERR_PTR(ret);
-
-	/* Allocate target SG list. */
 	st = kmalloc(sizeof(*st), GFP_KERNEL);
 	if (!st)
-		goto err_st_alloc;
+		goto err;
 
 	ret = sg_alloc_table(st, size, GFP_KERNEL);
 	if (ret)
-		goto err_sg_alloc;
-
-	/* Populate source page list from the object. */
-	i = 0;
-	for_each_sgt_dma(dma_addr, sgt_iter, obj->mm.pages)
-		page_addr_list[i++] = dma_addr;
+		goto err;
 
-	GEM_BUG_ON(i != n_pages);
 	st->nents = 0;
 	sg = st->sgl;
-
-	for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++) {
-		sg = rotate_pages(page_addr_list, rot_info->plane[i].offset,
-				  rot_info->plane[i].width, rot_info->plane[i].height,
-				  rot_info->plane[i].stride, st, sg);
-	}
-
-	DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
-		      obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
-
-	kvfree(page_addr_list);
+	for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++)
+		sg = rotate_pages(obj, &rot_info->plane[i], st, sg);
+	GEM_BUG_ON(st->nents != size);
 
 	return st;
 
-err_sg_alloc:
+err:
 	kfree(st);
-err_st_alloc:
-	kvfree(page_addr_list);
-
-	DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
-		      obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
-
 	return ERR_PTR(ret);
 }
 
-- 
2.15.0

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Drop the unbound page cache upon idling

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Drop the unbound page cache upon idling
URL   : https://patchwork.freedesktop.org/series/33817/
State : success

== Summary ==

Series 33817v1 series starting with [1/2] drm/i915: Drop the unbound page cache upon idling
https://patchwork.freedesktop.org/api/1.0/series/33817/revisions/1/mbox/

Test gem_sync:
        Subgroup basic-store-all:
                pass       -> FAIL       (fi-ivb-3520m) fdo#100007
Test kms_busy:
        Subgroup basic-flip-b:
                pass       -> FAIL       (fi-gdg-551) fdo#102654
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#102654 https://bugs.freedesktop.org/show_bug.cgi?id=102654
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:442s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:374s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:532s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:274s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:499s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:491s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:491s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:430s
fi-gdg-551       total:289  pass:177  dwarn:1   dfail:0   fail:2   skip:109 time:265s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:536s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:434s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:437s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:421s
fi-ivb-3520m     total:289  pass:259  dwarn:0   dfail:0   fail:1   skip:29  time:478s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:484s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:528s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:471s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:575s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:455s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:547s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:558s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:458s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:419s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:530s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:569s
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:495s

c46476e24d6432b5792ef63596a985848d122d50 drm-tip: 2017y-11m-14d-17h-03m-32s UTC integration manifest
37edd581e8ff drm/i915: Remove temporary allocation of dma addresses when rotating
9401693bec81 drm/i915: Drop the unbound page cache upon idling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7126/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [1/2] drm/i915: Drop the unbound page cache upon idling

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Drop the unbound page cache upon idling
URL   : https://patchwork.freedesktop.org/series/33817/
State : warning

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-a:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249
        Subgroup basic-modeset-a:
                pass       -> SKIP       (shard-hsw)
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-varying-size:
                fail       -> PASS       (shard-hsw)
Test kms_flip:
        Subgroup render-flip-vs-panning-interruptible:
                pass       -> SKIP       (shard-hsw)
Test kms_atomic_interruptible:
        Subgroup legacy-pageflip:
                pass       -> SKIP       (shard-hsw)
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252 +1

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2584 pass:1467 dwarn:3   dfail:1   fail:11  skip:1102 time:9394s
Blacklisted hosts:
shard-apl        total:2584 pass:1621 dwarn:2   dfail:1   fail:24  skip:936 time:13217s
shard-kbl        total:2565 pass:1704 dwarn:6   dfail:0   fail:25  skip:829 time:10606s
shard-snb        total:2584 pass:1210 dwarn:1   dfail:1   fail:13  skip:1359 time:7705s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7126/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Drop the unbound page cache upon idling

[Joonas Lahtinen]

On Tue, 2017-11-14 at 19:11 +0000, Chris Wilson wrote:
> When the GPU is idle, we can take a breather and release our hold on our
> many caches. One such cache is that we keep objects pinned in memory
> even when they are no longer accessible by the GPU, that cache is held
> until the system comes under memory pressure. As we are idle, we can be
> reasonably confident that the pages will not be used again in the near
> future, so let the system know it can recover the memory for its own
> purposes.
> 
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Remove temporary allocation of dma addresses when rotating

[Joonas Lahtinen]

On Tue, 2017-11-14 at 19:11 +0000, Chris Wilson wrote:
> The object already stores (computed on the fly) the index to dma address
> so use it instead of reallocating a large temporary array every time we
> bind a rotated framebuffer.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<SNIP>

> @@ -3665,27 +3665,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  }
>  
>  static struct scatterlist *
> -rotate_pages(const dma_addr_t *in, unsigned int offset,
> -	     unsigned int width, unsigned int height,
> -	     unsigned int stride,
> +rotate_pages(struct drm_i915_gem_object *obj,
> +	     const struct intel_rotation_plane_info *p,
>  	     struct sg_table *st, struct scatterlist *sg)
>  {
>  	unsigned int column, row;
> -	unsigned int src_idx;
>  
> -	for (column = 0; column < width; column++) {
> -		src_idx = stride * (height - 1) + column;
> -		for (row = 0; row < height; row++) {
> -			st->nents++;
> +	for (column = 0; column < p->width; column++) {
> +		unsigned long src_idx =
> +			p->stride * (p->height - 1) + column + p->offset;

Wouldn't you rather do offset + ...? It's the most dominating factor.

> +		for (row = 0; row < p->height; row++) {
> +			struct scatterlist *src;
> +			unsigned int n;
> +
> +			src = i915_gem_object_get_sg(obj, src_idx, &n);

It's bit confusing that the arguments are "n" and "offset" and "n" is
passed as "offset", so maybe s/n/sg_offset/. With that sorted out;

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Remove temporary allocation of dma addresses when rotating

[Chris Wilson]

Quoting Joonas Lahtinen (2017-11-16 10:43:42)
> On Tue, 2017-11-14 at 19:11 +0000, Chris Wilson wrote:
> > The object already stores (computed on the fly) the index to dma address
> > so use it instead of reallocating a large temporary array every time we
> > bind a rotated framebuffer.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> <SNIP>
> 
> > @@ -3665,27 +3665,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
> >  }
> >  
> >  static struct scatterlist *
> > -rotate_pages(const dma_addr_t *in, unsigned int offset,
> > -          unsigned int width, unsigned int height,
> > -          unsigned int stride,
> > +rotate_pages(struct drm_i915_gem_object *obj,
> > +          const struct intel_rotation_plane_info *p,
> >            struct sg_table *st, struct scatterlist *sg)
> >  {
> >       unsigned int column, row;
> > -     unsigned int src_idx;
> >  
> > -     for (column = 0; column < width; column++) {
> > -             src_idx = stride * (height - 1) + column;
> > -             for (row = 0; row < height; row++) {
> > -                     st->nents++;
> > +     for (column = 0; column < p->width; column++) {
> > +             unsigned long src_idx =
> > +                     p->stride * (p->height - 1) + column + p->offset;
> 
> Wouldn't you rather do offset + ...? It's the most dominating factor.

Yes. p->offset matches image in the (image, row, column) hierarchy. It
just happens that when I was playing with the selftests, I used very
small (and unrealistic) offsets.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx