[Mesa-dev] [PATCH] nv50: make sure to clear _all_ layers of all attachments

[Ilia Mirkin]

On Thu, Feb 13, 2014 at 9:57 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 13/02/14 10:13, Ilia Mirkin wrote:
>> Unfortunately there's only one RT_ARRAY_MODE setting for all
>> attachments, so clears were previously truncated to the minimum number
>> of layers any attachment had. Instead set the RT_ARRAY_MODE to 512 (the
>> max number of layers) before doing the clear. This fixes
>> gl-3.2-layered-rendering-clear-color-mismatched-layer-count.
>>
> Pardon my ignorance but how did you gather the maximum number of layers
> ? The headers list values up-to 0xffff.
>
> Guessing that setting the maximum number of layers is a slight overkill
> but I'm definitely not opposed to it.

The headers sure do...

[369050.901062] nouveau E[  PGRAPH][0000:02:00.0] DATA_ERROR INVALID_VALUE
[369050.901069] nouveau E[  PGRAPH][0000:02:00.0] ch 4 [0x003f92e000
gl-3.2-layered-[13468]] subc 3 class 0x8297 mthd 0x1224 data
0x0000ffff
[369178.121557] nouveau E[  PGRAPH][0000:02:00.0] DATA_ERROR INVALID_VALUE
[369178.121567] nouveau E[  PGRAPH][0000:02:00.0] ch 4 [0x003f92e000
gl-3.2-layered-[14400]] subc 3 class 0x8297 mthd 0x1224 data
0x00000201

And also

   case PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS:
      return 512;

So I'm pretty sure that 512 is max. I'll have to check whether the
rnndb can support a max value like that.

CLEAR_BUFFERS explicitly gets the layer id from the code, so we'll
never try to clear a layer that we don't think is there. The whole
problem originates from the issues around multiple attachments with
different numbers of layers. Setting the max just avoids the card from
checking whether the layer id is valid or not based on the
RT_ARRAY_MODE.

>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> index a22436b..68924c9 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> @@ -366,7 +366,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>>     PUSH_DATA (push, bo->offset + sf->offset);
>>     PUSH_DATA (push, nv50_format_table[dst->format].rt);
>>     PUSH_DATA (push, mt->level[sf->base.u.tex.level].tile_mode);
>> -   PUSH_DATA (push, 0);
>> +   PUSH_DATA (push, mt->layer_stride >> 2);
> I'm guessing that this hunk could be separated, as it does make sense to
> set the stride when doing the clears.

I guess it could be. But it's related to clearing and layers and I
noticed it when I was doing this exercise, so... seems reasonable to
throw it in here.

>
> Does this make make any difference wrt the test in question ?

Not in the least. The test goes down the nv50_clear path. No idea what
hits this path, but it's part of the pipe interface.

  -ilia