[Intel-gfx] [PATCH 19/29] drm/i915: turn bound_ggtt checks to bound_any

[Ben Widawsky]

On Tue, Aug 06, 2013 at 08:43:42PM +0200, Daniel Vetter wrote:
> On Wed, Jul 31, 2013 at 05:00:12PM -0700, Ben Widawsky wrote:
> > In some places, we want to know if an object is bound in any address
> > space, and not just the global GTT. This often applies when there is a
> > single global resource (object, pages, etc.)
> > 
> > function                             |      reason
> > --------------------------------------------------
> > i915_gem_object_is_inactive          | global object
> > i915_gem_object_put_pages            | object's pages
> > 915_gem_object_unpin                 | global object
> > i915_gem_execbuffer_unreserve_object | temporary until we plumb vma
> > pread/pread                          | object's domain
> 
> pread/pwrite isn't about the object's domain at all, but purely about
> synchronizing for outstanding rendering. Replacing the call to
> set_to_gtt_domain with a wait_rendering would imo improve code
> readability. Furthermore we could pimp pread to only block for outstanding
> writes and not for reads.

I might have been the first to trip over it, but this isn't my first
instance ;-).

> 
> Since you're not the first one to trip over this: Can I volunteer you for
> a follow-up patch to fix this?

Working on it now.

> 
> Otherwise patch looks good.
> -Daniel
> 
> > 
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c            | 12 ++++++------
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 1013105..d4d6444 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -122,7 +122,7 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
> >  static inline bool
> >  i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> >  {
> > -	return i915_gem_obj_ggtt_bound(obj) && !obj->active;
> > +	return i915_gem_obj_bound_any(obj) && !obj->active;
> >  }
> >  
> >  int
> > @@ -408,7 +408,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
> >  		 * anyway again before the next pread happens. */
> >  		if (obj->cache_level == I915_CACHE_NONE)
> >  			needs_clflush = 1;
> > -		if (i915_gem_obj_ggtt_bound(obj)) {
> > +		if (i915_gem_obj_bound_any(obj)) {
> >  			ret = i915_gem_object_set_to_gtt_domain(obj, false);
> >  			if (ret)
> >  				return ret;
> > @@ -725,7 +725,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
> >  		 * right away and we therefore have to clflush anyway. */
> >  		if (obj->cache_level == I915_CACHE_NONE)
> >  			needs_clflush_after = 1;
> > -		if (i915_gem_obj_ggtt_bound(obj)) {
> > +		if (i915_gem_obj_bound_any(obj)) {
> >  			ret = i915_gem_object_set_to_gtt_domain(obj, true);
> >  			if (ret)
> >  				return ret;
> > @@ -1659,7 +1659,7 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> >  	if (obj->pages_pin_count)
> >  		return -EBUSY;
> >  
> > -	BUG_ON(i915_gem_obj_ggtt_bound(obj));
> > +	BUG_ON(i915_gem_obj_bound_any(obj));
> >  
> >  	/* ->put_pages might need to allocate memory for the bit17 swizzle
> >  	 * array, hence protect them from being reaped by removing them from gtt
> > @@ -3301,7 +3301,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
> >  	int ret;
> >  
> >  	/* Not valid to be called on unbound objects. */
> > -	if (!i915_gem_obj_ggtt_bound(obj))
> > +	if (!i915_gem_obj_bound_any(obj))
> >  		return -EINVAL;
> >  
> >  	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
> > @@ -3725,7 +3725,7 @@ void
> >  i915_gem_object_unpin(struct drm_i915_gem_object *obj)
> >  {
> >  	BUG_ON(obj->pin_count == 0);
> > -	BUG_ON(!i915_gem_obj_ggtt_bound(obj));
> > +	BUG_ON(!i915_gem_obj_bound_any(obj));
> >  
> >  	if (--obj->pin_count == 0)
> >  		obj->pin_mappable = false;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 5e68f1e..64dc6b5 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -466,7 +466,7 @@ i915_gem_execbuffer_unreserve_object(struct drm_i915_gem_object *obj)
> >  {
> >  	struct drm_i915_gem_exec_object2 *entry;
> >  
> > -	if (!i915_gem_obj_ggtt_bound(obj))
> > +	if (!i915_gem_obj_bound_any(obj))
> >  		return;
> >  
> >  	entry = obj->exec_entry;
> > -- 
> > 1.8.3.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ben Widawsky, Intel Open Source Technology Center