All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Refactor common list iteration over GGTT vma
@ 2017-12-07 16:27 Chris Wilson
  2017-12-07 16:40 ` Ville Syrjälä
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Chris Wilson @ 2017-12-07 16:27 UTC (permalink / raw)
  To: intel-gfx

In quite a few places, we have a list iteration over the vma on an
object that only want to inspect GGTT vma. By construction, these are
placed at the start of the list, so we have copied that knowledge into
many callsites. Pull that knowledge back to i915_vma.h and provide a
for_each_ggtt_vma() to tidy up the code.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c    |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c        | 18 ++++--------------
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  5 +----
 drivers/gpu/drm/i915/i915_gem_tiling.c | 10 ++--------
 drivers/gpu/drm/i915/i915_vma.h        | 14 +++++++++++++-
 5 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 28294470ae31..7b41a1799a03 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -111,8 +111,8 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
 	u64 size = 0;
 	struct i915_vma *vma;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (i915_vma_is_ggtt(vma) && drm_mm_node_allocated(&vma->node))
+	for_each_ggtt_vma(vma, obj) {
+		if (drm_mm_node_allocated(&vma->node))
 			size += vma->node.size;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67dc11effc8e..c7b5db78fbb4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -714,10 +714,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (!i915_vma_is_ggtt(vma))
-				break;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (vma->iomap)
 				continue;
 
@@ -1569,10 +1566,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_is_active(vma))
 			continue;
 
@@ -2051,12 +2045,8 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	drm_vma_node_unmap(&obj->base.vma_node,
 			   obj->base.dev->anon_inode->i_mapping);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj)
 		i915_vma_unset_userfault(vma);
-	}
 }
 
 /**
@@ -3822,7 +3812,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			 * dropped the fence as all snoopable access is
 			 * supposed to be linear.
 			 */
-			list_for_each_entry(vma, &obj->vma_list, obj_link) {
+			for_each_ggtt_vma(vma, obj) {
 				ret = i915_vma_put_fence(vma);
 				if (ret)
 					return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1faea9a17b5a..d701b79b6319 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3623,10 +3623,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		bool ggtt_bound = false;
 		struct i915_vma *vma;
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (vma->vm != &ggtt->base)
-				continue;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (!i915_vma_unbind(vma))
 				continue;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index b85d7ebd9bee..d9dc9df523b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -205,10 +205,7 @@ i915_gem_object_fence_prepare(struct drm_i915_gem_object *obj,
 	if (tiling_mode == I915_TILING_NONE)
 		return 0;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_fence_prepare(vma, tiling_mode, stride))
 			continue;
 
@@ -285,10 +282,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	}
 	mutex_unlock(&obj->mm.lock);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		vma->fence_size =
 			i915_gem_fence_size(i915, vma->size, tiling, stride);
 		vma->fence_alignment =
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f636243eb8f7..d58da80c0dd2 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -408,5 +408,17 @@ i915_vma_unpin_fence(struct i915_vma *vma)
 		__i915_vma_unpin_fence(vma);
 }
 
-#endif
+/**
+ * for_each_ggtt_vma - Iterate over the GGTT VMA belonging to an object.
+ * V - the #i915_vma iterator
+ * OBJ - the #drm_i915_gem_object
+ *
+ * GGTT VMA are placed at the being of the object's vma_list, see
+ * vma_create(), so we can stop our walk as soon as we see a ppgtt VMA,
+ * or the list is empty ofc.
+ */
+#define for_each_ggtt_vma(V, OBJ) \
+	list_for_each_entry(V, &(OBJ)->vma_list, obj_link)		\
+		if (!i915_vma_is_ggtt(vma)) break; else
 
+#endif
-- 
2.15.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
@ 2017-12-07 16:40 ` Ville Syrjälä
  2017-12-07 16:46   ` Chris Wilson
  2017-12-07 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2017-12-07 16:40 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, Dec 07, 2017 at 04:27:17PM +0000, Chris Wilson wrote:
> In quite a few places, we have a list iteration over the vma on an
> object that only want to inspect GGTT vma. By construction, these are
> placed at the start of the list, so we have copied that knowledge into
> many callsites. Pull that knowledge back to i915_vma.h and provide a
> for_each_ggtt_vma() to tidy up the code.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c    |  4 ++--
>  drivers/gpu/drm/i915/i915_gem.c        | 18 ++++--------------
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |  5 +----
>  drivers/gpu/drm/i915/i915_gem_tiling.c | 10 ++--------
>  drivers/gpu/drm/i915/i915_vma.h        | 14 +++++++++++++-
>  5 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 28294470ae31..7b41a1799a03 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -111,8 +111,8 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
>  	u64 size = 0;
>  	struct i915_vma *vma;
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (i915_vma_is_ggtt(vma) && drm_mm_node_allocated(&vma->node))
> +	for_each_ggtt_vma(vma, obj) {
> +		if (drm_mm_node_allocated(&vma->node))
>  			size += vma->node.size;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 67dc11effc8e..c7b5db78fbb4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -714,10 +714,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
>  		intel_fb_obj_flush(obj,
>  				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
>  
> -		list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -			if (!i915_vma_is_ggtt(vma))
> -				break;
> -
> +		for_each_ggtt_vma(vma, obj) {
>  			if (vma->iomap)
>  				continue;
>  
> @@ -1569,10 +1566,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
>  
>  	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		if (i915_vma_is_active(vma))
>  			continue;
>  
> @@ -2051,12 +2045,8 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
>  	drm_vma_node_unmap(&obj->base.vma_node,
>  			   obj->base.dev->anon_inode->i_mapping);
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj)
>  		i915_vma_unset_userfault(vma);
> -	}
>  }
>  
>  /**
> @@ -3822,7 +3812,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  			 * dropped the fence as all snoopable access is
>  			 * supposed to be linear.
>  			 */
> -			list_for_each_entry(vma, &obj->vma_list, obj_link) {
> +			for_each_ggtt_vma(vma, obj) {
>  				ret = i915_vma_put_fence(vma);
>  				if (ret)
>  					return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1faea9a17b5a..d701b79b6319 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3623,10 +3623,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  		bool ggtt_bound = false;
>  		struct i915_vma *vma;
>  
> -		list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -			if (vma->vm != &ggtt->base)
> -				continue;
> -
> +		for_each_ggtt_vma(vma, obj) {
>  			if (!i915_vma_unbind(vma))
>  				continue;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index b85d7ebd9bee..d9dc9df523b5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -205,10 +205,7 @@ i915_gem_object_fence_prepare(struct drm_i915_gem_object *obj,
>  	if (tiling_mode == I915_TILING_NONE)
>  		return 0;
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		if (i915_vma_fence_prepare(vma, tiling_mode, stride))
>  			continue;
>  
> @@ -285,10 +282,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
>  	}
>  	mutex_unlock(&obj->mm.lock);
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		vma->fence_size =
>  			i915_gem_fence_size(i915, vma->size, tiling, stride);
>  		vma->fence_alignment =
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index f636243eb8f7..d58da80c0dd2 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -408,5 +408,17 @@ i915_vma_unpin_fence(struct i915_vma *vma)
>  		__i915_vma_unpin_fence(vma);
>  }
>  
> -#endif
> +/**
> + * for_each_ggtt_vma - Iterate over the GGTT VMA belonging to an object.
> + * V - the #i915_vma iterator
> + * OBJ - the #drm_i915_gem_object
> + *
> + * GGTT VMA are placed at the being of the object's vma_list, see
> + * vma_create(), so we can stop our walk as soon as we see a ppgtt VMA,
> + * or the list is empty ofc.
> + */
> +#define for_each_ggtt_vma(V, OBJ) \
> +	list_for_each_entry(V, &(OBJ)->vma_list, obj_link)		\
> +		if (!i915_vma_is_ggtt(vma)) break; else

for_each_if() ?

>  
> +#endif
> -- 
> 2.15.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:40 ` Ville Syrjälä
@ 2017-12-07 16:46   ` Chris Wilson
  2017-12-07 17:05     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-12-07 16:46 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Quoting Ville Syrjälä (2017-12-07 16:40:49)
> On Thu, Dec 07, 2017 at 04:27:17PM +0000, Chris Wilson wrote:
> > +#define for_each_ggtt_vma(V, OBJ) \
> > +     list_for_each_entry(V, &(OBJ)->vma_list, obj_link)              \
> > +             if (!i915_vma_is_ggtt(vma)) break; else
> 
> for_each_if() ?

for_each_if() doesn't perform a break.

for_each_until() ? I think that may be a bit too magical.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
  2017-12-07 16:40 ` Ville Syrjälä
@ 2017-12-07 16:48 ` Patchwork
  2017-12-07 17:49 ` [PATCH] " Daniel Vetter
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-12-07 16:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Refactor common list iteration over GGTT vma
URL   : https://patchwork.freedesktop.org/series/35048/
State : success

== Summary ==

Series 35048v1 drm/i915: Refactor common list iteration over GGTT vma
https://patchwork.freedesktop.org/api/1.0/series/35048/revisions/1/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:446s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:521s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:486s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:1   dfail:0   fail:0   skip:108 time:270s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:357s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:258s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:395s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:444s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:498s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:525s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:484s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:585s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:455s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:516s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:497s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:550s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:409s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:616s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:629s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s
fi-bxt-j4205 failed to collect. IGT log at Patchwork_7442/fi-bxt-j4205/igt.log

bdf9b36fd601c2aa06fb191b31d0bac1197db8d0 drm-tip: 2017y-12m-07d-14h-56m-01s UTC integration manifest
9d686b86a209 drm/i915: Refactor common list iteration over GGTT vma

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:46   ` Chris Wilson
@ 2017-12-07 17:05     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2017-12-07 17:05 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, Dec 07, 2017 at 04:46:37PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjälä (2017-12-07 16:40:49)
> > On Thu, Dec 07, 2017 at 04:27:17PM +0000, Chris Wilson wrote:
> > > +#define for_each_ggtt_vma(V, OBJ) \
> > > +     list_for_each_entry(V, &(OBJ)->vma_list, obj_link)              \
> > > +             if (!i915_vma_is_ggtt(vma)) break; else
> > 
> > for_each_if() ?
> 
> for_each_if() doesn't perform a break.

Ah. I probably should have read the comment :)

> 
> for_each_until() ? I think that may be a bit too magical.

Maybe. I guess open coding it here is fine until the pattern
starts to repeat all over the place.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
  2017-12-07 16:40 ` Ville Syrjälä
  2017-12-07 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-12-07 17:49 ` Daniel Vetter
  2017-12-07 21:11 ` [PATCH v2] " Chris Wilson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-12-07 17:49 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, Dec 07, 2017 at 04:27:17PM +0000, Chris Wilson wrote:
