All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>,
	Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH 6/9] drm/i915: Wrap VMA binding
Date: Wed, 7 May 2014 09:55:49 +0200	[thread overview]
Message-ID: <20140507075549.GO5730@phenom.ffwll.local> (raw)
In-Reply-To: <1399440098-17378-6-git-send-email-benjamin.widawsky@intel.com>

On Tue, May 06, 2014 at 10:21:35PM -0700, Ben Widawsky wrote:
> This will be useful for some upcoming patches which do more platform
> specific work. Having it in one central place just makes things a bit
> cleaner and easier.
> 
> There is a small functional change here. There are more calls to the
> tracepoints.
> 
> NOTE: I didn't actually end up using this patch for the intended purpose, but I
> thought it was a nice patch to keep around.
> 
> v2: s/i915_gem_bind_vma/i915_gem_vma_bind/
> s/i915_gem_unbind_vma/i915_gem_vma_unbind/
> (Chris)
> 
> v3: Missed one spot
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

You change the semantics of the vma bind/unbind tracepoints - previously
they've meant we reserve/unreserve a ppgtt address for an object, not
they're called every time we touch the ptes (which is what
vma_bind|unbind actually do). That doesn't look too terribly useful for
tracing any more.

What's the use case for these added tracepoints?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h            |  3 +++
>  drivers/gpu/drm/i915/i915_gem.c            | 13 ++++++-------
>  drivers/gpu/drm/i915/i915_gem_context.c    |  2 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
>  drivers/gpu/drm/i915/i915_gem_gtt.c        | 16 ++++++++++++++--
>  5 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c1d2bea..30cde3d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2264,6 +2264,9 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
>  			struct i915_address_space *vm);
>  unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
>  				struct i915_address_space *vm);
> +void i915_gem_vma_bind(struct i915_vma *vma, enum i915_cache_level,
> +		       unsigned flags);
> +void i915_gem_vma_unbind(struct i915_vma *vma);
>  struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
>  				     struct i915_address_space *vm);
>  struct i915_vma *
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e9599e9..1567911 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2771,7 +2771,7 @@ int i915_vma_unbind(struct i915_vma *vma)
>  
>  	trace_i915_vma_unbind(vma);
>  
> -	vma->unbind_vma(vma);
> +	i915_gem_vma_unbind(vma);
>  
>  	i915_gem_gtt_finish_object(obj);
>  
> @@ -3342,9 +3342,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>  
>  	WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable);
>  
> -	trace_i915_vma_bind(vma, flags);
> -	vma->bind_vma(vma, obj->cache_level,
> -		      flags & (PIN_MAPPABLE | PIN_GLOBAL) ? GLOBAL_BIND : 0);
> +	i915_gem_vma_bind(vma, obj->cache_level,
> +			  flags & (PIN_MAPPABLE | PIN_GLOBAL) ? GLOBAL_BIND : 0);
>  
>  	i915_gem_verify_gtt(dev);
>  	return vma;
> @@ -3548,8 +3547,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  
>  		list_for_each_entry(vma, &obj->vma_list, vma_link)
>  			if (drm_mm_node_allocated(&vma->node))
> -				vma->bind_vma(vma, cache_level,
> -					      obj->has_global_gtt_mapping ? GLOBAL_BIND : 0);
> +				i915_gem_vma_bind(vma, cache_level,
> +						  obj->has_global_gtt_mapping ? GLOBAL_BIND : 0);
>  	}
>  
>  	list_for_each_entry(vma, &obj->vma_list, vma_link)
> @@ -3914,7 +3913,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  	}
>  
>  	if (flags & PIN_GLOBAL && !obj->has_global_gtt_mapping)
> -		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
> +		i915_gem_vma_bind(vma, obj->cache_level, GLOBAL_BIND);
>  
>  	vma->pin_count++;
>  	if (flags & PIN_MAPPABLE)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index f77b4c1..00c7d88 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -655,7 +655,7 @@ static int do_switch(struct intel_ring_buffer *ring,
>  	if (!to->obj->has_global_gtt_mapping) {
>  		struct i915_vma *vma = i915_gem_obj_to_vma(to->obj,
>  							   &dev_priv->gtt.base);
> -		vma->bind_vma(vma, to->obj->cache_level, GLOBAL_BIND);
> +		i915_gem_vma_bind(vma, to->obj->cache_level, GLOBAL_BIND);
>  	}
>  
>  	if (!to->is_initialized || i915_gem_context_is_default(to))
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 6cc004f..211bbbd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -369,7 +369,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
>  		struct i915_vma *vma =
>  			list_first_entry(&target_i915_obj->vma_list,
>  					 typeof(*vma), vma_link);
> -		vma->bind_vma(vma, target_i915_obj->cache_level, GLOBAL_BIND);
> +		i915_gem_vma_bind(vma, target_i915_obj->cache_level,
> +				  GLOBAL_BIND);
>  	}
>  
>  	/* Validate that the target is in a valid r/w GPU domain */
> @@ -1266,7 +1267,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  		 * allocate space first */
>  		struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
>  		BUG_ON(!vma);
> -		vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND);
> +		i915_gem_vma_bind(vma, batch_obj->cache_level, GLOBAL_BIND);
>  	}
>  
>  	if (flags & I915_DISPATCH_SECURE)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 79ae83b..a66cb6a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1319,10 +1319,9 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  		 * without telling our object about it. So we need to fake it.
>  		 */
>  		obj->has_global_gtt_mapping = 0;
> -		vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
> +		i915_gem_vma_bind(vma, obj->cache_level, GLOBAL_BIND);
>  	}
>  
> -
>  	if (INTEL_INFO(dev)->gen >= 8) {
>  		gen8_setup_private_ppat(dev_priv);
>  		return;
> @@ -1986,6 +1985,19 @@ int i915_gem_gtt_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> +void i915_gem_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
> +		       unsigned flags)
> +{
> +	trace_i915_vma_bind(vma, flags);
> +	vma->bind_vma(vma, cache_level, flags);
> +}
> +
> +void i915_gem_vma_unbind(struct i915_vma *vma)
> +{
> +	trace_i915_vma_unbind(vma);
> +	vma->unbind_vma(vma);
> +}
> +
>  static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
>  					      struct i915_address_space *vm)
>  {
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@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

  reply	other threads:[~2014-05-07  7:55 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-07  5:21 [PATCH 1/9] drm/i915: Use topdown allocation for PPGTT PDEs on gen6/7 Ben Widawsky
2014-05-07  5:21 ` [PATCH 2/9] drm/i915: Extract node allocation from bind Ben Widawsky
2014-05-07  7:02   ` Chris Wilson
2014-05-07 15:45     ` Ben Widawsky
2014-05-07 15:53       ` Chris Wilson
2014-05-07 16:00         ` Ben Widawsky
2014-05-07 16:55           ` Chris Wilson
2014-05-07 17:30             ` Ben Widawsky
2014-05-07  5:21 ` [PATCH 3/9] drm/i915: WARN on unexpected return from drm_mm Ben Widawsky
2014-05-07  5:21 ` [PATCH 4/9] drm/i915: Limit the number of node allocation retries Ben Widawsky
2014-05-07  7:49   ` Daniel Vetter
2014-05-07 15:21     ` Ben Widawsky
2014-05-07  5:21 ` [PATCH 5/9] drm/i915: Use new drm node allocator for PPGTT Ben Widawsky
2014-05-07  5:21 ` [PATCH 6/9] drm/i915: Wrap VMA binding Ben Widawsky
2014-05-07  7:55   ` Daniel Vetter [this message]
2014-05-07 15:54     ` Ben Widawsky
2014-05-07 16:09       ` Daniel Vetter
2014-05-07  5:21 ` [PATCH 7/9] drm/i915: Make aliasing a 2nd class VM Ben Widawsky
2014-05-07  7:56   ` Daniel Vetter
2014-05-07  5:21 ` [PATCH 8/9] drm/i915: Make pin global flags explicit Ben Widawsky
2014-05-07  5:21 ` [PATCH 9/9] drm/i915: Split out aliasing binds Ben Widawsky
2014-05-07  7:59   ` Daniel Vetter
2014-05-07  7:44 ` [PATCH 1/9] drm/i915: Use topdown allocation for PPGTT PDEs on gen6/7 Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140507075549.GO5730@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=ben@bwidawsk.net \
    --cc=benjamin.widawsky@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.