[PATCH v3] drm/fourcc: document modifier uniqueness requirements

[Daniel Vetter]

On Fri, May 29, 2020 at 3:29 PM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Fri, May 29, 2020 at 4:59 AM Simon Ser <contact at emersion.fr> wrote:
> >
> > On Thursday, May 28, 2020 5:49 PM, Marek Olšák <maraeo at gmail.com> wrote:
> >
> > > On most hardware, there is a minimum pitch alignment for linear and
> > > any greater multiple of the alignment is fine.
> > >
> > > On Navi, the pitch in bytes for linear must be
> > > align(width * bpp / 8, 256). That's because the hw computes the pitch
> > > from the width and doesn't allow setting a custom pitch. For that
> > > reason, multi-GPU sharing might not be possible if the other GPU
> > > doesn't align the pitch in exactly the same way.
> >
> > OK. In this case I think it's fine to make the DMA-BUF import fail, as
> > we've suggested on IRC. The more-or-less planned fix for these buffer
> > sharing issues is to revive the buffer constraints proposal from the
> > allocator project. It's a lot of work though.
>
> I get that, but why explicitly limit modifiers then?  Shouldn't we try
> and do the best we can with what we have now?  If not the situation is
> not much better than what we have now.  Why go through the effort or
> adding modifer support in the first place if they are mostly useless?
> I don't quite get what we are trying to do with them.  What does this
> mean "Modifiers must uniquely encode buffer layout"?  We have a number
> of buffer layouts that are the same from a functional standpoint, but
> they have different alignment requirements depending on the chip and
> the number of memory channels, etc.  Would those be considered the
> same modifer?  If not, then we are sort of implicitly encoding
> alignment requirements into the modifier.

The risk is essentially that if you have these, and they still match,
then either you need to make sure every claims support for the full
set of modifiers, and it gets complicated. Or occasionally sharing
doesn't work when it should.

I'd say the more specific the format (extreme case is compression
format used by one vendor only), the more you can just bake in as
implicit assumptions and deal with the fallout. The stuff Simon patch
adds to the docs is also just general rules, there's always some
exceptions. Pretty much for every more complex set of modifiers we had
lengthy discussions to figure out what they should look like, and as
you can see from scrolling through drm_fourcc.h, there's a bunch of
fairly different approaches in there. One extreme is hw where you
enumerate 1-2 modifiers and done, not a single further constraint
beyond "must be aligned naturally to tiles", others are a _lot_ more
complicated. If the memory channels and stuff meaningfully impacts how
you can share buffers (and if that meaningfully changes across
different gpus, we don't want to encode everything, but only the stuff
that's actually in used and needed for sharing), then that's a good
reason to add them. But if that then results in no sharing possible
per modifiers, while the hw could do it, then that's not great either
(e.g. if you have integrated + discrete gpu in an optimus laptop, but
memory channels mismatch except for the buffer size you care about it
all lines up).
-Daniel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch