[Mesa-dev] [PATCH 02/17] ac/surface: don't set the display flag for obviously unsupported cases

[Michel Dänzer]

On 2018-04-06 07:14 PM, Marek Olšák wrote:
> On Fri, Apr 6, 2018 at 11:41 AM, Michel Dänzer <michel at daenzer.net> wrote:
> 
>> On 2018-04-06 03:25 PM, Marek Olšák wrote:
>>> On Thu, Apr 5, 2018, 3:09 AM Michel Dänzer <michel at daenzer.net> wrote:
>>>> On 2018-04-04 07:35 PM, Marek Olšák wrote:
>>>>> On Wed, Apr 4, 2018 at 9:01 AM, Michel Dänzer <michel at daenzer.net>
>>>> wrote:
>>>>>> On 2018-04-04 02:57 PM, Marek Olšák wrote:
>>>>>>> On Wed, Apr 4, 2018, 6:18 AM Michel Dänzer <michel at daenzer.net
>>>>>>> <mailto:michel at daenzer.net>> wrote:
>>>>>>>
>>>>>>>     On 2018-04-04 03:59 AM, Marek Olšák wrote:
>>>>>>>     > From: Marek Olšák <marek.olsak at amd.com <mailto:
>>>> marek.olsak at amd.com
>>>>>>>>
>>>>>>>     >
>>>>>>>     > This enables the tile swizzle for some cases of the displayable
>>>>>>>     micro mode,
>>>>>>>     > and it also fixes an addrlib assertion failure on Vega.
>>>>>>>     > ---
>>>>>>>     >  src/amd/common/ac_surface.c | 18 ++++++++++++++----
>>>>>>>     >  1 file changed, 14 insertions(+), 4 deletions(-)
>>>>>>>     >
>>>>>>>     > diff --git a/src/amd/common/ac_surface.c
>>>>>> b/src/amd/common/ac_surface.c
>>>>>>>     > index b294cd85259..2b20a553d51 100644
>>>>>>>     > --- a/src/amd/common/ac_surface.c
>>>>>>>     > +++ b/src/amd/common/ac_surface.c
>>>>>>>     > @@ -408,20 +408,29 @@ static unsigned
>>>>>>>     cik_get_macro_tile_index(struct radeon_surf *surf)
>>>>>>>     >       tileb = 8 * 8 * surf->bpe;
>>>>>>>     >       tileb = MIN2(surf->u.legacy.tile_split, tileb);
>>>>>>>     >
>>>>>>>     >       for (index = 0; tileb > 64; index++)
>>>>>>>     >               tileb >>= 1;
>>>>>>>     >
>>>>>>>     >       assert(index < 16);
>>>>>>>     >       return index;
>>>>>>>     >  }
>>>>>>>     >
>>>>>>>     > +static bool get_display_flag(const struct ac_surf_config
>>>> *config,
>>>>>>>     > +                          const struct radeon_surf *surf)
>>>>>>>     > +{
>>>>>>>     > +     return surf->flags & RADEON_SURF_SCANOUT &&
>>>>>>>     > +            !(surf->flags & RADEON_SURF_FMASK) &&
>>>>>>>     > +            config->info.samples <= 1 &&
>>>>>>>     > +            surf->bpe >= 4 && surf->bpe <= 8;
>>>>>>>
>>>>>>>     surf->bpe is the number of bytes used to store each pixel, right?
>>>> If
>>>>>> so,
>>>>>>>     this cannot exclude surf->bpe < 4, since 16 bpp and 8 bpp formats
>>>>>> can be
>>>>>>>     displayed.
>>>>>>>
>>>>>>>
>>>>>>> Sure, but what are the chances they will be displayed with the
>> current
>>>>>>> stack? GLX doesn't have 16bpp visuals for on-screen rendering.
>>>>>>
>>>>>> Maybe not when the X server runs at depth 24, but it can also run at
>>>>>> depths 8, 15 & 16, in which case displayable surfaces with bpe == 1
>> or 2
>>>>>> are needed even before GLX even comes into the picture.
>>>>>>
>>>>>
>>>>> OK. Let me ask differently. Do we wanna support displayable 8, 15, and
>> 16
>>>>> bpp?
>>>>
>>>> We do support it, it's not really a question of whether we want to
>>>> anymore. :)
>>>>
>>>>> Can we just say that we don't support those?
>>>>
>>>> I'm afraid we can't.
>>>>
>>>>
>>>> Which kind of surfaces are you trying to exclude like this? Maybe they
>>>> can be excluded in a different way.
>>>
>>> Currently just the MSAA resolve temporary destination buffer.
>>
>> Do those actually have surf->bpe < 4? Im not getting any hits with
>> glxgears -samples 8.
>>
> 
> The main purpose of the patch is to fix addrlib crashes on Vega when bpe ==
> 16. Everything else you see in the patch is just a bonus.

Then I'm confused what your last few posts in this thread were all about...


Anyway, with "surf->bpe >= 4" removed, this patch is

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer