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

[Chris Wilson]

On Thu, Apr 16, 2015 at 10:01:31AM +0200, Daniel Vetter wrote:
> 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.

Yeah, I'm just arguing that EXECBUF doesn't tell me anything at all
about what is happening. It is just not descriptive enough and is liable
to change meanings and end up in confusion.

How about PIN_GLOBAL (aka PIN_SYSTEM) and PIN_USER?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre