[Mesa-dev] [PATCH 3/3] panfrost: Add support for modifiers

[Daniel Stone]

Hi,

On Fri, 5 Jul 2019 at 14:51, Alyssa Rosenzweig
<alyssa.rosenzweig at collabora.com> wrote:
> > +     switch(whandle->modifier) {
> > +     case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP):
> > +             rsc->layout = PAN_AFBC;
> > +             break;
>
> Why ROCKCHIP in particular? AFBC fourcc codes are based on modifiers,
> and there are a *lot* of them. We need to figure out which upstream
> modifier PAN_AFBC actually enables and use that:
>
> [...]
>
> On other AFBC flags, I don't know. I'm also not sure if Arm would chime
> in one way or the other.
>
> Mainly the issue is that whandle->modifier could contain some other,
> non-Rockchip modifier, and then the whole stack collapses; AFBC
> modifiers are meant to be parsedd, not =='d. See the komeda driver in
> the kernel for reference.

Yes and no. See below.

> > +     if (want_afbc || (is_renderable && can_afbc && !is_texture))
>
> This doesn't seem quite correct. We preselect AFBC if:
>
>         - AFBC is explicitly requested, OR
>         - We're rendering to something that's not bound as a texture
>           that could be AFBC
>
> In case one, what happens if AFBC is explicitly requested but we don't
> support AFBC on that format? It should fallback to linear, or error out.
>
> In case two, what if the modifier for linear or tiled is explicitly set?
> Now we've given them AFBC when they've asked for something else;
> switching it up is only safe for internal BOs.
>
> The logic in this code block has only ever been for internal BOs, where
> it's up to *us* to pick a format. Imports are logically distinct, since
> it's now up to the modifiers. Two different problems, unless the
> modifier is a "don't care".

It's still up to you-the-driver to pick a modifier.

EGL/Wayland/KMS all advertise sets of modifiers to clients. In
general, clients don't try to reason about the contents of those lists
- if they have multiple lists, they might intersect them, or they
might create their own list-of-one to be LINEAR, or whatever.

Modifier lists are explicitly unsorted. The list provided is the
modifiers which are acceptable to use. The driver is expected to sort
(on whatever metric) for 'optimality'. If none of the modifiers
provided are usable, then resource creation is free to fail. If the
provided modifiers are not optimal, then the driver just has to select
that less-optimal modifier. The only no-no is to select a modifier
which is not in the provided list.

Cheers,
Daniel