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

[Bas Nieuwenhuizen]

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?
>
> 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