[Intel-gfx] [PATCH 06/15] drm/i915: Mark up obj->pin_global as being atomic

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Coming up next, we will want to manipulate the pin_global counter
> outside of the principle locks, so convert it to an atomic.
>

I am not yet up to speed on pin_global but
being part of a series where where this needs
to be atomic, the patch does what it says on the tin.

Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>


> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c       | 12 ++++++------
>  drivers/gpu/drm/i915/gem/i915_gem_object.h       |  3 ++-
>  drivers/gpu/drm/i915/gem/i915_gem_object_types.h |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c     |  2 +-
>  drivers/gpu/drm/i915/i915_debugfs.c              |  4 ++--
>  5 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index ef33001e8ddb..b7be9ad2b62c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -27,7 +27,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
>  
>  void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj)
>  {
> -	if (!READ_ONCE(obj->pin_global))
> +	if (!atomic_read(&obj->pin_global))
>  		return;
>  
>  	i915_gem_object_lock(obj);
> @@ -425,7 +425,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	/* Mark the global pin early so that we account for the
>  	 * display coherency whilst setting up the cache domains.
>  	 */
> -	obj->pin_global++;
> +	atomic_inc(&obj->pin_global);
>  
>  	/* The display engine is not coherent with the LLC cache on gen6.  As
>  	 * a result, we make sure that the pinning that is about to occur is
> @@ -475,7 +475,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	return vma;
>  
>  err_unpin_global:
> -	obj->pin_global--;
> +	atomic_dec(&obj->pin_global);
>  	return vma;
>  }
>  
> @@ -515,11 +515,11 @@ i915_gem_object_unpin_from_display_plane(struct i915_vma *vma)
>  
>  	assert_object_held(obj);
>  
> -	if (WARN_ON(obj->pin_global == 0))
> +	if (GEM_WARN_ON(!atomic_read(&obj->pin_global)))
>  		return;
>  
> -	if (--obj->pin_global == 0)
> -		vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
> +	if (atomic_dec_and_test(&obj->pin_global))
> +		vma->display_alignment = I915_GTT_MIN_ALIGNMENT; /* XXX race */
>  
>  	/* Bump the LRU to try and avoid premature eviction whilst flipping  */
>  	i915_gem_object_bump_inactive_ggtt(obj);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 5efb9936e05b..11bb1a412e4f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -406,7 +406,8 @@ static inline bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
>  	if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE))
>  		return true;
>  
> -	return obj->pin_global; /* currently in use by HW, keep flushed */
> +	/* Currently in use by HW (display engine)? Keep flushed. */
> +	return atomic_read(&obj->pin_global);
>  }
>  
>  static inline void __start_cpu_write(struct drm_i915_gem_object *obj)
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> index 511fce7913ec..a09e74e27734 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -153,7 +153,7 @@ struct drm_i915_gem_object {
>  	/** Count of VMA actually bound by this object */
>  	atomic_t bind_count;
>  	/** Count of how many global VMA are currently pinned for use by HW */
> -	unsigned int pin_global;
> +	atomic_t pin_global;
>  
>  	struct {
>  		struct mutex lock; /* protects the pages and their use */
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 94970f60d7f6..841f18386b54 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -78,7 +78,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
>  	 * To simplify the scan, and to avoid walking the list of vma under the
>  	 * object, we just check the count of its permanently pinned.
>  	 */
> -	if (READ_ONCE(obj->pin_global))
> +	if (atomic_read(&obj->pin_global))
>  		return false;
>  
>  	/* We can only return physical pages to the system if we can either
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e103fcba6435..c36538f1b36c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -79,7 +79,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
>  
>  static char get_pin_flag(struct drm_i915_gem_object *obj)
>  {
> -	return obj->pin_global ? 'p' : ' ';
> +	return atomic_read(&obj->pin_global) ? 'p' : ' ';
>  }
>  
>  static char get_tiling_flag(struct drm_i915_gem_object *obj)
> @@ -221,7 +221,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>  	seq_printf(m, " (pinned x %d)", pin_count);
>  	if (obj->stolen)
>  		seq_printf(m, " (stolen: %08llx)", obj->stolen->start);
> -	if (obj->pin_global)
> +	if (atomic_read(&obj->pin_global))
>  		seq_printf(m, " (global)");
>  
>  	engine = i915_gem_object_last_write_engine(obj);
> -- 
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx