Re: [PATCH 07/11] drm/i915: Fix up map and fenceable for VMA

From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 07/11] drm/i915: Fix up map and fenceable for VMA
Date: Tue, 9 Jul 2013 09:16:54 +0200	[thread overview]
Message-ID: <20130709071654.GR18285@phenom.ffwll.local> (raw)
In-Reply-To: <1373350122-5118-8-git-send-email-ben@bwidawsk.net>

On Mon, Jul 08, 2013 at 11:08:38PM -0700, Ben Widawsky wrote:
> formerly: "drm/i915: Create VMAs (part 3.5) - map and fenceable
> tracking"
> 
> The map_and_fenceable tracking is per object. GTT mapping, and fences
> only apply to global GTT. As such,  object operations which are not
> performed on the global GTT should not effect mappable or fenceable
> characteristics.
> 
> Functionally, this commit could very well be squashed in to the previous
> patch which updated object operations to take a VM argument.  This
> commit is split out because it's a bit tricky (or at least it was for
> me).
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 21015cd..501c590 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2635,7 +2635,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj,
>  
>  	trace_i915_gem_object_unbind(obj, vm);
>  
> -	if (obj->has_global_gtt_mapping)
> +	if (obj->has_global_gtt_mapping && i915_is_ggtt(vm))
>  		i915_gem_gtt_unbind_object(obj);

Wont this part be done as part of the global gtt clear_range callback?

>  	if (obj->has_aliasing_ppgtt_mapping) {
>  		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);

And I have a hunch that we should shovel the aliasing ppgtt clearing into
the ggtt write_ptes/clear_range callbacks, too. Once all this has settled
at least.

> @@ -2646,7 +2646,8 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj,
>  
>  	list_del(&obj->mm_list);
>  	/* Avoid an unnecessary call to unbind on rebind. */
> -	obj->map_and_fenceable = true;
> +	if (i915_is_ggtt(vm))
> +		obj->map_and_fenceable = true;
>  
>  	vma = i915_gem_obj_to_vma(obj, vm);
>  	list_del(&vma->vma_link);
> @@ -3213,7 +3214,9 @@ search_free:
>  		i915_is_ggtt(vm) &&
>  		vma->node.start + obj->base.size <= dev_priv->gtt.mappable_end;
>  
> -	obj->map_and_fenceable = mappable && fenceable;
> +	/* Map and fenceable only changes if the VM is the global GGTT */
> +	if (i915_is_ggtt(vm))
> +		obj->map_and_fenceable = mappable && fenceable;
>  
>  	trace_i915_gem_object_bind(obj, vm, map_and_fenceable);
>  	i915_gem_verify_gtt(dev);
> -- 
> 1.8.3.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