On 17/04/2020 17:07, Daniel Vetter wrote:
> On Thu, Apr 16, 2020 at 05:24:53PM +0200, Neil Armstrong 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.
>>
>> This introduces the basic layout composed of:
>> - a body content organized in 64x32 superblocks with 4096 bytes per
>> superblock in default mode.
>> - a 32 bytes per 128x64 header block
>>
>> This layout is tranferrable between Amlogic SoCs supporting this modifier.
>>
>> Tested-by: Kevin Hilman <khilman@xxxxxxxxxxxx>
>> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>> ---
>> include/uapi/drm/drm_fourcc.h | 39 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 8bc0b31597d8..a1b163a5641f 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,44 @@ 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.
>> + *
>> + * 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
>> + *
>> + * The first 8 bits of the mode defines the layout, then the following 8 bits
>> + * defines the options changing the layout.
>
> None of the modifiers you're doing seem to have these other 8 bits
> defined anywhere. And it's not encoded in your modifiers. Can't we just
> enumerate the ones we have/need and done?
It's introduced in patch 5
>
>> + *
>> + * Not all combinations are valid, and different SoCs may support different
>> + * combinations of layout and options.
>> + */
>> +#define DRM_FORMAT_MOD_AMLOGIC_FBC(__modes) fourcc_mod_code(AMLOGIC, __modes)
>> +
>> +/* Amlogic FBC Layouts */
>> +#define DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_MASK (0xf << 0)
>> +
>> +/*
>> + * Amlogic FBC Basic Layout
>> + *
>> + * The basic layout is composed of:
>> + * - a body content organized in 64x32 superblocks with 4096 bytes per
>> + * superblock in default mode.
>> + * - a 32 bytes per 128x64 header block
>> + *
>> + * This layout is transferrable between Amlogic SoCs supporting this modifier.
>> + */
>> +#define DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_BASIC (1ULL << 0)
>
> This is kinda confusing, since this isn't actually the modifier, but the
> mode of the modifer. Generally what we do is only define the former, with
> maybe some macros to extract stuff.
>
> To make this more mistake-proof I'd only define the full modifier code.
> Definitely don't add a #define with the DRM_FORMAT_MOD_ prefix which isn't
> actually a full modifier code.
Exact, I'll use the same scheme as AFBC: AMLOGIC_FBC_FORMAT_ ...
Neil
> -Daniel
>
>> +
>> #if defined(__cplusplus)
>> }
>> #endif
>> --
>> 2.22.0
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
