2023-03-24 15:16:43

by Paul Kocialkowski

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

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


2023-03-24 15:16:55

by Paul Kocialkowski

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

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

Signed-off-by: Paul Kocialkowski <[email protected]>
Co-authored-by: Adam Pigg <[email protected]>
Signed-off-by: Adam Pigg <[email protected]>
---
.../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

2023-03-24 15:17:04

by Paul Kocialkowski

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

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

Signed-off-by: Paul Kocialkowski <[email protected]>
Co-authored-by: Adam Pigg <[email protected]>
Signed-off-by: Adam Pigg <[email protected]>
---
.../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

2023-03-24 15:17:05

by Paul Kocialkowski

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

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 <[email protected]>
---
.../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

2023-03-25 07:28:57

by Jernej Škrabec

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

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
>
>


2023-03-25 07:29:07

by Jernej Škrabec

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

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 <[email protected]>
> Co-authored-by: Adam Pigg <[email protected]>
> Signed-off-by: Adam Pigg <[email protected]>

Reviewed-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2023-03-25 07:30:01

by Jernej Škrabec

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

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 <[email protected]>
> Co-authored-by: Adam Pigg <[email protected]>
> Signed-off-by: Adam Pigg <[email protected]>

Reviewed-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2023-03-25 21:32:57

by Laurent Pinchart

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

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 <[email protected]>
> ---
> .../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

2023-03-25 21:57:23

by Laurent Pinchart

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

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 <[email protected]>
> Co-authored-by: Adam Pigg <[email protected]>
> Signed-off-by: Adam Pigg <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

> ---
> .../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