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

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Sep 16, 2013 at 11:23:43AM -0700, Ben Widawsky wrote:
> 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.

Ben's right afaik, we need this kludge to keep snb happy. And we need
ppgtt to make kernel cs scanning possible, which we seem to need for
geomtry shaders or some other gl3.2/3 feature. So not much choice I'd say
...

-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch