[PATCH 0/5] media: meson: vdec: Add support for compressed framebuffer

[PATCH 0/5] media: meson: vdec: Add support for compressed framebuffer

[Neil Armstrong]

This patchset add support for compressed framebuffer while decoding VP9
8bit and 10bit streams.

First, it adds two generic Compressed Framebuffer pixel formats to be used
with a modifier when imported back in another subsystem like DRM/KMS.

These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
used to describe the underlying compressed buffers used for ARM Framebuffer
Compression. In the Amlogic case, the compression is different but the
underlying buffer components is the same.

Then, in order to handle Compressed Framebuffer support, we need to handle
the switch between 8bit and 10bit frame output.
Add the necessary changes to decode VP9 8bit and 10bit streams into
compressed buffers to be imported back into DRM/KMS using a modifier.

Finally, add the necessary to add support for negociating the compressed buffer
pixel format with the V4L2 M2M consumer, and allocating the right
buffers in this case.

Until a proper mechanism exists to pass a modifier along the pixel format,
only the generic V4L2_PIX_FMT_YUV420_8BIT and V4L2_PIX_FMT_YUV420_10BIT
format are passed in v4l2_pix_format_mplane struct for consumer.

Maxime Jourdan (5):
  media: videodev2: add Compressed Framebuffer pixel formats
  media: meson: vdec: handle bitdepth on source change
  media: meson: vdec: update compressed buffer helpers
  media: meson: vdec: add support for compressed output for VP9 decoder
  media: meson: vdec: handle compressed output pixel format negociation
    with consumer

 drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
 drivers/staging/media/meson/vdec/codec_h264.c |   3 +-
 .../media/meson/vdec/codec_hevc_common.c      | 133 +++++++-----------
 .../media/meson/vdec/codec_hevc_common.h      |  13 +-
 drivers/staging/media/meson/vdec/codec_vp9.c  |  29 ++--
 drivers/staging/media/meson/vdec/vdec.c       |  46 ++++++
 drivers/staging/media/meson/vdec/vdec.h       |   4 +
 .../staging/media/meson/vdec/vdec_helpers.c   |  68 +++++++--
 .../staging/media/meson/vdec/vdec_helpers.h   |  11 +-
 .../staging/media/meson/vdec/vdec_platform.c  |   9 +-
 include/uapi/linux/videodev2.h                |   9 ++
 11 files changed, 203 insertions(+), 124 deletions(-)

-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Neil Armstrong]

From: Maxime Jourdan <mjourdan@baylibre.com>

Add two generic Compressed Framebuffer pixel formats to be used
with a modifier when imported back in another subsystem like DRM/KMS.

These pixel formats represents generic 8bits and 10bits compressed buffers
with a vendor specific layout.

These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
used to describe the underlying compressed buffers used for ARM Framebuffer
Compression. In the Amlogic case, the compression is different but the
underlying buffer components is the same.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
 include/uapi/linux/videodev2.h       | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 2322f08a98be..8f14adfd5bc5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
 		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
 		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
+		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
+		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
 		default:
 			if (fmt->description[0])
 				return;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c3a1cf1c507f..90b9949acb8a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -705,6 +705,15 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
 #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
 
+/*
+ * Compressed Luminance+Chrominance meta-formats
+ * In these formats, the component ordering is specified (Y, followed by U
+ * then V), but the exact Linear layout is undefined.
+ * These formats can only be used with a non-Linear modifier.
+ */
+#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
+#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
+
 /*  Vendor-specific formats   */
 #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
 #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Nicolas Dufresne]

Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
> From: Maxime Jourdan <mjourdan@baylibre.com>
> 
> Add two generic Compressed Framebuffer pixel formats to be used
> with a modifier when imported back in another subsystem like DRM/KMS.
> 
> These pixel formats represents generic 8bits and 10bits compressed buffers
> with a vendor specific layout.
> 
> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
> used to describe the underlying compressed buffers used for ARM Framebuffer
> Compression. In the Amlogic case, the compression is different but the
> underlying buffer components is the same.
> 
> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>  include/uapi/linux/videodev2.h       | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 2322f08a98be..8f14adfd5bc5 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;

When I read the DRM documentation [0], I'm reading that YUV420_8BIT
definition matches V4L2_PIX_FMT_YVU420 and V4L2_PIX_FMT_YVU420M fully.
In fact, on DRM side, to represent that format you want to expose here,
they will strictly combine this generic format (documented un-
compressed) with a modifier generated with the macro
DRM_FORMAT_MOD_ARM_AFBC(*). And only the combination represent a unique
and share-able format.

In absence of modifier in V4L2 API, this compressed format should be
named accordingly to the compressed algorithm used (something like
FMT_YUV420_8BIT_AML_FBC). So I believe these format name cannot be
copied as-is like this, as they can only introduce more ambiguity in
the already quite hard to follow naming of pixel formats. In fact, it
is very common to see multiple different vendor compressions on the
same SoC, so I don't really believe a "generic" compressed format make
sense. To site one, the IMX8M, which got Verrisillicon/Vivante DEC400
on the GPU, and the Hantro G2 compression format. Both will apply to
NV12 class of format so in DRM they would be NV12 + modifier, and the
combination forms the unique format. Now, in term of sharing, they must
be differiented by userspace, as support for compression/decompression
is heterogeneous (in that case the GPU does not support Hantro G2
decompression or compression, and the VPU does not support DEC400).

I'll remind that the modifier implementation has great value and is
much more scalable then the current V4L2 approach. There has been some
early proposal for this, maybe it's time to prioritize because this
list will starts growing with hundred or even thousands or format,
which is clearly indicated by the increase of modifier generator macro
on the DRM side.

>  		default:
>  			if (fmt->description[0])
>  				return;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index c3a1cf1c507f..90b9949acb8a 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>  
> +/*
> + * Compressed Luminance+Chrominance meta-formats
> + * In these formats, the component ordering is specified (Y, followed by U
> + * then V), but the exact Linear layout is undefined.
> + * These formats can only be used with a non-Linear modifier.
> + */
> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
> +
>  /*  Vendor-specific formats   */
>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Nicolas Dufresne]

Le vendredi 05 juin 2020 à 11:35 -0400, Nicolas Dufresne a écrit :
> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
> > From: Maxime Jourdan <mjourdan@baylibre.com>
> > 
> > Add two generic Compressed Framebuffer pixel formats to be used
> > with a modifier when imported back in another subsystem like DRM/KMS.
> > 
> > These pixel formats represents generic 8bits and 10bits compressed buffers
> > with a vendor specific layout.
> > 
> > These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
> > used to describe the underlying compressed buffers used for ARM Framebuffer
> > Compression. In the Amlogic case, the compression is different but the
> > underlying buffer components is the same.
> > 
> > Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> >  include/uapi/linux/videodev2.h       | 9 +++++++++
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 2322f08a98be..8f14adfd5bc5 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> >  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
> >  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
> >  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
> > +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
> > +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> 
> When I read the DRM documentation [0], I'm reading that YUV420_8BIT
> definition matches V4L2_PIX_FMT_YVU420 and V4L2_PIX_FMT_YVU420M fully.
> In fact, on DRM side, to represent that format you want to expose here,
> they will strictly combine this generic format (documented un-
> compressed) with a modifier generated with the macro
> DRM_FORMAT_MOD_ARM_AFBC(*). And only the combination represent a unique
> and share-able format.

[0] https://www.kernel.org/doc/html/v5.5-rc1/gpu/afbc.html

> 
> In absence of modifier in V4L2 API, this compressed format should be
> named accordingly to the compressed algorithm used (something like
> FMT_YUV420_8BIT_AML_FBC). So I believe these format name cannot be
> copied as-is like this, as they can only introduce more ambiguity in
> the already quite hard to follow naming of pixel formats. In fact, it
> is very common to see multiple different vendor compressions on the
> same SoC, so I don't really believe a "generic" compressed format make
> sense. To site one, the IMX8M, which got Verrisillicon/Vivante DEC400
> on the GPU, and the Hantro G2 compression format. Both will apply to
> NV12 class of format so in DRM they would be NV12 + modifier, and the
> combination forms the unique format. Now, in term of sharing, they must
> be differiented by userspace, as support for compression/decompression
> is heterogeneous (in that case the GPU does not support Hantro G2
> decompression or compression, and the VPU does not support DEC400).
> 
> I'll remind that the modifier implementation has great value and is
> much more scalable then the current V4L2 approach. There has been some
> early proposal for this, maybe it's time to prioritize because this
> list will starts growing with hundred or even thousands or format,
> which is clearly indicated by the increase of modifier generator macro
> on the DRM side.
> 
> >  		default:
> >  			if (fmt->description[0])
> >  				return;
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index c3a1cf1c507f..90b9949acb8a 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -705,6 +705,15 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
> >  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
> >  
> > +/*
> > + * Compressed Luminance+Chrominance meta-formats
> > + * In these formats, the component ordering is specified (Y, followed by U
> > + * then V), but the exact Linear layout is undefined.
> > + * These formats can only be used with a non-Linear modifier.
> > + */
> > +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
> > +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
> > +
> >  /*  Vendor-specific formats   */
> >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> >  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Neil Armstrong]

Hi Nicolas,

On 05/06/2020 17:35, Nicolas Dufresne wrote:
> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>
>> Add two generic Compressed Framebuffer pixel formats to be used
>> with a modifier when imported back in another subsystem like DRM/KMS.
>>
>> These pixel formats represents generic 8bits and 10bits compressed buffers
>> with a vendor specific layout.
>>
>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>> used to describe the underlying compressed buffers used for ARM Framebuffer
>> Compression. In the Amlogic case, the compression is different but the
>> underlying buffer components is the same.
>>
>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 2322f08a98be..8f14adfd5bc5 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;

Seems we are totally on-par with the following :-)

> 
> When I read the DRM documentation [0], I'm reading that YUV420_8BIT
> definition matches V4L2_PIX_FMT_YVU420 and V4L2_PIX_FMT_YVU420M fully.
> In fact, on DRM side, to represent that format you want to expose here,
> they will strictly combine this generic format (documented un-
> compressed) with a modifier generated with the macro
> DRM_FORMAT_MOD_ARM_AFBC(*). And only the combination represent a unique
> and share-able format.

Yes, and this is exactly my goal here, and matches the Amlogic Framebuffer as
described in patch 4. The modifier patchset is at [1].

> 
> In absence of modifier in V4L2 API, this compressed format should be
> named accordingly to the compressed algorithm used (something like
> FMT_YUV420_8BIT_AML_FBC). 

It's even more complex, the modifier depends on the SoC revision, so we can
have up to6 different unique pixel format instead of 2 with a variable
modifier.

> So I believe these format name cannot be
> copied as-is like this, as they can only introduce more ambiguity in
> the already quite hard to follow naming of pixel formats. In fact, it
> is very common to see multiple different vendor compressions on the
> same SoC, so I don't really believe a "generic" compressed format make
> sense. To site one, the IMX8M, which got Verrisillicon/Vivante DEC400
> on the GPU, and the Hantro G2 compression format. Both will apply to
> NV12 class of format so in DRM they would be NV12 + modifier, and the
> combination forms the unique format. Now, in term of sharing, they must
> be differiented by userspace, as support for compression/decompression
> is heterogeneous (in that case the GPU does not support Hantro G2
> decompression or compression, and the VPU does not support DEC400).
> 
> I'll remind that the modifier implementation has great value and is
> much more scalable then the current V4L2 approach. There has been some
> early proposal for this, maybe it's time to prioritize because this
> list will starts growing with hundred or even thousands or format,
> which is clearly indicated by the increase of modifier generator macro
> on the DRM side.

Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
is decided, I'm stuck and this is the only intermediate solution I found.

We have a working solution with Boris's patchset with ext_fmt passing the
modifier to user-space.

but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
will still be needed if we pass the modifier with an extended format struct.

> 
>>  		default:
>>  			if (fmt->description[0])
>>  				return;
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index c3a1cf1c507f..90b9949acb8a 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>  
>> +/*
>> + * Compressed Luminance+Chrominance meta-formats
>> + * In these formats, the component ordering is specified (Y, followed by U
>> + * then V), but the exact Linear layout is undefined.
>> + * These formats can only be used with a non-Linear modifier.
>> + */
>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>> +
>>  /*  Vendor-specific formats   */
>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
> 

[1] https://patchwork.freedesktop.org/series/73722/#rev7

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Hans Verkuil]

On 08/06/2020 10:16, Neil Armstrong wrote:
> Hi Nicolas,
> 
> On 05/06/2020 17:35, Nicolas Dufresne wrote:
>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>>
>>> Add two generic Compressed Framebuffer pixel formats to be used
>>> with a modifier when imported back in another subsystem like DRM/KMS.
>>>
>>> These pixel formats represents generic 8bits and 10bits compressed buffers
>>> with a vendor specific layout.
>>>
>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>>> used to describe the underlying compressed buffers used for ARM Framebuffer
>>> Compression. In the Amlogic case, the compression is different but the
>>> underlying buffer components is the same.
>>>
>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> index 2322f08a98be..8f14adfd5bc5 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> 
> Seems we are totally on-par with the following :-)
> 
>>
>> When I read the DRM documentation [0], I'm reading that YUV420_8BIT
>> definition matches V4L2_PIX_FMT_YVU420 and V4L2_PIX_FMT_YVU420M fully.
>> In fact, on DRM side, to represent that format you want to expose here,
>> they will strictly combine this generic format (documented un-
>> compressed) with a modifier generated with the macro
>> DRM_FORMAT_MOD_ARM_AFBC(*). And only the combination represent a unique
>> and share-able format.
> 
> Yes, and this is exactly my goal here, and matches the Amlogic Framebuffer as
> described in patch 4. The modifier patchset is at [1].
> 
>>
>> In absence of modifier in V4L2 API, this compressed format should be
>> named accordingly to the compressed algorithm used (something like
>> FMT_YUV420_8BIT_AML_FBC). 
> 
> It's even more complex, the modifier depends on the SoC revision, so we can
> have up to6 different unique pixel format instead of 2 with a variable
> modifier.
> 
>> So I believe these format name cannot be
>> copied as-is like this, as they can only introduce more ambiguity in
>> the already quite hard to follow naming of pixel formats. In fact, it
>> is very common to see multiple different vendor compressions on the
>> same SoC, so I don't really believe a "generic" compressed format make
>> sense. To site one, the IMX8M, which got Verrisillicon/Vivante DEC400
>> on the GPU, and the Hantro G2 compression format. Both will apply to
>> NV12 class of format so in DRM they would be NV12 + modifier, and the
>> combination forms the unique format. Now, in term of sharing, they must
>> be differiented by userspace, as support for compression/decompression
>> is heterogeneous (in that case the GPU does not support Hantro G2
>> decompression or compression, and the VPU does not support DEC400).
>>
>> I'll remind that the modifier implementation has great value and is
>> much more scalable then the current V4L2 approach. There has been some
>> early proposal for this, maybe it's time to prioritize because this
>> list will starts growing with hundred or even thousands or format,
>> which is clearly indicated by the increase of modifier generator macro
>> on the DRM side.
> 
> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
> is decided, I'm stuck and this is the only intermediate solution I found.

We can safely assume that drm fourcc and v4l2 fourcc won't be merged.

There is too much divergence and not enough interest in creating common
fourccs.

But we *do* want to share the modifiers.

> 
> We have a working solution with Boris's patchset with ext_fmt passing the
> modifier to user-space.
> 
> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
> will still be needed if we pass the modifier with an extended format struct.

We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
bother with this, it is much easier to just create a conversion table in the
kernel docs.

So don't block on this, I would really prefer if the ext_fmt series is picked
up again and rebased and reposted and then worked on. The stateless codec support
is taking less time (it's shaping up well) so there is more time to work on this.

I believe we really need this since v4l2_buffer and v4l2_format are a real mess.

Regards,

	Hans

> 
>>
>>>  		default:
>>>  			if (fmt->description[0])
>>>  				return;
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index c3a1cf1c507f..90b9949acb8a 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>>  
>>> +/*
>>> + * Compressed Luminance+Chrominance meta-formats
>>> + * In these formats, the component ordering is specified (Y, followed by U
>>> + * then V), but the exact Linear layout is undefined.
>>> + * These formats can only be used with a non-Linear modifier.
>>> + */
>>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>>> +
>>>  /*  Vendor-specific formats   */
>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
>>
> 
> [1] https://patchwork.freedesktop.org/series/73722/#rev7
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Neil Armstrong]

On 08/06/2020 11:26, Hans Verkuil wrote:
> On 08/06/2020 10:16, Neil Armstrong wrote:
>> Hi Nicolas,
>>
>> On 05/06/2020 17:35, Nicolas Dufresne wrote:
>>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>>>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>>>
>>>> Add two generic Compressed Framebuffer pixel formats to be used
>>>> with a modifier when imported back in another subsystem like DRM/KMS.
>>>>
>>>> These pixel formats represents generic 8bits and 10bits compressed buffers
>>>> with a vendor specific layout.
>>>>
>>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>>>> used to describe the underlying compressed buffers used for ARM Framebuffer
>>>> Compression. In the Amlogic case, the compression is different but the
>>>> underlying buffer components is the same.
>>>>
>>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>>>  2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> index 2322f08a98be..8f14adfd5bc5 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>>>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>>>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;

[..]

>>>
>>> I'll remind that the modifier implementation has great value and is
>>> much more scalable then the current V4L2 approach. There has been some
>>> early proposal for this, maybe it's time to prioritize because this
>>> list will starts growing with hundred or even thousands or format,
>>> which is clearly indicated by the increase of modifier generator macro
>>> on the DRM side.
>>
>> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
>> is decided, I'm stuck and this is the only intermediate solution I found.
> 
> We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
> 
> There is too much divergence and not enough interest in creating common
> fourccs.
> 
> But we *do* want to share the modifiers.
> 
>>
>> We have a working solution with Boris's patchset with ext_fmt passing the
>> modifier to user-space.
>>
>> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
>> will still be needed if we pass the modifier with an extended format struct.
> 
> We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
> bother with this, it is much easier to just create a conversion table in the
> kernel docs.
> 
> So don't block on this, I would really prefer if the ext_fmt series is picked
> up again and rebased and reposted and then worked on. The stateless codec support
> is taking less time (it's shaping up well) so there is more time to work on this.

Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.

Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
patch along the new ext_fmt and shared modifiers ?

Neil

> 
> I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
> 
> Regards,
> 
> 	Hans
> 
>>
>>>
>>>>  		default:
>>>>  			if (fmt->description[0])
>>>>  				return;
>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>> index c3a1cf1c507f..90b9949acb8a 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>>>  
>>>> +/*
>>>> + * Compressed Luminance+Chrominance meta-formats
>>>> + * In these formats, the component ordering is specified (Y, followed by U
>>>> + * then V), but the exact Linear layout is undefined.
>>>> + * These formats can only be used with a non-Linear modifier.
>>>> + */
>>>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>>>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>>>> +
>>>>  /*  Vendor-specific formats   */
>>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
>>>
>>
>> [1] https://patchwork.freedesktop.org/series/73722/#rev7
>>
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Hans Verkuil]

On 08/06/2020 16:14, Neil Armstrong wrote:
> On 08/06/2020 11:26, Hans Verkuil wrote:
>> On 08/06/2020 10:16, Neil Armstrong wrote:
>>> Hi Nicolas,
>>>
>>> On 05/06/2020 17:35, Nicolas Dufresne wrote:
>>>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>>>>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>>>>
>>>>> Add two generic Compressed Framebuffer pixel formats to be used
>>>>> with a modifier when imported back in another subsystem like DRM/KMS.
>>>>>
>>>>> These pixel formats represents generic 8bits and 10bits compressed buffers
>>>>> with a vendor specific layout.
>>>>>
>>>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>>>>> used to describe the underlying compressed buffers used for ARM Framebuffer
>>>>> Compression. In the Amlogic case, the compression is different but the
>>>>> underlying buffer components is the same.
>>>>>
>>>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>> ---
>>>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>>>>  2 files changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> index 2322f08a98be..8f14adfd5bc5 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>>>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>>>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>>>>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>>>>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> 
> [..]
> 
>>>>
>>>> I'll remind that the modifier implementation has great value and is
>>>> much more scalable then the current V4L2 approach. There has been some
>>>> early proposal for this, maybe it's time to prioritize because this
>>>> list will starts growing with hundred or even thousands or format,
>>>> which is clearly indicated by the increase of modifier generator macro
>>>> on the DRM side.
>>>
>>> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
>>> is decided, I'm stuck and this is the only intermediate solution I found.
>>
>> We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
>>
>> There is too much divergence and not enough interest in creating common
>> fourccs.
>>
>> But we *do* want to share the modifiers.
>>
>>>
>>> We have a working solution with Boris's patchset with ext_fmt passing the
>>> modifier to user-space.
>>>
>>> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
>>> will still be needed if we pass the modifier with an extended format struct.
>>
>> We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
>> bother with this, it is much easier to just create a conversion table in the
>> kernel docs.
>>
>> So don't block on this, I would really prefer if the ext_fmt series is picked
>> up again and rebased and reposted and then worked on. The stateless codec support
>> is taking less time (it's shaping up well) so there is more time to work on this.
> 
> Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.
> 
> Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
> patch along the new ext_fmt and shared modifiers ?

So to be clear the DRM_FORMAT_YUV420_8BIT/10BIT fourccs define that this is a
buffer containing compressed YUV420 in 8 or 10 bit and the modifier tells userspace
which compression is used, right?

And we would add V4L2_PIX_FMT_YUV420_8BIT/_10BIT that, I assume, use the same
fourcc values as the DRM variants?

Since these fourccs are basically useless without V4L2 modifier support it would
only make sense in combination with the ext_fmt series.

Regards,

	Hans

> 
> Neil
> 
>>
>> I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>>>
>>>>>  		default:
>>>>>  			if (fmt->description[0])
>>>>>  				return;
>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>> index c3a1cf1c507f..90b9949acb8a 100644
>>>>> --- a/include/uapi/linux/videodev2.h
>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>>>>  
>>>>> +/*
>>>>> + * Compressed Luminance+Chrominance meta-formats
>>>>> + * In these formats, the component ordering is specified (Y, followed by U
>>>>> + * then V), but the exact Linear layout is undefined.
>>>>> + * These formats can only be used with a non-Linear modifier.
>>>>> + */
>>>>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>>>>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>>>>> +
>>>>>  /*  Vendor-specific formats   */
>>>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
>>>>
>>>
>>> [1] https://patchwork.freedesktop.org/series/73722/#rev7
>>>
>>
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Nicolas Dufresne]

Le lundi 08 juin 2020 à 16:43 +0200, Hans Verkuil a écrit :
> On 08/06/2020 16:14, Neil Armstrong wrote:
> > On 08/06/2020 11:26, Hans Verkuil wrote:
> > > On 08/06/2020 10:16, Neil Armstrong wrote:
> > > > Hi Nicolas,
> > > > 
> > > > On 05/06/2020 17:35, Nicolas Dufresne wrote:
> > > > > Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
> > > > > > From: Maxime Jourdan <mjourdan@baylibre.com>
> > > > > > 
> > > > > > Add two generic Compressed Framebuffer pixel formats to be used
> > > > > > with a modifier when imported back in another subsystem like DRM/KMS.
> > > > > > 
> > > > > > These pixel formats represents generic 8bits and 10bits compressed buffers
> > > > > > with a vendor specific layout.
> > > > > > 
> > > > > > These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
> > > > > > used to describe the underlying compressed buffers used for ARM Framebuffer
> > > > > > Compression. In the Amlogic case, the compression is different but the
> > > > > > underlying buffer components is the same.
> > > > > > 
> > > > > > Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> > > > > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > > > > > ---
> > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> > > > > >  include/uapi/linux/videodev2.h       | 9 +++++++++
> > > > > >  2 files changed, 11 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > index 2322f08a98be..8f14adfd5bc5 100644
> > > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > > > > >  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
> > > > > >  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
> > > > > >  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
> > > > > > +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
> > > > > > +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> > 
> > [..]
> > 
> > > > > I'll remind that the modifier implementation has great value and is
> > > > > much more scalable then the current V4L2 approach. There has been some
> > > > > early proposal for this, maybe it's time to prioritize because this
> > > > > list will starts growing with hundred or even thousands or format,
> > > > > which is clearly indicated by the increase of modifier generator macro
> > > > > on the DRM side.
> > > > 
> > > > Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
> > > > is decided, I'm stuck and this is the only intermediate solution I found.
> > > 
> > > We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
> > > 
> > > There is too much divergence and not enough interest in creating common
> > > fourccs.
> > > 
> > > But we *do* want to share the modifiers.
> > > 
> > > > We have a working solution with Boris's patchset with ext_fmt passing the
> > > > modifier to user-space.
> > > > 
> > > > but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
> > > > will still be needed if we pass the modifier with an extended format struct.
> > > 
> > > We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
> > > bother with this, it is much easier to just create a conversion table in the
> > > kernel docs.
> > > 
> > > So don't block on this, I would really prefer if the ext_fmt series is picked
> > > up again and rebased and reposted and then worked on. The stateless codec support
> > > is taking less time (it's shaping up well) so there is more time to work on this.
> > 
> > Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.
> > 
> > Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
> > patch along the new ext_fmt and shared modifiers ?
> 
> So to be clear the DRM_FORMAT_YUV420_8BIT/10BIT fourccs define that this is a
> buffer containing compressed YUV420 in 8 or 10 bit and the modifier tells userspace
> which compression is used, right?
> 
> And we would add V4L2_PIX_FMT_YUV420_8BIT/_10BIT that, I assume, use the same
> fourcc values as the DRM variants?
> 
> Since these fourccs are basically useless without V4L2 modifier support it would
> only make sense in combination with the ext_fmt series.

I personally still think that adding these fourcc will just create a
source of confusion and that fourcc should not be tried to be matched
at the cost of tripling the already duplicated pixel formats. Userspace
already need to implement translation anyway.

On DRM side, new fourcc was not create for NV12+modifier, I don't see
why planar YUV420 has to be different, with or without ext_fmt.

Nicolas
 
> 
> Regards,
> 
> 	Hans
> 
> > Neil
> > 
> > > I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
> > > 
> > > Regards,
> > > 
> > > 	Hans
> > > 
> > > > > >  		default:
> > > > > >  			if (fmt->description[0])
> > > > > >  				return;
> > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > > > > index c3a1cf1c507f..90b9949acb8a 100644
> > > > > > --- a/include/uapi/linux/videodev2.h
> > > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > > @@ -705,6 +705,15 @@ struct v4l2_pix_format {
> > > > > >  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
> > > > > >  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
> > > > > >  
> > > > > > +/*
> > > > > > + * Compressed Luminance+Chrominance meta-formats
> > > > > > + * In these formats, the component ordering is specified (Y, followed by U
> > > > > > + * then V), but the exact Linear layout is undefined.
> > > > > > + * These formats can only be used with a non-Linear modifier.
> > > > > > + */
> > > > > > +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
> > > > > > +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
> > > > > > +
> > > > > >  /*  Vendor-specific formats   */
> > > > > >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> > > > > >  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
> > > > 
> > > > [1] https://patchwork.freedesktop.org/series/73722/#rev7
> > > > 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Neil Armstrong]

Hi Nicolas,

On 08/06/2020 20:59, Nicolas Dufresne wrote:
> Le lundi 08 juin 2020 à 16:43 +0200, Hans Verkuil a écrit :
>> On 08/06/2020 16:14, Neil Armstrong wrote:
>>> On 08/06/2020 11:26, Hans Verkuil wrote:
>>>> On 08/06/2020 10:16, Neil Armstrong wrote:
>>>>> Hi Nicolas,
>>>>>
>>>>> On 05/06/2020 17:35, Nicolas Dufresne wrote:
>>>>>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>>>>>>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>>>>>>
>>>>>>> Add two generic Compressed Framebuffer pixel formats to be used
>>>>>>> with a modifier when imported back in another subsystem like DRM/KMS.
>>>>>>>
>>>>>>> These pixel formats represents generic 8bits and 10bits compressed buffers
>>>>>>> with a vendor specific layout.
>>>>>>>
>>>>>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>>>>>>> used to describe the underlying compressed buffers used for ARM Framebuffer
>>>>>>> Compression. In the Amlogic case, the compression is different but the
>>>>>>> underlying buffer components is the same.
>>>>>>>
>>>>>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>>>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>>>> ---
>>>>>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>>>>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>>>>>>  2 files changed, 11 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>>>> index 2322f08a98be..8f14adfd5bc5 100644
>>>>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>>>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>>>>>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>>>>>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>>>>>>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>>>>>>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
>>>
>>> [..]
>>>
>>>>>> I'll remind that the modifier implementation has great value and is
>>>>>> much more scalable then the current V4L2 approach. There has been some
>>>>>> early proposal for this, maybe it's time to prioritize because this
>>>>>> list will starts growing with hundred or even thousands or format,
>>>>>> which is clearly indicated by the increase of modifier generator macro
>>>>>> on the DRM side.
>>>>>
>>>>> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
>>>>> is decided, I'm stuck and this is the only intermediate solution I found.
>>>>
>>>> We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
>>>>
>>>> There is too much divergence and not enough interest in creating common
>>>> fourccs.
>>>>
>>>> But we *do* want to share the modifiers.
>>>>
>>>>> We have a working solution with Boris's patchset with ext_fmt passing the
>>>>> modifier to user-space.
>>>>>
>>>>> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
>>>>> will still be needed if we pass the modifier with an extended format struct.
>>>>
>>>> We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
>>>> bother with this, it is much easier to just create a conversion table in the
>>>> kernel docs.
>>>>
>>>> So don't block on this, I would really prefer if the ext_fmt series is picked
>>>> up again and rebased and reposted and then worked on. The stateless codec support
>>>> is taking less time (it's shaping up well) so there is more time to work on this.
>>>
>>> Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.
>>>
>>> Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
>>> patch along the new ext_fmt and shared modifiers ?
>>
>> So to be clear the DRM_FORMAT_YUV420_8BIT/10BIT fourccs define that this is a
>> buffer containing compressed YUV420 in 8 or 10 bit and the modifier tells userspace
>> which compression is used, right?
>>
>> And we would add V4L2_PIX_FMT_YUV420_8BIT/_10BIT that, I assume, use the same
>> fourcc values as the DRM variants?
>>
>> Since these fourccs are basically useless without V4L2 modifier support it would
>> only make sense in combination with the ext_fmt series.
> 
> I personally still think that adding these fourcc will just create a
> source of confusion and that fourcc should not be tried to be matched
> at the cost of tripling the already duplicated pixel formats. Userspace
> already need to implement translation anyway.

By using the same fourcc + modifiers, the translation table would only be needed
for v4l2-specific fourcc, by reusing the same it's not necessary anymore.
We have a really simple ffmpeg implementation using ext_fmt, and it makes it
generic.

> 
> On DRM side, new fourcc was not create for NV12+modifier, I don't see
> why planar YUV420 has to be different, with or without ext_fmt.

These V4L2_PIX_FMT_YUV420_8BIT/_10BIT were added because of the compressed nature
of buffers. It's not because of the modifiers, modifiers can be used we any fourcc
to define vendor specific layout requirements or changes, but for compressed the
underlying YUV buffer cannot be physically described by any YUV420 fourcc, so
ARM introduced these fourcc to describe a virtual YUV420 8 or 10bit buffer which
physical layout is defined by the modifier.
They could have re-used DRM_FORMAT_YUV420, but it's a 2 plane fourcc, and the other
describe a true single or multiple plane layout which are simply not true with
ARM AFBC or Amlogic FBC.

Neil

> 
> Nicolas
>  
>>
>> Regards,
>>
>> 	Hans
>>
>>> Neil
>>>
>>>> I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>>>>>  		default:
>>>>>>>  			if (fmt->description[0])
>>>>>>>  				return;
>>>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>>>> index c3a1cf1c507f..90b9949acb8a 100644
>>>>>>> --- a/include/uapi/linux/videodev2.h
>>>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>>>>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>>>>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>>>>>>  
>>>>>>> +/*
>>>>>>> + * Compressed Luminance+Chrominance meta-formats
>>>>>>> + * In these formats, the component ordering is specified (Y, followed by U
>>>>>>> + * then V), but the exact Linear layout is undefined.
>>>>>>> + * These formats can only be used with a non-Linear modifier.
>>>>>>> + */
>>>>>>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>>>>>>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>>>>>>> +
>>>>>>>  /*  Vendor-specific formats   */
>>>>>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>>>>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
>>>>>
>>>>> [1] https://patchwork.freedesktop.org/series/73722/#rev7
>>>>>
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Ezequiel Garcia]

Adding Helen to the discussion.

On Tue, 9 Jun 2020 at 04:43, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi Nicolas,
>
> On 08/06/2020 20:59, Nicolas Dufresne wrote:
> > Le lundi 08 juin 2020 à 16:43 +0200, Hans Verkuil a écrit :
> >> On 08/06/2020 16:14, Neil Armstrong wrote:
> >>> On 08/06/2020 11:26, Hans Verkuil wrote:
> >>>> On 08/06/2020 10:16, Neil Armstrong wrote:
> >>>>> Hi Nicolas,
> >>>>>
> >>>>> On 05/06/2020 17:35, Nicolas Dufresne wrote:
> >>>>>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
> >>>>>>> From: Maxime Jourdan <mjourdan@baylibre.com>
> >>>>>>>
> >>>>>>> Add two generic Compressed Framebuffer pixel formats to be used
> >>>>>>> with a modifier when imported back in another subsystem like DRM/KMS.
> >>>>>>>
> >>>>>>> These pixel formats represents generic 8bits and 10bits compressed buffers
> >>>>>>> with a vendor specific layout.
> >>>>>>>
> >>>>>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
> >>>>>>> used to describe the underlying compressed buffers used for ARM Framebuffer
> >>>>>>> Compression. In the Amlogic case, the compression is different but the
> >>>>>>> underlying buffer components is the same.
> >>>>>>>
> >>>>>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> >>>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >>>>>>> ---
> >>>>>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> >>>>>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
> >>>>>>>  2 files changed, 11 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> >>>>>>> index 2322f08a98be..8f14adfd5bc5 100644
> >>>>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> >>>>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> >>>>>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> >>>>>>>                 case V4L2_PIX_FMT_S5C_UYVY_JPG: descr = "S5C73MX interleaved UYVY/JPEG"; break;
> >>>>>>>                 case V4L2_PIX_FMT_MT21C:        descr = "Mediatek Compressed Format"; break;
> >>>>>>>                 case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
> >>>>>>> +               case V4L2_PIX_FMT_YUV420_8BIT:  descr = "Compressed YUV 4:2:0 8-bit Format"; break;
> >>>>>>> +               case V4L2_PIX_FMT_YUV420_10BIT: descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> >>>
> >>> [..]
> >>>
> >>>>>> I'll remind that the modifier implementation has great value and is
> >>>>>> much more scalable then the current V4L2 approach. There has been some
> >>>>>> early proposal for this, maybe it's time to prioritize because this
> >>>>>> list will starts growing with hundred or even thousands or format,
> >>>>>> which is clearly indicated by the increase of modifier generator macro
> >>>>>> on the DRM side.
> >>>>>
> >>>>> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
> >>>>> is decided, I'm stuck and this is the only intermediate solution I found.
> >>>>
> >>>> We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
> >>>>
> >>>> There is too much divergence and not enough interest in creating common
> >>>> fourccs.
> >>>>
> >>>> But we *do* want to share the modifiers.
> >>>>
> >>>>> We have a working solution with Boris's patchset with ext_fmt passing the
> >>>>> modifier to user-space.
> >>>>>
> >>>>> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
> >>>>> will still be needed if we pass the modifier with an extended format struct.
> >>>>
> >>>> We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
> >>>> bother with this, it is much easier to just create a conversion table in the
> >>>> kernel docs.
> >>>>
> >>>> So don't block on this, I would really prefer if the ext_fmt series is picked
> >>>> up again and rebased and reposted and then worked on. The stateless codec support
> >>>> is taking less time (it's shaping up well) so there is more time to work on this.
> >>>
> >>> Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.
> >>>
> >>> Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
> >>> patch along the new ext_fmt and shared modifiers ?
> >>
> >> So to be clear the DRM_FORMAT_YUV420_8BIT/10BIT fourccs define that this is a
> >> buffer containing compressed YUV420 in 8 or 10 bit and the modifier tells userspace
> >> which compression is used, right?
> >>
> >> And we would add V4L2_PIX_FMT_YUV420_8BIT/_10BIT that, I assume, use the same
> >> fourcc values as the DRM variants?
> >>
> >> Since these fourccs are basically useless without V4L2 modifier support it would
> >> only make sense in combination with the ext_fmt series.
> >
> > I personally still think that adding these fourcc will just create a
> > source of confusion and that fourcc should not be tried to be matched
> > at the cost of tripling the already duplicated pixel formats. Userspace
> > already need to implement translation anyway.
>
> By using the same fourcc + modifiers, the translation table would only be needed
> for v4l2-specific fourcc, by reusing the same it's not necessary anymore.
> We have a really simple ffmpeg implementation using ext_fmt, and it makes it
> generic.
>
> >
> > On DRM side, new fourcc was not create for NV12+modifier, I don't see
> > why planar YUV420 has to be different, with or without ext_fmt.
>
> These V4L2_PIX_FMT_YUV420_8BIT/_10BIT were added because of the compressed nature
> of buffers. It's not because of the modifiers, modifiers can be used we any fourcc
> to define vendor specific layout requirements or changes, but for compressed the
> underlying YUV buffer cannot be physically described by any YUV420 fourcc, so
> ARM introduced these fourcc to describe a virtual YUV420 8 or 10bit buffer which
> physical layout is defined by the modifier.
> They could have re-used DRM_FORMAT_YUV420, but it's a 2 plane fourcc, and the other
> describe a true single or multiple plane layout which are simply not true with
> ARM AFBC or Amlogic FBC.
>
> Neil
>
> >
> > Nicolas
> >
> >>
> >> Regards,
> >>
> >>      Hans
> >>
> >>> Neil
> >>>
> >>>> I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
> >>>>
> >>>> Regards,
> >>>>
> >>>>    Hans
> >>>>
> >>>>>>>                 default:
> >>>>>>>                         if (fmt->description[0])
> >>>>>>>                                 return;
> >>>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >>>>>>> index c3a1cf1c507f..90b9949acb8a 100644
> >>>>>>> --- a/include/uapi/linux/videodev2.h
> >>>>>>> +++ b/include/uapi/linux/videodev2.h
> >>>>>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
> >>>>>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
> >>>>>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
> >>>>>>>
> >>>>>>> +/*
> >>>>>>> + * Compressed Luminance+Chrominance meta-formats
> >>>>>>> + * In these formats, the component ordering is specified (Y, followed by U
> >>>>>>> + * then V), but the exact Linear layout is undefined.
> >>>>>>> + * These formats can only be used with a non-Linear modifier.
> >>>>>>> + */
> >>>>>>> +#define V4L2_PIX_FMT_YUV420_8BIT       v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
> >>>>>>> +#define V4L2_PIX_FMT_YUV420_10BIT      v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
> >>>>>>> +
> >>>>>>>  /*  Vendor-specific formats   */
> >>>>>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> >>>>>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
> >>>>>
> >>>>> [1] https://patchwork.freedesktop.org/series/73722/#rev7
> >>>>>
> >
>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Nicolas Dufresne]

Le mardi 09 juin 2020 à 07:28 -0300, Ezequiel Garcia a écrit :
> Adding Helen to the discussion.
> 
> On Tue, 9 Jun 2020 at 04:43, Neil Armstrong <narmstrong@baylibre.com> wrote:
> > Hi Nicolas,
> > 
> > On 08/06/2020 20:59, Nicolas Dufresne wrote:
> > > Le lundi 08 juin 2020 à 16:43 +0200, Hans Verkuil a écrit :
> > > > On 08/06/2020 16:14, Neil Armstrong wrote:
> > > > > On 08/06/2020 11:26, Hans Verkuil wrote:
> > > > > > On 08/06/2020 10:16, Neil Armstrong wrote:
> > > > > > > Hi Nicolas,
> > > > > > > 
> > > > > > > On 05/06/2020 17:35, Nicolas Dufresne wrote:
> > > > > > > > Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
> > > > > > > > > From: Maxime Jourdan <mjourdan@baylibre.com>
> > > > > > > > > 
> > > > > > > > > Add two generic Compressed Framebuffer pixel formats to be used
> > > > > > > > > with a modifier when imported back in another subsystem like DRM/KMS.
> > > > > > > > > 
> > > > > > > > > These pixel formats represents generic 8bits and 10bits compressed buffers
> > > > > > > > > with a vendor specific layout.
> > > > > > > > > 
> > > > > > > > > These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
> > > > > > > > > used to describe the underlying compressed buffers used for ARM Framebuffer
> > > > > > > > > Compression. In the Amlogic case, the compression is different but the
> > > > > > > > > underlying buffer components is the same.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> > > > > > > > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> > > > > > > > >  include/uapi/linux/videodev2.h       | 9 +++++++++
> > > > > > > > >  2 files changed, 11 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > > > > index 2322f08a98be..8f14adfd5bc5 100644
> > > > > > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > > > > @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > > > > > > > >                 case V4L2_PIX_FMT_S5C_UYVY_JPG: descr = "S5C73MX interleaved UYVY/JPEG"; break;
> > > > > > > > >                 case V4L2_PIX_FMT_MT21C:        descr = "Mediatek Compressed Format"; break;
> > > > > > > > >                 case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
> > > > > > > > > +               case V4L2_PIX_FMT_YUV420_8BIT:  descr = "Compressed YUV 4:2:0 8-bit Format"; break;
> > > > > > > > > +               case V4L2_PIX_FMT_YUV420_10BIT: descr = "Compressed YUV 4:2:0 10-bit Format"; break;
> > > > > 
> > > > > [..]
> > > > > 
> > > > > > > > I'll remind that the modifier implementation has great value and is
> > > > > > > > much more scalable then the current V4L2 approach. There has been some
> > > > > > > > early proposal for this, maybe it's time to prioritize because this
> > > > > > > > list will starts growing with hundred or even thousands or format,
> > > > > > > > which is clearly indicated by the increase of modifier generator macro
> > > > > > > > on the DRM side.
> > > > > > > 
> > > > > > > Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
> > > > > > > is decided, I'm stuck and this is the only intermediate solution I found.
> > > > > > 
> > > > > > We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
> > > > > > 
> > > > > > There is too much divergence and not enough interest in creating common
> > > > > > fourccs.
> > > > > > 
> > > > > > But we *do* want to share the modifiers.
> > > > > > 
> > > > > > > We have a working solution with Boris's patchset with ext_fmt passing the
> > > > > > > modifier to user-space.
> > > > > > > 
> > > > > > > but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
> > > > > > > will still be needed if we pass the modifier with an extended format struct.
> > > > > > 
> > > > > > We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
> > > > > > bother with this, it is much easier to just create a conversion table in the
> > > > > > kernel docs.
> > > > > > 
> > > > > > So don't block on this, I would really prefer if the ext_fmt series is picked
> > > > > > up again and rebased and reposted and then worked on. The stateless codec support
> > > > > > is taking less time (it's shaping up well) so there is more time to work on this.
> > > > > 
> > > > > Ok, I already starting discussing with Helen Koike about the ext_fnt re-spin.
> > > > > 
> > > > > Should I re-introduce different v4l2 pixfmt for these DRM YUV420_*BIT or I can keep this
> > > > > patch along the new ext_fmt and shared modifiers ?
> > > > 
> > > > So to be clear the DRM_FORMAT_YUV420_8BIT/10BIT fourccs define that this is a
> > > > buffer containing compressed YUV420 in 8 or 10 bit and the modifier tells userspace
> > > > which compression is used, right?
> > > > 
> > > > And we would add V4L2_PIX_FMT_YUV420_8BIT/_10BIT that, I assume, use the same
> > > > fourcc values as the DRM variants?
> > > > 
> > > > Since these fourccs are basically useless without V4L2 modifier support it would
> > > > only make sense in combination with the ext_fmt series.
> > > 
> > > I personally still think that adding these fourcc will just create a
> > > source of confusion and that fourcc should not be tried to be matched
> > > at the cost of tripling the already duplicated pixel formats. Userspace
> > > already need to implement translation anyway.
> > 
> > By using the same fourcc + modifiers, the translation table would only be needed
> > for v4l2-specific fourcc, by reusing the same it's not necessary anymore.
> > We have a really simple ffmpeg implementation using ext_fmt, and it makes it
> > generic.

But it's not overall generic because "generic" userspace still need to
handle legacy, like the Samsung NV12 tiled format. There is no way to
avoid translation while supporting multiple HW (even with modifiers).
And adding a subset of generic fourcc that duplicates some existing
pixel format seems like the wrong way. Quickly this will grow fourcc
collision between DRM and V4L2.

> > 
> > > On DRM side, new fourcc was not create for NV12+modifier, I don't see
> > > why planar YUV420 has to be different, with or without ext_fmt.
> > 
> > These V4L2_PIX_FMT_YUV420_8BIT/_10BIT were added because of the compressed nature
> > of buffers. It's not because of the modifiers, modifiers can be used we any fourcc
> > to define vendor specific layout requirements or changes, but for compressed the
> > underlying YUV buffer cannot be physically described by any YUV420 fourcc, so
> > ARM introduced these fourcc to describe a virtual YUV420 8 or 10bit buffer which
> > physical layout is defined by the modifier.
> > They could have re-used DRM_FORMAT_YUV420, but it's a 2 plane fourcc, and the other
> > describe a true single or multiple plane layout which are simply not true with
> > ARM AFBC or Amlogic FBC.

As far as I'm concern, this argument does not hold, as NV12 is still
used with compressed modifiers on other plaforms (even if NV12 is not
by "nature" compressed) and only 1 out of the two NV12 formats we got
in V4L2 matches the DRM fourcc. So it might be "generic" to your
specific SoC, but that's not real.

In any case, I still believe the naming is completely misleading if
this is to indicate a buffer format which is compressed, and not
supposed to be used for linear YUV420p (which already have two fourcc
in V4L2 for).

> > 
> > Neil
> > 
> > > Nicolas
> > > 
> > > > Regards,
> > > > 
> > > >      Hans
> > > > 
> > > > > Neil
> > > > > 
> > > > > > I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
> > > > > > 
> > > > > > Regards,
> > > > > > 
> > > > > >    Hans
> > > > > > 
> > > > > > > > >                 default:
> > > > > > > > >                         if (fmt->description[0])
> > > > > > > > >                                 return;
> > > > > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > > > > > > > index c3a1cf1c507f..90b9949acb8a 100644
> > > > > > > > > --- a/include/uapi/linux/videodev2.h
> > > > > > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > > > > > @@ -705,6 +705,15 @@ struct v4l2_pix_format {
> > > > > > > > >  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
> > > > > > > > >  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
> > > > > > > > > 
> > > > > > > > > +/*
> > > > > > > > > + * Compressed Luminance+Chrominance meta-formats
> > > > > > > > > + * In these formats, the component ordering is specified (Y, followed by U
> > > > > > > > > + * then V), but the exact Linear layout is undefined.
> > > > > > > > > + * These formats can only be used with a non-Linear modifier.
> > > > > > > > > + */
> > > > > > > > > +#define V4L2_PIX_FMT_YUV420_8BIT       v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
> > > > > > > > > +#define V4L2_PIX_FMT_YUV420_10BIT      v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
> > > > > > > > > +
> > > > > > > > >  /*  Vendor-specific formats   */
> > > > > > > > >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> > > > > > > > >  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
> > > > > > > 
> > > > > > > [1] https://patchwork.freedesktop.org/series/73722/#rev7
> > > > > > > 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/5] media: videodev2: add Compressed Framebuffer pixel formats

[Helen Koike]

On 6/8/20 6:26 AM, Hans Verkuil wrote:
> On 08/06/2020 10:16, Neil Armstrong wrote:
>> Hi Nicolas,
>>
>> On 05/06/2020 17:35, Nicolas Dufresne wrote:
>>> Le jeudi 04 juin 2020 à 15:53 +0200, Neil Armstrong a écrit :
>>>> From: Maxime Jourdan <mjourdan@baylibre.com>
>>>>
>>>> Add two generic Compressed Framebuffer pixel formats to be used
>>>> with a modifier when imported back in another subsystem like DRM/KMS.
>>>>
>>>> These pixel formats represents generic 8bits and 10bits compressed buffers
>>>> with a vendor specific layout.
>>>>
>>>> These are aligned with the DRM_FORMAT_YUV420_8BIT and DRM_FORMAT_YUV420_10BIT
>>>> used to describe the underlying compressed buffers used for ARM Framebuffer
>>>> Compression. In the Amlogic case, the compression is different but the
>>>> underlying buffer components is the same.
>>>>
>>>> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>>>>  include/uapi/linux/videodev2.h       | 9 +++++++++
>>>>  2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> index 2322f08a98be..8f14adfd5bc5 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> @@ -1447,6 +1447,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>>>>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
>>>>  		case V4L2_PIX_FMT_SUNXI_TILED_NV12: descr = "Sunxi Tiled NV12 Format"; break;
>>>> +		case V4L2_PIX_FMT_YUV420_8BIT:	descr = "Compressed YUV 4:2:0 8-bit Format"; break;
>>>> +		case V4L2_PIX_FMT_YUV420_10BIT:	descr = "Compressed YUV 4:2:0 10-bit Format"; break;
>>
>> Seems we are totally on-par with the following :-)
>>
>>>
>>> When I read the DRM documentation [0], I'm reading that YUV420_8BIT
>>> definition matches V4L2_PIX_FMT_YVU420 and V4L2_PIX_FMT_YVU420M fully.
>>> In fact, on DRM side, to represent that format you want to expose here,
>>> they will strictly combine this generic format (documented un-
>>> compressed) with a modifier generated with the macro
>>> DRM_FORMAT_MOD_ARM_AFBC(*). And only the combination represent a unique
>>> and share-able format.
>>
>> Yes, and this is exactly my goal here, and matches the Amlogic Framebuffer as
>> described in patch 4. The modifier patchset is at [1].
>>
>>>
>>> In absence of modifier in V4L2 API, this compressed format should be
>>> named accordingly to the compressed algorithm used (something like
>>> FMT_YUV420_8BIT_AML_FBC). 
>>
>> It's even more complex, the modifier depends on the SoC revision, so we can
>> have up to6 different unique pixel format instead of 2 with a variable
>> modifier.
>>
>>> So I believe these format name cannot be
>>> copied as-is like this, as they can only introduce more ambiguity in
>>> the already quite hard to follow naming of pixel formats. In fact, it
>>> is very common to see multiple different vendor compressions on the
>>> same SoC, so I don't really believe a "generic" compressed format make
>>> sense. To site one, the IMX8M, which got Verrisillicon/Vivante DEC400
>>> on the GPU, and the Hantro G2 compression format. Both will apply to
>>> NV12 class of format so in DRM they would be NV12 + modifier, and the
>>> combination forms the unique format. Now, in term of sharing, they must
>>> be differiented by userspace, as support for compression/decompression
>>> is heterogeneous (in that case the GPU does not support Hantro G2
>>> decompression or compression, and the VPU does not support DEC400).
>>>
>>> I'll remind that the modifier implementation has great value and is
>>> much more scalable then the current V4L2 approach. There has been some
>>> early proposal for this, maybe it's time to prioritize because this
>>> list will starts growing with hundred or even thousands or format,
>>> which is clearly indicated by the increase of modifier generator macro
>>> on the DRM side.
>>
>> Yes, but until the migration of drm_fourcc and v4l2 fourcc into a common one
>> is decided, I'm stuck and this is the only intermediate solution I found.
> 
> We can safely assume that drm fourcc and v4l2 fourcc won't be merged.
> 
> There is too much divergence and not enough interest in creating common
> fourccs.
> 
> But we *do* want to share the modifiers.
> 
>>
>> We have a working solution with Boris's patchset with ext_fmt passing the
>> modifier to user-space.
>>
>> but anyway, since the goal is to merge the fourcc between DRM & V4L2, these YUV420_*BIT
>> will still be needed if we pass the modifier with an extended format struct.
> 
> We tried merging fourccs but that ran into resistance. Frankly, I wouldn't
> bother with this, it is much easier to just create a conversion table in the
> kernel docs.
> 
> So don't block on this, I would really prefer if the ext_fmt series is picked
> up again and rebased and reposted and then worked on.

jfyi, I picked it up, and should post a new version of the RFC soonish.

Regards,
Helen

> The stateless codec support
> is taking less time (it's shaping up well) so there is more time to work on this.
> 
> I believe we really need this since v4l2_buffer and v4l2_format are a real mess.
> 
> Regards,
> 
> 	Hans
> 
>>
>>>
>>>>  		default:
>>>>  			if (fmt->description[0])
>>>>  				return;
>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>> index c3a1cf1c507f..90b9949acb8a 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -705,6 +705,15 @@ struct v4l2_pix_format {
>>>>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>>>>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>>>>  
>>>> +/*
>>>> + * Compressed Luminance+Chrominance meta-formats
>>>> + * In these formats, the component ordering is specified (Y, followed by U
>>>> + * then V), but the exact Linear layout is undefined.
>>>> + * These formats can only be used with a non-Linear modifier.
>>>> + */
>>>> +#define V4L2_PIX_FMT_YUV420_8BIT	v4l2_fourcc('Y', 'U', '0', '8') /* 1-plane YUV 4:2:0 8-bit */
>>>> +#define V4L2_PIX_FMT_YUV420_10BIT	v4l2_fourcc('Y', 'U', '1', '0') /* 1-plane YUV 4:2:0 10-bit */
>>>> +
>>>>  /*  Vendor-specific formats   */
>>>>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
>>>>  #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
>>>
>>
>> [1] https://patchwork.freedesktop.org/series/73722/#rev7
>>
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/5] media: meson: vdec: handle bitdepth on source change

[Neil Armstrong]

From: Maxime Jourdan <mjourdan@baylibre.com>

In order to handle Compressed Framebuffer support, we need to handle
the switch between 8bit and 10bit frame output.

This handles the bitdepth in the codec amvdec_src_change() call to handle
a source change/decode resume when the stream bitdepth changes.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/staging/media/meson/vdec/codec_h264.c   |  3 ++-
 drivers/staging/media/meson/vdec/codec_vp9.c    |  3 ++-
 drivers/staging/media/meson/vdec/vdec.h         |  1 +
 drivers/staging/media/meson/vdec/vdec_helpers.c | 10 ++++++----
 drivers/staging/media/meson/vdec/vdec_helpers.h |  3 ++-
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/codec_h264.c b/drivers/staging/media/meson/vdec/codec_h264.c
index c61128fc4bb9..d53c9a464bde 100644
--- a/drivers/staging/media/meson/vdec/codec_h264.c
+++ b/drivers/staging/media/meson/vdec/codec_h264.c
@@ -353,7 +353,8 @@ static void codec_h264_src_change(struct amvdec_session *sess)
 		frame_width, frame_height, crop_right, crop_bottom);
 
 	codec_h264_set_par(sess);
-	amvdec_src_change(sess, frame_width, frame_height, h264->max_refs + 5);
+	amvdec_src_change(sess, frame_width, frame_height,
+			  h264->max_refs + 5, 8);
 }
 
 /*
diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
index 897f5d7a6aad..4c6a644ab1a7 100644
--- a/drivers/staging/media/meson/vdec/codec_vp9.c
+++ b/drivers/staging/media/meson/vdec/codec_vp9.c
@@ -2132,7 +2132,8 @@ static irqreturn_t codec_vp9_threaded_isr(struct amvdec_session *sess)
 
 	codec_vp9_fetch_rpm(sess);
 	if (codec_vp9_process_rpm(vp9)) {
-		amvdec_src_change(sess, vp9->width, vp9->height, 16);
+		amvdec_src_change(sess, vp9->width, vp9->height, 16,
+				  vp9->is_10bit ? 10 : 8);
 
 		/* No frame is actually processed */
 		vp9->cur_frame = NULL;
diff --git a/drivers/staging/media/meson/vdec/vdec.h b/drivers/staging/media/meson/vdec/vdec.h
index f95445ac0658..e3e4af73447a 100644
--- a/drivers/staging/media/meson/vdec/vdec.h
+++ b/drivers/staging/media/meson/vdec/vdec.h
@@ -234,6 +234,7 @@ struct amvdec_session {
 	u32 width;
 	u32 height;
 	u32 colorspace;
+	u32 bitdepth;
 	u8 ycbcr_enc;
 	u8 quantization;
 	u8 xfer_func;
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.c b/drivers/staging/media/meson/vdec/vdec_helpers.c
index 7f07a9175815..eed7a929c5d0 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.c
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.c
@@ -436,7 +436,7 @@ void amvdec_set_par_from_dar(struct amvdec_session *sess,
 EXPORT_SYMBOL_GPL(amvdec_set_par_from_dar);
 
 void amvdec_src_change(struct amvdec_session *sess, u32 width,
-		       u32 height, u32 dpb_size)
+		       u32 height, u32 dpb_size, u32 bitdepth)
 {
 	static const struct v4l2_event ev = {
 		.type = V4L2_EVENT_SOURCE_CHANGE,
@@ -451,7 +451,8 @@ void amvdec_src_change(struct amvdec_session *sess, u32 width,
 	if (sess->streamon_cap &&
 	    sess->width == width &&
 	    sess->height == height &&
-	    dpb_size <= sess->num_dst_bufs) {
+	    dpb_size <= sess->num_dst_bufs &&
+	    sess->bitdepth == bitdepth) {
 		sess->fmt_out->codec_ops->resume(sess);
 		return;
 	}
@@ -460,9 +461,10 @@ void amvdec_src_change(struct amvdec_session *sess, u32 width,
 	sess->width = width;
 	sess->height = height;
 	sess->status = STATUS_NEEDS_RESUME;
+	sess->bitdepth = bitdepth;
 
-	dev_dbg(sess->core->dev, "Res. changed (%ux%u), DPB size %u\n",
-		width, height, dpb_size);
+	dev_dbg(sess->core->dev, "Res. changed (%ux%u), DPB %u, bitdepth %u\n",
+		width, height, dpb_size, bitdepth);
 	v4l2_event_queue_fh(&sess->fh, &ev);
 }
 EXPORT_SYMBOL_GPL(amvdec_src_change);
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.h b/drivers/staging/media/meson/vdec/vdec_helpers.h
index cfaed52ab526..f059cf195cca 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.h
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.h
@@ -76,9 +76,10 @@ void amvdec_set_par_from_dar(struct amvdec_session *sess,
  * @width: picture width detected by the hardware
  * @height: picture height detected by the hardware
  * @dpb_size: Decoded Picture Buffer size (= amount of buffers for decoding)
+ * @bitdepth: Bit depth (usually 10 or 8) of the coded content
  */
 void amvdec_src_change(struct amvdec_session *sess, u32 width,
-		       u32 height, u32 dpb_size);
+		       u32 height, u32 dpb_size, u32 bitdepth);
 
 /**
  * amvdec_abort() - Abort the current decoding session
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 3/5] media: meson: vdec: update compressed buffer helpers

[Neil Armstrong]

From: Maxime Jourdan <mjourdan@baylibre.com>

The actual compressed buffer helpers were very basic and only used
to enabled downsampling when decoding a 10bit stream.

Update and rename these helpers to handle the complete compressed buffer
output buffer size and alignment for 8bit and 10bit streams.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../media/meson/vdec/codec_hevc_common.c      | 28 ++++++++++++---
 drivers/staging/media/meson/vdec/codec_vp9.c  |  7 ++--
 .../staging/media/meson/vdec/vdec_helpers.c   | 35 +++++++++++++------
 .../staging/media/meson/vdec/vdec_helpers.h   |  8 +++--
 4 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/codec_hevc_common.c b/drivers/staging/media/meson/vdec/codec_hevc_common.c
index 0315cc0911cd..c9bf67aa2668 100644
--- a/drivers/staging/media/meson/vdec/codec_hevc_common.c
+++ b/drivers/staging/media/meson/vdec/codec_hevc_common.c
@@ -30,8 +30,11 @@ const u16 vdec_hevc_parser_cmd[] = {
 void codec_hevc_setup_decode_head(struct amvdec_session *sess, int is_10bit)
 {
 	struct amvdec_core *core = sess->core;
-	u32 body_size = amvdec_am21c_body_size(sess->width, sess->height);
-	u32 head_size = amvdec_am21c_head_size(sess->width, sess->height);
+	u32 use_mmu = codec_hevc_use_mmu(core->platform->revision,
+					 sess->pixfmt_cap, is_10bit);
+	u32 body_size = amvdec_amfbc_body_size(sess->width, sess->height,
+					       is_10bit, use_mmu);
+	u32 head_size = amvdec_amfbc_head_size(sess->width, sess->height);
 
 	if (!codec_hevc_use_fbc(sess->pixfmt_cap, is_10bit)) {
 		/* Enable 2-plane reference read mode */
@@ -154,7 +157,12 @@ void codec_hevc_free_fbc_buffers(struct amvdec_session *sess,
 				 struct codec_hevc_common *comm)
 {
 	struct device *dev = sess->core->dev;
-	u32 am21_size = amvdec_am21c_size(sess->width, sess->height);
+	u32 use_mmu = codec_hevc_use_mmu(sess->core->platform->revision,
+					 sess->pixfmt_cap,
+					 sess->bitdepth == 10 ? 1 : 0);
+	u32 am21_size = amvdec_amfbc_size(sess->width, sess->height,
+					  sess->bitdepth == 10 ? 1 : 0,
+					  use_mmu);
 	int i;
 
 	for (i = 0; i < MAX_REF_PIC_NUM; ++i) {
@@ -173,7 +181,12 @@ static int codec_hevc_alloc_fbc_buffers(struct amvdec_session *sess,
 {
 	struct device *dev = sess->core->dev;
 	struct v4l2_m2m_buffer *buf;
-	u32 am21_size = amvdec_am21c_size(sess->width, sess->height);
+	u32 use_mmu = codec_hevc_use_mmu(sess->core->platform->revision,
+					 sess->pixfmt_cap,
+					 sess->bitdepth == 10 ? 1 : 0);
+	u32 am21_size = amvdec_amfbc_size(sess->width, sess->height,
+					  sess->bitdepth == 10 ? 1 : 0,
+					  use_mmu);
 
 	v4l2_m2m_for_each_dst_buf(sess->m2m_ctx, buf) {
 		u32 idx = buf->vb.vb2_buf.index;
@@ -280,7 +293,12 @@ void codec_hevc_fill_mmu_map(struct amvdec_session *sess,
 			     struct codec_hevc_common *comm,
 			     struct vb2_buffer *vb)
 {
-	u32 size = amvdec_am21c_size(sess->width, sess->height);
+	u32 use_mmu = codec_hevc_use_mmu(sess->core->platform->revision,
+					 sess->pixfmt_cap,
+					 sess->bitdepth == 10 ? 1 : 0);
+	u32 size = amvdec_amfbc_size(sess->width, sess->height,
+				     sess->bitdepth == 10 ? 1 : 0,
+				     use_mmu);
 	u32 nb_pages = size / PAGE_SIZE;
 	u32 *mmu_map = comm->mmu_map_vaddr;
 	u32 first_page;
diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
index 4c6a644ab1a7..a810c50f3a39 100644
--- a/drivers/staging/media/meson/vdec/codec_vp9.c
+++ b/drivers/staging/media/meson/vdec/codec_vp9.c
@@ -1147,6 +1147,8 @@ static void codec_vp9_set_mc(struct amvdec_session *sess,
 			     struct codec_vp9 *vp9)
 {
 	struct amvdec_core *core = sess->core;
+	u32 use_mmu = codec_hevc_use_mmu(core->platform->revision,
+					 sess->pixfmt_cap, vp9->is_10bit);
 	u32 scale = 0;
 	u32 sz;
 	int i;
@@ -1166,8 +1168,9 @@ static void codec_vp9_set_mc(struct amvdec_session *sess,
 		    vp9->frame_refs[i]->height != vp9->height)
 			scale = 1;
 
-		sz = amvdec_am21c_body_size(vp9->frame_refs[i]->width,
-					    vp9->frame_refs[i]->height);
+		sz = amvdec_amfbc_body_size(vp9->frame_refs[i]->width,
+					    vp9->frame_refs[i]->height,
+					    vp9->is_10bit, use_mmu);
 
 		amvdec_write_dos(core, VP9D_MPP_REFINFO_DATA,
 				 vp9->frame_refs[i]->width);
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.c b/drivers/staging/media/meson/vdec/vdec_helpers.c
index eed7a929c5d0..320cac1ed03e 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.c
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.c
@@ -50,32 +50,47 @@ void amvdec_write_parser(struct amvdec_core *core, u32 reg, u32 val)
 }
 EXPORT_SYMBOL_GPL(amvdec_write_parser);
 
-/* 4 KiB per 64x32 block */
-u32 amvdec_am21c_body_size(u32 width, u32 height)
+/* AMFBC body is made out of 64x32 blocks with varying block size */
+u32 amvdec_amfbc_body_size(u32 width, u32 height, u32 is_10bit, u32 use_mmu)
 {
 	u32 width_64 = ALIGN(width, 64) / 64;
 	u32 height_32 = ALIGN(height, 32) / 32;
+	u32 blk_size = 4096;
 
-	return SZ_4K * width_64 * height_32;
+	if (!is_10bit) {
+		if (use_mmu)
+			blk_size = 3200;
+		else
+			blk_size = 3072;
+	}
+
+	return blk_size * width_64 * height_32;
 }
-EXPORT_SYMBOL_GPL(amvdec_am21c_body_size);
+EXPORT_SYMBOL_GPL(amvdec_amfbc_body_size);
 
 /* 32 bytes per 128x64 block */
-u32 amvdec_am21c_head_size(u32 width, u32 height)
+u32 amvdec_amfbc_head_size(u32 width, u32 height)
 {
 	u32 width_128 = ALIGN(width, 128) / 128;
 	u32 height_64 = ALIGN(height, 64) / 64;
 
 	return 32 * width_128 * height_64;
 }
-EXPORT_SYMBOL_GPL(amvdec_am21c_head_size);
+EXPORT_SYMBOL_GPL(amvdec_amfbc_head_size);
+
+u32 amvdec_amfbc_size(u32 width, u32 height, u32 is_10bit, u32 use_mmu)
+{
+	return ALIGN(amvdec_amfbc_body_size(width, height, is_10bit, use_mmu) +
+		     amvdec_amfbc_head_size(width, height), SZ_64K);
+}
+EXPORT_SYMBOL_GPL(amvdec_amfbc_size);
 
-u32 amvdec_am21c_size(u32 width, u32 height)
+u32 amvdec_is_dst_fbc(struct amvdec_session *sess)
 {
-	return ALIGN(amvdec_am21c_body_size(width, height) +
-		     amvdec_am21c_head_size(width, height), SZ_64K);
+	return sess->pixfmt_cap == V4L2_PIX_FMT_YUV420_8BIT ||
+	       sess->pixfmt_cap == V4L2_PIX_FMT_YUV420_10BIT;
 }
-EXPORT_SYMBOL_GPL(amvdec_am21c_size);
+EXPORT_SYMBOL_GPL(amvdec_is_dst_fbc);
 
 static int canvas_alloc(struct amvdec_session *sess, u8 *canvas_id)
 {
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.h b/drivers/staging/media/meson/vdec/vdec_helpers.h
index f059cf195cca..c1666125fe4c 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.h
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.h
@@ -27,9 +27,11 @@ void amvdec_clear_dos_bits(struct amvdec_core *core, u32 reg, u32 val);
 u32 amvdec_read_parser(struct amvdec_core *core, u32 reg);
 void amvdec_write_parser(struct amvdec_core *core, u32 reg, u32 val);
 
-u32 amvdec_am21c_body_size(u32 width, u32 height);
-u32 amvdec_am21c_head_size(u32 width, u32 height);
-u32 amvdec_am21c_size(u32 width, u32 height);
+/* Helpers for the Amlogic compressed framebuffer format */
+u32 amvdec_amfbc_body_size(u32 width, u32 height, u32 is_10bit, u32 use_mmu);
+u32 amvdec_amfbc_head_size(u32 width, u32 height);
+u32 amvdec_amfbc_size(u32 width, u32 height, u32 is_10bit, u32 use_mmu);
+u32 amvdec_is_dst_fbc(struct amvdec_session *sess);
 
 /**
  * amvdec_dst_buf_done_idx() - Signal that a buffer is done decoding
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 4/5] media: meson: vdec: add support for compressed output for VP9 decoder

[Neil Armstrong]

From: Maxime Jourdan <mjourdan@baylibre.com>

Add the necessary changes to decode VP9 8bit and 10bit streams into
compressed buffers to be imported back into DRM/KMS using a modifier.

On GXL/GXM platforms, the VP9 decoder will output a basic Framebuffer
Compressed layout, with a memory saving option when decoding 8bit to
better align the compressed macroblocks. This layout includes the buffer
content and an header desscribing the compressed buffer.

On G12A and later, the VP9 decoder will output "Scatter" layout, meaning
the header buffer will contain references to the internal memory decoder
workspace and frame memory to construct a compressed framebuffer.

The compressed layout has been described in the DRM Modifier patchset
at [1].

[1] https://patchwork.freedesktop.org/series/73722/#rev7

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../media/meson/vdec/codec_hevc_common.c      | 112 ++++++------------
 .../media/meson/vdec/codec_hevc_common.h      |  13 +-
 drivers/staging/media/meson/vdec/codec_vp9.c  |  19 +--
 3 files changed, 47 insertions(+), 97 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/codec_hevc_common.c b/drivers/staging/media/meson/vdec/codec_hevc_common.c
index c9bf67aa2668..73dae40b3319 100644
--- a/drivers/staging/media/meson/vdec/codec_hevc_common.c
+++ b/drivers/staging/media/meson/vdec/codec_hevc_common.c
@@ -42,9 +42,17 @@ void codec_hevc_setup_decode_head(struct amvdec_session *sess, int is_10bit)
 		return;
 	}
 
+	/* enable mem saving mode for 8-bit */
+	if (!is_10bit)
+		amvdec_write_dos_bits(core, HEVC_SAO_CTRL5, BIT(9));
+	else
+		amvdec_clear_dos_bits(core, HEVC_SAO_CTRL5, BIT(9));
+
 	if (codec_hevc_use_mmu(core->platform->revision,
 			       sess->pixfmt_cap, is_10bit))
 		amvdec_write_dos(core, HEVCD_MPP_DECOMP_CTL1, BIT(4));
+	else if (!is_10bit)
+		amvdec_write_dos(core, HEVCD_MPP_DECOMP_CTL1, BIT(3));
 	else
 		amvdec_write_dos(core, HEVCD_MPP_DECOMP_CTL1, 0);
 
@@ -76,7 +84,7 @@ static void codec_hevc_setup_buffers_gxbb(struct amvdec_session *sess,
 
 		idx = vb->index;
 
-		if (codec_hevc_use_downsample(sess->pixfmt_cap, is_10bit))
+		if (codec_hevc_use_fbc(sess->pixfmt_cap, is_10bit))
 			buf_y_paddr = comm->fbc_buffer_paddr[idx];
 		else
 			buf_y_paddr = vb2_dma_contig_plane_dma_addr(vb, 0);
@@ -117,7 +125,6 @@ static void codec_hevc_setup_buffers_gxl(struct amvdec_session *sess,
 {
 	struct amvdec_core *core = sess->core;
 	struct v4l2_m2m_buffer *buf;
-	u32 revision = core->platform->revision;
 	u32 pixfmt_cap = sess->pixfmt_cap;
 	int i;
 
@@ -130,9 +137,7 @@ static void codec_hevc_setup_buffers_gxl(struct amvdec_session *sess,
 		dma_addr_t buf_uv_paddr = 0;
 		u32 idx = vb->index;
 
-		if (codec_hevc_use_mmu(revision, pixfmt_cap, is_10bit))
-			buf_y_paddr = comm->mmu_header_paddr[idx];
-		else if (codec_hevc_use_downsample(pixfmt_cap, is_10bit))
+		if (codec_hevc_use_downsample(pixfmt_cap, is_10bit))
 			buf_y_paddr = comm->fbc_buffer_paddr[idx];
 		else
 			buf_y_paddr = vb2_dma_contig_plane_dma_addr(vb, 0);
@@ -173,6 +178,14 @@ void codec_hevc_free_fbc_buffers(struct amvdec_session *sess,
 			comm->fbc_buffer_vaddr[i] = NULL;
 		}
 	}
+
+	if (comm->mmu_map_vaddr) {
+		dma_free_coherent(dev, MMU_MAP_SIZE,
+				  comm->mmu_map_vaddr,
+				  comm->mmu_map_paddr);
+		comm->mmu_map_vaddr = NULL;
+	}
+
 }
 EXPORT_SYMBOL_GPL(codec_hevc_free_fbc_buffers);
 
@@ -205,79 +218,29 @@ static int codec_hevc_alloc_fbc_buffers(struct amvdec_session *sess,
 	return 0;
 }
 
-void codec_hevc_free_mmu_headers(struct amvdec_session *sess,
-				 struct codec_hevc_common *comm)
-{
-	struct device *dev = sess->core->dev;
-	int i;
-
-	for (i = 0; i < MAX_REF_PIC_NUM; ++i) {
-		if (comm->mmu_header_vaddr[i]) {
-			dma_free_coherent(dev, MMU_COMPRESS_HEADER_SIZE,
-					  comm->mmu_header_vaddr[i],
-					  comm->mmu_header_paddr[i]);
-			comm->mmu_header_vaddr[i] = NULL;
-		}
-	}
-
-	if (comm->mmu_map_vaddr) {
-		dma_free_coherent(dev, MMU_MAP_SIZE,
-				  comm->mmu_map_vaddr,
-				  comm->mmu_map_paddr);
-		comm->mmu_map_vaddr = NULL;
-	}
-}
-EXPORT_SYMBOL_GPL(codec_hevc_free_mmu_headers);
-
-static int codec_hevc_alloc_mmu_headers(struct amvdec_session *sess,
-					struct codec_hevc_common *comm)
-{
-	struct device *dev = sess->core->dev;
-	struct v4l2_m2m_buffer *buf;
-
-	comm->mmu_map_vaddr = dma_alloc_coherent(dev, MMU_MAP_SIZE,
-						 &comm->mmu_map_paddr,
-						 GFP_KERNEL);
-	if (!comm->mmu_map_vaddr)
-		return -ENOMEM;
-
-	v4l2_m2m_for_each_dst_buf(sess->m2m_ctx, buf) {
-		u32 idx = buf->vb.vb2_buf.index;
-		dma_addr_t paddr;
-		void *vaddr = dma_alloc_coherent(dev, MMU_COMPRESS_HEADER_SIZE,
-						 &paddr, GFP_KERNEL);
-		if (!vaddr) {
-			codec_hevc_free_mmu_headers(sess, comm);
-			return -ENOMEM;
-		}
-
-		comm->mmu_header_vaddr[idx] = vaddr;
-		comm->mmu_header_paddr[idx] = paddr;
-	}
-
-	return 0;
-}
-
 int codec_hevc_setup_buffers(struct amvdec_session *sess,
 			     struct codec_hevc_common *comm,
 			     int is_10bit)
 {
 	struct amvdec_core *core = sess->core;
+	struct device *dev = core->dev;
 	int ret;
 
-	if (codec_hevc_use_downsample(sess->pixfmt_cap, is_10bit)) {
-		ret = codec_hevc_alloc_fbc_buffers(sess, comm);
-		if (ret)
-			return ret;
+	if (codec_hevc_use_mmu(core->platform->revision,
+			       sess->pixfmt_cap, is_10bit)) {
+		comm->mmu_map_vaddr = dma_alloc_coherent(dev, MMU_MAP_SIZE,
+							 &comm->mmu_map_paddr,
+							 GFP_KERNEL);
+		if (!comm->mmu_map_vaddr)
+			return -ENOMEM;
 	}
 
 	if (codec_hevc_use_mmu(core->platform->revision,
-			       sess->pixfmt_cap, is_10bit)) {
-		ret = codec_hevc_alloc_mmu_headers(sess, comm);
-		if (ret) {
-			codec_hevc_free_fbc_buffers(sess, comm);
+			       sess->pixfmt_cap, is_10bit) ||
+	    codec_hevc_use_downsample(sess->pixfmt_cap, is_10bit)) {
+		ret = codec_hevc_alloc_fbc_buffers(sess, comm);
+		if (ret)
 			return ret;
-		}
 	}
 
 	if (core->platform->revision == VDEC_REVISION_GXBB)
@@ -291,24 +254,19 @@ EXPORT_SYMBOL_GPL(codec_hevc_setup_buffers);
 
 void codec_hevc_fill_mmu_map(struct amvdec_session *sess,
 			     struct codec_hevc_common *comm,
-			     struct vb2_buffer *vb)
+			     struct vb2_buffer *vb,
+			     u32 is_10bit)
 {
 	u32 use_mmu = codec_hevc_use_mmu(sess->core->platform->revision,
-					 sess->pixfmt_cap,
-					 sess->bitdepth == 10 ? 1 : 0);
-	u32 size = amvdec_amfbc_size(sess->width, sess->height,
-				     sess->bitdepth == 10 ? 1 : 0,
+					 sess->pixfmt_cap, is_10bit);
+	u32 size = amvdec_amfbc_size(sess->width, sess->height, is_10bit,
 				     use_mmu);
 	u32 nb_pages = size / PAGE_SIZE;
 	u32 *mmu_map = comm->mmu_map_vaddr;
 	u32 first_page;
 	u32 i;
 
-	if (sess->pixfmt_cap == V4L2_PIX_FMT_NV12M)
-		first_page = comm->fbc_buffer_paddr[vb->index] >> PAGE_SHIFT;
-	else
-		first_page = vb2_dma_contig_plane_dma_addr(vb, 0) >> PAGE_SHIFT;
-
+	first_page = comm->fbc_buffer_paddr[vb->index] >> PAGE_SHIFT;
 	for (i = 0; i < nb_pages; ++i)
 		mmu_map[i] = first_page + i;
 }
diff --git a/drivers/staging/media/meson/vdec/codec_hevc_common.h b/drivers/staging/media/meson/vdec/codec_hevc_common.h
index 88e4379ba1ee..5a3c6520940f 100644
--- a/drivers/staging/media/meson/vdec/codec_hevc_common.h
+++ b/drivers/staging/media/meson/vdec/codec_hevc_common.h
@@ -22,9 +22,6 @@ struct codec_hevc_common {
 	void      *fbc_buffer_vaddr[MAX_REF_PIC_NUM];
 	dma_addr_t fbc_buffer_paddr[MAX_REF_PIC_NUM];
 
-	void      *mmu_header_vaddr[MAX_REF_PIC_NUM];
-	dma_addr_t mmu_header_paddr[MAX_REF_PIC_NUM];
-
 	void      *mmu_map_vaddr;
 	dma_addr_t mmu_map_paddr;
 };
@@ -32,14 +29,15 @@ struct codec_hevc_common {
 /* Returns 1 if we must use framebuffer compression */
 static inline int codec_hevc_use_fbc(u32 pixfmt, int is_10bit)
 {
-	/* TOFIX: Handle Amlogic Compressed buffer for 8bit also */
-	return is_10bit;
+	return pixfmt == V4L2_PIX_FMT_YUV420_8BIT ||
+	       pixfmt == V4L2_PIX_FMT_YUV420_10BIT ||
+	       is_10bit;
 }
 
 /* Returns 1 if we are decoding 10-bit but outputting 8-bit NV12 */
 static inline int codec_hevc_use_downsample(u32 pixfmt, int is_10bit)
 {
-	return is_10bit;
+	return pixfmt == V4L2_PIX_FMT_NV12M && is_10bit;
 }
 
 /* Returns 1 if we are decoding using the IOMMU */
@@ -66,6 +64,7 @@ int codec_hevc_setup_buffers(struct amvdec_session *sess,
 
 void codec_hevc_fill_mmu_map(struct amvdec_session *sess,
 			     struct codec_hevc_common *comm,
-			     struct vb2_buffer *vb);
+			     struct vb2_buffer *vb,
+			     u32 is_10bit);
 
 #endif
diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
index a810c50f3a39..e193a2f9de9f 100644
--- a/drivers/staging/media/meson/vdec/codec_vp9.c
+++ b/drivers/staging/media/meson/vdec/codec_vp9.c
@@ -458,12 +458,6 @@ struct codec_vp9 {
 	struct list_head ref_frames_list;
 	u32 frames_num;
 
-	/* In case of downsampling (decoding with FBC but outputting in NV12M),
-	 * we need to allocate additional buffers for FBC.
-	 */
-	void      *fbc_buffer_vaddr[MAX_REF_PIC_NUM];
-	dma_addr_t fbc_buffer_paddr[MAX_REF_PIC_NUM];
-
 	int ref_frame_map[REF_FRAMES];
 	int next_ref_frame_map[REF_FRAMES];
 	struct vp9_frame *frame_refs[REFS_PER_FRAME];
@@ -901,11 +895,8 @@ static void codec_vp9_set_sao(struct amvdec_session *sess,
 		buf_y_paddr =
 		       vb2_dma_contig_plane_dma_addr(vb, 0);
 
-	if (codec_hevc_use_fbc(sess->pixfmt_cap, vp9->is_10bit)) {
-		val = amvdec_read_dos(core, HEVC_SAO_CTRL5) & ~0xff0200;
-		amvdec_write_dos(core, HEVC_SAO_CTRL5, val);
+	if (codec_hevc_use_fbc(sess->pixfmt_cap, vp9->is_10bit))
 		amvdec_write_dos(core, HEVC_CM_BODY_START_ADDR, buf_y_paddr);
-	}
 
 	if (sess->pixfmt_cap == V4L2_PIX_FMT_NV12M) {
 		buf_y_paddr =
@@ -921,7 +912,7 @@ static void codec_vp9_set_sao(struct amvdec_session *sess,
 	if (codec_hevc_use_mmu(core->platform->revision, sess->pixfmt_cap,
 			       vp9->is_10bit)) {
 		amvdec_write_dos(core, HEVC_CM_HEADER_START_ADDR,
-				 vp9->common.mmu_header_paddr[vb->index]);
+				 vb2_dma_contig_plane_dma_addr(vb, 0));
 		/* use HEVC_CM_HEADER_START_ADDR */
 		amvdec_write_dos_bits(core, HEVC_SAO_CTRL5, BIT(10));
 	}
@@ -956,7 +947,8 @@ static void codec_vp9_set_sao(struct amvdec_session *sess,
 		val &= ~0x3;
 		if (!codec_hevc_use_fbc(sess->pixfmt_cap, vp9->is_10bit))
 			val |= BIT(0); /* disable cm compression */
-		/* TOFIX: Handle Amlogic Framebuffer compression */
+		else if (amvdec_is_dst_fbc(sess))
+			val |= BIT(1); /* Disable double write */
 	}
 
 	amvdec_write_dos(core, HEVC_SAO_CTRL1, val);
@@ -1286,7 +1278,8 @@ static void codec_vp9_process_frame(struct amvdec_session *sess)
 	if (codec_hevc_use_mmu(core->platform->revision, sess->pixfmt_cap,
 			       vp9->is_10bit))
 		codec_hevc_fill_mmu_map(sess, &vp9->common,
-					&vp9->cur_frame->vbuf->vb2_buf);
+					&vp9->cur_frame->vbuf->vb2_buf,
+					vp9->is_10bit);
 
 	intra_only = param->p.show_frame ? 0 : param->p.intra_only;
 
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 5/5] media: meson: vdec: handle compressed output pixel format negociation with consumer

[Neil Armstrong]

From: Maxime Jourdan <mjourdan@baylibre.com>

Add the necessary to add support for negociating the compressed buffer
pixel format with the V4L2 M2M consumer, and allocating the right
buffers in this case.

Until a proper mechanism exists to pass a modifier along the pixel format,
only the generic V4L2_PIX_FMT_YUV420_8BIT and V4L2_PIX_FMT_YUV420_10BIT
format are passed in v4l2_pix_format_mplane struct for consumer.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../media/meson/vdec/codec_hevc_common.c      |  1 -
 drivers/staging/media/meson/vdec/vdec.c       | 46 +++++++++++++++++++
 drivers/staging/media/meson/vdec/vdec.h       |  3 ++
 .../staging/media/meson/vdec/vdec_helpers.c   | 23 ++++++++++
 .../staging/media/meson/vdec/vdec_platform.c  |  9 ++--
 5 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/codec_hevc_common.c b/drivers/staging/media/meson/vdec/codec_hevc_common.c
index 73dae40b3319..78fada7b8fa9 100644
--- a/drivers/staging/media/meson/vdec/codec_hevc_common.c
+++ b/drivers/staging/media/meson/vdec/codec_hevc_common.c
@@ -10,7 +10,6 @@
 #include "vdec_helpers.h"
 #include "hevc_regs.h"
 
-#define MMU_COMPRESS_HEADER_SIZE 0x48000
 #define MMU_MAP_SIZE 0x4800
 
 const u16 vdec_hevc_parser_cmd[] = {
diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 3040136ceb77..9fb075f69cb9 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -192,6 +192,7 @@ static int vdec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
 {
 	struct amvdec_session *sess = vb2_get_drv_priv(q);
 	u32 output_size = amvdec_get_output_size(sess);
+	u32 revision = sess->core->platform->revision;
 
 	if (*num_planes) {
 		switch (q->type) {
@@ -215,6 +216,12 @@ static int vdec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
 				    sizes[2] < output_size / 4)
 					return -EINVAL;
 				break;
+			case V4L2_PIX_FMT_YUV420_8BIT:
+			case V4L2_PIX_FMT_YUV420_10BIT:
+				if (*num_planes != 1 ||
+				    sizes[0] < MMU_COMPRESS_HEADER_SIZE)
+					return -EINVAL;
+				break;
 			default:
 				return -EINVAL;
 			}
@@ -244,6 +251,24 @@ static int vdec_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
 			sizes[2] = output_size / 4;
 			*num_planes = 3;
 			break;
+		case V4L2_PIX_FMT_YUV420_8BIT:
+			if (revision >= VDEC_REVISION_G12A)
+				sizes[0] = MMU_COMPRESS_HEADER_SIZE;
+			else
+				sizes[0] = amvdec_amfbc_size(sess->width,
+							     sess->height,
+							     0, 0);
+			*num_planes = 1;
+			break;
+		case V4L2_PIX_FMT_YUV420_10BIT:
+			if (revision >= VDEC_REVISION_G12A)
+				sizes[0] = MMU_COMPRESS_HEADER_SIZE;
+			else
+				sizes[0] = amvdec_amfbc_size(sess->width,
+							     sess->height,
+							     1, 0);
+			*num_planes = 1;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -496,6 +521,7 @@ vdec_try_fmt_common(struct amvdec_session *sess, u32 size,
 	struct v4l2_plane_pix_format *pfmt = pixmp->plane_fmt;
 	const struct amvdec_format *fmts = sess->core->platform->formats;
 	const struct amvdec_format *fmt_out = NULL;
+	u32 revision = sess->core->platform->revision;
 	u32 output_size = 0;
 
 	memset(pfmt[0].reserved, 0, sizeof(pfmt[0].reserved));
@@ -548,6 +574,26 @@ vdec_try_fmt_common(struct amvdec_session *sess, u32 size,
 			pfmt[2].sizeimage = output_size / 2;
 			pfmt[2].bytesperline = ALIGN(pixmp->width, 32) / 2;
 			pixmp->num_planes = 3;
+		} else if (pixmp->pixelformat == V4L2_PIX_FMT_YUV420_8BIT) {
+			if (revision >= VDEC_REVISION_G12A) {
+				pfmt[0].sizeimage = MMU_COMPRESS_HEADER_SIZE;
+			} else {
+				pfmt[0].sizeimage =
+					amvdec_amfbc_size(pixmp->width,
+							  pixmp->height, 0, 0);
+				pfmt[0].bytesperline = pixmp->width;
+			}
+			pixmp->num_planes = 1;
+		} else if (pixmp->pixelformat == V4L2_PIX_FMT_YUV420_10BIT) {
+			if (revision >= VDEC_REVISION_G12A) {
+				pfmt[0].sizeimage = MMU_COMPRESS_HEADER_SIZE;
+			} else {
+				pfmt[0].sizeimage =
+					amvdec_amfbc_size(pixmp->width,
+							  pixmp->height, 1, 0);
+				pfmt[0].bytesperline = pixmp->width;
+			}
+			pixmp->num_planes = 1;
 		}
 	}
 
diff --git a/drivers/staging/media/meson/vdec/vdec.h b/drivers/staging/media/meson/vdec/vdec.h
index e3e4af73447a..1412054a70c4 100644
--- a/drivers/staging/media/meson/vdec/vdec.h
+++ b/drivers/staging/media/meson/vdec/vdec.h
@@ -17,6 +17,9 @@
 
 #include "vdec_platform.h"
 
+/* MMU header size for codecs using the IOMMU + FBC */
+#define MMU_COMPRESS_HEADER_SIZE 0x48000
+
 /* 32 buffers in 3-plane YUV420 */
 #define MAX_CANVAS (32 * 3)
 
diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.c b/drivers/staging/media/meson/vdec/vdec_helpers.c
index 320cac1ed03e..7166605b89ae 100644
--- a/drivers/staging/media/meson/vdec/vdec_helpers.c
+++ b/drivers/staging/media/meson/vdec/vdec_helpers.c
@@ -299,6 +299,22 @@ static void dst_buf_done(struct amvdec_session *sess,
 		vbuf->vb2_buf.planes[1].bytesused = output_size / 4;
 		vbuf->vb2_buf.planes[2].bytesused = output_size / 4;
 		break;
+	case V4L2_PIX_FMT_YUV420_8BIT:
+		if (sess->core->platform->revision >= VDEC_REVISION_G12A)
+			vbuf->vb2_buf.planes[0].bytesused =
+				MMU_COMPRESS_HEADER_SIZE;
+		else
+			vbuf->vb2_buf.planes[0].bytesused =
+			   amvdec_amfbc_size(sess->width, sess->height, 0, 0);
+		break;
+	case V4L2_PIX_FMT_YUV420_10BIT:
+		if (sess->core->platform->revision >= VDEC_REVISION_G12A)
+			vbuf->vb2_buf.planes[0].bytesused =
+				MMU_COMPRESS_HEADER_SIZE;
+		else
+			vbuf->vb2_buf.planes[0].bytesused =
+			   amvdec_amfbc_size(sess->width, sess->height, 1, 0);
+		break;
 	}
 
 	vbuf->vb2_buf.timestamp = timestamp;
@@ -478,6 +494,13 @@ void amvdec_src_change(struct amvdec_session *sess, u32 width,
 	sess->status = STATUS_NEEDS_RESUME;
 	sess->bitdepth = bitdepth;
 
+	if (sess->pixfmt_cap == V4L2_PIX_FMT_YUV420_8BIT &&
+	    bitdepth == 10)
+		sess->pixfmt_cap = V4L2_PIX_FMT_YUV420_10BIT;
+	else if (sess->pixfmt_cap == V4L2_PIX_FMT_YUV420_10BIT &&
+		 bitdepth == 8)
+		sess->pixfmt_cap = V4L2_PIX_FMT_YUV420_8BIT;
+
 	dev_dbg(sess->core->dev, "Res. changed (%ux%u), DPB %u, bitdepth %u\n",
 		width, height, dpb_size, bitdepth);
 	v4l2_event_queue_fh(&sess->fh, &ev);
diff --git a/drivers/staging/media/meson/vdec/vdec_platform.c b/drivers/staging/media/meson/vdec/vdec_platform.c
index eabbebab2da2..efc090d2a3bb 100644
--- a/drivers/staging/media/meson/vdec/vdec_platform.c
+++ b/drivers/staging/media/meson/vdec/vdec_platform.c
@@ -61,7 +61,8 @@ static const struct amvdec_format vdec_formats_gxl[] = {
 		.vdec_ops = &vdec_hevc_ops,
 		.codec_ops = &codec_vp9_ops,
 		.firmware_path = "meson/vdec/gxl_vp9.bin",
-		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, 0 },
+		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, V4L2_PIX_FMT_YUV420_8BIT,
+				 V4L2_PIX_FMT_YUV420_10BIT, 0 },
 		.flags = V4L2_FMT_FLAG_COMPRESSED |
 			 V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
@@ -149,7 +150,8 @@ static const struct amvdec_format vdec_formats_g12a[] = {
 		.vdec_ops = &vdec_hevc_ops,
 		.codec_ops = &codec_vp9_ops,
 		.firmware_path = "meson/vdec/g12a_vp9.bin",
-		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, 0 },
+		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, V4L2_PIX_FMT_YUV420_8BIT,
+				 V4L2_PIX_FMT_YUV420_10BIT, 0 },
 		.flags = V4L2_FMT_FLAG_COMPRESSED |
 			 V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
@@ -199,7 +201,8 @@ static const struct amvdec_format vdec_formats_sm1[] = {
 		.vdec_ops = &vdec_hevc_ops,
 		.codec_ops = &codec_vp9_ops,
 		.firmware_path = "meson/vdec/sm1_vp9_mmu.bin",
-		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, 0 },
+		.pixfmts_cap = { V4L2_PIX_FMT_NV12M, V4L2_PIX_FMT_YUV420_8BIT,
+				 V4L2_PIX_FMT_YUV420_10BIT, 0 },
 		.flags = V4L2_FMT_FLAG_COMPRESSED |
 			 V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic