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

[Oded Gabbay]

On Mar 4, 2016 7:53 PM, "Oded Gabbay" <oded.gabbay at gmail.com> wrote:
>
> 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
Marek,
fwiw, 3 weeks ago was the first time I ever saw the r600g driver, so my
questions maybe a bit basic.

Oded
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20160304/1355195f/attachment-0001.html>