Re: [PATCH 11/17] drm/i915: Fix up the vma aliasing ppgtt binding

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 11/17] drm/i915: Fix up the vma aliasing ppgtt binding
Date: Thu, 16 Apr 2015 10:01:31 +0200	[thread overview]
Message-ID: <20150416080131.GG6092@phenom.ffwll.local> (raw)
In-Reply-To: <20150415104702.GW16216@nuc-i3427.alporthouse.com>

On Wed, Apr 15, 2015 at 11:47:02AM +0100, Chris Wilson wrote:
> On Tue, Apr 14, 2015 at 05:35:21PM +0200, Daniel Vetter wrote:
> > Currently we have the problem that the decision whether ptes need to
> > be (re)written is splattered all over the codebase. Move all that into
> > i915_vma_bind. This needs a few changes:
> > - Just reuse the PIN_* flags for i915_vma_bind and do the conversion
> >   to vma->bound in there to avoid duplicating the conversion code all
> >   over.
> > - We need to make binding for EXECBUF (i.e. pick aliasing ppgtt if
> >   around) explicit, add PIN_EXECBUF for that.
> 
> I am in favour of making the PIN_GLOBAL | PIN_LOCAL explicit, but
> PIN_EXECBUF doesn't seem descriptive of what happens, nor why it should
> be execbuf specific. Just use PIN_LOCAL with the execbuf oring in
> PIN_GLOBAL as it needs for workarounds + relocations.

Well I wasnt' too happy with LOCAL_BIND tbh since that sounds awfully
close to gen1 local memory ;-) My idea behind PIN_EXECBUF (and hiding the
ggtt vs. aliasing ppgtt binding in the low-level code) is that imo
conceptually global vs. aliasing ppgtt is just a pte bit with crazy
storage. We might as well have 2 bits to control access for priviledged
clients and for unpriviledge GT access, and in that case it would make
sense to hide that in the lower levels. ggtt+aliasing ppgtt isnt' anything
else, except for a rather peculariar storage format of these two bits.

Essentially we have:
- PIN_GLOBAL: Please set up pte so that system agent, display and
  priviledged GT can access it.
- PIN_EXECBUF: Please set up pte so that unpriveledged GT access is
  possible.

Maybe PIN_UNPRIV_GT or something else would be better? Pushing the
decision of how that access control is down up into the execbuf code is
imo a layering violation and exactly the kind of trouble that imo has lead
to rebinding or too-much-binding bugs we've had. With my approach it's all
together in the low-level gtt binding code.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx