[libcamera-devel,v2,2/2] pipeline: rkisp1: Add support for YUV422
diff mbox series

Message ID 20220719074013.846726-2-paul.elder@ideasonboard.com
State New
Headers show
Series
  • [libcamera-devel,v2,1/2] pipeline: rkisp1: Add output support for YUV420 and YVU420
Related show

Commit Message

Paul Elder July 19, 2022, 7:40 a.m. UTC
YUV422 is already supported as an output format by the rkisp1 driver.
Add them to the pipeline handler to support them in libcamera as well.

YVU422 is also supported by the driver, but there only exists a
multiplanar V4L2 format, which libcamera is currently unable to map to
from the libcamera YVU422 format. This will be fixed later.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2:
- remove YVU422
- reorder formats
---
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi July 19, 2022, 10:05 a.m. UTC | #1
Hi Paul,

On Tue, Jul 19, 2022 at 04:40:13PM +0900, Paul Elder via libcamera-devel wrote:
> YUV422 is already supported as an output format by the rkisp1 driver.
> Add them to the pipeline handler to support them in libcamera as well.
>
> YVU422 is also supported by the driver, but there only exists a
> multiplanar V4L2 format, which libcamera is currently unable to map to
> from the libcamera YVU422 format. This will be fixed later.

That's peculiar!

The driver supports (for both the main and self path)

V4L2_PIX_FMT_YUV422P,
V4L2_PIX_FMT_YVU422M,

so the YUV component ordering is supported throught the contiguous planes
variant, while the YVU permutation goes through non-contiguous one.

Is there a reason ? Is the driver correct I wonder ?

>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---
> Changes in v2:
> - remove YVU422
> - reorder formats
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index c070929d..856554b3 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -209,7 +209,7 @@ void RkISP1Path::stop()
>  namespace {
>  constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
>  constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> -constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> +constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
>  	formats::YUYV,
>  	formats::NV16,
>  	formats::NV61,
> @@ -217,13 +217,14 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
>  	formats::NV12,
>  	formats::YUV420,
>  	formats::YVU420,
> +	formats::YUV422,
>  	formats::R8,
>  	/* \todo Add support for RAW formats. */
>  };
>
>  constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
>  constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
> -constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> +constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
>  	formats::YUYV,
>  	formats::NV16,
>  	formats::NV61,
> @@ -231,6 +232,7 @@ constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
>  	formats::NV12,
>  	formats::YUV420,
>  	formats::YVU420,
> +	formats::YUV422,
>  	formats::R8,
>  	formats::RGB565,
>  	formats::XRGB8888,
> --
> 2.30.2
>
Nicolas Dufresne via libcamera-devel July 19, 2022, 11:26 a.m. UTC | #2
Hi Jacopo,

On Tue, Jul 19, 2022 at 12:05:00PM +0200, Jacopo Mondi wrote:
> Hi Paul,
> 
> On Tue, Jul 19, 2022 at 04:40:13PM +0900, Paul Elder via libcamera-devel wrote:
> > YUV422 is already supported as an output format by the rkisp1 driver.
> > Add them to the pipeline handler to support them in libcamera as well.
> >
> > YVU422 is also supported by the driver, but there only exists a
> > multiplanar V4L2 format, which libcamera is currently unable to map to
> > from the libcamera YVU422 format. This will be fixed later.
> 
> That's peculiar!
> 
> The driver supports (for both the main and self path)
> 
> V4L2_PIX_FMT_YUV422P,
> V4L2_PIX_FMT_YVU422M,
> 
> so the YUV component ordering is supported throught the contiguous planes
> variant, while the YVU permutation goes through non-contiguous one.
> 
> Is there a reason ? Is the driver correct I wonder ?

It's not the driver; it's that YVU422P doesn't exist as a V4L2 format.


Paul