> In quite a few places, we have a list iteration over the vma on an
> object that only want to inspect GGTT vma. By construction, these are
> placed at the start of the list, so we have copied that knowledge into
> many callsites. Pull that knowledge back to i915_vma.h and provide a
> for_each_ggtt_vma() to tidy up the code.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c    |  4 ++--
>  drivers/gpu/drm/i915/i915_gem.c        | 18 ++++--------------
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |  5 +----
>  drivers/gpu/drm/i915/i915_gem_tiling.c | 10 ++--------
>  drivers/gpu/drm/i915/i915_vma.h        | 14 +++++++++++++-
>  5 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 28294470ae31..7b41a1799a03 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -111,8 +111,8 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
>  	u64 size = 0;
>  	struct i915_vma *vma;
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (i915_vma_is_ggtt(vma) && drm_mm_node_allocated(&vma->node))
> +	for_each_ggtt_vma(vma, obj) {
> +		if (drm_mm_node_allocated(&vma->node))
>  			size += vma->node.size;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 67dc11effc8e..c7b5db78fbb4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -714,10 +714,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
>  		intel_fb_obj_flush(obj,
>  				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
>  
> -		list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -			if (!i915_vma_is_ggtt(vma))
> -				break;
> -
> +		for_each_ggtt_vma(vma, obj) {
>  			if (vma->iomap)
>  				continue;
>  
> @@ -1569,10 +1566,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
>  
>  	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		if (i915_vma_is_active(vma))
>  			continue;
>  
> @@ -2051,12 +2045,8 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
>  	drm_vma_node_unmap(&obj->base.vma_node,
>  			   obj->base.dev->anon_inode->i_mapping);
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj)
>  		i915_vma_unset_userfault(vma);
> -	}
>  }
>  
>  /**
> @@ -3822,7 +3812,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  			 * dropped the fence as all snoopable access is
>  			 * supposed to be linear.
>  			 */
> -			list_for_each_entry(vma, &obj->vma_list, obj_link) {
> +			for_each_ggtt_vma(vma, obj) {
>  				ret = i915_vma_put_fence(vma);
>  				if (ret)
>  					return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1faea9a17b5a..d701b79b6319 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3623,10 +3623,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  		bool ggtt_bound = false;
>  		struct i915_vma *vma;
>  
> -		list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -			if (vma->vm != &ggtt->base)
> -				continue;
> -
> +		for_each_ggtt_vma(vma, obj) {
>  			if (!i915_vma_unbind(vma))
>  				continue;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index b85d7ebd9bee..d9dc9df523b5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -205,10 +205,7 @@ i915_gem_object_fence_prepare(struct drm_i915_gem_object *obj,
>  	if (tiling_mode == I915_TILING_NONE)
>  		return 0;
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		if (i915_vma_fence_prepare(vma, tiling_mode, stride))
>  			continue;
>  
> @@ -285,10 +282,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
>  	}
>  	mutex_unlock(&obj->mm.lock);
>  
> -	list_for_each_entry(vma, &obj->vma_list, obj_link) {
> -		if (!i915_vma_is_ggtt(vma))
> -			break;
> -
> +	for_each_ggtt_vma(vma, obj) {
>  		vma->fence_size =
>  			i915_gem_fence_size(i915, vma->size, tiling, stride);
>  		vma->fence_alignment =
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index f636243eb8f7..d58da80c0dd2 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -408,5 +408,17 @@ i915_vma_unpin_fence(struct i915_vma *vma)
>  		__i915_vma_unpin_fence(vma);
>  }
>  
> -#endif
> +/**
> + * for_each_ggtt_vma - Iterate over the GGTT VMA belonging to an object.
> + * V - the #i915_vma iterator
> + * OBJ - the #drm_i915_gem_object
> + *
> + * GGTT VMA are placed at the being of the object's vma_list, see
> + * vma_create(), so we can stop our walk as soon as we see a ppgtt VMA,
> + * or the list is empty ofc.
> + */
> +#define for_each_ggtt_vma(V, OBJ) \
> +	list_for_each_entry(V, &(OBJ)->vma_list, obj_link)		\
> +		if (!i915_vma_is_ggtt(vma)) break; else

Definitely like that we document this assumption a bit better and
encapsulate it.

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

>  
> +#endif
> -- 
> 2.15.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
                   ` (2 preceding siblings ...)
  2017-12-07 17:49 ` [PATCH] " Daniel Vetter
@ 2017-12-07 21:11 ` Chris Wilson
  2017-12-07 21:14 ` [PATCH v3] " Chris Wilson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-12-07 21:11 UTC (permalink / raw)
  To: intel-gfx

In quite a few places, we have a list iteration over the vma on an
object that only want to inspect GGTT vma. By construction, these are
placed at the start of the list, so we have copied that knowledge into
many callsites. Pull that knowledge back to i915_vma.h and provide a
for_each_ggtt_vma() to tidy up the code.

v2: Add a backreference from vma_create() to remind ourselves why we put
ggtt vma at the head of the obj->vma_list (and ppgtt vma at the tail).

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c    |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c        | 18 ++++--------------
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  5 +----
 drivers/gpu/drm/i915/i915_gem_tiling.c | 10 ++--------
 drivers/gpu/drm/i915/i915_vma.c        |  6 ++++++
 drivers/gpu/drm/i915/i915_vma.h        | 16 +++++++++++++++-
 6 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 28294470ae31..7b41a1799a03 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -111,8 +111,8 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
 	u64 size = 0;
 	struct i915_vma *vma;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (i915_vma_is_ggtt(vma) && drm_mm_node_allocated(&vma->node))
+	for_each_ggtt_vma(vma, obj) {
+		if (drm_mm_node_allocated(&vma->node))
 			size += vma->node.size;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67dc11effc8e..c7b5db78fbb4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -714,10 +714,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (!i915_vma_is_ggtt(vma))
-				break;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (vma->iomap)
 				continue;
 
@@ -1569,10 +1566,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_is_active(vma))
 			continue;
 
@@ -2051,12 +2045,8 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	drm_vma_node_unmap(&obj->base.vma_node,
 			   obj->base.dev->anon_inode->i_mapping);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj)
 		i915_vma_unset_userfault(vma);
-	}
 }
 
 /**
@@ -3822,7 +3812,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			 * dropped the fence as all snoopable access is
 			 * supposed to be linear.
 			 */
-			list_for_each_entry(vma, &obj->vma_list, obj_link) {
+			for_each_ggtt_vma(vma, obj) {
 				ret = i915_vma_put_fence(vma);
 				if (ret)
 					return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 209bb111f652..5e7efbbac9f7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3620,10 +3620,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		bool ggtt_bound = false;
 		struct i915_vma *vma;
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (vma->vm != &ggtt->base)
-				continue;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (!i915_vma_unbind(vma))
 				continue;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index b85d7ebd9bee..d9dc9df523b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -205,10 +205,7 @@ i915_gem_object_fence_prepare(struct drm_i915_gem_object *obj,
 	if (tiling_mode == I915_TILING_NONE)
 		return 0;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_fence_prepare(vma, tiling_mode, stride))
 			continue;
 
@@ -285,10 +282,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	}
 	mutex_unlock(&obj->mm.lock);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		vma->fence_size =
 			i915_gem_fence_size(i915, vma->size, tiling, stride);
 		vma->fence_alignment =
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 0ebd75693505..92c11e70fea4 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -142,6 +142,12 @@ vma_create(struct drm_i915_gem_object *obj,
 								i915_gem_object_get_stride(obj));
 		GEM_BUG_ON(!is_power_of_2(vma->fence_alignment));
 
+		/*
+		 * We put the GGTT vma at the start of the vma-list, followed
+		 * by the ppGGTT vma. This allows us to break early when
+		 * iterating over only the GGTT vma for an object, see
+		 * for_each_ggtt_vma()
+		 */
 		vma->flags |= I915_VMA_GGTT;
 		list_add(&vma->obj_link, &obj->vma_list);
 	} else {
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f636243eb8f7..4ac9e307a6dd 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -408,5 +408,19 @@ i915_vma_unpin_fence(struct i915_vma *vma)
 		__i915_vma_unpin_fence(vma);
 }
 
-#endif
+#define for_each_until(cond) if (cond) break; else
+
+/**
+ * for_each_ggtt_vma - Iterate over the GGTT VMA belonging to an object.
+ * @V: the #i915_vma iterator
+ * @OBJ: the #drm_i915_gem_object
+ *
+ * GGTT VMA are placed at the being of the object's vma_list, see
+ * vma_create(), so we can stop our walk as soon as we see a ppgtt VMA,
+ * or the list is empty ofc.
+ */
+#define for_each_ggtt_vma(V, OBJ) \
+	list_for_each_entry(V, &(OBJ)->vma_list, obj_link)		\
+		for_each_until(!i915_vma_is_ggtt(vma))
 
+#endif
-- 
2.15.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3] drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
                   ` (3 preceding siblings ...)
  2017-12-07 21:11 ` [PATCH v2] " Chris Wilson
