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@xxxxxxxxxxxxx> wrote: > > > On 03/27/2014 03:44 PM, Daniel Vetter wrote: > > > On Thu, Mar 27, 2014 at 10:34 PM, Kenneth Graunke <kenneth@xxxxxxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx