Re: [PATCH 4/6] drm/i915: Add bind/unbind object functions to VM

[Ben Widawsky <ben@bwidawsk.net>]

On Mon, Sep 16, 2013 at 10:25:28AM +0100, Chris Wilson wrote:
> On Sat, Sep 14, 2013 at 03:03:17PM -0700, Ben Widawsky wrote:
> > +static void gen6_ggtt_bind_vma(struct i915_vma *vma,
> > +			       enum i915_cache_level cache_level,
> > +			       u32 flags)
> > +{
> > +	struct drm_device *dev = vma->vm->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_i915_gem_object *obj = vma->obj;
> > +	const unsigned long entry = vma->node.start >> PAGE_SHIFT;
> > +
> > +	/* If there is an aliasing PPGTT, and the user didn't explicitly ask for
> > +	 * the global, just use aliasing */
> > +	if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) {
> > +		/* If the object is unbound, or we're change the cache bits */
> > +		if (!obj->has_global_gtt_mapping ||
> > +		    (cache_level != obj->cache_level)) {
> > +			gen6_ggtt_insert_entries(vma->vm, obj->pages, entry,
> > +						 cache_level);
> > +			obj->has_global_gtt_mapping = 1;
> > +		}
> > +	}
> > +
> > +	/* If put the mapping in the aliasing PPGTT as well as Global if we have
> > +	 * aliasing, but the user requested global. */
> 
> Why? As a proponent of full-ppgtt I thought you would be envisoning a
> future where the aliasing_ppgtt was used far less (i.e. never), and the
> ggtt would only continue to be used for the truly global entries such as
> scanouts, contexts, pdes, execlists etc.
> 

Firstly, I've still yet to expose the grand plan at this point in the
series, so I am not really certain if you're just complaining for the
fun of it, or what. I'd like to make everything functionally the same,
just with VMA support.

Secondly, I was under the impression that for Sandybridge we had to have
all global mappings in the aliasing to support PIPE_CONTROL, or some
command like that. It's a bit mixed up in my head atm, and I'm too lazy
to look at the exact reason.

Finally, see firstly. I'll try to rip it out later on if it's possible.

[snip]

> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ben Widawsky, Intel Open Source Technology Center