[Mesa-stable] [Mesa-dev] [PATCH] nvc0: don't make use of push hint if there are no non-const user vbos

[Ilia Mirkin]

On Sun, Jun 19, 2016 at 9:56 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> I don't know if you remember, but I suggested exactly the same patch few
> days ago when I was looking at the Talos issue, but I was not really sure if
> this was the "correct" fix.

Sorry, I had no recollection of that - I only remember the patch that
you sent to the list, which effectively set vbo_push_hint to 0 for
everything.

>
> Anyways, this patch looks fine to me (even if we don't exactly know why it
> works this way).
>
> This will also improve performance in Talos because the manual push path is
> sometimes slow and that arbitrary limit should be improved.

Well, it's especially slow when it's used when it shouldn't be - like
when there are no user buffers. Which is 99% of modern applications,
since user buffers were nixed in core contexts as I recall.

>
> We still need to fix the remaining issue (ie. the green wall thing) but this
> one is most likely totally unrelated to the push path.
>
> Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

Thanks!

>
>
> On 06/19/2016 06:34 AM, Ilia Mirkin wrote:
>>
>> This makes the check match up what we do on nv50 as well - there's no
>> point in switching over the push path if everything's in managed
>> buffers. This can happen when a shader uses a vertex without an enabled
>> array - we end up passing it a constant attribute.
>>
>> This also has the effect of "fixing" some flickering in Talos. I have no
>> idea why. I've stared at the push logic forwards, backwards, and
>> sideways. By always forcing the push path (which is slow), the
>> flickering also goes away, but other rendering is still wrong
>> (specifically draw 383068 as identified in the bug). However by not
>> switching over to the push path, draw 383068 is correct.
>>
>> Note that other flickering remains in Talos, like the red/green
>> walls/floors. This takes care of the shadow flickering though.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90513
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c
>> b/src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c
>> index 7662bdd..cae621c 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c
>> @@ -441,8 +441,10 @@ nvc0_vertex_arrays_validate(struct nvc0_context
>> *nvc0)
>>     if (unlikely(vertex->need_conversion) ||
>>         unlikely(nvc0->vertprog->vp.edgeflag < PIPE_MAX_ATTRIBS)) {
>>        vbo_mode = 3;
>> +   } else if (nvc0->vbo_user & ~nvc0->constant_vbos) {
>> +      vbo_mode = nvc0->vbo_push_hint ? 1 : 0;
>>     } else {
>> -      vbo_mode = (nvc0->vbo_user && nvc0->vbo_push_hint) ? 1 : 0;
>> +      vbo_mode = 0;
>>     }
>>     const_vbos = vbo_mode ? 0 : nvc0->constant_vbos;
>>
>>
>