Re: [PATCH 1/4] drm/fourcc: Add modifier definitions for describing Amlogic Video Framebuffer Compression

[Pekka Paalanen]

On Tue, 3 Mar 2020 15:25:41 +0200
Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote:

> On Tue, 3 Mar 2020 12:37:16 +0100
> Daniel Vetter <daniel@xxxxxxxx> wrote:
> 
> > On Tue, Mar 3, 2020 at 11:53 AM Brian Starkey <brian.starkey@xxxxxxx> wrote:  
> > >
> > > Hi,
> > >
> > > On Tue, Mar 03, 2020 at 12:10:29PM +0200, Pekka Paalanen wrote:    
> > > > On Fri, 21 Feb 2020 10:08:42 +0100
> > > > Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:
> > > >    
...
> > > > > +/*
> > > > > + * Amlogic Video Framebuffer Compression modifiers
> > > > > + *
> > > > > + * Amlogic uses a proprietary lossless image compression protocol and format
> > > > > + * for their hardware video codec accelerators, either video decoders or
> > > > > + * video input encoders.
> > > > > + *
> > > > > + * It considerably reduces memory bandwidth while writing and reading
> > > > > + * frames in memory.
> > > > > + * Implementation details may be platform and SoC specific, and shared
> > > > > + * between the producer and the decoder on the same platform.    
> > > >
> > > > Hi,
> > > >
> > > > after a lengthy IRC discussion on #dri-devel, this "may be platform and
> > > > SoC specific" is a problem.
> > > >
> > > > It can be an issue in two ways:
> > > >
> > > > - If something in the data acts like a sub-modifier, then advertising
> > > >   support for one modifier does not really tell if the data layout is
> > > >   supported or not.
> > > >
> > > > - If you need to know the platform and/or SoC to be able to interpret
> > > >   the data, it means the modifier is ill-defined and cannot be used in
> > > >   inter-machine communication (e.g. Pipewire).
> > > >    
> > >
> > > Playing devil's advocate, the comment sounds similar to
> > > I915_FORMAT_MOD_{X,Y}_TILED:
> > >
> > >  * This format is highly platforms specific and not useful for cross-driver
> > >  * sharing. It exists since on a given platform it does uniquely identify the
> > >  * layout in a simple way for i915-specific userspace.    
> > 
> > Yeah which we regret now. We need to now roll out a new set of
> > modifiers for at least some of the differences in these on the
> > modern-ish chips (the old crap is pretty much lost cause anyway).
> > 
> > This was kinda a nasty hack to smooth things over since we have epic
> > amounts of userspace, but it's really not a great idea (and no one
> > else really has epic amounts of existing userspace that uses tiling
> > flags everywhere, this is all new code).
> > -Daniel
> >   
> > > Isn't the statement that this for sharing between producer and decoder
> > > _on the same platform_ a similar clause with the same effect?
> > >
> > > What advantage is there to exposing the gory details? For Arm AFBC
> > > it's necessary because IP on the SoC can be (likely to be) from
> > > different vendors with different capabilities.
> > >
> > > If this is only for talking between Amlogic IP on the same SoC, and
> > > those devices support all the same "flavours", I don't see what is
> > > gained by making userspace care about internals.    
> > 
> > The trouble is if you mix&match IP cores, and one of them supports
> > flavours A, B, C and the other C, D, E. But all you have is a single
> > magic modifier for "whatever the flavour is that soc prefers". So
> > someone gets to stuff this in DT.
> > 
> > Also eventually, maybe, perhaps ARM does grow up into the
> > client/server space with add-on pcie graphics, and at least for client
> > you very often end up with integrated + add-in pcie gpu. At that point
> > you really can't have magic per-soc modifiers anymore.  
> 
> Hi,
> 
> I also heard that Pipewire will copy buffers and modifiers verbatim
> from one machine to another when streaming across network, assuming
> that the same modifier means the same thing on all machines.[Citation needed]
> 
> If that is something that must not be done with DRM modifiers, then
> please contact them and document that.

Sorry, it's waypipe, not pipewire:
https://gitlab.freedesktop.org/mstoeckl/waypipe/


Thanks,
pq

Attachment: pgp32kCEnA43D.pgp
Description: OpenPGP digital signature