On Tue, Mar 03, 2020 at 12:37:16PM +0100, Daniel Vetter wrote:
On Tue, Mar 3, 2020 at 11:53 AM Brian Starkey brian.starkey@arm.com 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@baylibre.com wrote:
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.
The underlying storage is considered to be 3 components, 8bit or 10-bit per component, YCbCr 420, single plane :
- DRM_FORMAT_YUV420_8BIT
- DRM_FORMAT_YUV420_10BIT
This modifier will be notably added to DMA-BUF frames imported from the V4L2 Amlogic VDEC decoder.
At least two options are supported :
- Scatter mode: the buffer is filled with a IOMMU scatter table referring to the encoder current memory layout. This mode if more efficient in terms of memory allocation but frames are not dumpable and only valid during until the buffer is freed and back in control of the encoder
- Memory saving: when the pixel bpp is 8b, the size of the superblock can be reduced, thus saving memory.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
include/uapi/drm/drm_fourcc.h | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 8bc0b31597d8..8a6e87bacadb 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -309,6 +309,7 @@ extern "C" { #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07 #define DRM_FORMAT_MOD_VENDOR_ARM 0x08 #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 +#define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
/* add more to the end as needed */
@@ -804,6 +805,61 @@ extern "C" { */ #define DRM_FORMAT_MOD_ALLWINNER_TILED fourcc_mod_code(ALLWINNER, 1)
+/*
- 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.
Yes, if incompatible support levels are possible, then they must be described, no disagreement there. That's why AFBC is so explicit.
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.
I don't entirely agree. This is only relevant for modifiers which might be used between the PCIe GPU and the SoC (in your example). Per-SoC modifiers still work, they just lose meaning at the SoC boundary.
Looking at the description of DRM_FORMAT_MOD_AMLOGIC_FBC_SCATTER in particular, it sounds like that would never be shareable even if it had a more "complete" modifier.
If people get confused I'm happy to add a "WARNING: This was a dumb idea for backwards compat with legacy code, no one with new stuff ever repeat it" to the i915 modifers. -Daniel
I think marking it as non-preferred (and why) would be a good idea, so as not to use it as an example.
Cheers, -Brian
Thanks, -Brian
Neil mentioned the data contains a "header" that further specifies things, but there is no specification about the header itself. Therefore I don't think we can even know if the header contains something that acts like a sub-modifier or not.
All this sounds like the modifier definitions here are not enough to fully interpret the data. At the very least I would expect a reference to a document explaining the "header", or even better, a kernel ReST doc.
I wonder if this is at all suitable as a DRM format modifier as is. I have been assuming that a modifier together with all the usual FB parameters should be enough to interpret the stored data, but in this case I have doubt it actually is.
I have no problem with proprietary data layouts as long as they are fully specified.
I do feel like I would not be able to write a software decoder for this set of modifiers given the details below.
Thanks, pq
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch