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

[Michel Dänzer]

On 2018-04-05 11:04 PM, Marek Olšák wrote:
> On Thu, Apr 5, 2018 at 4:47 PM, Marek Olšák <maraeo at gmail.com> wrote:
> 
>> On Thu, Apr 5, 2018 at 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.
>>>
>>
>> Can we drop support for those formats? i.e. if somebody uses them, the
>> person will get garbage.

Sounds pretty user-unfriendly.


>> How likely is it that somebody will use bpp <= 16 for display nowadays?
>>
> 
> Or put differently: How likely is it that somebody will notice it on
> supported distros and supported window managers?

It's not related to that but to configuration. You can try it yourself
with Xorg -depth 16/15/8.


Can you answer my question, instead of asking more yourself?


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