Re: [PATCH] drm/i915: Refactor common list iteration over GGTT vma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxx>

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux