diff mbox series

[v5,1/8] drm/fourcc: Add modifier definitions for describing Amlogic Video Framebuffer Compression

Message ID 20200416152500.29429-2-narmstrong@baylibre.com (mailing list archive)
State Superseded
Headers show
Series drm/meson: add support for Amlogic Video FBC | expand

Commit Message

Neil Armstrong April 16, 2020, 3:24 p.m. UTC 
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@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 include/uapi/drm/drm_fourcc.h | 39 +++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Daniel Vetter April 17, 2020, 3:07 p.m. UTC | #1 
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@baylibre.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  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?

> + *
> + * 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.
-Daniel

> +
>  #if defined(__cplusplus)
>  }
>  #endif
> -- 
> 2.22.0
>
Neil Armstrong April 17, 2020, 4:05 p.m. UTC | #2 
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@baylibre.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  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
>>
>
Neil Armstrong April 17, 2020, 4:11 p.m. UTC | #3 
On 17/04/2020 18:05, Neil Armstrong wrote:
> 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@baylibre.com>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  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

I did slit the options/layout for the last one: SCATTER so I could apply the BASIC and the option
first then continue the discussion on the second SCATTER layout.

So maybe I should add the option in the first patch.

Neil
Daniel Vetter April 17, 2020, 6:14 p.m. UTC | #4 
On Fri, Apr 17, 2020 at 6:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> 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@baylibre.com>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  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

Hm must have been blind, I overlooked the << 8 shift. I'd just do a
macro which encoders all fields into the modifier, instead of
hand-rolling this.

> >
> >> + *
> >> + * 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_ ...

Yup there's a number of parametried modifiers. As long as the stuff
you get from a DRM_FORMAT_MOD_ ... macro is a full modifier with
everything it should be all fine.
-Daniel

> Neil
>
> > -Daniel
> >
> >> +
> >>  #if defined(__cplusplus)
> >>  }
> >>  #endif
> >> --
> >> 2.22.0
> >>
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Neil Armstrong April 20, 2020, 10:21 a.m. UTC | #5 
On 17/04/2020 20:14, Daniel Vetter wrote:
> On Fri, Apr 17, 2020 at 6:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 17/04/2020 17:07, Daniel Vetter wrote:

[...]

> 
> Yup there's a number of parametried modifiers. As long as the stuff
> you get from a DRM_FORMAT_MOD_ ... macro is a full modifier with
> everything it should be all fine.

Would something like that be ok ?

#define __fourcc_mod_amlogic_layout_mask 0xf
#define __fourcc_mod_amlogic_options_shift 8
#define __fourcc_mod_amlogic_options_mask 0xf

#define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
	fourcc_mod_code(AMLOGIC, \
			((__layout) & __fourcc_mod_amlogic_layout_mask) | \
			((options) & __fourcc_mod_amlogic_options_mask \
			 << __fourcc_mod_amlogic_options_shift))

/* Amlogic FBC Layouts */

/* bla */
#define AMLOGIC_FBC_LAYOUT_BASIC		(1ULL)

/* bla */
#define AMLOGIC_FBC_LAYOUT_SCATTER		(2ULL)

/* Amlogic FBC Layout Options Bit Mask */

/* bla */
#define AMLOGIC_FBC_OPTION_MEM_SAVING		(1ULL << 0)

Neil

> -Daniel
> 
>> Neil
>>
>>> -Daniel
>>>
>>>> +
>>>>  #if defined(__cplusplus)
>>>>  }
>>>>  #endif
>>>> --
>>>> 2.22.0
>>>>
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
>
Daniel Vetter April 21, 2020, 12:16 p.m. UTC | #6 
On Mon, Apr 20, 2020 at 12:21:24PM +0200, Neil Armstrong wrote:
> On 17/04/2020 20:14, Daniel Vetter wrote:
> > On Fri, Apr 17, 2020 at 6:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> On 17/04/2020 17:07, Daniel Vetter wrote:
> 
> [...]
> 
> > 
> > Yup there's a number of parametried modifiers. As long as the stuff
> > you get from a DRM_FORMAT_MOD_ ... macro is a full modifier with
> > everything it should be all fine.
> 
> Would something like that be ok ?
> 
> #define __fourcc_mod_amlogic_layout_mask 0xf
> #define __fourcc_mod_amlogic_options_shift 8
> #define __fourcc_mod_amlogic_options_mask 0xf
> 
> #define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
> 	fourcc_mod_code(AMLOGIC, \
> 			((__layout) & __fourcc_mod_amlogic_layout_mask) | \
> 			((options) & __fourcc_mod_amlogic_options_mask \
> 			 << __fourcc_mod_amlogic_options_shift))
> 
> /* Amlogic FBC Layouts */
> 
> /* bla */
> #define AMLOGIC_FBC_LAYOUT_BASIC		(1ULL)
> 
> /* bla */
> #define AMLOGIC_FBC_LAYOUT_SCATTER		(2ULL)
> 
> /* Amlogic FBC Layout Options Bit Mask */
> 
> /* bla */
> #define AMLOGIC_FBC_OPTION_MEM_SAVING		(1ULL << 0)

lgtm.
-Daniel

> 
> Neil
> 
> > -Daniel
> > 
> >> Neil
> >>
> >>> -Daniel
> >>>
> >>>> +
> >>>>  #if defined(__cplusplus)
> >>>>  }
> >>>>  #endif
> >>>> --
> >>>> 2.22.0
> >>>>
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > 
> > 
>
Neil Armstrong April 21, 2020, 1:14 p.m. UTC | #7 
On 21/04/2020 14:16, Daniel Vetter wrote:
> On Mon, Apr 20, 2020 at 12:21:24PM +0200, Neil Armstrong wrote:
>> On 17/04/2020 20:14, Daniel Vetter wrote:
>>> On Fri, Apr 17, 2020 at 6:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>>
>>>> On 17/04/2020 17:07, Daniel Vetter wrote:
>>
>> [...]
>>
>>>
>>> Yup there's a number of parametried modifiers. As long as the stuff
>>> you get from a DRM_FORMAT_MOD_ ... macro is a full modifier with
>>> everything it should be all fine.
>>
>> Would something like that be ok ?
>>
>> #define __fourcc_mod_amlogic_layout_mask 0xf
>> #define __fourcc_mod_amlogic_options_shift 8
>> #define __fourcc_mod_amlogic_options_mask 0xf
>>
>> #define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
>> 	fourcc_mod_code(AMLOGIC, \
>> 			((__layout) & __fourcc_mod_amlogic_layout_mask) | \
>> 			((options) & __fourcc_mod_amlogic_options_mask \
>> 			 << __fourcc_mod_amlogic_options_shift))
>>
>> /* Amlogic FBC Layouts */
>>
>> /* bla */
>> #define AMLOGIC_FBC_LAYOUT_BASIC		(1ULL)
>>
>> /* bla */
>> #define AMLOGIC_FBC_LAYOUT_SCATTER		(2ULL)
>>
>> /* Amlogic FBC Layout Options Bit Mask */
>>
>> /* bla */
>> #define AMLOGIC_FBC_OPTION_MEM_SAVING		(1ULL << 0)
> 
> lgtm.
> -Daniel

Thanks,

Re-sending with this.

Neil

> 
>>
>> Neil
>>
>>> -Daniel
>>>
>>>> Neil
>>>>
>>>>> -Daniel
>>>>>
>>>>>> +
>>>>>>  #if defined(__cplusplus)
>>>>>>  }
>>>>>>  #endif
>>>>>> --
>>>>>> 2.22.0
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
>
diff mbox series

Patch

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.
+ *
+ * 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)
+
 #if defined(__cplusplus)
 }
 #endif