[Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling

[Neil Roberts]

> On Fri, Nov 07, 2014 at 03:28:01PM +0000, Neil Roberts wrote:

>> Unfortunately these two source registers are not in the whitelist of
>> available registers in the kernel driver so this needs a kernel patch
>> to work. This patch tries to check whether it is possible to write to
>> this register by creating a DRM buffer with a single command to write
>> to the register and then trying to exec it. The return value of the
>> exec function (and hence the ioctl) is checked.
>> 
>> There is a function with a similar goal just above to check whether
>> the OACONTROL register can be used. This works by putting the test
>> command in the regular batch buffer and trying to compare whether the
>> value was successfully written. I couldn't get this approach to work
>> because intel_batchbuffer_flush actually calls exit() if the ioctl
>> fails. I am suspicious that this approach doesn't work for OACONTROL
>> either.

Daniel Vetter <daniel at ffwll.ch> writes:

> Atm the cmd parser for gen7 validates the batch and rejects it if there's
> something in there it doesn't like. But it doesn't grant any additional
> privs. Hence
> - OACONTROL passes since it's in the whitelist, but since the cmd parser
>   doesn't grant the needed privs the writes would need.
> - your new regs aren't in the whitelist, so the execbuf fails with
>   -EINVAL.
>
> We have a cmd parser version which we intend to bump every time we add new
> registers, so probably better to check that. And I guess we'd need one to
> indicate that the cmd parser actually does something useful. Probably best
> done with just another bump. And if we do that we could replace the
> current trick mesa uses with just a getparam query - the getparam is fixed
> so either returns -EINVAL on old kernels or the cmd parser version.

I'm not really sure if I understand your reply correctly. Did you see
that I also posted the corresponding kernel patch? I was testing my
patch on top of the drm-intel-next branch with IvyBridge. If I don't
apply the patch then I get EINVAL and the register load doesn't work. If
I do apply it then everything seems to work and the Piglit tests pass.
Therefore I'm assuming that the only thing that's stopping the register
load from working without the patch is the whitelist in the command
parser so adding the registers to the whitelist should be enough to get
it working.

It looks like the PRM for Haswell says that MI_LOAD_REGISTER_MEM is
converted to no-op for non-privileged buffers. However I can't find any
mention of this for IvyBridge. Does that mean it's allowed on IvyBridge
but it won't work on Haswell? I haven't tested it on Haswell yet.

However, as well as checking the command parser version, the patch for
Mesa only enables using the predicate registers if it determines it can
do a register write from an untrusted buffer. This is done via
can_do_pipelined_register_writes which already existed to determine
whether we can do indirect renders. So presumably checking for this and
checking for version 2 of the command parser would be enough to ensure
that we can load into the predicate register. Presumably it would also
start working magically on Haswell if we did something to disable the
hardware command parser and only relied on the kernel command parser. Is
that the plan?

The bit I mentioned about OACONTROL was just saying that the method of
detecting whether we can write to OACONTROL specifically doesn't work.
This is because writing to a register that is not in the whitelist
returns EINVAL and Mesa calls exit(1) when drm_intel_gem_bo_context_exec
fails. I used a different method to detect whether we can write to
MI_PREDICATE_SRC0 by manually calling drm_intel_gem_bo_context_exec to
avoid the call to exit(1). I think the code to check whether OACONTROL
is accessible is wrong because the process will have already exited
before it gets a chance to check whether the load worked. In v2 of the
patch I made it check the cmd parser version as you suggested. We should
probably do something similar for OACONTROL, but that is a separate
issue.

Regards,
- Neil