Re: [PATCH 12/17] drm/i915: Arm cmd parser with aliasng ppgtt only

Daniel Vetter
2015-04-15T10:26:43Z

On Wed, Apr 15, 2015 at 11:07:15AM +0100, Chris Wilson wrote:
> On Wed, Apr 15, 2015 at 11:43:25AM +0200, Daniel Vetter wrote:
> > On Tue, Apr 14, 2015 at 07:10:30PM +0100, Chris Wilson wrote:
> > > On Tue, Apr 14, 2015 at 05:35:22PM +0200, Daniel Vetter wrote:
> > > > With the binding regression from the original full ppgtt patches
> > > > fixed we can throw the switch. Yay!
> > >
> > > This changelog is misleading. The validation part of the command parser
> > > has been running for some time, with people starting to notice the
> > > performance regressions. What is being turned on here is the enabling
> > > part to allow userspace to do more. So shouldn't that also be a bump in
> > > the command parser version?
> >
> > mesa has independent checks that the register writes go through, so just
> > switching the cmd parser to permission granting mode should be all that's
> > neeeded really.
>
> But the issue is the hardware would allow the writes anyway, and that
> this patch has no actual effect since mesa can already do pipelined
> register writes (at least on ivb/byt).

Yeah it's only interesting for hsw really.

> > And yes the cmd parser is enabled already, I thought "to arm" does convey
> > that it's now going from dummy mode to live.
>
> "Arm cmd parser" reads to me as a passive actor (parser is just a reader
> and doesn't suggest that it enables anything).
>
> "Enable cmd parser to do secure batch promotion for aliasing ppgtt"
>
> or perhaps
>
> "Now witness the firepower of this fully ARMED and OPERATIONAL cmdparser"

Yeah not my best commit summary ever. I'll go with the first suggestion.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation