[Mesa-dev] [PATCH] i965: Skip register write checks if cmd_parser_version >= 2.

[Francisco Jerez]

Daniel Vetter <daniel at ffwll.ch> writes:

> On Tue, Oct 25, 2016 at 11:16:56AM -0700, Francisco Jerez wrote:
>> Kenneth Graunke <kenneth at whitecape.org> writes:
>> 
>> > If the kernel advertises a new enough command parser version, then we
>> > can just assume that register writes will work and not bother executing
>> > commands on the GPU to test it.
>> >
>> > This should speed up context creation.
>> >
>> > From the command parser version history (i915_cmd_parser.c):
>> >
>> >   * 1. Initial version. Checks batches and reports violations, but leaves
>> >   *    hardware parsing enabled (so does not allow new use cases).
>> >   * 2. Allow access to the MI_PREDICATE_SRC0 and
>> >   *    MI_PREDICATE_SRC1 registers.
>> >
>> > Both of the things we check for were added before version 2, but
>> > 1 doesn't sound sufficient.  The point is to skip work on modern
>> > kernels, so requiring version 2 will work.
>> >
>> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> > ---
>> >  src/mesa/drivers/dri/i965/intel_extensions.c | 8 ++++++++
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
>> > index 66079b5..2a5a3b7 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_extensions.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
>> > @@ -46,6 +46,10 @@ can_do_pipelined_register_writes(struct brw_context *brw)
>> >     if (brw->gen != 7)
>> >        return true;
>> >  
>> > +   /* No need to execute commands to check if the kernel advertises it. */
>> > +   if (brw->screen->cmd_parser_version >= 2)
>> > +      return true;
>> > +
>> >     static int result = -1;
>> >     if (result != -1)
>> >        return result;
>> > @@ -106,6 +110,10 @@ can_write_oacontrol(struct brw_context *brw)
>> >     if (brw->gen < 6 || brw->gen >= 8)
>> >        return false;
>> >  
>> > +   /* No need to execute commands to check if the kernel advertises it. */
>> > +   if (brw->screen->cmd_parser_version >= 2)
>> > +      return true;
>> > +
>> 
>> I'm not sure this gives you any guarantees that the command parser is
>> going to be active?  Has the command parser been enabled by default on
>> all kernel versions exposing cmd_parser_version >= 2?  Wouldn't this
>> break things if the command parser is disabled in the kernel command
>> line?  [Currently it would just cause some extensions to be disabled]
>
> We have the stance that touching any module option is an unsupported
> configuration, don't do that (and we'll ingore bug reports). Either way,
> the kernel reports 0 when the cmd parser is disabled.

I don't think it reports 0 in that case -- At least it didn't last time
I checked.  And ISTR that the command parser wasn't actually in control
on at least some platforms until PPGTT was enabled, so I doubt that
'cmd_parser_version >= 2' gives you any guarantees that the whitelisted
commands are actually going to get through the hardware command checker.

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161026/d439ebc8/attachment.sig>