Kenneth Graunke <kenn...@whitecape.org> writes:
> It might be nice to create a brw_load_register_mem64 function, for
> symmetry with brw_store_register_mem64 - we might want to reuse it
> elsewhere someday.
Ok, that sounds sensible.
> One interesting quirk: the two halves of your register write may land
> in two separate batchbuffers, since it's done with two BEGIN_BATCH /
> ADVANCE_BATCH blocks (each of which only reserves space for one LRM).
Ah right, yes, good point.
> */ goes on its own line, here and elsewhere.
Oops, yes.
>> + } else {
>> + if (brw->ctx.Query.CondRenderQuery) {
>> + perf_debug("Conditional rendering is implemented in software and
>> may "
>> + "stall.\n");
>> + }
>> +
>> + return _mesa_check_conditional_render(&brw->ctx);
>
> I'd put this in the same block as the perf_debug and just do 'return
> true' here - we can save the function call and redundant query check in
> the common case (and this is a really hot path).
Ok, sounds good.
>> @@ -333,6 +335,9 @@ intelInitExtensions(struct gl_context *ctx)
>> ctx->Extensions.ARB_transform_feedback2 = true;
>> ctx->Extensions.ARB_transform_feedback3 = true;
>> ctx->Extensions.ARB_transform_feedback_instanced = true;
>> +
>> + if (brw->intelScreen->cmd_parser_version >= 2)
>> + brw->predicate.supported = true;
>
> So, this is insufficient for Haswell. There was not a version bump when
> it actually started working (I think Daniel assumed we didn't need it,
> since we were already attempting to write registers ourselves.)
>
> I think the best plan of action is to submit a kernel patch bumping the
> command parser version number to 4, then change this to:
>
> const int cmd_parser_version = brw->intelScreen->cmd_parser_version;
>
> if (cmd_parser_version >= (brw->is_haswell ? 4 : 2))
> brw->predicate.supported = true;
I don't think this is necessary. It's a bit hard to tell from the patch
diff but this hunk is inside an if statement like this:
if (brw->gen >= 7) {
/* ... */
if (can_do_pipelined_register_writes(brw)) {
/* ... transform feedback stuff ... */
if (brw->intelScreen->cmd_parser_version >= 2)
brw->predicate.supported = true;
}
/* ... */
}
Therefore it will only try to use the predicate register if the command
parser is at least version 2 *and* we can do register writes. I don't
think there is any version of the kernel where this wouldn't correctly
detect whether it can write to the predicate register.
If you agree with this then I'll post a v4 with all the other changes.
In the meantime there is a WIP branch of it here:
https://github.com/bpeel/mesa/tree/wip/conditional-render
Many thanks for the detailed review.
Regards,
- Neil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
