[Intel-gfx] [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.

[Chris Wilson]

On Fri, May 16, 2014 at 12:05:45PM -0700, Jesse Barnes wrote:
> On Thu, 27 Mar 2014 16:22:44 -0700
> Kenneth Graunke <kenneth at whitecape.org> wrote:
> 
> > On 03/27/2014 03:44 PM, Daniel Vetter wrote:
> > > On Thu, Mar 27, 2014 at 10:34 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > >> Why are we parsing batches with I915_EXEC_SECURE at all?  Secure batches
> > >> are only issued from trusted code which is guaranteed to be running as
> > >> root.  I don't see any benefit to scanning those batches, and there's
> > >> definitely overhead.
> > >>
> > >> I mean, sure, it may be reasonable in the short term as a way to test
> > >> the command parser, but I certainly hope we don't *ship* that.
> > > 
> > > Everyone runs X as root, but I kinda want X to also be able to run as
> > > non-root. The cmd parser has a special list of drm master register
> > > lists which should allow this, but if we just bypass the cmd parser
> > > for all normal X installs we'll have 0 test coverage on this. Which
> > > means broken like hell.
> > > 
> > > Hence I actually intend to ship this, yes. Chris doesn't like it either really.
> > > -Daniel
> > 
> > Seriously?  Hurt performance on every user's system just so you can test
> > things?  That a classic case of the tail wagging the dog.
> > 
> > Why not make a i915.enable_cmd_parser=2 value which enables it all the
> > time and use that when running igt?  Clearly being able to test this is
> > valuable, but enabling it universally is *not* OK.
> 
> Daniel, I'm not sure what you mean by 0 coverage.  Surely DRI clients
> count for something?  And X shouldn't be submitting all its batches
> with the secure bit set, right?  If so, we ought to fix that and only
> use it for ones where it's necessary (e.g. wait events or similar).  I
> agree with Ken and Chris here.
> 
> Chris?

We haven't even fixed the major regression from enabling FBC. What's
another massive slowdown?

Yes, X only sets the secure bit when it pokes the display registers, and
those registers should be privileged even with a cmd parser in place
(which they are).

Daniel's argument presumes that we haven't been patching out the
cmd parser all this time anyway.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre