[PATCH 0/9] media: sun6i-csi/isp: Implement MC I/O support

[PATCH 0/9] media: sun6i-csi/isp: Implement MC I/O support

[Paul Kocialkowski]

This series is a follow-up to Adam Pigg's "suns6-csi changes to support
libcamera" series, with the same purpose.

As discussed in the original thread, it takes a different approach
and ensures input/output format matching is maintained without
regression.

New v4l2 format info is also added about unusual formats used by the
driver so that no specific logic is required to handle them.

The same functionality is also added to the sun6i-isp driver.

Paul Kocialkowski (9):
  media: v4l2: Add RGB565X pixel format to v4l2 format info
  media: v4l2: Add NV12_16L16 pixel format to v4l2 format info
  media: v4l2: Introduce compressed pixel encoding definition and helper
  media: v4l2: Add JPEG pixel format to v4l2 format info
  media: sun6i-csi: capture: Rework and separate format validation
  media: sun6i-csi: capture: Implement MC I/O with extended enum_fmt
  media: sun6i-csi: capture: Implement enum_framesizes
  media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt
  media: sun6i-isp: capture: Implement enum_framesizes

 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 157 ++++++++++++------
 drivers/media/v4l2-core/v4l2-common.c         |   6 +
 .../media/sunxi/sun6i-isp/sun6i_isp_capture.c |  35 +++-
 include/media/v4l2-common.h                   |   7 +
 4 files changed, 154 insertions(+), 51 deletions(-)

-- 
2.39.2

[PATCH 1/9] media: v4l2: Add RGB565X pixel format to v4l2 format info

[Paul Kocialkowski]

Represent the RGB565X pixel format in the v4l2 format info table.
This is a big-endian variant of the already present RGB565 format,
which has the same characteristics.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 40f56e044640..3d044b31caad 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -250,6 +250,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		{ .format = V4L2_PIX_FMT_ABGR32,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_BGRA32,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_RGB565X, .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 
-- 
2.39.2

Re: [PATCH 1/9] media: v4l2: Add RGB565X pixel format to v4l2 format info

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:20 CET je Paul Kocialkowski napisal(a):
> Represent the RGB565X pixel format in the v4l2 format info table.
> This is a big-endian variant of the already present RGB565 format,
> which has the same characteristics.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 1/9] media: v4l2: Add RGB565X pixel format to v4l2 format info

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:20PM +0100, Paul Kocialkowski wrote:
> Represent the RGB565X pixel format in the v4l2 format info table.
> This is a big-endian variant of the already present RGB565 format,
> which has the same characteristics.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 40f56e044640..3d044b31caad 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -250,6 +250,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_ABGR32,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_BGRA32,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> +		{ .format = V4L2_PIX_FMT_RGB565X, .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  

-- 
Regards,

Laurent Pinchart

[PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Paul Kocialkowski]

Represent the NV12_16L16 pixel format in the v4l2 format info table.
This is a 16x16 tiled version of NV12.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 3d044b31caad..5101989716aa 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		/* Tiled YUV formats */
 		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
 		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV12_16L16,	.pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2,
+		  .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },
 
 		/* YUV planar formats, non contiguous variant */
 		{ .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
-- 
2.39.2

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:21 CET je Paul Kocialkowski napisal(a):
> Represent the NV12_16L16 pixel format in the v4l2 format info table.
> This is a 16x16 tiled version of NV12.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:21PM +0100, Paul Kocialkowski wrote:
> Represent the NV12_16L16 pixel format in the v4l2 format info table.
> This is a 16x16 tiled version of NV12.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 3d044b31caad..5101989716aa 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		/* Tiled YUV formats */
>  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
>  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> +		{ .format = V4L2_PIX_FMT_NV12_16L16,	.pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2,
> +		  .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },

Not necessarily related to this patch, but I'm a bit puzzled by why
V4L2_PIX_FMT_NV12_4L4 doesn't list block sizes.

>  
>  		/* YUV planar formats, non contiguous variant */
>  		{ .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Nicolas Dufresne]

Le samedi 25 mars 2023 à 23:01 +0200, Laurent Pinchart a écrit :
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Fri, Mar 24, 2023 at 04:12:21PM +0100, Paul Kocialkowski wrote:
> > Represent the NV12_16L16 pixel format in the v4l2 format info table.
> > This is a 16x16 tiled version of NV12.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 3d044b31caad..5101989716aa 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> >  		/* Tiled YUV formats */
> >  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> >  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> > +		{ .format = V4L2_PIX_FMT_NV12_16L16,	.pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2,
> > +		  .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },
> 
> Not necessarily related to this patch, but I'm a bit puzzled by why
> V4L2_PIX_FMT_NV12_4L4 doesn't list block sizes.

It looks like Ezequiel introduced that initially, but didn't introduce any tiled
format, as a side effect, we missed it and no one ever used it.

In practice, its not dramatic, since most of the time, the alignment needed is
bigger then the block (specially with only 4x4 tiles), but we should certainly
fix that, thanks for spotting.

> 
> >  
> >  		/* YUV planar formats, non contiguous variant */
> >  		{ .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
> 

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Laurent Pinchart]

Hi Nicolas,

On Fri, Mar 31, 2023 at 02:54:20PM -0400, Nicolas Dufresne wrote:
> Le samedi 25 mars 2023 à 23:01 +0200, Laurent Pinchart a écrit :
> > On Fri, Mar 24, 2023 at 04:12:21PM +0100, Paul Kocialkowski wrote:
> > > Represent the NV12_16L16 pixel format in the v4l2 format info table.
> > > This is a 16x16 tiled version of NV12.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-common.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > > index 3d044b31caad..5101989716aa 100644
> > > --- a/drivers/media/v4l2-core/v4l2-common.c
> > > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > > @@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> > >  		/* Tiled YUV formats */
> > >  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> > >  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> > > +		{ .format = V4L2_PIX_FMT_NV12_16L16,	.pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2,
> > > +		  .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },
> > 
> > Not necessarily related to this patch, but I'm a bit puzzled by why
> > V4L2_PIX_FMT_NV12_4L4 doesn't list block sizes.
> 
> It looks like Ezequiel introduced that initially, but didn't introduce any tiled
> format, as a side effect, we missed it and no one ever used it.
> 
> In practice, its not dramatic, since most of the time, the alignment needed is
> bigger then the block (specially with only 4x4 tiles), but we should certainly
> fix that, thanks for spotting.

Just to make sure this won't fall through the cracks, will you send a
patch ?

> > >  
> > >  		/* YUV planar formats, non contiguous variant */
> > >  		{ .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Nicolas Dufresne]

Hi Laurent,

Le mer. 5 avr. 2023, 00 h 21, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> a écrit :
> Hi Nicolas,
> 
> On Fri, Mar 31, 2023 at 02:54:20PM -0400, Nicolas Dufresne wrote:
> > Le samedi 25 mars 2023 à 23:01 +0200, Laurent Pinchart a écrit :
> > > On Fri, Mar 24, 2023 at 04:12:21PM +0100, Paul Kocialkowski wrote:
> > > > Represent the NV12_16L16 pixel format in the v4l2 format info table.
> > > > This is a 16x16 tiled version of NV12.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > ---
> > > >  drivers/media/v4l2-core/v4l2-common.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-
core/v4l2-common.c
> > > > index 3d044b31caad..5101989716aa 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-common.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > > > @@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32
format)
> > > >           /* Tiled YUV formats */
> > > >           { .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc =
V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 },
.hdiv = 2, .vdiv = 2 },
> > > >           { .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc =
V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 },
.hdiv = 2, .vdiv = 2 },
> > > > +         { .format = V4L2_PIX_FMT_NV12_16L16,    .pixel_enc =
V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 },
.hdiv = 2, .vdiv = 2,
> > > > +           .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },
> > > 
> > > Not necessarily related to this patch, but I'm a bit puzzled by why
> > > V4L2_PIX_FMT_NV12_4L4 doesn't list block sizes.
> > 
> > It looks like Ezequiel introduced that initially, but didn't introduce any
tiled
> > format, as a side effect, we missed it and no one ever used it.
> > 
> > In practice, its not dramatic, since most of the time, the alignment needed
is
> > bigger then the block (specially with only 4x4 tiles), but we should
certainly
> > fix that, thanks for spotting.
> 
> Just to make sure this won't fall through the cracks, will you send a
> patch ?

I didn't had any immediate plan to fix it (lack of time, absence of related
bugs). Andrzej, do you happen to have some time ? The SUNXI pixel format should
have that too, along with some NXP format.

regards,
Nicolas
> 
> > > >  
> > > >           /* YUV planar formats, non contiguous variant */
> > > >           { .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 2/9] media: v4l2: Add NV12_16L16 pixel format to v4l2 format info

[Nicolas Dufresne]

Le vendredi 24 mars 2023 à 16:12 +0100, Paul Kocialkowski a écrit :
> Represent the NV12_16L16 pixel format in the v4l2 format info table.
> This is a 16x16 tiled version of NV12.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/v4l2-core/v4l2-common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 3d044b31caad..5101989716aa 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -280,6 +280,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		/* Tiled YUV formats */
>  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
>  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .hdiv = 2, .vdiv = 2 },
> +		{ .format = V4L2_PIX_FMT_NV12_16L16,	.pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2,
> +		  .block_w = { 16, 16, 0, 0 }, .block_h = { 16, 16, 0, 0 } },
>  
>  		/* YUV planar formats, non contiguous variant */
>  		{ .format = V4L2_PIX_FMT_YUV420M, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },

[PATCH 3/9] media: v4l2: Introduce compressed pixel encoding definition and helper

[Paul Kocialkowski]

Some pixel formats (such as JPEG) have their data compressed and
encoded in a specific way, which is not directly YUV, RGB or Bayer.

Add a new definition and helper for compressed pixel encoding to
represent this situation.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 include/media/v4l2-common.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1bdaea248089..37554bc10e2a 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -465,12 +465,14 @@ int v4l2_s_parm_cap(struct video_device *vdev,
  * @V4L2_PIXEL_ENC_YUV:		Pixel encoding is YUV
  * @V4L2_PIXEL_ENC_RGB:		Pixel encoding is RGB
  * @V4L2_PIXEL_ENC_BAYER:	Pixel encoding is Bayer
+ * @V4L2_PIXEL_ENC_COMPRESSED:	Pixel encoding is compressed
  */
 enum v4l2_pixel_encoding {
 	V4L2_PIXEL_ENC_UNKNOWN = 0,
 	V4L2_PIXEL_ENC_YUV = 1,
 	V4L2_PIXEL_ENC_RGB = 2,
 	V4L2_PIXEL_ENC_BAYER = 3,
+	V4L2_PIXEL_ENC_COMPRESSED = 4,
 };
 
 /**
@@ -512,6 +514,11 @@ static inline bool v4l2_is_format_bayer(const struct v4l2_format_info *f)
 	return f && f->pixel_enc == V4L2_PIXEL_ENC_BAYER;
 }
 
+static inline bool v4l2_is_format_compressed(const struct v4l2_format_info *f)
+{
+	return f && f->pixel_enc == V4L2_PIXEL_ENC_COMPRESSED;
+}
+
 const struct v4l2_format_info *v4l2_format_info(u32 format);
 void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
 				    const struct v4l2_frmsize_stepwise *frmsize);
-- 
2.39.2

Re: [PATCH 3/9] media: v4l2: Introduce compressed pixel encoding definition and helper

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:22 CET je Paul Kocialkowski napisal(a):
> Some pixel formats (such as JPEG) have their data compressed and
> encoded in a specific way, which is not directly YUV, RGB or Bayer.
> 
> Add a new definition and helper for compressed pixel encoding to
> represent this situation.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

[Paul Kocialkowski]

Represent the JPEG pixel format in the v4l2 format info table.

Note that the bpp is set to 1 which is not a proper way to estimate
the needed buffer size for a given JPEG image. However the compression
ratios of JPEG typically allow fitting the image in a smaller size,
even though extra metadata could increase the total size by an
arbitrary amount. Thus it is not a perfectly safe way to calculate the
size of a JPEG buffer for given dimensions but it still provides a
reasonable approach.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 5101989716aa..8b2f201a8918 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -317,6 +317,9 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		{ .format = V4L2_PIX_FMT_SGBRG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_SGRBG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_SRGGB12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+
+		/* Compressed formats */
+		{ .format = V4L2_PIX_FMT_JPEG,	.pixel_enc = V4L2_PIXEL_ENC_COMPRESSED, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 	};
 	unsigned int i;
 
-- 
2.39.2

Re: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:23 CET je Paul Kocialkowski napisal(a):
> Represent the JPEG pixel format in the v4l2 format info table.
> 
> Note that the bpp is set to 1 which is not a proper way to estimate
> the needed buffer size for a given JPEG image. However the compression
> ratios of JPEG typically allow fitting the image in a smaller size,
> even though extra metadata could increase the total size by an
> arbitrary amount. Thus it is not a perfectly safe way to calculate the
> size of a JPEG buffer for given dimensions but it still provides a
> reasonable approach.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

[Nicolas Dufresne]

Le vendredi 24 mars 2023 à 16:12 +0100, Paul Kocialkowski a écrit :
> Represent the JPEG pixel format in the v4l2 format info table.
> 
> Note that the bpp is set to 1 which is not a proper way to estimate
> the needed buffer size for a given JPEG image. However the compression
> ratios of JPEG typically allow fitting the image in a smaller size,
> even though extra metadata could increase the total size by an
> arbitrary amount. Thus it is not a perfectly safe way to calculate the
> size of a JPEG buffer for given dimensions but it still provides a
> reasonable approach.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 5101989716aa..8b2f201a8918 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -317,6 +317,9 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_SGBRG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_SGRBG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_SRGGB12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> +
> +		/* Compressed formats */
> +		{ .format = V4L2_PIX_FMT_JPEG,	.pixel_enc = V4L2_PIXEL_ENC_COMPRESSED, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },

This is ackward, at best. Guesstimating the compressed buffer size is a valid
use case, but this table and the related helper function seems badly suited.
When I look at the stateless decoders that do that, they take into account the
fact that the compression can handle different bit depth, and difference
subsampling (choma idc). In this implementation, JPEG is reduce to 4:4:4, 8bit.

I'd like to reject this change, and recommand coming back with a suitable
integration, so that it can be special cased and the driver can communicate the
required information to narrow down the estimate. And this way, you are actually
making it usable for other compressed format like H.264, HEVC, VP8, VP9 AV1 etc.

Nicolas

Nacked in that form.

> 
>  	};
>  	unsigned int i;
>  

[PATCH 5/9] media: sun6i-csi: capture: Rework and separate format validation

[Paul Kocialkowski]

Introduce a new sun6i_csi_capture_format_check helper to indicate
whether a set of pixel format and mbus code are compatible.
Most of the logic is taken from sun6i_csi_capture_link_validate,
with extra checks added along the way.

Note that v4l2_format_info is now used for all pixel formats
since they should all be listed in the helper at this point.

The motivation behind this change is to pave the way for supporting
the mc-style enum_fmt.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 95 ++++++++++---------
 1 file changed, 49 insertions(+), 46 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
index cf6aadbc130b..6ce7f1d3ed57 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -327,6 +327,52 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code)
 	return false;
 }
 
+static bool sun6i_csi_capture_format_check(u32 pixelformat, u32 mbus_code)
+{
+	const struct sun6i_csi_capture_format *capture_format;
+	const struct sun6i_csi_bridge_format *bridge_format;
+	const struct v4l2_format_info *format_info;
+
+	format_info = v4l2_format_info(pixelformat);
+	if (WARN_ON(!format_info))
+		return false;
+
+	capture_format = sun6i_csi_capture_format_find(pixelformat);
+	if (WARN_ON(!capture_format))
+		return false;
+
+	bridge_format = sun6i_csi_bridge_format_find(mbus_code);
+	if (WARN_ON(!bridge_format))
+		return false;
+
+	/* Raw input is required for non-YUV formats. */
+	if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW &&
+	    (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER ||
+	     format_info->pixel_enc == V4L2_PIXEL_ENC_RGB ||
+	     format_info->pixel_enc == V4L2_PIXEL_ENC_COMPRESSED))
+		return false;
+
+	if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) {
+		/* YUV input is required for YUV pixels. */
+		if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 &&
+		    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422)
+			return false;
+
+		/* YUV420 input can't produce (upsampled) YUV422 output. */
+		if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 &&
+		    format_info->vdiv == 1)
+			return false;
+	}
+
+	/* Raw input requires a 1:1 match between input and output. */
+	if ((bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW ||
+	     capture_format->input_format_raw) &&
+	    !sun6i_csi_capture_format_match(pixelformat, mbus_code))
+			return false;
+
+	return true;
+}
+
 /* Capture */
 
 static void
@@ -890,28 +936,16 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
 		media_entity_to_video_device(link->sink->entity);
 	struct sun6i_csi_device *csi_dev = video_get_drvdata(video_dev);
 	struct v4l2_device *v4l2_dev = csi_dev->v4l2_dev;
-	const struct sun6i_csi_capture_format *capture_format;
-	const struct sun6i_csi_bridge_format *bridge_format;
 	unsigned int capture_width, capture_height;
 	unsigned int bridge_width, bridge_height;
-	const struct v4l2_format_info *format_info;
 	u32 pixelformat, capture_field;
 	u32 mbus_code, bridge_field;
-	bool match;
 
 	sun6i_csi_capture_dimensions(csi_dev, &capture_width, &capture_height);
-
 	sun6i_csi_capture_format(csi_dev, &pixelformat, &capture_field);
-	capture_format = sun6i_csi_capture_format_find(pixelformat);
-	if (WARN_ON(!capture_format))
-		return -EINVAL;
 
 	sun6i_csi_bridge_dimensions(csi_dev, &bridge_width, &bridge_height);
-
 	sun6i_csi_bridge_format(csi_dev, &mbus_code, &bridge_field);
-	bridge_format = sun6i_csi_bridge_format_find(mbus_code);
-	if (WARN_ON(!bridge_format))
-		return -EINVAL;
 
 	/* No cropping/scaling is supported. */
 	if (capture_width != bridge_width || capture_height != bridge_height) {
@@ -922,43 +956,12 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
 		return -EINVAL;
 	}
 
-	format_info = v4l2_format_info(pixelformat);
-	/* Some formats are not listed. */
-	if (!format_info)
-		return 0;
-
-	if (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER &&
-	    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW)
-		goto invalid;
-
-	if (format_info->pixel_enc == V4L2_PIXEL_ENC_RGB &&
-	    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW)
-		goto invalid;
-
-	if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) {
-		if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 &&
-		    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422)
-			goto invalid;
-
-		/* YUV420 input can't produce YUV422 output. */
-		if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 &&
-		    format_info->vdiv == 1)
-			goto invalid;
-	}
-
-	/* With raw input mode, we need a 1:1 match between input and output. */
-	if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW ||
-	    capture_format->input_format_raw) {
-		match = sun6i_csi_capture_format_match(pixelformat, mbus_code);
-		if (!match)
-			goto invalid;
+	if (!sun6i_csi_capture_format_check(pixelformat, mbus_code)) {
+		v4l2_err(v4l2_dev, "invalid input/output format combination\n");
+		return -EINVAL;
 	}
 
 	return 0;
-
-invalid:
-	v4l2_err(v4l2_dev, "invalid input/output format combination\n");
-	return -EINVAL;
 }
 
 static const struct media_entity_operations sun6i_csi_capture_media_ops = {
-- 
2.39.2

Re: [PATCH 5/9] media: sun6i-csi: capture: Rework and separate format validation

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:24 CET je Paul Kocialkowski napisal(a):
> Introduce a new sun6i_csi_capture_format_check helper to indicate
> whether a set of pixel format and mbus code are compatible.
> Most of the logic is taken from sun6i_csi_capture_link_validate,
> with extra checks added along the way.
> 
> Note that v4l2_format_info is now used for all pixel formats
> since they should all be listed in the helper at this point.
> 
> The motivation behind this change is to pave the way for supporting
> the mc-style enum_fmt.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 5/9] media: sun6i-csi: capture: Rework and separate format validation

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:24PM +0100, Paul Kocialkowski wrote:
> Introduce a new sun6i_csi_capture_format_check helper to indicate
> whether a set of pixel format and mbus code are compatible.
> Most of the logic is taken from sun6i_csi_capture_link_validate,
> with extra checks added along the way.
> 
> Note that v4l2_format_info is now used for all pixel formats
> since they should all be listed in the helper at this point.
> 
> The motivation behind this change is to pave the way for supporting
> the mc-style enum_fmt.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 95 ++++++++++---------
>  1 file changed, 49 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index cf6aadbc130b..6ce7f1d3ed57 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -327,6 +327,52 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code)
>  	return false;
>  }
>  
> +static bool sun6i_csi_capture_format_check(u32 pixelformat, u32 mbus_code)
> +{
> +	const struct sun6i_csi_capture_format *capture_format;
> +	const struct sun6i_csi_bridge_format *bridge_format;

I think I would have named those output_format and input_format
respectively, as "capture" and "bridge" take a bit more mental
processing when reading the code. That may be because I'm not familiar
with the driver though, so feel free to ignore this.

> +	const struct v4l2_format_info *format_info;
> +
> +	format_info = v4l2_format_info(pixelformat);
> +	if (WARN_ON(!format_info))
> +		return false;
> +
> +	capture_format = sun6i_csi_capture_format_find(pixelformat);
> +	if (WARN_ON(!capture_format))
> +		return false;
> +
> +	bridge_format = sun6i_csi_bridge_format_find(mbus_code);
> +	if (WARN_ON(!bridge_format))
> +		return false;
> +
> +	/* Raw input is required for non-YUV formats. */
> +	if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW &&
> +	    (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER ||
> +	     format_info->pixel_enc == V4L2_PIXEL_ENC_RGB ||
> +	     format_info->pixel_enc == V4L2_PIXEL_ENC_COMPRESSED))

Unless I'm mistaken, the compressed format check is new. You could split
it to a separate patch, or at least mention it in the commit message.

> +		return false;
> +
> +	if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) {
> +		/* YUV input is required for YUV pixels. */
> +		if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 &&
> +		    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422)
> +			return false;
> +
> +		/* YUV420 input can't produce (upsampled) YUV422 output. */
> +		if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 &&
> +		    format_info->vdiv == 1)
> +			return false;
> +	}
> +
> +	/* Raw input requires a 1:1 match between input and output. */
> +	if ((bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW ||
> +	     capture_format->input_format_raw) &&
> +	    !sun6i_csi_capture_format_match(pixelformat, mbus_code))
> +			return false;

Wrong indentation.

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	return true;
> +}
> +
>  /* Capture */
>  
>  static void
> @@ -890,28 +936,16 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
>  		media_entity_to_video_device(link->sink->entity);
>  	struct sun6i_csi_device *csi_dev = video_get_drvdata(video_dev);
>  	struct v4l2_device *v4l2_dev = csi_dev->v4l2_dev;
> -	const struct sun6i_csi_capture_format *capture_format;
> -	const struct sun6i_csi_bridge_format *bridge_format;
>  	unsigned int capture_width, capture_height;
>  	unsigned int bridge_width, bridge_height;
> -	const struct v4l2_format_info *format_info;
>  	u32 pixelformat, capture_field;
>  	u32 mbus_code, bridge_field;
> -	bool match;
>  
>  	sun6i_csi_capture_dimensions(csi_dev, &capture_width, &capture_height);
> -
>  	sun6i_csi_capture_format(csi_dev, &pixelformat, &capture_field);
> -	capture_format = sun6i_csi_capture_format_find(pixelformat);
> -	if (WARN_ON(!capture_format))
> -		return -EINVAL;
>  
>  	sun6i_csi_bridge_dimensions(csi_dev, &bridge_width, &bridge_height);
> -
>  	sun6i_csi_bridge_format(csi_dev, &mbus_code, &bridge_field);
> -	bridge_format = sun6i_csi_bridge_format_find(mbus_code);
> -	if (WARN_ON(!bridge_format))
> -		return -EINVAL;
>  
>  	/* No cropping/scaling is supported. */
>  	if (capture_width != bridge_width || capture_height != bridge_height) {
> @@ -922,43 +956,12 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
>  		return -EINVAL;
>  	}
>  
> -	format_info = v4l2_format_info(pixelformat);
> -	/* Some formats are not listed. */
> -	if (!format_info)
> -		return 0;
> -
> -	if (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER &&
> -	    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW)
> -		goto invalid;
> -
> -	if (format_info->pixel_enc == V4L2_PIXEL_ENC_RGB &&
> -	    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW)
> -		goto invalid;
> -
> -	if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) {
> -		if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 &&
> -		    bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422)
> -			goto invalid;
> -
> -		/* YUV420 input can't produce YUV422 output. */
> -		if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 &&
> -		    format_info->vdiv == 1)
> -			goto invalid;
> -	}
> -
> -	/* With raw input mode, we need a 1:1 match between input and output. */
> -	if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW ||
> -	    capture_format->input_format_raw) {
> -		match = sun6i_csi_capture_format_match(pixelformat, mbus_code);
> -		if (!match)
> -			goto invalid;
> +	if (!sun6i_csi_capture_format_check(pixelformat, mbus_code)) {
> +		v4l2_err(v4l2_dev, "invalid input/output format combination\n");
> +		return -EINVAL;
>  	}
>  
>  	return 0;
> -
> -invalid:
> -	v4l2_err(v4l2_dev, "invalid input/output format combination\n");
> -	return -EINVAL;
>  }
>  
>  static const struct media_entity_operations sun6i_csi_capture_media_ops = {

-- 
Regards,

Laurent Pinchart

[PATCH 6/9] media: sun6i-csi: capture: Implement MC I/O with extended enum_fmt

[Paul Kocialkowski]

This driver needs the media-controller API to operate and should not be
considered as a video-device-centric implementation.

Properly report the IO_MC device cap and extend the enum_fmt
implementation to support enumeration with an explicit mbus_code.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 36 ++++++++++++++++---
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
index 6ce7f1d3ed57..9627030ff060 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -777,13 +777,40 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *private,
 				      struct v4l2_fmtdesc *fmtdesc)
 {
 	u32 index = fmtdesc->index;
+	u32 mbus_code = fmtdesc->mbus_code;
+	unsigned int index_valid = 0;
+	unsigned int i;
+
+	if (!mbus_code) {
+		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
+			return -EINVAL;
+
+		fmtdesc->pixelformat =
+			sun6i_csi_capture_formats[index].pixelformat;
+
+		return 0;
+	}
+
+	/* Check capture pixel format matching with mbus code. */
 
-	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
+	if (!sun6i_csi_bridge_format_find(mbus_code))
 		return -EINVAL;
 
-	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
+	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) {
+		u32 pixelformat = sun6i_csi_capture_formats[i].pixelformat;
 
-	return 0;
+		if (!sun6i_csi_capture_format_check(pixelformat, mbus_code))
+			continue;
+
+		if (index == index_valid) {
+			fmtdesc->pixelformat = pixelformat;
+			return 0;
+		}
+
+		index_valid++;
+	}
+
+	return -EINVAL;
 }
 
 static int sun6i_csi_capture_g_fmt(struct file *file, void *private,
@@ -1039,7 +1066,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
 
 	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
 		sizeof(video_dev->name));
-	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+				 V4L2_CAP_IO_MC;
 	video_dev->vfl_dir = VFL_DIR_RX;
 	video_dev->release = video_device_release_empty;
 	video_dev->fops = &sun6i_csi_capture_fops;
-- 
2.39.2

Re: [PATCH 6/9] media: sun6i-csi: capture: Implement MC I/O with extended enum_fmt

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:25 CET je Paul Kocialkowski napisal(a):
> This driver needs the media-controller API to operate and should not be
> considered as a video-device-centric implementation.
> 
> Properly report the IO_MC device cap and extend the enum_fmt
> implementation to support enumeration with an explicit mbus_code.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 36 ++++++++++++++++---
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index 6ce7f1d3ed57..9627030ff060 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -777,13 +777,40 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *private,
>  				      struct v4l2_fmtdesc *fmtdesc)
>  {
>  	u32 index = fmtdesc->index;
> +	u32 mbus_code = fmtdesc->mbus_code;

Nit: reverse christmas tree.

Other than that:

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> +	unsigned int index_valid = 0;
> +	unsigned int i;
> +
> +	if (!mbus_code) {
> +		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +			return -EINVAL;
> +
> +		fmtdesc->pixelformat =
> +			sun6i_csi_capture_formats[index].pixelformat;
> +
> +		return 0;
> +	}
> +
> +	/* Check capture pixel format matching with mbus code. */
>  
> -	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +	if (!sun6i_csi_bridge_format_find(mbus_code))
>  		return -EINVAL;
>  
> -	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
> +	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) {
> +		u32 pixelformat = sun6i_csi_capture_formats[i].pixelformat;
>  
> -	return 0;
> +		if (!sun6i_csi_capture_format_check(pixelformat, mbus_code))
> +			continue;
> +
> +		if (index == index_valid) {
> +			fmtdesc->pixelformat = pixelformat;
> +			return 0;
> +		}
> +
> +		index_valid++;
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static int sun6i_csi_capture_g_fmt(struct file *file, void *private,
> @@ -1039,7 +1066,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
>  
>  	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
>  		sizeof(video_dev->name));
> -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +				 V4L2_CAP_IO_MC;
>  	video_dev->vfl_dir = VFL_DIR_RX;
>  	video_dev->release = video_device_release_empty;
>  	video_dev->fops = &sun6i_csi_capture_fops;
> -- 
> 2.39.2
> 
> 

Re: [PATCH 6/9] media: sun6i-csi: capture: Implement MC I/O with extended enum_fmt

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:25PM +0100, Paul Kocialkowski wrote:
> This driver needs the media-controller API to operate and should not be
> considered as a video-device-centric implementation.
> 
> Properly report the IO_MC device cap and extend the enum_fmt
> implementation to support enumeration with an explicit mbus_code.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 36 ++++++++++++++++---
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index 6ce7f1d3ed57..9627030ff060 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -777,13 +777,40 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *private,
>  				      struct v4l2_fmtdesc *fmtdesc)
>  {
>  	u32 index = fmtdesc->index;
> +	u32 mbus_code = fmtdesc->mbus_code;
> +	unsigned int index_valid = 0;
> +	unsigned int i;
> +
> +	if (!mbus_code) {
> +		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +			return -EINVAL;
> +
> +		fmtdesc->pixelformat =
> +			sun6i_csi_capture_formats[index].pixelformat;
> +
> +		return 0;
> +	}
> +
> +	/* Check capture pixel format matching with mbus code. */
>  
> -	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +	if (!sun6i_csi_bridge_format_find(mbus_code))
>  		return -EINVAL;
>  
> -	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
> +	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) {
> +		u32 pixelformat = sun6i_csi_capture_formats[i].pixelformat;
>  
> -	return 0;
> +		if (!sun6i_csi_capture_format_check(pixelformat, mbus_code))
> +			continue;

I would probably have added compatible media bus codes to the
sun6i_csi_capture_format structure. This should work though, even if it
is more CPU-intensive. I'm OK with either option.

> +
> +		if (index == index_valid) {

You can replace this with

		if (index == 0) {

and

		index_valid++;

with

		index--;

below, and drop the index_valid variable.

> +			fmtdesc->pixelformat = pixelformat;
> +			return 0;
> +		}
> +
> +		index_valid++;
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static int sun6i_csi_capture_g_fmt(struct file *file, void *private,
> @@ -1039,7 +1066,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
>  
>  	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
>  		sizeof(video_dev->name));
> -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +				 V4L2_CAP_IO_MC;
>  	video_dev->vfl_dir = VFL_DIR_RX;
>  	video_dev->release = video_device_release_empty;

-- 
Regards,

Laurent Pinchart

[PATCH 7/9] media: sun6i-csi: capture: Implement enum_framesizes

[Paul Kocialkowski]

Report available frame sizes as a continuous range between the
hardware min/max limits.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Co-authored-by: Adam Pigg <adam@piggz.co.uk>
Signed-off-by: Adam Pigg <adam@piggz.co.uk>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
index 9627030ff060..31ba83014086 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -847,6 +847,30 @@ static int sun6i_csi_capture_try_fmt(struct file *file, void *private,
 	return 0;
 }
 
+static int
+sun6i_csi_capture_enum_framesizes(struct file *file, void *fh,
+				  struct v4l2_frmsizeenum *frmsizeenum)
+{
+	const struct sun6i_csi_capture_format *format;
+
+	if (frmsizeenum->index > 0)
+		return -EINVAL;
+
+	format = sun6i_csi_capture_format_find(frmsizeenum->pixel_format);
+	if (!format)
+		return -EINVAL;
+
+	frmsizeenum->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
+	frmsizeenum->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
+	frmsizeenum->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
+	frmsizeenum->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
+	frmsizeenum->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
+	frmsizeenum->stepwise.step_width = 1;
+	frmsizeenum->stepwise.step_height = 1;
+
+	return 0;
+}
+
 static int sun6i_csi_capture_enum_input(struct file *file, void *private,
 					struct v4l2_input *input)
 {
@@ -884,6 +908,8 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
 	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,
 
+	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesizes,
+
 	.vidioc_enum_input		= sun6i_csi_capture_enum_input,
 	.vidioc_g_input			= sun6i_csi_capture_g_input,
 	.vidioc_s_input			= sun6i_csi_capture_s_input,
-- 
2.39.2

Re: [PATCH 7/9] media: sun6i-csi: capture: Implement enum_framesizes

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:26 CET je Paul Kocialkowski napisal(a):
> Report available frame sizes as a continuous range between the
> hardware min/max limits.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Co-authored-by: Adam Pigg <adam@piggz.co.uk>
> Signed-off-by: Adam Pigg <adam@piggz.co.uk>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 7/9] media: sun6i-csi: capture: Implement enum_framesizes

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:26PM +0100, Paul Kocialkowski wrote:
> Report available frame sizes as a continuous range between the
> hardware min/max limits.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Co-authored-by: Adam Pigg <adam@piggz.co.uk>
> Signed-off-by: Adam Pigg <adam@piggz.co.uk>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index 9627030ff060..31ba83014086 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -847,6 +847,30 @@ static int sun6i_csi_capture_try_fmt(struct file *file, void *private,
>  	return 0;
>  }
>  
> +static int
> +sun6i_csi_capture_enum_framesizes(struct file *file, void *fh,
> +				  struct v4l2_frmsizeenum *frmsizeenum)
> +{
> +	const struct sun6i_csi_capture_format *format;
> +
> +	if (frmsizeenum->index > 0)
> +		return -EINVAL;
> +
> +	format = sun6i_csi_capture_format_find(frmsizeenum->pixel_format);
> +	if (!format)
> +		return -EINVAL;
> +
> +	frmsizeenum->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
> +	frmsizeenum->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
> +	frmsizeenum->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
> +	frmsizeenum->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
> +	frmsizeenum->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
> +	frmsizeenum->stepwise.step_width = 1;
> +	frmsizeenum->stepwise.step_height = 1;
> +
> +	return 0;
> +}
> +
>  static int sun6i_csi_capture_enum_input(struct file *file, void *private,
>  					struct v4l2_input *input)
>  {
> @@ -884,6 +908,8 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
>  	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
>  	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,
>  
> +	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesizes,
> +
>  	.vidioc_enum_input		= sun6i_csi_capture_enum_input,
>  	.vidioc_g_input			= sun6i_csi_capture_g_input,
>  	.vidioc_s_input			= sun6i_csi_capture_s_input,

-- 
Regards,

Laurent Pinchart

[PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt

[Paul Kocialkowski]

This driver needs the media-controller API to operate and should not be
considered as a video-device-centric implementation.

Properly report the IO_MC device cap and extend the enum_fmt
implementation to support enumeration with an explicit mbus_code.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c    | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
index 1595a9607775..5160b93b69ff 100644
--- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
+++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
@@ -439,6 +439,12 @@ static int sun6i_isp_capture_enum_fmt(struct file *file, void *private,
 				      struct v4l2_fmtdesc *fmtdesc)
 {
 	u32 index = fmtdesc->index;
+	u32 mbus_code = fmtdesc->mbus_code;
+
+	if (mbus_code && !sun6i_isp_proc_format_find(mbus_code))
+		return -EINVAL;
+
+	/* Capture format is independent from proc format. */
 
 	if (index >= ARRAY_SIZE(sun6i_isp_capture_formats))
 		return -EINVAL;
@@ -685,7 +691,8 @@ int sun6i_isp_capture_setup(struct sun6i_isp_device *isp_dev)
 
 	strscpy(video_dev->name, SUN6I_ISP_CAPTURE_NAME,
 		sizeof(video_dev->name));
-	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+				 V4L2_CAP_IO_MC;
 	video_dev->vfl_dir = VFL_DIR_RX;
 	video_dev->release = video_device_release_empty;
 	video_dev->fops = &sun6i_isp_capture_fops;
-- 
2.39.2

Re: [PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:27 CET je Paul Kocialkowski napisal(a):
> This driver needs the media-controller API to operate and should not be
> considered as a video-device-centric implementation.
> 
> Properly report the IO_MC device cap and extend the enum_fmt
> implementation to support enumeration with an explicit mbus_code.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c    | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> index 1595a9607775..5160b93b69ff 100644
> --- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> +++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> @@ -439,6 +439,12 @@ static int sun6i_isp_capture_enum_fmt(struct file *file, void *private,
>  				      struct v4l2_fmtdesc *fmtdesc)
>  {
>  	u32 index = fmtdesc->index;
> +	u32 mbus_code = fmtdesc->mbus_code;

Nit: reverse christmas tree

Other than that:
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> +
> +	if (mbus_code && !sun6i_isp_proc_format_find(mbus_code))
> +		return -EINVAL;
> +
> +	/* Capture format is independent from proc format. */
>  
>  	if (index >= ARRAY_SIZE(sun6i_isp_capture_formats))
>  		return -EINVAL;
> @@ -685,7 +691,8 @@ int sun6i_isp_capture_setup(struct sun6i_isp_device *isp_dev)
>  
>  	strscpy(video_dev->name, SUN6I_ISP_CAPTURE_NAME,
>  		sizeof(video_dev->name));
> -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +				 V4L2_CAP_IO_MC;
>  	video_dev->vfl_dir = VFL_DIR_RX;
>  	video_dev->release = video_device_release_empty;
>  	video_dev->fops = &sun6i_isp_capture_fops;
> -- 
> 2.39.2
> 
> 

Re: [PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Mar 24, 2023 at 04:12:27PM +0100, Paul Kocialkowski wrote:
> This driver needs the media-controller API to operate and should not be
> considered as a video-device-centric implementation.
> 
> Properly report the IO_MC device cap and extend the enum_fmt
> implementation to support enumeration with an explicit mbus_code.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c    | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> index 1595a9607775..5160b93b69ff 100644
> --- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> +++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
> @@ -439,6 +439,12 @@ static int sun6i_isp_capture_enum_fmt(struct file *file, void *private,
>  				      struct v4l2_fmtdesc *fmtdesc)
>  {
>  	u32 index = fmtdesc->index;
> +	u32 mbus_code = fmtdesc->mbus_code;
> +
> +	if (mbus_code && !sun6i_isp_proc_format_find(mbus_code))
> +		return -EINVAL;
> +

This doesn't look right. As far as I understand,
sun6i_isp_proc_format_find() looks up media bus codes for the ISP sink
pad. Here you enuemrate pixel formats of the ISP output, so the media
bus code given by userspace corresponds to the ISP source pad.

I've had a look at sun6i_isp_proc_set_fmt() to see what media bus codes
are used on the ISP output, and couldn't figure it out as it seems
incorrectly implemented :-) The function doesn't check format->pad.

> +	/* Capture format is independent from proc format. */
>  
>  	if (index >= ARRAY_SIZE(sun6i_isp_capture_formats))
>  		return -EINVAL;
> @@ -685,7 +691,8 @@ int sun6i_isp_capture_setup(struct sun6i_isp_device *isp_dev)
>  
>  	strscpy(video_dev->name, SUN6I_ISP_CAPTURE_NAME,
>  		sizeof(video_dev->name));
> -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +				 V4L2_CAP_IO_MC;
>  	video_dev->vfl_dir = VFL_DIR_RX;
>  	video_dev->release = video_device_release_empty;
>  	video_dev->fops = &sun6i_isp_capture_fops;

-- 
Regards,

Laurent Pinchart

[PATCH 9/9] media: sun6i-isp: capture: Implement enum_framesizes

[Paul Kocialkowski]

Report available frame sizes as a continuous range between the
hardware min/max limits.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Co-authored-by: Adam Pigg <adam@piggz.co.uk>
Signed-off-by: Adam Pigg <adam@piggz.co.uk>
---
 .../media/sunxi/sun6i-isp/sun6i_isp_capture.c | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
index 5160b93b69ff..a368f90a9beb 100644
--- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
+++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c
@@ -487,6 +487,30 @@ static int sun6i_isp_capture_try_fmt(struct file *file, void *private,
 	return 0;
 }
 
+static int
+sun6i_isp_capture_enum_framesizes(struct file *file, void *fh,
+				  struct v4l2_frmsizeenum *frmsizeenum)
+{
+	const struct sun6i_isp_capture_format *format;
+
+	if (frmsizeenum->index > 0)
+		return -EINVAL;
+
+	format = sun6i_isp_capture_format_find(frmsizeenum->pixel_format);
+	if (!format)
+		return -EINVAL;
+
+	frmsizeenum->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
+	frmsizeenum->stepwise.min_width = SUN6I_ISP_CAPTURE_WIDTH_MIN;
+	frmsizeenum->stepwise.max_width = SUN6I_ISP_CAPTURE_WIDTH_MAX;
+	frmsizeenum->stepwise.min_height = SUN6I_ISP_CAPTURE_HEIGHT_MIN;
+	frmsizeenum->stepwise.max_height = SUN6I_ISP_CAPTURE_HEIGHT_MAX;
+	frmsizeenum->stepwise.step_width = 1;
+	frmsizeenum->stepwise.step_height = 1;
+
+	return 0;
+}
+
 static int sun6i_isp_capture_enum_input(struct file *file, void *private,
 					struct v4l2_input *input)
 {
@@ -524,6 +548,8 @@ static const struct v4l2_ioctl_ops sun6i_isp_capture_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap		= sun6i_isp_capture_s_fmt,
 	.vidioc_try_fmt_vid_cap		= sun6i_isp_capture_try_fmt,
 
+	.vidioc_enum_framesizes		= sun6i_isp_capture_enum_framesizes,
+
 	.vidioc_enum_input		= sun6i_isp_capture_enum_input,
 	.vidioc_g_input			= sun6i_isp_capture_g_input,
 	.vidioc_s_input			= sun6i_isp_capture_s_input,
-- 
2.39.2

Re: [PATCH 9/9] media: sun6i-isp: capture: Implement enum_framesizes

[Jernej Škrabec]

Dne petek, 24. marec 2023 ob 16:12:28 CET je Paul Kocialkowski napisal(a):
> Report available frame sizes as a continuous range between the
> hardware min/max limits.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Co-authored-by: Adam Pigg <adam@piggz.co.uk>
> Signed-off-by: Adam Pigg <adam@piggz.co.uk>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej