[Mesa-dev] [PATCH] amd/surface: fix setting of ADDR2_SURFACE_FLAGS::color

[Marek Olšák]

On Thu, Dec 20, 2018, 4:05 AM Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl
wrote:

> On Thu, Dec 20, 2018 at 9:00 AM Marek Olšák <maraeo at gmail.com> wrote:
> >
> > I prefer Nicolai's patch because it's shorter and doesn't need driver
> changes.
> >
> > Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>
> This patch still needs driver changes because radv will render to a
> compressed surface as r32g32b32a32, so allocating a non-renderable
> surface without driver changes goes against actual usage?
>

Ok. I just meant the ac_surface interface change.

Marek

>
> > Marek
> >
> > On Tue, Dec 18, 2018 at 12:50 PM Haehnle, Nicolai <
> Nicolai.Haehnle at amd.com> wrote:
> >>
> >> On 18.12.18 18:36, Bas Nieuwenhuizen wrote:
> >> > Hi Nicolai,
> >> >
> >> > I happened to be writing something similar which also fixes up radv to
> >> > never render to those surfaces as r32g32b32a32:
> >> > https://patchwork.freedesktop.org/series/54172/ I can split out the
> >> > radv specific stuff and this one is r-b after than.
> >>
> >> Oh, I missed that, sorry. I don't particularly care about which approach
> >> to this is taken.
> >>
> >> Cheers,
> >> Nicolai
> >>
> >>
> >>
> >> >
> >> > Thanks,
> >> > Bas
> >> >
> >> > On Tue, Dec 18, 2018 at 5:37 PM Nicolai Hähnle <nhaehnle at gmail.com>
> wrote:
> >> >>
> >> >> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> >> >>
> >> >> In the gfx9 addrlib, this bit has been clarified as meaning that
> >> >> the surface can be used as a color buffer (render target).
> >> >>
> >> >> Setting this for compressed surfaces triggers a workaround that
> >> >> is only required for surfaces that can be render targets, and ends
> >> >> up breaking the 16-byte-per-block case.
> >> >>
> >> >> Fixes
> dEQP-VK.pipeline.image.suballocation.sampling_type.combined.view_type.3d.format.etc2_r8g8b8a8_srgb_block.count_1.size.11x11x11
> and others
> >> >>
> >> >> Note that there are other related bits which we don't set as intended
> >> >> by the interface, notably the 'unordered' bit, which is meant to
> >> >> indicate use as a shader image. It may be worth cleaning that up at
> some
> >> >> point after proper testing.
> >> >>
> >> >> Reported-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> >> >> Fixes: 776b9113656 ("amd/addrlib: update Mesa's copy of addrlib")
> >> >> ---
> >> >>   src/amd/common/ac_surface.c | 5 ++---
> >> >>   1 file changed, 2 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/src/amd/common/ac_surface.c
> b/src/amd/common/ac_surface.c
> >> >> index d8d927ee1c5..d647bd523f9 100644
> >> >> --- a/src/amd/common/ac_surface.c
> >> >> +++ b/src/amd/common/ac_surface.c
> >> >> @@ -1405,25 +1405,24 @@ static int gfx9_compute_surface(ADDR_HANDLE
> addrlib,
> >> >>                  case 16:
> >> >>                          assert(!(surf->flags &
> RADEON_SURF_Z_OR_SBUFFER));
> >> >>                          AddrSurfInfoIn.format =
> ADDR_FMT_32_32_32_32;
> >> >>                          break;
> >> >>                  default:
> >> >>                          assert(0);
> >> >>                  }
> >> >>                  AddrSurfInfoIn.bpp = surf->bpe * 8;
> >> >>          }
> >> >>
> >> >> -       AddrSurfInfoIn.flags.color = !(surf->flags &
> RADEON_SURF_Z_OR_SBUFFER);
> >> >> +       AddrSurfInfoIn.flags.color = !compressed && !(surf->flags &
> RADEON_SURF_Z_OR_SBUFFER);
> >> >>          AddrSurfInfoIn.flags.depth = (surf->flags &
> RADEON_SURF_ZBUFFER) != 0;
> >> >>          AddrSurfInfoIn.flags.display = get_display_flag(config,
> surf);
> >> >> -       /* flags.texture currently refers to TC-compatible HTILE */
> >> >> -       AddrSurfInfoIn.flags.texture = AddrSurfInfoIn.flags.color ||
> >> >> +       AddrSurfInfoIn.flags.texture = AddrSurfInfoIn.flags.color ||
> compressed ||
> >> >>                                         surf->flags &
> RADEON_SURF_TC_COMPATIBLE_HTILE;
> >> >>          AddrSurfInfoIn.flags.opt4space = 1;
> >> >>
> >> >>          AddrSurfInfoIn.numMipLevels = config->info.levels;
> >> >>          AddrSurfInfoIn.numSamples = MAX2(1, config->info.samples);
> >> >>          AddrSurfInfoIn.numFrags = AddrSurfInfoIn.numSamples;
> >> >>
> >> >>          if (!(surf->flags & RADEON_SURF_Z_OR_SBUFFER))
> >> >>                  AddrSurfInfoIn.numFrags = MAX2(1,
> config->info.storage_samples);
> >> >>
> >> >> --
> >> >> 2.19.1
> >> >>
> >> >> _______________________________________________
> >> >> mesa-dev mailing list
> >> >> mesa-dev at lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181220/7a554240/attachment.html>