> 
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - remove YVU422
> > - reorder formats
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > index c070929d..856554b3 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > @@ -209,7 +209,7 @@ void RkISP1Path::stop()
> >  namespace {
> >  constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> >  constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> > -constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > +constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
> >  	formats::YUYV,
> >  	formats::NV16,
> >  	formats::NV61,
> > @@ -217,13 +217,14 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> >  	formats::NV12,
> >  	formats::YUV420,
> >  	formats::YVU420,
> > +	formats::YUV422,
> >  	formats::R8,
> >  	/* \todo Add support for RAW formats. */
> >  };
> >
> >  constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> >  constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
> > -constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > +constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
> >  	formats::YUYV,
> >  	formats::NV16,
> >  	formats::NV61,
> > @@ -231,6 +232,7 @@ constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> >  	formats::NV12,
> >  	formats::YUV420,
> >  	formats::YVU420,
> > +	formats::YUV422,
> >  	formats::R8,
> >  	formats::RGB565,
> >  	formats::XRGB8888,
> > --
> > 2.30.2
> >
Jacopo Mondi July 19, 2022, 12:40 p.m. UTC | #3
Hi PAul

On Tue, Jul 19, 2022 at 08:26:14PM +0900, paul.elder@ideasonboard.com wrote:
> Hi Jacopo,
>
> On Tue, Jul 19, 2022 at 12:05:00PM +0200, Jacopo Mondi wrote:
> > Hi Paul,
> >
> > On Tue, Jul 19, 2022 at 04:40:13PM +0900, Paul Elder via libcamera-devel wrote:
> > > YUV422 is already supported as an output format by the rkisp1 driver.
> > > Add them to the pipeline handler to support them in libcamera as well.
> > >
> > > YVU422 is also supported by the driver, but there only exists a
> > > multiplanar V4L2 format, which libcamera is currently unable to map to
> > > from the libcamera YVU422 format. This will be fixed later.
> >
> > That's peculiar!
> >
> > The driver supports (for both the main and self path)
> >
> > V4L2_PIX_FMT_YUV422P,
> > V4L2_PIX_FMT_YVU422M,
> >
> > so the YUV component ordering is supported throught the contiguous planes
> > variant, while the YVU permutation goes through non-contiguous one.
> >
> > Is there a reason ? Is the driver correct I wonder ?
>
> It's not the driver; it's that YVU422P doesn't exist as a V4L2 format.
>

Ok, the real question is then what the HW supports, contiguous or
non-contiguous ? The fact the YUV422P varian is used when the YUV422M
exists makes me thing also YVU422 should be reported through the 'P'
variant.

Should the format be added to the videodev2.h header ?
>
> Paul
>
> >
> > >
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > >
> > > ---
> > > Changes in v2:
> > > - remove YVU422
> > > - reorder formats
> > > ---
> > >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > index c070929d..856554b3 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > @@ -209,7 +209,7 @@ void RkISP1Path::stop()
> > >  namespace {
> > >  constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> > >  constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> > > -constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > > +constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
> > >  	formats::YUYV,
> > >  	formats::NV16,
> > >  	formats::NV61,
> > > @@ -217,13 +217,14 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > >  	formats::NV12,
> > >  	formats::YUV420,
> > >  	formats::YVU420,
> > > +	formats::YUV422,
> > >  	formats::R8,
> > >  	/* \todo Add support for RAW formats. */
> > >  };
> > >
> > >  constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> > >  constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
> > > -constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > > +constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
> > >  	formats::YUYV,
> > >  	formats::NV16,
> > >  	formats::NV61,
> > > @@ -231,6 +232,7 @@ constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > >  	formats::NV12,
> > >  	formats::YUV420,
> > >  	formats::YVU420,
> > > +	formats::YUV422,
> > >  	formats::R8,
> > >  	formats::RGB565,
> > >  	formats::XRGB8888,
> > > --
> > > 2.30.2
> > >
Nicolas Dufresne via libcamera-devel July 19, 2022, 1:19 p.m. UTC | #4
Hi Jacopo,

On Tue, Jul 19, 2022 at 02:40:33PM +0200, Jacopo Mondi wrote:
> Hi PAul
> 
> On Tue, Jul 19, 2022 at 08:26:14PM +0900, paul.elder@ideasonboard.com wrote:
> > Hi Jacopo,
> >
> > On Tue, Jul 19, 2022 at 12:05:00PM +0200, Jacopo Mondi wrote:
> > > Hi Paul,
> > >
> > > On Tue, Jul 19, 2022 at 04:40:13PM +0900, Paul Elder via libcamera-devel wrote:
> > > > YUV422 is already supported as an output format by the rkisp1 driver.
> > > > Add them to the pipeline handler to support them in libcamera as well.
> > > >
> > > > YVU422 is also supported by the driver, but there only exists a
> > > > multiplanar V4L2 format, which libcamera is currently unable to map to
> > > > from the libcamera YVU422 format. This will be fixed later.
> > >
> > > That's peculiar!
> > >
> > > The driver supports (for both the main and self path)
> > >
> > > V4L2_PIX_FMT_YUV422P,
> > > V4L2_PIX_FMT_YVU422M,
> > >
> > > so the YUV component ordering is supported throught the contiguous planes
> > > variant, while the YVU permutation goes through non-contiguous one.
> > >
> > > Is there a reason ? Is the driver correct I wonder ?
> >
> > It's not the driver; it's that YVU422P doesn't exist as a V4L2 format.
> >
> 
> Ok, the real question is then what the HW supports, contiguous or
> non-contiguous ? The fact the YUV422P varian is used when the YUV422M

Both.

> exists makes me thing also YVU422 should be reported through the 'P'
> variant.
> 
> Should the format be added to the videodev2.h header ?

Maybe? The i.MX3x IPU dma and some Marvell display controllers define
their own YVU422P formats. But also not many drivers seem to support
YVU422M anyway? Although maybe that doesn't mean much, since YUV422M
isn't very popular even though YUV422P is.


Paul

> >
> > Paul
> >
> > >
> > > >
> > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > >
> > > > ---
> > > > Changes in v2:
> > > > - remove YVU422
> > > > - reorder formats
> > > > ---
> > > >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > index c070929d..856554b3 100644
> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > @@ -209,7 +209,7 @@ void RkISP1Path::stop()
> > > >  namespace {
> > > >  constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> > > >  constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> > > > -constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > > > +constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
> > > >  	formats::YUYV,
> > > >  	formats::NV16,
> > > >  	formats::NV61,
> > > > @@ -217,13 +217,14 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > > >  	formats::NV12,
> > > >  	formats::YUV420,
> > > >  	formats::YVU420,
> > > > +	formats::YUV422,
> > > >  	formats::R8,
> > > >  	/* \todo Add support for RAW formats. */
> > > >  };
> > > >
> > > >  constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> > > >  constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
> > > > -constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > > > +constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
> > > >  	formats::YUYV,
> > > >  	formats::NV16,
> > > >  	formats::NV61,
> > > > @@ -231,6 +232,7 @@ constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > > >  	formats::NV12,
> > > >  	formats::YUV420,
> > > >  	formats::YVU420,
> > > > +	formats::YUV422,
> > > >  	formats::R8,
> > > >  	formats::RGB565,
> > > >  	formats::XRGB8888,
> > > > --
> > > > 2.30.2
> > > >
Laurent Pinchart July 19, 2022, 10:41 p.m. UTC | #5
Hi,

On Tue, Jul 19, 2022 at 10:19:29PM +0900, Paul Elder via libcamera-devel wrote:
> On Tue, Jul 19, 2022 at 02:40:33PM +0200, Jacopo Mondi wrote:
> > On Tue, Jul 19, 2022 at 08:26:14PM +0900, paul.elder@ideasonboard.com wrote:
> > > On Tue, Jul 19, 2022 at 12:05:00PM +0200, Jacopo Mondi wrote:
> > > > On Tue, Jul 19, 2022 at 04:40:13PM +0900, Paul Elder via libcamera-devel wrote:
> > > > > YUV422 is already supported as an output format by the rkisp1 driver.
> > > > > Add them to the pipeline handler to support them in libcamera as well.
> > > > >
> > > > > YVU422 is also supported by the driver, but there only exists a
> > > > > multiplanar V4L2 format, which libcamera is currently unable to map to
> > > > > from the libcamera YVU422 format. This will be fixed later.

Why is that ?

> > > > That's peculiar!
> > > >
> > > > The driver supports (for both the main and self path)
> > > >
> > > > V4L2_PIX_FMT_YUV422P,
> > > > V4L2_PIX_FMT_YVU422M,
> > > >
> > > > so the YUV component ordering is supported throught the contiguous planes
> > > > variant, while the YVU permutation goes through non-contiguous one.
> > > >
> > > > Is there a reason ? Is the driver correct I wonder ?
> > >
> > > It's not the driver; it's that YVU422P doesn't exist as a V4L2 format.
> > 
> > Ok, the real question is then what the HW supports, contiguous or
> > non-contiguous ? The fact the YUV422P varian is used when the YUV422M
> 
> Both.
> 
> > exists makes me thing also YVU422 should be reported through the 'P'
> > variant.
> > 
> > Should the format be added to the videodev2.h header ?
> 
> Maybe? The i.MX3x IPU dma and some Marvell display controllers define
> their own YVU422P formats. But also not many drivers seem to support
> YVU422M anyway? Although maybe that doesn't mean much, since YUV422M
> isn't very popular even though YUV422P is.

We could extend V4L2 with missing contiguous planar formats indeed, but
I'd rather see this contiguous/non-contiguous separation go away in
V4L2. Unless there's a strong need for V4L2_PIX_FMT_YVU422P, I'd leave
it out for now.

> > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > > >
> > > > > ---
> > > > > Changes in v2:
> > > > > - remove YVU422
> > > > > - reorder formats
> > > > > ---
> > > > >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 6 ++++--
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > > index c070929d..856554b3 100644
> > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > > > > @@ -209,7 +209,7 @@ void RkISP1Path::stop()
> > > > >  namespace {
> > > > >  constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> > > > >  constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> > > > > -constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > > > > +constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
> > > > >  	formats::YUYV,
> > > > >  	formats::NV16,
> > > > >  	formats::NV61,
> > > > > @@ -217,13 +217,14 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
> > > > >  	formats::NV12,
> > > > >  	formats::YUV420,
> > > > >  	formats::YVU420,
> > > > > +	formats::YUV422,
> > > > >  	formats::R8,
> > > > >  	/* \todo Add support for RAW formats. */
> > > > >  };
> > > > >
> > > > >  constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> > > > >  constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
> > > > > -constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > > > > +constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
> > > > >  	formats::YUYV,
> > > > >  	formats::NV16,
> > > > >  	formats::NV61,
> > > > > @@ -231,6 +232,7 @@ constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
> > > > >  	formats::NV12,
> > > > >  	formats::YUV420,
> > > > >  	formats::YVU420,
> > > > > +	formats::YUV422,
> > > > >  	formats::R8,
> > > > >  	formats::RGB565,
> > > > >  	formats::XRGB8888,

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index c070929d..856554b3 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -209,7 +209,7 @@  void RkISP1Path::stop()
 namespace {
 constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
 constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
-constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
+constexpr std::array<PixelFormat, 9> RKISP1_RSZ_MP_FORMATS{
 	formats::YUYV,
 	formats::NV16,
 	formats::NV61,
@@ -217,13 +217,14 @@  constexpr std::array<PixelFormat, 8> RKISP1_RSZ_MP_FORMATS{
 	formats::NV12,
 	formats::YUV420,
 	formats::YVU420,
+	formats::YUV422,
 	formats::R8,
 	/* \todo Add support for RAW formats. */
 };
 
 constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
 constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
-constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
+constexpr std::array<PixelFormat, 11> RKISP1_RSZ_SP_FORMATS{
 	formats::YUYV,
 	formats::NV16,
 	formats::NV61,
@@ -231,6 +232,7 @@  constexpr std::array<PixelFormat, 10> RKISP1_RSZ_SP_FORMATS{
 	formats::NV12,
 	formats::YUV420,
 	formats::YVU420,
+	formats::YUV422,
 	formats::R8,
 	formats::RGB565,
 	formats::XRGB8888,