@ 2017-12-07 21:14 ` Chris Wilson
  2017-12-07 21:46 ` ✓ Fi.CI.BAT: success for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-12-07 21:14 UTC (permalink / raw)
  To: intel-gfx

In quite a few places, we have a list iteration over the vma on an
object that only want to inspect GGTT vma. By construction, these are
placed at the start of the list, so we have copied that knowledge into
many callsites. Pull that knowledge back to i915_vma.h and provide a
for_each_ggtt_vma() to tidy up the code.

v2: Add a backreference from vma_create() to remind ourselves why we put
ggtt vma at the head of the obj->vma_list (and ppgtt vma at the tail).
v3: Fixup s/vma/V/

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c    |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c        | 18 ++++--------------
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  5 +----
 drivers/gpu/drm/i915/i915_gem_tiling.c | 10 ++--------
 drivers/gpu/drm/i915/i915_vma.c        |  6 ++++++
 drivers/gpu/drm/i915/i915_vma.h        | 16 +++++++++++++++-
 6 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 28294470ae31..7b41a1799a03 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -111,8 +111,8 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
 	u64 size = 0;
 	struct i915_vma *vma;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (i915_vma_is_ggtt(vma) && drm_mm_node_allocated(&vma->node))
+	for_each_ggtt_vma(vma, obj) {
+		if (drm_mm_node_allocated(&vma->node))
 			size += vma->node.size;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67dc11effc8e..c7b5db78fbb4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -714,10 +714,7 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (!i915_vma_is_ggtt(vma))
-				break;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (vma->iomap)
 				continue;
 
@@ -1569,10 +1566,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_is_active(vma))
 			continue;
 
