<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Hey Pam!</div><div class="gmail_quote"><br></div><div class="gmail_quote">A quick skim through this looks fairly good.  I'll be honest, I expected to have some quick comments on the v1 but I'm not seeing anything immediately.  That said, I've got to think about it fairly hard to convince myself that it's actually correct.  The i965 drawing logic isn't really my area.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">On Thu, Aug 31, 2017 at 11:26 AM, Ilia Mirkin <span dir="ltr"><<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Aug 31, 2017 at 1:45 PM, Bas Nieuwenhuizen<br>
<<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br>
><br>
><br>
> On Wed, Aug 30, 2017, at 14:59, Ilia Mirkin wrote:<br>
>> On Wed, Aug 30, 2017 at 3:17 AM, Bas Nieuwenhuizen<br>
>> <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br>
>> > So as a random drive-by review, I think the risk in this implementation<br>
>> > is that apps just set maxdrawcount to some high value . If I'm reading<br>
>> > the spec correctly there is no real bound on the value except for the<br>
>> > max-representable value for the integer.  Also AFAIK the AMD and NVidia<br>
>> > implementation don't really get slower if you specify maxdrawcount very<br>
>><br>
>> Can't speak for AMD, but this is actually extremely similar (in<br>
>> principle) to how this is handled on NVIDIA. It's a little cleaner<br>
>> since there's a nice macro, but since macros can't have a variable<br>
>> number of arguments, you have to feed it maxdrawcount args. Admittedly<br>
>> this is the nouveau impl, but I'm almost certain that the blob does it<br>
>> in a similar way.<br>
><br>
> I thought, you had a macro that looped, but seems that was a<br>
> misunderstanding on my side.  If nvidia does it that way, then I suppose<br>
> there is little risk of games expecting something more efficient. Never<br>
> mind my comments then.<br>
<br>
</span>There indeed is a macro that loops. However that macro takes a number<br>
of parameters determined at the time of cmd stream generation. Also<br>
the indirect draw arguments have to be added into the cmd stream (via<br>
IB references). Here's how nouveau does it:<br>
<br>
<a href="https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c#n798" rel="noreferrer" target="_blank">https://cgit.freedesktop.org/<wbr>mesa/mesa/tree/src/gallium/<wbr>drivers/nouveau/nvc0/nvc0_vbo.<wbr>c#n798</a><br>
<br>
I don't remember if I looked precisely at what NVIDIA does, but I<br>
definitely can't think of another way. Even if you use the command<br>
which takes the argument length as a separate word, that won't be<br>
right since it's count * N arguments. macros can't read arbitrary<br>
things either, it has to come out of the cmdstream.<br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">I'll agree that this solution is suboptimal.  However, it's really the best we can do without getting crazy.  A "real" implementation would require a self-modifying command buffer which is a sketchy idea in the best of circumstances. :/<br></div></div>