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

[Marek Olšák]

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.

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.

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.

Marek