@@ -2051,12 +2045,8 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	drm_vma_node_unmap(&obj->base.vma_node,
 			   obj->base.dev->anon_inode->i_mapping);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj)
 		i915_vma_unset_userfault(vma);
-	}
 }
 
 /**
@@ -3822,7 +3812,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			 * dropped the fence as all snoopable access is
 			 * supposed to be linear.
 			 */
-			list_for_each_entry(vma, &obj->vma_list, obj_link) {
+			for_each_ggtt_vma(vma, obj) {
 				ret = i915_vma_put_fence(vma);
 				if (ret)
 					return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 209bb111f652..5e7efbbac9f7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3620,10 +3620,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		bool ggtt_bound = false;
 		struct i915_vma *vma;
 
-		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (vma->vm != &ggtt->base)
-				continue;
-
+		for_each_ggtt_vma(vma, obj) {
 			if (!i915_vma_unbind(vma))
 				continue;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index b85d7ebd9bee..d9dc9df523b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -205,10 +205,7 @@ i915_gem_object_fence_prepare(struct drm_i915_gem_object *obj,
 	if (tiling_mode == I915_TILING_NONE)
 		return 0;
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		if (i915_vma_fence_prepare(vma, tiling_mode, stride))
 			continue;
 
@@ -285,10 +282,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	}
 	mutex_unlock(&obj->mm.lock);
 
-	list_for_each_entry(vma, &obj->vma_list, obj_link) {
-		if (!i915_vma_is_ggtt(vma))
-			break;
-
+	for_each_ggtt_vma(vma, obj) {
 		vma->fence_size =
 			i915_gem_fence_size(i915, vma->size, tiling, stride);
 		vma->fence_alignment =
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 0ebd75693505..92c11e70fea4 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -142,6 +142,12 @@ vma_create(struct drm_i915_gem_object *obj,
 								i915_gem_object_get_stride(obj));
 		GEM_BUG_ON(!is_power_of_2(vma->fence_alignment));
 
+		/*
+		 * We put the GGTT vma at the start of the vma-list, followed
+		 * by the ppGGTT vma. This allows us to break early when
+		 * iterating over only the GGTT vma for an object, see
+		 * for_each_ggtt_vma()
+		 */
 		vma->flags |= I915_VMA_GGTT;
 		list_add(&vma->obj_link, &obj->vma_list);
 	} else {
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f636243eb8f7..fd5b84904f7c 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -408,5 +408,19 @@ i915_vma_unpin_fence(struct i915_vma *vma)
 		__i915_vma_unpin_fence(vma);
 }
 
-#endif
+#define for_each_until(cond) if (cond) break; else
+
+/**
+ * for_each_ggtt_vma - Iterate over the GGTT VMA belonging to an object.
+ * @V: the #i915_vma iterator
+ * @OBJ: the #drm_i915_gem_object
+ *
+ * GGTT VMA are placed at the being of the object's vma_list, see
+ * vma_create(), so we can stop our walk as soon as we see a ppgtt VMA,
+ * or the list is empty ofc.
+ */
+#define for_each_ggtt_vma(V, OBJ) \
+	list_for_each_entry(V, &(OBJ)->vma_list, obj_link)		\
+		for_each_until(!i915_vma_is_ggtt(V))
 
+#endif
-- 
2.15.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Refactor common list iteration over GGTT vma (rev3)
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
                   ` (4 preceding siblings ...)
  2017-12-07 21:14 ` [PATCH v3] " Chris Wilson
@ 2017-12-07 21:46 ` Patchwork
  2017-12-07 21:56 ` ✗ Fi.CI.IGT: failure for drm/i915: Refactor common list iteration over GGTT vma Patchwork
  2017-12-08  0:32 ` ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-12-07 21:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Refactor common list iteration over GGTT vma (rev3)
URL   : https://patchwork.freedesktop.org/series/35048/
State : success

== Summary ==

Series 35048v3 drm/i915: Refactor common list iteration over GGTT vma
https://patchwork.freedesktop.org/api/1.0/series/35048/revisions/3/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledy:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:387s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:526s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:284s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:507s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:495s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:480s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:177  dwarn:1   dfail:0   fail:2   skip:108 time:272s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:366s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:260s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:399s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:456s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:542s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:531s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:595s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:460s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:542s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:531s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:450s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:426s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:621s
fi-cnl-y         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:25 
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:492s

fb6aa7cd0d6bfa86efb7a87938733f1272414690 drm-tip: 2017y-12m-07d-17h-43m-29s UTC integration manifest
686c1ccfeb57 drm/i915: Refactor common list iteration over GGTT vma

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915: Refactor common list iteration over GGTT vma
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
                   ` (5 preceding siblings ...)
  2017-12-07 21:46 ` ✓ Fi.CI.BAT: success for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
@ 2017-12-07 21:56 ` Patchwork
  2017-12-08  0:32 ` ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-12-07 21:56 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Refactor common list iteration over GGTT vma
URL   : https://patchwork.freedesktop.org/series/35048/
State : failure

== Summary ==

Test drv_suspend:
        Subgroup forcewake:
                pass       -> SKIP       (shard-snb)
                skip       -> PASS       (shard-hsw)
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-toggle:
                pass       -> SKIP       (shard-snb)
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-b:
                pass       -> SKIP       (shard-snb)
        Subgroup extended-modeset-hang-oldfb-render-a:
                pass       -> SKIP       (shard-snb)
Test kms_plane_multiple:
        Subgroup legacy-pipe-a-tiling-none:
                pass       -> SKIP       (shard-snb)
Test kms_flip:
        Subgroup plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                pass       -> SKIP       (shard-snb)
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                skip       -> PASS       (shard-snb) fdo#102365
                pass       -> SKIP       (shard-hsw) fdo#103540
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-hsw) fdo#104009
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-indfb-draw-blt:
                pass       -> FAIL       (shard-hsw)

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009

shard-hsw        total:2679 pass:1532 dwarn:2   dfail:0   fail:12  skip:1133 time:9402s
shard-snb        total:2679 pass:1302 dwarn:2   dfail:0   fail:11  skip:1364 time:8039s
Blacklisted hosts:
shard-kbl        total:2602 pass:1742 dwarn:1   dfail:0   fail:25  skip:832 time:10544s

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3)
  2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
                   ` (6 preceding siblings ...)
  2017-12-07 21:56 ` ✗ Fi.CI.IGT: failure for drm/i915: Refactor common list iteration over GGTT vma Patchwork
@ 2017-12-08  0:32 ` Patchwork
  2017-12-08  0:44   ` Chris Wilson
  7 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2017-12-08  0:32 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Refactor common list iteration over GGTT vma (rev3)
URL   : https://patchwork.freedesktop.org/series/35048/
State : warning

== Summary ==

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                incomplete -> PASS       (shard-hsw)
Test kms_chv_cursor_fail:
        Subgroup pipe-b-128x128-right-edge:
                pass       -> SKIP       (shard-snb)
Test kms_draw_crc:
        Subgroup fill-fb:
                pass       -> SKIP       (shard-snb)
        Subgroup draw-method-rgb565-pwrite-xtiled:
                pass       -> SKIP       (shard-snb)
Test kms_plane_multiple:
        Subgroup atomic-pipe-b-tiling-x:
                pass       -> SKIP       (shard-snb)
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-hsw) fdo#104009
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2658 pass:1522 dwarn:2   dfail:0   fail:8   skip:1125 time:9088s
shard-snb        total:2679 pass:1304 dwarn:1   dfail:0   fail:12  skip:1362 time:8128s
Blacklisted hosts:
shard-apl        total:2679 pass:1678 dwarn:1   dfail:0   fail:23  skip:977 time:13740s
shard-kbl        total:2612 pass:1751 dwarn:2   dfail:0   fail:27  skip:830 time:10256s

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3)
  2017-12-08  0:32 ` ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
@ 2017-12-08  0:44   ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-12-08  0:44 UTC (permalink / raw)
  To: Patchwork; +Cc: intel-gfx

Quoting Patchwork (2017-12-08 00:32:47)
> == Series Details ==
> 
> Series: drm/i915: Refactor common list iteration over GGTT vma (rev3)
> URL   : https://patchwork.freedesktop.org/series/35048/
> State : warning
> 
> == Summary ==
> 
> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-b:
>                 incomplete -> PASS       (shard-hsw)
> Test kms_chv_cursor_fail:
>         Subgroup pipe-b-128x128-right-edge:
>                 pass       -> SKIP       (shard-snb)
> Test kms_draw_crc:
>         Subgroup fill-fb:
>                 pass       -> SKIP       (shard-snb)
>         Subgroup draw-method-rgb565-pwrite-xtiled:
>                 pass       -> SKIP       (shard-snb)
> Test kms_plane_multiple:
>         Subgroup atomic-pipe-b-tiling-x:
>                 pass       -> SKIP       (shard-snb)
> Test gem_tiled_swapping:
>         Subgroup non-threaded:
>                 pass       -> INCOMPLETE (shard-hsw) fdo#104009
> Test drv_module_reload:
>         Subgroup basic-reload:
>                 pass       -> DMESG-WARN (shard-hsw) fdo#102707
> Test kms_frontbuffer_tracking:
>         Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
>                 fail       -> PASS       (shard-snb) fdo#101623
> Test kms_setmode:
>         Subgroup basic:
>                 fail       -> PASS       (shard-hsw) fdo#99912

Whatever bug may be lurking hides very well.

Thanks for the suggestion and review, applied.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-12-08  0:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-07 16:27 [PATCH] drm/i915: Refactor common list iteration over GGTT vma Chris Wilson
2017-12-07 16:40 ` Ville Syrjälä
2017-12-07 16:46   ` Chris Wilson
2017-12-07 17:05     ` Ville Syrjälä
2017-12-07 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-07 17:49 ` [PATCH] " Daniel Vetter
2017-12-07 21:11 ` [PATCH v2] " Chris Wilson
2017-12-07 21:14 ` [PATCH v3] " Chris Wilson
2017-12-07 21:46 ` ✓ Fi.CI.BAT: success for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
2017-12-07 21:56 ` ✗ Fi.CI.IGT: failure for drm/i915: Refactor common list iteration over GGTT vma Patchwork
2017-12-08  0:32 ` ✗ Fi.CI.IGT: warning for drm/i915: Refactor common list iteration over GGTT vma (rev3) Patchwork
2017-12-08  0:44   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.