[Mesa-dev] [PATCH] r600g: Adjust pipe format when decompressing depth in BE

[Oded Gabbay]

On Fri, Mar 4, 2016 at 6:59 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Fri, Mar 4, 2016 at 4:56 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>> On Fri, Mar 4, 2016 at 2:19 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>> Note that the DB only supports tiling and separate depth and stencil, so
>>> it's unmappable. Before transfers and sometimes even texturing, the buffer
>>> must be copied via the DB->CB path, because CB supports both interleaved and
>>> linear layouts. The result is the flushed texture. The goal here is to
>>> ensure the flushed texture uses the correct format.
>>>
>>> Marek
>>>
>> Marek,
>> Thanks for the info, makes the code more clear :)
>>
>> I can do what you asked, but frankly, I don't think it looks better:
>>
>> @@ -2657,9 +2657,15 @@ uint32_t r600_translate_colorformat(enum
>> chip_class chip, enum pipe_format forma
>>                                         return V_0280A0_COLOR_32_32;
>>                         }
>>                 } else if (HAS_SIZE(8,24,0,0)) {
>> -                       return V_0280A0_COLOR_24_8;
>> +                       if (R600_BIG_ENDIAN)
>> +                               return V_0280A0_COLOR_8_24;
>> +                       else
>> +                               return V_0280A0_COLOR_24_8;
>>                 } else if (HAS_SIZE(24,8,0,0)) {
>> -                       return V_0280A0_COLOR_8_24;
>> +                       if (R600_BIG_ENDIAN)
>> +                               return V_0280A0_COLOR_24_8;
>> +                       else
>> +                               return V_0280A0_COLOR_8_24;
>>                 }
>>                 break;
>>         case 3:
>>
>>
>> @@ -1296,7 +1296,11 @@ unsigned r600_translate_colorswap(enum
>> pipe_format format)
>>                          (HAS_SWIZZLE(0,Y) && HAS_SWIZZLE(1,NONE)) ||
>>                          (HAS_SWIZZLE(0,NONE) && HAS_SWIZZLE(1,X))) {
>>                         entry = 2;
>> +#ifdef PIPE_ARCH_LITTLE_ENDIAN
>>                         ret = V_0280A0_SWAP_STD_REV; /* YX__ */
>> +#else
>> +                       ret = V_0280A0_SWAP_STD; /* YX__ */
>> +#endif
>>                 } else if (HAS_SWIZZLE(0,X) && HAS_SWIZZLE(3,Y)) {
>>                         entry = 3;
>>                         ret = V_0280A0_SWAP_ALT; /* X__Y */
>>
>>
>> Actually I think it looks worse as we need to intervene in two places
>> to get the same result I got with just switching the pipe format of
>> the CB. And I still didn't check what happens with textures.
>>
>> In any case, this is basically a workaround, because gallium still
>> thinks it uses the PIPE_FORMAT_Z24_UNORM_S8_UINT format, while we
>> program the GPU with the PIPE_FORMAT_S8_UINT_Z24_UNORM parameters.
>>
>> I think that if we use a workaround, better use a cleaner/simpler one.
>
> Have you tested entire piglit with your first workaround? I doubt it
> passes depth texturing tests.

No. I only focused so far on getting gl-1.0-readpixsanity to work...

piglit run on BE r600g is in such worse shape, there is no way of
telling. It crashes after about 300 tests (out of 8600) and in those
300 tests, about 50 are failing, and I get dmesg errors from the
kernel driver.
Really, BE r600g is so broken, that there can be no baseline until I
manage to fix things so I get at least an entire piglit run without
dmesg errors and computer crashes. And I think I'm not anywhere near
that yet.

I'm sure I will find out that some of my current fixes don't match all
of the cases and they will need better tweaking. I don't see that as
problematic.

>
> The problem with hacking the DB->CB decompression blit path is that
> it's sometimes used for texturing and other times it's not, and it
> depends on the format and the chip. The other option is that the
> DB->CB copy is not used for texturing if the hardware supports
> in-place DB decompression; if so, the DB->CB copy is only used for
> downloads. The result is that we have to maintain 2 texturing
> codepaths: one that uses the original texture used by DB and
> decompressed in-place, and one that is the result of the DB->CB copy.

I assume you mean r600_blit_decompress_depth() and
r600_blit_decompress_depth_in_place() ?

>
> If you want to change the format of the flushed texture, fine, but you
> should change it for all users of that codepath, not just texture
> downloads.

I'm not sure what you mean by that. Could you please give me an
example or point me to the relevant functions ?

Thanks,

     Oded
>
> Marek