* [PATCH 0/3] suns6-csi changes to support libcamera
@ 2023-01-06 19:40 adam
2023-01-06 19:40 ` [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match adam
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: adam @ 2023-01-06 19:40 UTC (permalink / raw)
To: linux-media
Cc: yong.deng, mchehab, linux-sunxi, paul.kocialkowski,
laurent.pinchart, Adam Pigg
From: Adam Pigg <adam@piggz.co.uk>
This 3 patch set adds V4L2_CAP_IO_MC and vidioc_enum_framesizes support
to the sun6i-csi driver, allowing the Pinephone rear camera (ov5640) to
be supported.
These have been developed with the guidance of Laurent Pinchart, and
been tested by capturing frames from the libcamera cam utility.
In addition to these, a patch to change the v4l gain mode in the ov5640
driver, and the 2 format propagation patches all by Laurnet are
required.
Adam Pigg (3):
media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match
media: sun6i-csi: implement V4L2_CAP_IO_MC
media: sun6i-csi: implement vidioc_enum_framesizes
.../sunxi/sun6i-csi/sun6i_csi_capture.c | 215 +++++++++---------
.../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +-
2 files changed, 103 insertions(+), 118 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-01-06 19:40 [PATCH 0/3] suns6-csi changes to support libcamera adam @ 2023-01-06 19:40 ` adam 2023-01-06 23:31 ` Laurent Pinchart 2023-01-06 19:40 ` [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC adam ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: adam @ 2023-01-06 19:40 UTC (permalink / raw) To: linux-media Cc: yong.deng, mchehab, linux-sunxi, paul.kocialkowski, laurent.pinchart, Adam Pigg From: Adam Pigg <adam@piggz.co.uk> Merged the two format arrays into sun6i_csi_capture_formats as a pre-requisite to implementing V4L2_CAP_IO_MC Signed-off-by: Adam Pigg <adam@piggz.co.uk> --- .../sunxi/sun6i-csi/sun6i_csi_capture.c | 155 +++++------------- .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- 2 files changed, 46 insertions(+), 115 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 03d4adec054c..69578075421c 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c @@ -22,6 +22,8 @@ /* Helpers */ +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} + void sun6i_csi_capture_dimensions(struct sun6i_csi_device *csi_dev, unsigned int *width, unsigned int *height) { @@ -49,72 +51,86 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { .pixelformat = V4L2_PIX_FMT_SBGGR8, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8), }, { .pixelformat = V4L2_PIX_FMT_SGBRG8, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8), }, { .pixelformat = V4L2_PIX_FMT_SGRBG8, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8), }, { .pixelformat = V4L2_PIX_FMT_SRGGB8, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8), }, { .pixelformat = V4L2_PIX_FMT_SBGGR10, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10), }, { .pixelformat = V4L2_PIX_FMT_SGBRG10, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10), }, { .pixelformat = V4L2_PIX_FMT_SGRBG10, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10), }, { .pixelformat = V4L2_PIX_FMT_SRGGB10, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10), }, { .pixelformat = V4L2_PIX_FMT_SBGGR12, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12), }, { .pixelformat = V4L2_PIX_FMT_SGBRG12, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12), }, { .pixelformat = V4L2_PIX_FMT_SGRBG12, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12), }, { .pixelformat = V4L2_PIX_FMT_SRGGB12, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12), }, /* RGB */ { .pixelformat = V4L2_PIX_FMT_RGB565, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE), }, { .pixelformat = V4L2_PIX_FMT_RGB565X, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_BE), }, /* YUV422 */ { @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, .input_format_raw = true, .hsize_len_factor = 2, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, + MEDIA_BUS_FMT_YUYV8_1X16), }, { .pixelformat = V4L2_PIX_FMT_YVYU, @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, .input_format_raw = true, .hsize_len_factor = 2, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, + MEDIA_BUS_FMT_YVYU8_1X16), }, { .pixelformat = V4L2_PIX_FMT_UYVY, @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, .input_format_raw = true, .hsize_len_factor = 2, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, + MEDIA_BUS_FMT_UYVY8_1X16), }, { .pixelformat = V4L2_PIX_FMT_VYUY, @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, .input_format_raw = true, .hsize_len_factor = 2, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, + MEDIA_BUS_FMT_VYUY8_1X16), }, { .pixelformat = V4L2_PIX_FMT_NV16, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_NV61, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, .input_yuv_seq_invert = true, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_YUV422P, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, + .mbus_codes = 0, }, /* YUV420 */ { .pixelformat = V4L2_PIX_FMT_NV12_16L16, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_NV12, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_NV21, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, .input_yuv_seq_invert = true, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_YUV420, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, + .mbus_codes = 0, }, { .pixelformat = V4L2_PIX_FMT_YVU420, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, .input_yuv_seq_invert = true, + .mbus_codes = 0, }, /* Compressed */ { .pixelformat = V4L2_PIX_FMT_JPEG, .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), }, }; @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format *sun6i_csi_capture_format_find(u32 pixelformat) return NULL; } -/* RAW formats need an exact match between pixel and mbus formats. */ -static const -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] = { - /* YUV420 */ - { - .pixelformat = V4L2_PIX_FMT_YUYV, - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, - }, - { - .pixelformat = V4L2_PIX_FMT_YUYV, - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, - }, - { - .pixelformat = V4L2_PIX_FMT_YVYU, - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, - }, - { - .pixelformat = V4L2_PIX_FMT_YVYU, - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, - }, - { - .pixelformat = V4L2_PIX_FMT_UYVY, - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, - }, - { - .pixelformat = V4L2_PIX_FMT_UYVY, - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, - }, - { - .pixelformat = V4L2_PIX_FMT_VYUY, - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, - }, - { - .pixelformat = V4L2_PIX_FMT_VYUY, - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, - }, - /* RGB */ - { - .pixelformat = V4L2_PIX_FMT_RGB565, - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, - }, - { - .pixelformat = V4L2_PIX_FMT_RGB565X, - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, - }, - /* Bayer */ - { - .pixelformat = V4L2_PIX_FMT_SBGGR8, - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, - }, - { - .pixelformat = V4L2_PIX_FMT_SGBRG8, - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, - }, - { - .pixelformat = V4L2_PIX_FMT_SGRBG8, - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, - }, - { - .pixelformat = V4L2_PIX_FMT_SRGGB8, - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, - }, - { - .pixelformat = V4L2_PIX_FMT_SBGGR10, - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, - }, - { - .pixelformat = V4L2_PIX_FMT_SGBRG10, - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, - }, - { - .pixelformat = V4L2_PIX_FMT_SGRBG10, - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, - }, - { - .pixelformat = V4L2_PIX_FMT_SRGGB10, - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, - }, - { - .pixelformat = V4L2_PIX_FMT_SBGGR12, - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, - }, - { - .pixelformat = V4L2_PIX_FMT_SGBRG12, - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, - }, - { - .pixelformat = V4L2_PIX_FMT_SGRBG12, - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, - }, - { - .pixelformat = V4L2_PIX_FMT_SRGGB12, - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, - }, - /* Compressed */ - { - .pixelformat = V4L2_PIX_FMT_JPEG, - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, - }, -}; - static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code) { - unsigned int i; - - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { - const struct sun6i_csi_capture_format_match *match = - &sun6i_csi_capture_format_matches[i]; - - if (match->pixelformat == pixelformat && - match->mbus_code == mbus_code) - return true; + unsigned int i, j; + + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { + const struct sun6i_csi_capture_format *format = + &sun6i_csi_capture_formats[i]; + + if (format->pixelformat == pixelformat) { + for (j = 0; format->mbus_codes[j]; j++) { + if (mbus_code == format->mbus_codes[j]) + return true; + } + } } return false; diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index 3ee5ccefbd10..0484942834e3 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { bool input_yuv_seq_invert; bool input_format_raw; u32 hsize_len_factor; -}; - -struct sun6i_csi_capture_format_match { - u32 pixelformat; - u32 mbus_code; + const u32 *mbus_codes; }; #undef current -- 2.39.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-01-06 19:40 ` [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match adam @ 2023-01-06 23:31 ` Laurent Pinchart 2023-01-08 18:23 ` Adam Pigg 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2023-01-06 23:31 UTC (permalink / raw) To: adam; +Cc: linux-media, yong.deng, mchehab, linux-sunxi, paul.kocialkowski Hi Adam, Thank you for the patch. On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > From: Adam Pigg <adam@piggz.co.uk> > > Merged the two format arrays into sun6i_csi_capture_formats as a > pre-requisite to implementing V4L2_CAP_IO_MC I'll point to https://cbea.ms/git-commit/ if you haven't read it yet. You can ignore the "Limit the subject line to 50 characters" rule and go up to the normal 72 characters limit for commit messages, as 50 doesn't include the prefixes commonly used in kernel commit messages. For this specific patch, I would write Information about media bus formats and pixel formats supported by the driver is split between the sun6i_csi_capture_formats and sun6i_csi_capture_format_matches arrays. This makes it difficult to map media bus formats to pixel formats when enumerating the supported pixel formats by walking the sun6i_csi_capture_formats array. To prepare for support of media bus format support in sun6i_csi_capture_enum_fmt(), merge the two arrays toegether. An extra paragraph could be added to explain *how* this is being done, but the implementation is straightforward enough to not require that. > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > --- > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 155 +++++------------- > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > 2 files changed, 46 insertions(+), 115 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 03d4adec054c..69578075421c 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > @@ -22,6 +22,8 @@ > > /* Helpers */ > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > + > void sun6i_csi_capture_dimensions(struct sun6i_csi_device *csi_dev, > unsigned int *width, unsigned int *height) > { > @@ -49,72 +51,86 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { > .pixelformat = V4L2_PIX_FMT_SBGGR8, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8), > }, > { > .pixelformat = V4L2_PIX_FMT_SGBRG8, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8), > }, > { > .pixelformat = V4L2_PIX_FMT_SGRBG8, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8), > }, > { > .pixelformat = V4L2_PIX_FMT_SRGGB8, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8), > }, > { > .pixelformat = V4L2_PIX_FMT_SBGGR10, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10), > }, > { > .pixelformat = V4L2_PIX_FMT_SGBRG10, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10), > }, > { > .pixelformat = V4L2_PIX_FMT_SGRBG10, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10), > }, > { > .pixelformat = V4L2_PIX_FMT_SRGGB10, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10), > }, > { > .pixelformat = V4L2_PIX_FMT_SBGGR12, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12), > }, > { > .pixelformat = V4L2_PIX_FMT_SGBRG12, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12), > }, > { > .pixelformat = V4L2_PIX_FMT_SGRBG12, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12), > }, > { > .pixelformat = V4L2_PIX_FMT_SRGGB12, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12), > }, > /* RGB */ > { > .pixelformat = V4L2_PIX_FMT_RGB565, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE), > }, > { > .pixelformat = V4L2_PIX_FMT_RGB565X, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_BE), > }, > /* YUV422 */ > { > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > .input_format_raw = true, > .hsize_len_factor = 2, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > + MEDIA_BUS_FMT_YUYV8_1X16), > }, > { > .pixelformat = V4L2_PIX_FMT_YVYU, > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > .input_format_raw = true, > .hsize_len_factor = 2, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > + MEDIA_BUS_FMT_YVYU8_1X16), > }, > { > .pixelformat = V4L2_PIX_FMT_UYVY, > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > .input_format_raw = true, > .hsize_len_factor = 2, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > + MEDIA_BUS_FMT_UYVY8_1X16), > }, > { > .pixelformat = V4L2_PIX_FMT_VYUY, > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = { > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > .input_format_raw = true, > .hsize_len_factor = 2, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > + MEDIA_BUS_FMT_VYUY8_1X16), > }, > { > .pixelformat = V4L2_PIX_FMT_NV16, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > + .mbus_codes = 0, I don't think this is correct. To produce semi-planar or multi-planar YUV formats, I believe the CSI needs YUV input. This should thus be (unless I'm mistaken) .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_VYUY8_2X8, MEDIA_BUS_FMT_VYUY8_1X16, MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_YUYV8_1X16, MEDIA_BUS_FMT_YVYU8_2X8, MEDIA_BUS_FMT_YVYU8_1X16), and same below. Paul, could you confirm this ? I'm a bit surprised that the CSI can't shuffle the YUV components for packed YUYV formats, but so be it if that's a hardware limitation. I'm also thinking that a subsequent patch could drop the raw check from sun6i_csi_capture_link_validate(): - /* 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, - fmt.format.code); - if (!match) - goto invalid; - } + /* Make sure the media bus code and pixel format are compatible. */ + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); + if (!match) + goto invalid; > }, > { > .pixelformat = V4L2_PIX_FMT_NV61, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > .input_yuv_seq_invert = true, > + .mbus_codes = 0, > }, > { > .pixelformat = V4L2_PIX_FMT_YUV422P, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > + .mbus_codes = 0, > }, > /* YUV420 */ > { > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > + .mbus_codes = 0, > }, > { > .pixelformat = V4L2_PIX_FMT_NV12, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > + .mbus_codes = 0, > }, > { > .pixelformat = V4L2_PIX_FMT_NV21, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > .input_yuv_seq_invert = true, > + .mbus_codes = 0, > }, > > { > .pixelformat = V4L2_PIX_FMT_YUV420, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > + .mbus_codes = 0, > }, > { > .pixelformat = V4L2_PIX_FMT_YVU420, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > .input_yuv_seq_invert = true, > + .mbus_codes = 0, > }, > /* Compressed */ > { > .pixelformat = V4L2_PIX_FMT_JPEG, > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > }, > }; > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format *sun6i_csi_capture_format_find(u32 pixelformat) > return NULL; > } > > -/* RAW formats need an exact match between pixel and mbus formats. */ > -static const > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] = { > - /* YUV420 */ > - { > - .pixelformat = V4L2_PIX_FMT_YUYV, > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_YUYV, > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_YVYU, > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_YVYU, > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_UYVY, > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_UYVY, > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_VYUY, > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_VYUY, > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > - }, > - /* RGB */ > - { > - .pixelformat = V4L2_PIX_FMT_RGB565, > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_RGB565X, > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > - }, > - /* Bayer */ > - { > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > - }, > - { > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > - }, > - /* Compressed */ > - { > - .pixelformat = V4L2_PIX_FMT_JPEG, > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > - }, > -}; > - > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code) > { > - unsigned int i; > - > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > - const struct sun6i_csi_capture_format_match *match = > - &sun6i_csi_capture_format_matches[i]; > - > - if (match->pixelformat == pixelformat && > - match->mbus_code == mbus_code) > - return true; > + unsigned int i, j; > + > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > + const struct sun6i_csi_capture_format *format = > + &sun6i_csi_capture_formats[i]; > + > + if (format->pixelformat == pixelformat) { > + for (j = 0; format->mbus_codes[j]; j++) { > + if (mbus_code == format->mbus_codes[j]) > + return true; > + } > + } > } > > return false; > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > index 3ee5ccefbd10..0484942834e3 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > bool input_yuv_seq_invert; > bool input_format_raw; > u32 hsize_len_factor; > -}; > - > -struct sun6i_csi_capture_format_match { > - u32 pixelformat; > - u32 mbus_code; > + const u32 *mbus_codes; > }; > > #undef current -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-01-06 23:31 ` Laurent Pinchart @ 2023-01-08 18:23 ` Adam Pigg 2023-01-08 20:26 ` Laurent Pinchart 0 siblings, 1 reply; 18+ messages in thread From: Adam Pigg @ 2023-01-08 18:23 UTC (permalink / raw) To: Laurent Pinchart Cc: linux-media, yong.deng, mchehab, linux-sunxi, paul.kocialkowski [-- Attachment #1: Type: text/plain, Size: 16262 bytes --] On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > Hi Adam, > > Thank you for the patch. > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > From: Adam Pigg <adam@piggz.co.uk> > > > > Merged the two format arrays into sun6i_csi_capture_formats as a > > pre-requisite to implementing V4L2_CAP_IO_MC > > I'll point to https://cbea.ms/git-commit/ if you haven't read it yet. > You can ignore the "Limit the subject line to 50 characters" rule and go > up to the normal 72 characters limit for commit messages, as 50 doesn't > include the prefixes commonly used in kernel commit messages. > > For this specific patch, I would write > > Information about media bus formats and pixel formats supported by the > driver is split between the sun6i_csi_capture_formats and > sun6i_csi_capture_format_matches arrays. This makes it difficult to map > media bus formats to pixel formats when enumerating the supported pixel > formats by walking the sun6i_csi_capture_formats array. To prepare for > support of media bus format support in sun6i_csi_capture_enum_fmt(), > merge the two arrays toegether. > > > An extra paragraph could be added to explain *how* this is being done, > but the implementation is straightforward enough to not require that. > > > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > > --- > > > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 155 +++++------------- > > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > > 2 files changed, 46 insertions(+), 115 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 > > 03d4adec054c..69578075421c 100644 > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > @@ -22,6 +22,8 @@ > > > > /* Helpers */ > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > + > > > > void sun6i_csi_capture_dimensions(struct sun6i_csi_device *csi_dev, > > > > unsigned int *width, unsigned int *height) > > > > { > > > > @@ -49,72 +51,86 @@ static const struct sun6i_csi_capture_format > > sun6i_csi_capture_formats[] = {> > > .pixelformat = V4L2_PIX_FMT_SBGGR8, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGBRG8, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGRBG8, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SRGGB8, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SBGGR10, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGBRG10, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGRBG10, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SRGGB10, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SBGGR12, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGBRG12, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SGRBG12, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_SRGGB12, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12), > > > > }, > > /* RGB */ > > { > > > > .pixelformat = V4L2_PIX_FMT_RGB565, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_RGB565X, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_BE), > > > > }, > > /* YUV422 */ > > { > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > sun6i_csi_capture_formats[] = {> > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > .input_format_raw = true, > > .hsize_len_factor = 2, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > sun6i_csi_capture_formats[] = {> > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > .input_format_raw = true, > > .hsize_len_factor = 2, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > sun6i_csi_capture_formats[] = {> > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > .input_format_raw = true, > > .hsize_len_factor = 2, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > sun6i_csi_capture_formats[] = {> > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > .input_format_raw = true, > > .hsize_len_factor = 2, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > + .mbus_codes = 0, > > I don't think this is correct. To produce semi-planar or multi-planar > YUV formats, I believe the CSI needs YUV input. This should thus be > (unless I'm mistaken) > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > MEDIA_BUS_FMT_UYVY8_1X16, > MEDIA_BUS_FMT_VYUY8_2X8, > MEDIA_BUS_FMT_VYUY8_1X16, > MEDIA_BUS_FMT_YUYV8_2X8, > MEDIA_BUS_FMT_YUYV8_1X16, > MEDIA_BUS_FMT_YVYU8_2X8, > MEDIA_BUS_FMT_YVYU8_1X16), > > and same below. > Hi Laurent Thanks for the help and tips. Ive made all the other changes, which can be viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ kernel-megi/commits/pinephone-libcamera Im just not quite sure on this one. I think my implementation of merging the arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches there is no mapping for the *NV formats, and the remaining ones ive set to 0? Thanks > Paul, could you confirm this ? > > I'm a bit surprised that the CSI can't shuffle the YUV components for > packed YUYV formats, but so be it if that's a hardware limitation. > > I'm also thinking that a subsequent patch could drop the raw check from > sun6i_csi_capture_link_validate(): > > - /* 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, > - fmt.format.code); > - if (!match) > - goto invalid; > - } > + /* Make sure the media bus code and pixel format are compatible. */ > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > + if (!match) > + goto invalid; > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > .input_yuv_seq_invert = true, > > > > + .mbus_codes = 0, > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > + .mbus_codes = 0, > > > > }, > > /* YUV420 */ > > { > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > + .mbus_codes = 0, > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > + .mbus_codes = 0, > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > .input_yuv_seq_invert = true, > > > > + .mbus_codes = 0, > > > > }, > > > > { > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > + .mbus_codes = 0, > > > > }, > > { > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > .input_yuv_seq_invert = true, > > > > + .mbus_codes = 0, > > > > }, > > /* Compressed */ > > { > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > }, > > > > }; > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > return NULL; > > > > } > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > -static const > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > = { - /* YUV420 */ > > - { > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > - }, > > - /* RGB */ > > - { > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > - }, > > - /* Bayer */ > > - { > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > - }, > > - { > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > - }, > > - /* Compressed */ > > - { > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > - }, > > -}; > > - > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > mbus_code) > > { > > > > - unsigned int i; > > - > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > - const struct sun6i_csi_capture_format_match *match = > > - &sun6i_csi_capture_format_matches[i]; > > - > > - if (match->pixelformat == pixelformat && > > - match->mbus_code == mbus_code) > > - return true; > > + unsigned int i, j; > > + > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > + const struct sun6i_csi_capture_format *format = > > + &sun6i_csi_capture_formats[i]; > > + > > + if (format->pixelformat == pixelformat) { > > + for (j = 0; format->mbus_codes[j]; j++) { > > + if (mbus_code == format- >mbus_codes[j]) > > + return true; > > + } > > + } > > > > } > > > > return false; > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > 3ee5ccefbd10..0484942834e3 100644 > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > bool input_yuv_seq_invert; > > bool input_format_raw; > > u32 hsize_len_factor; > > > > -}; > > - > > -struct sun6i_csi_capture_format_match { > > - u32 pixelformat; > > - u32 mbus_code; > > + const u32 *mbus_codes; > > > > }; > > > > #undef current [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-01-08 18:23 ` Adam Pigg @ 2023-01-08 20:26 ` Laurent Pinchart 2023-02-17 18:20 ` Laurent Pinchart 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2023-01-08 20:26 UTC (permalink / raw) To: Adam Pigg; +Cc: linux-media, yong.deng, mchehab, linux-sunxi, paul.kocialkowski Hi Adam, On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > > From: Adam Pigg <adam@piggz.co.uk> > > > > > > Merged the two format arrays into sun6i_csi_capture_formats as a > > > pre-requisite to implementing V4L2_CAP_IO_MC > > > > I'll point to https://cbea.ms/git-commit/ if you haven't read it yet. > > You can ignore the "Limit the subject line to 50 characters" rule and go > > up to the normal 72 characters limit for commit messages, as 50 doesn't > > include the prefixes commonly used in kernel commit messages. > > > > For this specific patch, I would write > > > > Information about media bus formats and pixel formats supported by the > > driver is split between the sun6i_csi_capture_formats and > > sun6i_csi_capture_format_matches arrays. This makes it difficult to map > > media bus formats to pixel formats when enumerating the supported pixel > > formats by walking the sun6i_csi_capture_formats array. To prepare for > > support of media bus format support in sun6i_csi_capture_enum_fmt(), > > merge the two arrays toegether. > > > > > > An extra paragraph could be added to explain *how* this is being done, > > but the implementation is straightforward enough to not require that. > > > > > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > > > --- > > > > > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 155 +++++------------- > > > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > > > 2 files changed, 46 insertions(+), 115 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 > > > 03d4adec054c..69578075421c 100644 > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > @@ -22,6 +22,8 @@ > > > > > > /* Helpers */ > > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > > + > > > > > > void sun6i_csi_capture_dimensions(struct sun6i_csi_device *csi_dev, > > > > > > unsigned int *width, unsigned int *height) > > > > > > { > > > > > > @@ -49,72 +51,86 @@ static const struct sun6i_csi_capture_format > > > sun6i_csi_capture_formats[] = {> > > > .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12), > > > > > > }, > > > /* RGB */ > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_RGB565, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_RGB565X, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_BE), > > > > > > }, > > > /* YUV422 */ > > > { > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > sun6i_csi_capture_formats[] = {> > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > .input_format_raw = true, > > > .hsize_len_factor = 2, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > sun6i_csi_capture_formats[] = {> > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > .input_format_raw = true, > > > .hsize_len_factor = 2, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > sun6i_csi_capture_formats[] = { > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > .input_format_raw = true, > > > .hsize_len_factor = 2, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > sun6i_csi_capture_formats[] = {> > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > .input_format_raw = true, > > > .hsize_len_factor = 2, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > + .mbus_codes = 0, > > > > I don't think this is correct. To produce semi-planar or multi-planar > > YUV formats, I believe the CSI needs YUV input. This should thus be > > (unless I'm mistaken) > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > MEDIA_BUS_FMT_UYVY8_1X16, > > MEDIA_BUS_FMT_VYUY8_2X8, > > MEDIA_BUS_FMT_VYUY8_1X16, > > MEDIA_BUS_FMT_YUYV8_2X8, > > MEDIA_BUS_FMT_YUYV8_1X16, > > MEDIA_BUS_FMT_YVYU8_2X8, > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > and same below. > > > Hi Laurent > > Thanks for the help and tips. Ive made all the other changes, which can be > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > kernel-megi/commits/pinephone-libcamera > > Im just not quite sure on this one. I think my implementation of merging the > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > there is no mapping for the *NV formats, and the remaining ones ive set to 0? The current implementation allows writing multi-planar formats (e.g. NV12) to memory when the input of the CSI is a YUV media bus format (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent NV12 from being enumerated when using media bus-based enumeration of pixel formats, so userspace won't see NV12 as being available. It would be fine fixing that issue in a separate patch on top of this one, but I though you could as well do both in one go. > > Paul, could you confirm this ? > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > packed YUYV formats, but so be it if that's a hardware limitation. > > > > I'm also thinking that a subsequent patch could drop the raw check from > > sun6i_csi_capture_link_validate(): > > > > - /* 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, > > - fmt.format.code); > > - if (!match) > > - goto invalid; > > - } > > + /* Make sure the media bus code and pixel format are compatible. */ > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > + if (!match) > > + goto invalid; > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > .input_yuv_seq_invert = true, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > /* YUV420 */ > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > .input_yuv_seq_invert = true, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > .input_yuv_seq_invert = true, > > > > > > + .mbus_codes = 0, > > > > > > }, > > > /* Compressed */ > > > { > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > }, > > > > > > }; > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > return NULL; > > > > > > } > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > -static const > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > = { - /* YUV420 */ > > > - { > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > - }, > > > - /* RGB */ > > > - { > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > - }, > > > - /* Bayer */ > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > - }, > > > - { > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > - }, > > > - /* Compressed */ > > > - { > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > - }, > > > -}; > > > - > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > mbus_code) > > > { > > > > > > - unsigned int i; > > > - > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > - const struct sun6i_csi_capture_format_match *match = > > > - &sun6i_csi_capture_format_matches[i]; > > > - > > > - if (match->pixelformat == pixelformat && > > > - match->mbus_code == mbus_code) > > > - return true; > > > + unsigned int i, j; > > > + > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > + const struct sun6i_csi_capture_format *format = > > > + &sun6i_csi_capture_formats[i]; > > > + > > > + if (format->pixelformat == pixelformat) { > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > + if (mbus_code == format->mbus_codes[j]) > > > + return true; > > > + } > > > + } > > > > > > } > > > > > > return false; > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > 3ee5ccefbd10..0484942834e3 100644 > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > bool input_yuv_seq_invert; > > > bool input_format_raw; > > > u32 hsize_len_factor; > > > > > > -}; > > > - > > > -struct sun6i_csi_capture_format_match { > > > - u32 pixelformat; > > > - u32 mbus_code; > > > + const u32 *mbus_codes; > > > > > > }; > > > > > > #undef current -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-01-08 20:26 ` Laurent Pinchart @ 2023-02-17 18:20 ` Laurent Pinchart 2023-02-22 10:39 ` Paul Kocialkowski 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2023-02-17 18:20 UTC (permalink / raw) To: paul.kocialkowski; +Cc: Adam Pigg, linux-media, yong.deng, mchehab, linux-sunxi Hi Paul, Could you share your opinion on the question below ? On Sun, Jan 08, 2023 at 10:26:09PM +0200, Laurent Pinchart wrote: > On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > > > From: Adam Pigg <adam@piggz.co.uk> > > > > > > > > Merged the two format arrays into sun6i_csi_capture_formats as a > > > > pre-requisite to implementing V4L2_CAP_IO_MC > > > > > > I'll point to https://cbea.ms/git-commit/ if you haven't read it yet. > > > You can ignore the "Limit the subject line to 50 characters" rule and go > > > up to the normal 72 characters limit for commit messages, as 50 doesn't > > > include the prefixes commonly used in kernel commit messages. > > > > > > For this specific patch, I would write > > > > > > Information about media bus formats and pixel formats supported by the > > > driver is split between the sun6i_csi_capture_formats and > > > sun6i_csi_capture_format_matches arrays. This makes it difficult to map > > > media bus formats to pixel formats when enumerating the supported pixel > > > formats by walking the sun6i_csi_capture_formats array. To prepare for > > > support of media bus format support in sun6i_csi_capture_enum_fmt(), > > > merge the two arrays toegether. > > > > > > > > > An extra paragraph could be added to explain *how* this is being done, > > > but the implementation is straightforward enough to not require that. > > > > > > > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > > > > --- > > > > > > > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 155 +++++------------- > > > > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > > > > 2 files changed, 46 insertions(+), 115 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 > > > > 03d4adec054c..69578075421c 100644 > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > > @@ -22,6 +22,8 @@ > > > > > > > > /* Helpers */ > > > > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > > > + > > > > > > > > void sun6i_csi_capture_dimensions(struct sun6i_csi_device *csi_dev, > > > > > > > > unsigned int *width, unsigned int *height) > > > > > > > > { > > > > > > > > @@ -49,72 +51,86 @@ static const struct sun6i_csi_capture_format > > > > sun6i_csi_capture_formats[] = {> > > > > .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_10, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_10, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_12, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_12, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12), > > > > > > > > }, > > > > /* RGB */ > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_RGB565, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RGB565, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RGB565, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_BE), > > > > > > > > }, > > > > /* YUV422 */ > > > > { > > > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > > sun6i_csi_capture_formats[] = {> > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > .input_format_raw = true, > > > > .hsize_len_factor = 2, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > > sun6i_csi_capture_formats[] = {> > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > .input_format_raw = true, > > > > .hsize_len_factor = 2, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > > sun6i_csi_capture_formats[] = { > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > .input_format_raw = true, > > > > .hsize_len_factor = 2, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > > sun6i_csi_capture_formats[] = {> > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > .input_format_raw = true, > > > > .hsize_len_factor = 2, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > > + .mbus_codes = 0, > > > > > > I don't think this is correct. To produce semi-planar or multi-planar > > > YUV formats, I believe the CSI needs YUV input. This should thus be > > > (unless I'm mistaken) > > > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > MEDIA_BUS_FMT_UYVY8_1X16, > > > MEDIA_BUS_FMT_VYUY8_2X8, > > > MEDIA_BUS_FMT_VYUY8_1X16, > > > MEDIA_BUS_FMT_YUYV8_2X8, > > > MEDIA_BUS_FMT_YUYV8_1X16, > > > MEDIA_BUS_FMT_YVYU8_2X8, > > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > and same below. > > > > > Hi Laurent > > > > Thanks for the help and tips. Ive made all the other changes, which can be > > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > > kernel-megi/commits/pinephone-libcamera > > > > Im just not quite sure on this one. I think my implementation of merging the > > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > > there is no mapping for the *NV formats, and the remaining ones ive set to 0? > > The current implementation allows writing multi-planar formats (e.g. > NV12) to memory when the input of the CSI is a YUV media bus format > (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent > NV12 from being enumerated when using media bus-based enumeration of > pixel formats, so userspace won't see NV12 as being available. > > It would be fine fixing that issue in a separate patch on top of this > one, but I though you could as well do both in one go. Adam, you mentioned that NV12 and NV16 "don't work". Could you elaborate and explain what you've tried exactly ? > > > Paul, could you confirm this ? > > > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > > packed YUYV formats, but so be it if that's a hardware limitation. > > > > > > I'm also thinking that a subsequent patch could drop the raw check from > > > sun6i_csi_capture_link_validate(): > > > > > > - /* 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, > > > - fmt.format.code); > > > - if (!match) > > > - goto invalid; > > > - } > > > + /* Make sure the media bus code and pixel format are compatible. */ > > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > > + if (!match) > > > + goto invalid; > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > .input_yuv_seq_invert = true, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > /* YUV420 */ > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > .input_yuv_seq_invert = true, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > .input_yuv_seq_invert = true, > > > > > > > > + .mbus_codes = 0, > > > > > > > > }, > > > > /* Compressed */ > > > > { > > > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > > > }, > > > > > > > > }; > > > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > > return NULL; > > > > > > > > } > > > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > > -static const > > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > > = { - /* YUV420 */ > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > > - }, > > > > - /* RGB */ > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > > - }, > > > > - /* Bayer */ > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > > - }, > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > > - }, > > > > - /* Compressed */ > > > > - { > > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > > - }, > > > > -}; > > > > - > > > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > > mbus_code) > > > > { > > > > > > > > - unsigned int i; > > > > - > > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > > - const struct sun6i_csi_capture_format_match *match = > > > > - &sun6i_csi_capture_format_matches[i]; > > > > - > > > > - if (match->pixelformat == pixelformat && > > > > - match->mbus_code == mbus_code) > > > > - return true; > > > > + unsigned int i, j; > > > > + > > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > > + const struct sun6i_csi_capture_format *format = > > > > + &sun6i_csi_capture_formats[i]; > > > > + > > > > + if (format->pixelformat == pixelformat) { > > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > > + if (mbus_code == format->mbus_codes[j]) > > > > + return true; > > > > + } > > > > + } > > > > > > > > } > > > > > > > > return false; > > > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > > 3ee5ccefbd10..0484942834e3 100644 > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > > > bool input_yuv_seq_invert; > > > > bool input_format_raw; > > > > u32 hsize_len_factor; > > > > > > > > -}; > > > > - > > > > -struct sun6i_csi_capture_format_match { > > > > - u32 pixelformat; > > > > - u32 mbus_code; > > > > + const u32 *mbus_codes; > > > > > > > > }; > > > > > > > > #undef current -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-02-17 18:20 ` Laurent Pinchart @ 2023-02-22 10:39 ` Paul Kocialkowski 2023-03-21 17:50 ` Adam Pigg 2023-03-25 23:17 ` Laurent Pinchart 0 siblings, 2 replies; 18+ messages in thread From: Paul Kocialkowski @ 2023-02-22 10:39 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Adam Pigg, linux-media, yong.deng, mchehab, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 15871 bytes --] Hi Adam, Laurent, On Fri 17 Feb 23, 20:20, Laurent Pinchart wrote: > Could you share your opinion on the question below ? Yeah sorry for the long delay here. It's taken me a while to dive back into this topic. My first impression is that I would rather be in favor of keeping a dynamic approach like what's already in sun6i_csi_capture_link_validate and extract the mbus/pixfmt matching logic from there. I would be happy to craft a patch in that direction, but maybe you'd like to have a try at it (since it's your series). Just let me know, I think I can do it pretty quickly now that I have everything back in mind. I could also add some comment about the general hardware mechanism/limitations. The hardware is a bit odd in a few ways and I found that the explicit combinatory approach wasn't a very good fit (but obviously that's an open topic for discussions). The general idea is that the SUN6I_CSI_INPUT_FMT_YUV420/422 can only be used to produce outputs on 2 or 3 planes, but not packed YUV. There's also no explicit hardware reordering of the chroma components, so we need to lie about the input order (input_yuv_seq_invert) to achieve inverted chroma components on the output format. In order to produce packed data, we can only rely on SUN6I_CSI_INPUT_FMT_RAW which provides no reordering mechanism. This is why it made good sense to me to only have an explicit matching table for this case and rely on more general logic that reflects the hardware capabilities otherwise. > On Sun, Jan 08, 2023 at 10:26:09PM +0200, Laurent Pinchart wrote: > > On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > > > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: [...] > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} Cosmetic suggestion to stay consistent with the rest: #define SUN6I_CSI_CAPTURE_MBUS_CODES(mbus_codes...) \ (const u32[]) { mbus_codes, 0 } Also it would look better in sun6i_csi_capture.h. But really I would be happier with a dynamic approach. [...] > > > > > /* YUV422 */ > > > > > { > > > > > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > > > sun6i_csi_capture_formats[] = {> > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > .input_format_raw = true, > > > > > .hsize_len_factor = 2, > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > > > sun6i_csi_capture_formats[] = {> > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > .input_format_raw = true, > > > > > .hsize_len_factor = 2, > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > > > sun6i_csi_capture_formats[] = { > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > .input_format_raw = true, > > > > > .hsize_len_factor = 2, > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > > > sun6i_csi_capture_formats[] = {> > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > .input_format_raw = true, > > > > > .hsize_len_factor = 2, > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > I don't think this is correct. To produce semi-planar or multi-planar > > > > YUV formats, I believe the CSI needs YUV input. This should thus be > > > > (unless I'm mistaken) You are correct. > > > > > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > MEDIA_BUS_FMT_UYVY8_1X16, > > > > MEDIA_BUS_FMT_VYUY8_2X8, > > > > MEDIA_BUS_FMT_VYUY8_1X16, > > > > MEDIA_BUS_FMT_YUYV8_2X8, > > > > MEDIA_BUS_FMT_YUYV8_1X16, > > > > MEDIA_BUS_FMT_YVYU8_2X8, > > > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > and same below. All of the YUV420/422 pixel formats on 2 or 3 planes can take all of the supports packed 16-bit YUV bus formats, which is why it doesn't seem very graceful to have an explicit list. > > > > > > > Hi Laurent > > > > > > Thanks for the help and tips. Ive made all the other changes, which can be > > > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > > > kernel-megi/commits/pinephone-libcamera > > > > > > Im just not quite sure on this one. I think my implementation of merging the > > > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > > > there is no mapping for the *NV formats, and the remaining ones ive set to 0? > > > > The current implementation allows writing multi-planar formats (e.g. > > NV12) to memory when the input of the CSI is a YUV media bus format > > (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent > > NV12 from being enumerated when using media bus-based enumeration of > > pixel formats, so userspace won't see NV12 as being available. > > > > It would be fine fixing that issue in a separate patch on top of this > > one, but I though you could as well do both in one go. > > Adam, you mentioned that NV12 and NV16 "don't work". Could you elaborate > and explain what you've tried exactly ? We definitely need to keep the ability to produce NV12, NV16 and friends from YUV bus formats. > > > > Paul, could you confirm this ? > > > > > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > > > packed YUYV formats, but so be it if that's a hardware limitation. Yep that is correct, it's a hardware limitation. Cheers, Paul > > > > I'm also thinking that a subsequent patch could drop the raw check from > > > > sun6i_csi_capture_link_validate(): > > > > > > > > - /* 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, > > > > - fmt.format.code); > > > > - if (!match) > > > > - goto invalid; > > > > - } > > > > + /* Make sure the media bus code and pixel format are compatible. */ > > > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > > > + if (!match) > > > > + goto invalid; > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > /* YUV420 */ > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > }, > > > > > /* Compressed */ > > > > > { > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > > > > > }, > > > > > > > > > > }; > > > > > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > > > return NULL; > > > > > > > > > > } > > > > > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > > > -static const > > > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > > > = { - /* YUV420 */ > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > > > - }, > > > > > - /* RGB */ > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > > > - }, > > > > > - /* Bayer */ > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > > > - }, > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > > > - }, > > > > > - /* Compressed */ > > > > > - { > > > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > > > - }, > > > > > -}; > > > > > - > > > > > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > > > mbus_code) > > > > > { > > > > > > > > > > - unsigned int i; > > > > > - > > > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > > > - const struct sun6i_csi_capture_format_match *match = > > > > > - &sun6i_csi_capture_format_matches[i]; > > > > > - > > > > > - if (match->pixelformat == pixelformat && > > > > > - match->mbus_code == mbus_code) > > > > > - return true; > > > > > + unsigned int i, j; > > > > > + > > > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > > > + const struct sun6i_csi_capture_format *format = > > > > > + &sun6i_csi_capture_formats[i]; > > > > > + > > > > > + if (format->pixelformat == pixelformat) { > > > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > > > + if (mbus_code == format->mbus_codes[j]) > > > > > + return true; > > > > > + } > > > > > + } > > > > > > > > > > } > > > > > > > > > > return false; > > > > > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > > > 3ee5ccefbd10..0484942834e3 100644 > > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > > > > > bool input_yuv_seq_invert; > > > > > bool input_format_raw; > > > > > u32 hsize_len_factor; > > > > > > > > > > -}; > > > > > - > > > > > -struct sun6i_csi_capture_format_match { > > > > > - u32 pixelformat; > > > > > - u32 mbus_code; > > > > > + const u32 *mbus_codes; > > > > > > > > > > }; > > > > > > > > > > #undef current > > -- > Regards, > > Laurent Pinchart -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-02-22 10:39 ` Paul Kocialkowski @ 2023-03-21 17:50 ` Adam Pigg 2023-03-23 12:55 ` Paul Kocialkowski 2023-03-25 23:17 ` Laurent Pinchart 1 sibling, 1 reply; 18+ messages in thread From: Adam Pigg @ 2023-03-21 17:50 UTC (permalink / raw) To: Paul Kocialkowski Cc: Laurent Pinchart, linux-media, yong.deng, mchehab, linux-sunxi On Wed, 22 Feb 2023 at 10:39, Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Hi Adam, Laurent, > > On Fri 17 Feb 23, 20:20, Laurent Pinchart wrote: > > Could you share your opinion on the question below ? > > Yeah sorry for the long delay here. It's taken me a while to dive back into > this topic. > > My first impression is that I would rather be in favor of keeping a dynamic > approach like what's already in sun6i_csi_capture_link_validate and extract > the mbus/pixfmt matching logic from there. > > I would be happy to craft a patch in that direction, but maybe you'd like > to have a try at it (since it's your series). Just let me know, I think I can > do it pretty quickly now that I have everything back in mind. I could also > add some comment about the general hardware mechanism/limitations. > Hi Paul Apologies for missing this email! As you seem to know the hardware much better, and I was only getting it working together with help from others, im more than happy for you to work up the changes, and it will likely result in a much shorter change cycle! Would that be for this patch only in the series, or the other 2 also? Thanks > The hardware is a bit odd in a few ways and I found that the explicit > combinatory approach wasn't a very good fit (but obviously that's an open topic > for discussions). > > The general idea is that the SUN6I_CSI_INPUT_FMT_YUV420/422 can only be used > to produce outputs on 2 or 3 planes, but not packed YUV. There's also no > explicit hardware reordering of the chroma components, so we need to lie about > the input order (input_yuv_seq_invert) to achieve inverted chroma components > on the output format. > > In order to produce packed data, we can only rely on SUN6I_CSI_INPUT_FMT_RAW > which provides no reordering mechanism. This is why it made good sense to me > to only have an explicit matching table for this case and rely on more general > logic that reflects the hardware capabilities otherwise. > > > On Sun, Jan 08, 2023 at 10:26:09PM +0200, Laurent Pinchart wrote: > > > On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > > > > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > > > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > [...] > > > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > Cosmetic suggestion to stay consistent with the rest: > #define SUN6I_CSI_CAPTURE_MBUS_CODES(mbus_codes...) \ > (const u32[]) { mbus_codes, 0 } > > Also it would look better in sun6i_csi_capture.h. > > But really I would be happier with a dynamic approach. > > [...] > > > > > > > /* YUV422 */ > > > > > > { > > > > > > > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = { > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > I don't think this is correct. To produce semi-planar or multi-planar > > > > > YUV formats, I believe the CSI needs YUV input. This should thus be > > > > > (unless I'm mistaken) > > You are correct. > > > > > > > > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > MEDIA_BUS_FMT_UYVY8_1X16, > > > > > MEDIA_BUS_FMT_VYUY8_2X8, > > > > > MEDIA_BUS_FMT_VYUY8_1X16, > > > > > MEDIA_BUS_FMT_YUYV8_2X8, > > > > > MEDIA_BUS_FMT_YUYV8_1X16, > > > > > MEDIA_BUS_FMT_YVYU8_2X8, > > > > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > and same below. > > All of the YUV420/422 pixel formats on 2 or 3 planes can take all of the > supports packed 16-bit YUV bus formats, which is why it doesn't seem very > graceful to have an explicit list. > > > > > > > > > > Hi Laurent > > > > > > > > Thanks for the help and tips. Ive made all the other changes, which can be > > > > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > > > > kernel-megi/commits/pinephone-libcamera > > > > > > > > Im just not quite sure on this one. I think my implementation of merging the > > > > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > > > > there is no mapping for the *NV formats, and the remaining ones ive set to 0? > > > > > > The current implementation allows writing multi-planar formats (e.g. > > > NV12) to memory when the input of the CSI is a YUV media bus format > > > (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent > > > NV12 from being enumerated when using media bus-based enumeration of > > > pixel formats, so userspace won't see NV12 as being available. > > > > > > It would be fine fixing that issue in a separate patch on top of this > > > one, but I though you could as well do both in one go. > > > > Adam, you mentioned that NV12 and NV16 "don't work". Could you elaborate > > and explain what you've tried exactly ? > > We definitely need to keep the ability to produce NV12, NV16 and friends from > YUV bus formats. > > > > > > Paul, could you confirm this ? > > > > > > > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > > > > packed YUYV formats, but so be it if that's a hardware limitation. > > Yep that is correct, it's a hardware limitation. > > Cheers, > > Paul > > > > > > I'm also thinking that a subsequent patch could drop the raw check from > > > > > sun6i_csi_capture_link_validate(): > > > > > > > > > > - /* 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, > > > > > - fmt.format.code); > > > > > - if (!match) > > > > > - goto invalid; > > > > > - } > > > > > + /* Make sure the media bus code and pixel format are compatible. */ > > > > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > > > > + if (!match) > > > > > + goto invalid; > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > /* YUV420 */ > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > /* Compressed */ > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > > > > > > > }, > > > > > > > > > > > > }; > > > > > > > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > > > > return NULL; > > > > > > > > > > > > } > > > > > > > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > > > > -static const > > > > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > > > > = { - /* YUV420 */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > > > > - }, > > > > > > - /* RGB */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > > > > - }, > > > > > > - /* Bayer */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > > > > - }, > > > > > > - /* Compressed */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > > > > - }, > > > > > > -}; > > > > > > - > > > > > > > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > > > > mbus_code) > > > > > > { > > > > > > > > > > > > - unsigned int i; > > > > > > - > > > > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > > > > - const struct sun6i_csi_capture_format_match *match = > > > > > > - &sun6i_csi_capture_format_matches[i]; > > > > > > - > > > > > > - if (match->pixelformat == pixelformat && > > > > > > - match->mbus_code == mbus_code) > > > > > > - return true; > > > > > > + unsigned int i, j; > > > > > > + > > > > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > > > > + const struct sun6i_csi_capture_format *format = > > > > > > + &sun6i_csi_capture_formats[i]; > > > > > > + > > > > > > + if (format->pixelformat == pixelformat) { > > > > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > > > > + if (mbus_code == format->mbus_codes[j]) > > > > > > + return true; > > > > > > + } > > > > > > + } > > > > > > > > > > > > } > > > > > > > > > > > > return false; > > > > > > > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > > > > 3ee5ccefbd10..0484942834e3 100644 > > > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > > > > > > > bool input_yuv_seq_invert; > > > > > > bool input_format_raw; > > > > > > u32 hsize_len_factor; > > > > > > > > > > > > -}; > > > > > > - > > > > > > -struct sun6i_csi_capture_format_match { > > > > > > - u32 pixelformat; > > > > > > - u32 mbus_code; > > > > > > + const u32 *mbus_codes; > > > > > > > > > > > > }; > > > > > > > > > > > > #undef current > > > > -- > > Regards, > > > > Laurent Pinchart > > -- > Paul Kocialkowski, Bootlin > Embedded Linux and kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-03-21 17:50 ` Adam Pigg @ 2023-03-23 12:55 ` Paul Kocialkowski 0 siblings, 0 replies; 18+ messages in thread From: Paul Kocialkowski @ 2023-03-23 12:55 UTC (permalink / raw) To: Adam Pigg; +Cc: Laurent Pinchart, linux-media, yong.deng, mchehab, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 21559 bytes --] Hi Adam, On Tue 21 Mar 23, 17:50, Adam Pigg wrote: > On Wed, 22 Feb 2023 at 10:39, Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi Adam, Laurent, > > > > On Fri 17 Feb 23, 20:20, Laurent Pinchart wrote: > > > Could you share your opinion on the question below ? > > > > Yeah sorry for the long delay here. It's taken me a while to dive back into > > this topic. > > > > My first impression is that I would rather be in favor of keeping a dynamic > > approach like what's already in sun6i_csi_capture_link_validate and extract > > the mbus/pixfmt matching logic from there. > > > > I would be happy to craft a patch in that direction, but maybe you'd like > > to have a try at it (since it's your series). Just let me know, I think I can > > do it pretty quickly now that I have everything back in mind. I could also > > add some comment about the general hardware mechanism/limitations. > > > > Hi Paul > > Apologies for missing this email! > > As you seem to know the hardware much better, and I was only getting > it working together with > help from others, im more than happy for you to work up the changes, > and it will likely result in > a much shorter change cycle! Glad to hear! I have some patches working already :) > Would that be for this patch only in the series, or the other 2 also? That would be for the enum_fmt mbus_code support as well as enum_framesizes. For the latter, I've made little change to your code so I will add your Co-authored-by and Signed-off-by lines. The same changes also apply to the sun6i-isp driver, which I try to keep in sync with sun6i-csi. Will send the patches soon (probably tomorrow). Cheers! Paul > Thanks > > > The hardware is a bit odd in a few ways and I found that the explicit > > combinatory approach wasn't a very good fit (but obviously that's an open topic > > for discussions). > > > > The general idea is that the SUN6I_CSI_INPUT_FMT_YUV420/422 can only be used > > to produce outputs on 2 or 3 planes, but not packed YUV. There's also no > > explicit hardware reordering of the chroma components, so we need to lie about > > the input order (input_yuv_seq_invert) to achieve inverted chroma components > > on the output format. > > > > In order to produce packed data, we can only rely on SUN6I_CSI_INPUT_FMT_RAW > > which provides no reordering mechanism. This is why it made good sense to me > > to only have an explicit matching table for this case and rely on more general > > logic that reflects the hardware capabilities otherwise. > > > > > On Sun, Jan 08, 2023 at 10:26:09PM +0200, Laurent Pinchart wrote: > > > > On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > > > > > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > > > > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > > > [...] > > > > > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > > > Cosmetic suggestion to stay consistent with the rest: > > #define SUN6I_CSI_CAPTURE_MBUS_CODES(mbus_codes...) \ > > (const u32[]) { mbus_codes, 0 } > > > > Also it would look better in sun6i_csi_capture.h. > > > > But really I would be happier with a dynamic approach. > > > > [...] > > > > > > > > > /* YUV422 */ > > > > > > > { > > > > > > > > > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > .input_format_raw = true, > > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > > > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > .input_format_raw = true, > > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > > > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > > > > > sun6i_csi_capture_formats[] = { > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > .input_format_raw = true, > > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > > > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > .input_format_raw = true, > > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > I don't think this is correct. To produce semi-planar or multi-planar > > > > > > YUV formats, I believe the CSI needs YUV input. This should thus be > > > > > > (unless I'm mistaken) > > > > You are correct. > > > > > > > > > > > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > MEDIA_BUS_FMT_UYVY8_1X16, > > > > > > MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > MEDIA_BUS_FMT_VYUY8_1X16, > > > > > > MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > MEDIA_BUS_FMT_YUYV8_1X16, > > > > > > MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > > > and same below. > > > > All of the YUV420/422 pixel formats on 2 or 3 planes can take all of the > > supports packed 16-bit YUV bus formats, which is why it doesn't seem very > > graceful to have an explicit list. > > > > > > > > > > > > > Hi Laurent > > > > > > > > > > Thanks for the help and tips. Ive made all the other changes, which can be > > > > > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > > > > > kernel-megi/commits/pinephone-libcamera > > > > > > > > > > Im just not quite sure on this one. I think my implementation of merging the > > > > > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > > > > > there is no mapping for the *NV formats, and the remaining ones ive set to 0? > > > > > > > > The current implementation allows writing multi-planar formats (e.g. > > > > NV12) to memory when the input of the CSI is a YUV media bus format > > > > (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent > > > > NV12 from being enumerated when using media bus-based enumeration of > > > > pixel formats, so userspace won't see NV12 as being available. > > > > > > > > It would be fine fixing that issue in a separate patch on top of this > > > > one, but I though you could as well do both in one go. > > > > > > Adam, you mentioned that NV12 and NV16 "don't work". Could you elaborate > > > and explain what you've tried exactly ? > > > > We definitely need to keep the ability to produce NV12, NV16 and friends from > > YUV bus formats. > > > > > > > > Paul, could you confirm this ? > > > > > > > > > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > > > > > packed YUYV formats, but so be it if that's a hardware limitation. > > > > Yep that is correct, it's a hardware limitation. > > > > Cheers, > > > > Paul > > > > > > > > I'm also thinking that a subsequent patch could drop the raw check from > > > > > > sun6i_csi_capture_link_validate(): > > > > > > > > > > > > - /* 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, > > > > > > - fmt.format.code); > > > > > > - if (!match) > > > > > > - goto invalid; > > > > > > - } > > > > > > + /* Make sure the media bus code and pixel format are compatible. */ > > > > > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > > > > > + if (!match) > > > > > > + goto invalid; > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > /* YUV420 */ > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > > > }, > > > > > > > /* Compressed */ > > > > > > > { > > > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > > > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > > > > > return NULL; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > > > > > -static const > > > > > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > > > > > = { - /* YUV420 */ > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > > > > > - }, > > > > > > > - /* RGB */ > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > > > > > - }, > > > > > > > - /* Bayer */ > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > > > > > - }, > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > > > > > - }, > > > > > > > - /* Compressed */ > > > > > > > - { > > > > > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > > > > > - }, > > > > > > > -}; > > > > > > > - > > > > > > > > > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > > > > > mbus_code) > > > > > > > { > > > > > > > > > > > > > > - unsigned int i; > > > > > > > - > > > > > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > > > > > - const struct sun6i_csi_capture_format_match *match = > > > > > > > - &sun6i_csi_capture_format_matches[i]; > > > > > > > - > > > > > > > - if (match->pixelformat == pixelformat && > > > > > > > - match->mbus_code == mbus_code) > > > > > > > - return true; > > > > > > > + unsigned int i, j; > > > > > > > + > > > > > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > > > > > + const struct sun6i_csi_capture_format *format = > > > > > > > + &sun6i_csi_capture_formats[i]; > > > > > > > + > > > > > > > + if (format->pixelformat == pixelformat) { > > > > > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > > > > > + if (mbus_code == format->mbus_codes[j]) > > > > > > > + return true; > > > > > > > + } > > > > > > > + } > > > > > > > > > > > > > > } > > > > > > > > > > > > > > return false; > > > > > > > > > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > > > > > 3ee5ccefbd10..0484942834e3 100644 > > > > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > > > > > > > > > bool input_yuv_seq_invert; > > > > > > > bool input_format_raw; > > > > > > > u32 hsize_len_factor; > > > > > > > > > > > > > > -}; > > > > > > > - > > > > > > > -struct sun6i_csi_capture_format_match { > > > > > > > - u32 pixelformat; > > > > > > > - u32 mbus_code; > > > > > > > + const u32 *mbus_codes; > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > #undef current > > > > > > -- > > > Regards, > > > > > > Laurent Pinchart > > > > -- > > Paul Kocialkowski, Bootlin > > Embedded Linux and kernel engineering > > https://bootlin.com -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match 2023-02-22 10:39 ` Paul Kocialkowski 2023-03-21 17:50 ` Adam Pigg @ 2023-03-25 23:17 ` Laurent Pinchart 1 sibling, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2023-03-25 23:17 UTC (permalink / raw) To: Paul Kocialkowski; +Cc: Adam Pigg, linux-media, yong.deng, mchehab, linux-sunxi Hi Paul, On Wed, Feb 22, 2023 at 11:39:42AM +0100, Paul Kocialkowski wrote: > On Fri 17 Feb 23, 20:20, Laurent Pinchart wrote: > > Could you share your opinion on the question below ? > > Yeah sorry for the long delay here. It's taken me a while to dive back into > this topic. > > My first impression is that I would rather be in favor of keeping a dynamic > approach like what's already in sun6i_csi_capture_link_validate and extract > the mbus/pixfmt matching logic from there. > > I would be happy to craft a patch in that direction, but maybe you'd like > to have a try at it (since it's your series). Just let me know, I think I can > do it pretty quickly now that I have everything back in mind. I could also > add some comment about the general hardware mechanism/limitations. > > The hardware is a bit odd in a few ways and I found that the explicit > combinatory approach wasn't a very good fit (but obviously that's an open topic > for discussions). > > The general idea is that the SUN6I_CSI_INPUT_FMT_YUV420/422 can only be used > to produce outputs on 2 or 3 planes, but not packed YUV. There's also no > explicit hardware reordering of the chroma components, so we need to lie about > the input order (input_yuv_seq_invert) to achieve inverted chroma components > on the output format. > > In order to produce packed data, we can only rely on SUN6I_CSI_INPUT_FMT_RAW > which provides no reordering mechanism. This is why it made good sense to me > to only have an explicit matching table for this case and rely on more general > logic that reflects the hardware capabilities otherwise. The explicit logic is more CPU-intensive so I have a small preference for adding mbus_codes to the sun6i_csi_capture_format structure. It's true that there will be some duplication of data, but I don't think it will consume lots of memory. As mentioned in the review of the series you've recently sent, I'm fine with both approaches, so you can decide. > > On Sun, Jan 08, 2023 at 10:26:09PM +0200, Laurent Pinchart wrote: > > > On Sun, Jan 08, 2023 at 06:23:56PM +0000, Adam Pigg wrote: > > > > On Friday, 6 January 2023 23:31:48 GMT Laurent Pinchart wrote: > > > > > On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@piggz.co.uk wrote: > > [...] > > > > > > > +#define SUN6I_BUS_FMTS(fmt...) (const u32[]) {fmt, 0} > > Cosmetic suggestion to stay consistent with the rest: > #define SUN6I_CSI_CAPTURE_MBUS_CODES(mbus_codes...) \ > (const u32[]) { mbus_codes, 0 } > > Also it would look better in sun6i_csi_capture.h. > > But really I would be happier with a dynamic approach. > > [...] > > > > > > > /* YUV422 */ > > > > > > { > > > > > > > > > > > > @@ -123,6 +139,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > + MEDIA_BUS_FMT_YUYV8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > > > > > > > @@ -130,6 +148,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > + MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > > > > > > > @@ -137,6 +157,8 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = { > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > + MEDIA_BUS_FMT_UYVY8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > > > > > > > @@ -144,57 +166,68 @@ static const struct sun6i_csi_capture_format > > > > > > sun6i_csi_capture_formats[] = {> > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > .input_format_raw = true, > > > > > > .hsize_len_factor = 2, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > + MEDIA_BUS_FMT_VYUY8_1X16), > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV16, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > I don't think this is correct. To produce semi-planar or multi-planar > > > > > YUV formats, I believe the CSI needs YUV input. This should thus be > > > > > (unless I'm mistaken) > > You are correct. > > > > > > > > > > > .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_UYVY8_2X8, > > > > > MEDIA_BUS_FMT_UYVY8_1X16, > > > > > MEDIA_BUS_FMT_VYUY8_2X8, > > > > > MEDIA_BUS_FMT_VYUY8_1X16, > > > > > MEDIA_BUS_FMT_YUYV8_2X8, > > > > > MEDIA_BUS_FMT_YUYV8_1X16, > > > > > MEDIA_BUS_FMT_YVYU8_2X8, > > > > > MEDIA_BUS_FMT_YVYU8_1X16), > > > > > > > > > > and same below. > > All of the YUV420/422 pixel formats on 2 or 3 planes can take all of the > supports packed 16-bit YUV bus formats, which is why it doesn't seem very > graceful to have an explicit list. > > > > > > > > > > Hi Laurent > > > > > > > > Thanks for the help and tips. Ive made all the other changes, which can be > > > > viewed here until i resubmit them https://github.com/sailfish-on-dontbeevil/ > > > > kernel-megi/commits/pinephone-libcamera > > > > > > > > Im just not quite sure on this one. I think my implementation of merging the > > > > arrays keeps the previous mapping right? In sun6i_csi_capture_format_matches > > > > there is no mapping for the *NV formats, and the remaining ones ive set to 0? > > > > > > The current implementation allows writing multi-planar formats (e.g. > > > NV12) to memory when the input of the CSI is a YUV media bus format > > > (e.g. YUYV8_1X16). This patch doesn't change that, but it will prevent > > > NV12 from being enumerated when using media bus-based enumeration of > > > pixel formats, so userspace won't see NV12 as being available. > > > > > > It would be fine fixing that issue in a separate patch on top of this > > > one, but I though you could as well do both in one go. > > > > Adam, you mentioned that NV12 and NV16 "don't work". Could you elaborate > > and explain what you've tried exactly ? > > We definitely need to keep the ability to produce NV12, NV16 and friends from > YUV bus formats. > > > > > > Paul, could you confirm this ? > > > > > > > > > > I'm a bit surprised that the CSI can't shuffle the YUV components for > > > > > packed YUYV formats, but so be it if that's a hardware limitation. > > Yep that is correct, it's a hardware limitation. > > Cheers, > > Paul > > > > > > I'm also thinking that a subsequent patch could drop the raw check from > > > > > sun6i_csi_capture_link_validate(): > > > > > > > > > > - /* 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, > > > > > - fmt.format.code); > > > > > - if (!match) > > > > > - goto invalid; > > > > > - } > > > > > + /* Make sure the media bus code and pixel format are compatible. */ > > > > > + match = sun6i_csi_capture_format_match(pixelformat, fmt.format.code); > > > > > + if (!match) > > > > > + goto invalid; > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV61, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422SP, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV422P, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV422P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV422P, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > /* YUV420 */ > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12_16L16, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420MB, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420MB, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV12, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_NV21, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420SP, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420SP, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YUV420, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_YVU420, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_YUV420P, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_YUV420P, > > > > > > .input_yuv_seq_invert = true, > > > > > > > > > > > > + .mbus_codes = 0, > > > > > > > > > > > > }, > > > > > > /* Compressed */ > > > > > > { > > > > > > > > > > > > .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > .output_format_frame = SUN6I_CSI_OUTPUT_FMT_FRAME_RAW_8, > > > > > > .output_format_field = SUN6I_CSI_OUTPUT_FMT_FIELD_RAW_8, > > > > > > > > > > > > + .mbus_codes = SUN6I_BUS_FMTS(MEDIA_BUS_FMT_JPEG_1X8), > > > > > > > > > > > > }, > > > > > > > > > > > > }; > > > > > > > > > > > > @@ -210,118 +243,20 @@ struct sun6i_csi_capture_format > > > > > > *sun6i_csi_capture_format_find(u32 pixelformat)> > > > > > > return NULL; > > > > > > > > > > > > } > > > > > > > > > > > > -/* RAW formats need an exact match between pixel and mbus formats. */ > > > > > > -static const > > > > > > -struct sun6i_csi_capture_format_match sun6i_csi_capture_format_matches[] > > > > > > = { - /* YUV420 */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YUYV, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_YVYU, > > > > > > - .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_UYVY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_2X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_VYUY, > > > > > > - .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16, > > > > > > - }, > > > > > > - /* RGB */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565, > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_RGB565X, > > > > > > - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE, > > > > > > - }, > > > > > > - /* Bayer */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB8, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB10, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SBGGR12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGBRG12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SGRBG12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12, > > > > > > - }, > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_SRGGB12, > > > > > > - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12, > > > > > > - }, > > > > > > - /* Compressed */ > > > > > > - { > > > > > > - .pixelformat = V4L2_PIX_FMT_JPEG, > > > > > > - .mbus_code = MEDIA_BUS_FMT_JPEG_1X8, > > > > > > - }, > > > > > > -}; > > > > > > - > > > > > > > > > > > > static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 > > > > > > mbus_code) > > > > > > { > > > > > > > > > > > > - unsigned int i; > > > > > > - > > > > > > - for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) { > > > > > > - const struct sun6i_csi_capture_format_match *match = > > > > > > - &sun6i_csi_capture_format_matches[i]; > > > > > > - > > > > > > - if (match->pixelformat == pixelformat && > > > > > > - match->mbus_code == mbus_code) > > > > > > - return true; > > > > > > + unsigned int i, j; > > > > > > + > > > > > > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > > > > > > + const struct sun6i_csi_capture_format *format = > > > > > > + &sun6i_csi_capture_formats[i]; > > > > > > + > > > > > > + if (format->pixelformat == pixelformat) { > > > > > > + for (j = 0; format->mbus_codes[j]; j++) { > > > > > > + if (mbus_code == format->mbus_codes[j]) > > > > > > + return true; > > > > > > + } > > > > > > + } > > > > > > > > > > > > } > > > > > > > > > > > > return false; > > > > > > > > > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h index > > > > > > 3ee5ccefbd10..0484942834e3 100644 > > > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.h > > > > > > @@ -27,11 +27,7 @@ struct sun6i_csi_capture_format { > > > > > > > > > > > > bool input_yuv_seq_invert; > > > > > > bool input_format_raw; > > > > > > u32 hsize_len_factor; > > > > > > > > > > > > -}; > > > > > > - > > > > > > -struct sun6i_csi_capture_format_match { > > > > > > - u32 pixelformat; > > > > > > - u32 mbus_code; > > > > > > + const u32 *mbus_codes; > > > > > > > > > > > > }; > > > > > > > > > > > > #undef current -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC 2023-01-06 19:40 [PATCH 0/3] suns6-csi changes to support libcamera adam 2023-01-06 19:40 ` [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match adam @ 2023-01-06 19:40 ` adam 2023-01-06 23:34 ` Laurent Pinchart 2023-01-06 19:40 ` [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes adam 2023-01-06 23:10 ` [PATCH 0/3] suns6-csi changes to support libcamera piggz1 3 siblings, 1 reply; 18+ messages in thread From: adam @ 2023-01-06 19:40 UTC (permalink / raw) To: linux-media Cc: yong.deng, mchehab, linux-sunxi, paul.kocialkowski, laurent.pinchart, Adam Pigg From: Adam Pigg <adam@piggz.co.uk> modify sun6i_csi_capture_enum_fmt to check the mbus_codes from the pixel format array for the supplied format Signed-off-by: Adam Pigg <adam@piggz.co.uk> --- .../sunxi/sun6i-csi/sun6i_csi_capture.c | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 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 69578075421c..54beba4d2b2f 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c @@ -666,13 +666,43 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *private, struct v4l2_fmtdesc *fmtdesc) { u32 index = fmtdesc->index; + unsigned int i; if (index >= ARRAY_SIZE(sun6i_csi_capture_formats)) return -EINVAL; - fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat; + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { + const struct sun6i_csi_capture_format *format = + &sun6i_csi_capture_formats[i]; - return 0; + /* + * If a media bus code is specified, only consider formats that + * match it. + */ + if (fmtdesc->mbus_code) { + unsigned int j; + + if (!format->mbus_codes) + continue; + + for (j = 0; format->mbus_codes[j]; j++) { + if (fmtdesc->mbus_code == format->mbus_codes[j]) + break; + } + + if (!format->mbus_codes[j]) + continue; + } + + if (index == 0) { + fmtdesc->pixelformat = format->pixelformat; + return 0; + } + + index--; + } + + return -EINVAL; } static int sun6i_csi_capture_g_fmt(struct file *file, void *private, @@ -978,7 +1008,7 @@ 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.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC 2023-01-06 19:40 ` [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC adam @ 2023-01-06 23:34 ` Laurent Pinchart 0 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2023-01-06 23:34 UTC (permalink / raw) To: adam; +Cc: linux-media, yong.deng, mchehab, linux-sunxi, paul.kocialkowski Hi Adam, Thank you for the patch. On Fri, Jan 06, 2023 at 07:40:37PM +0000, adam@piggz.co.uk wrote: > From: Adam Pigg <adam@piggz.co.uk> > > modify sun6i_csi_capture_enum_fmt to check the mbus_codes from the > pixel format array for the supplied format The commit message should explain why. Please also start sentences with a capital letter, and end them with a period. > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > --- > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 36 +++++++++++++++++-- > 1 file changed, 33 insertions(+), 3 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 69578075421c..54beba4d2b2f 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > @@ -666,13 +666,43 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *private, > struct v4l2_fmtdesc *fmtdesc) > { > u32 index = fmtdesc->index; > + unsigned int i; > > if (index >= ARRAY_SIZE(sun6i_csi_capture_formats)) > return -EINVAL; > > - fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat; > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_formats); i++) { > + const struct sun6i_csi_capture_format *format = > + &sun6i_csi_capture_formats[i]; > > - return 0; > + /* > + * If a media bus code is specified, only consider formats that > + * match it. > + */ > + if (fmtdesc->mbus_code) { > + unsigned int j; > + > + if (!format->mbus_codes) > + continue; > + > + for (j = 0; format->mbus_codes[j]; j++) { > + if (fmtdesc->mbus_code == format->mbus_codes[j]) > + break; > + } > + > + if (!format->mbus_codes[j]) > + continue; > + } > + > + if (index == 0) { > + fmtdesc->pixelformat = format->pixelformat; > + return 0; > + } > + > + index--; > + } > + > + return -EINVAL; > } > > static int sun6i_csi_capture_g_fmt(struct file *file, void *private, > @@ -978,7 +1008,7 @@ 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; Let's avoid long lines: video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | V4L2_CAP_IO_MC; With this change and with an updated commit message you can add my Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> in the next version of this patch. > video_dev->vfl_dir = VFL_DIR_RX; > video_dev->release = video_device_release_empty; > video_dev->fops = &sun6i_csi_capture_fops; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes 2023-01-06 19:40 [PATCH 0/3] suns6-csi changes to support libcamera adam 2023-01-06 19:40 ` [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match adam 2023-01-06 19:40 ` [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC adam @ 2023-01-06 19:40 ` adam 2023-01-06 23:35 ` Laurent Pinchart 2023-01-06 23:10 ` [PATCH 0/3] suns6-csi changes to support libcamera piggz1 3 siblings, 1 reply; 18+ messages in thread From: adam @ 2023-01-06 19:40 UTC (permalink / raw) To: linux-media Cc: yong.deng, mchehab, linux-sunxi, paul.kocialkowski, laurent.pinchart, Adam Pigg From: Adam Pigg <adam@piggz.co.uk> Create sun6i_csi_capture_enum_framesizes which defines the min and max frame sizes Signed-off-by: Adam Pigg <adam@piggz.co.uk> --- .../sunxi/sun6i-csi/sun6i_csi_capture.c | 24 +++++++++++++++++++ 1 file changed, 24 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 54beba4d2b2f..2be761e6b650 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c @@ -739,6 +739,29 @@ 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 *fsize) +{ + const struct sun6i_csi_capture_format *format; + + if (fsize->index > 0) + return -EINVAL; + + format = sun6i_csi_capture_format_find(fsize->pixel_format); + if (!format) + return -EINVAL; + + fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; + fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN; + fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX; + fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN; + fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX; + fsize->stepwise.step_width = 1; + fsize->stepwise.step_height = 1; + + return 0; +} + static int sun6i_csi_capture_enum_input(struct file *file, void *private, struct v4l2_input *input) { @@ -775,6 +798,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = { .vidioc_g_fmt_vid_cap = sun6i_csi_capture_g_fmt, .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, -- 2.39.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes 2023-01-06 19:40 ` [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes adam @ 2023-01-06 23:35 ` Laurent Pinchart 2023-02-22 10:43 ` Paul Kocialkowski 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2023-01-06 23:35 UTC (permalink / raw) To: adam; +Cc: linux-media, yong.deng, mchehab, linux-sunxi, paul.kocialkowski Hi Adam, Thank you for the patch. On Fri, Jan 06, 2023 at 07:40:38PM +0000, adam@piggz.co.uk wrote: > From: Adam Pigg <adam@piggz.co.uk> > > Create sun6i_csi_capture_enum_framesizes which defines the min > and max frame sizes With the commit message updated (see review of 1/3), Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > --- > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 24 +++++++++++++++++++ > 1 file changed, 24 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 54beba4d2b2f..2be761e6b650 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > @@ -739,6 +739,29 @@ 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 *fsize) > +{ > + const struct sun6i_csi_capture_format *format; > + > + if (fsize->index > 0) > + return -EINVAL; > + > + format = sun6i_csi_capture_format_find(fsize->pixel_format); > + if (!format) > + return -EINVAL; > + > + fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; > + fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN; > + fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX; > + fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN; > + fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX; > + fsize->stepwise.step_width = 1; > + fsize->stepwise.step_height = 1; > + > + return 0; > +} > + > static int sun6i_csi_capture_enum_input(struct file *file, void *private, > struct v4l2_input *input) > { > @@ -775,6 +798,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = { > .vidioc_g_fmt_vid_cap = sun6i_csi_capture_g_fmt, > .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, -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes 2023-01-06 23:35 ` Laurent Pinchart @ 2023-02-22 10:43 ` Paul Kocialkowski 2023-03-25 23:12 ` Laurent Pinchart 0 siblings, 1 reply; 18+ messages in thread From: Paul Kocialkowski @ 2023-02-22 10:43 UTC (permalink / raw) To: Laurent Pinchart; +Cc: adam, linux-media, yong.deng, mchehab, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 3042 bytes --] Hi, On Sat 07 Jan 23, 01:35, Laurent Pinchart wrote: > Hi Adam, > > Thank you for the patch. > > On Fri, Jan 06, 2023 at 07:40:38PM +0000, adam@piggz.co.uk wrote: > > From: Adam Pigg <adam@piggz.co.uk> > > > > Create sun6i_csi_capture_enum_framesizes which defines the min > > and max frame sizes > > With the commit message updated (see review of 1/3), > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'm always a bit confused regarding how such an ioctl's behavior should depend on the attached subdev. Is it well-defined behavior that this here is only about the receiver part and has nothing to do with what the connected sensor? > > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > > --- > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 24 +++++++++++++++++++ > > 1 file changed, 24 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 54beba4d2b2f..2be761e6b650 100644 > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > @@ -739,6 +739,29 @@ 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 *fsize) A cosmetic/consistency suggestion would be to name this variable "frmsize" to reuse part of the name of the structure, which is what I've done in other places of the driver. Cheers, Paul > > +{ > > + const struct sun6i_csi_capture_format *format; > > + > > + if (fsize->index > 0) > > + return -EINVAL; > > + > > + format = sun6i_csi_capture_format_find(fsize->pixel_format); > > + if (!format) > > + return -EINVAL; > > + > > + fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; > > + fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN; > > + fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX; > > + fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN; > > + fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX; > > + fsize->stepwise.step_width = 1; > > + fsize->stepwise.step_height = 1; > > + > > + return 0; > > +} > > + > > static int sun6i_csi_capture_enum_input(struct file *file, void *private, > > struct v4l2_input *input) > > { > > @@ -775,6 +798,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = { > > .vidioc_g_fmt_vid_cap = sun6i_csi_capture_g_fmt, > > .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, > > -- > Regards, > > Laurent Pinchart -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes 2023-02-22 10:43 ` Paul Kocialkowski @ 2023-03-25 23:12 ` Laurent Pinchart 0 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2023-03-25 23:12 UTC (permalink / raw) To: Paul Kocialkowski; +Cc: adam, linux-media, yong.deng, mchehab, linux-sunxi Hi Paul, On Wed, Feb 22, 2023 at 11:43:06AM +0100, Paul Kocialkowski wrote: > On Sat 07 Jan 23, 01:35, Laurent Pinchart wrote: > > On Fri, Jan 06, 2023 at 07:40:38PM +0000, adam@piggz.co.uk wrote: > > > From: Adam Pigg <adam@piggz.co.uk> > > > > > > Create sun6i_csi_capture_enum_framesizes which defines the min > > > and max frame sizes > > > > With the commit message updated (see review of 1/3), > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'm always a bit confused regarding how such an ioctl's behavior should depend > on the attached subdev. Is it well-defined behavior that this here is only > about the receiver part and has nothing to do with what the connected sensor? That's right. For MC-based drivers, enumerating pixel formats and frame sizes on a video node should return the formats and sizes intrinsically supported by the video node (in most cases, that maps to the DMA engine at the hardware level) without considering connected subdevs. > > > Signed-off-by: Adam Pigg <adam@piggz.co.uk> > > > --- > > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 24 +++++++++++++++++++ > > > 1 file changed, 24 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 54beba4d2b2f..2be761e6b650 100644 > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > > > @@ -739,6 +739,29 @@ 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 *fsize) > > A cosmetic/consistency suggestion would be to name this variable "frmsize" to > reuse part of the name of the structure, which is what I've done in other places > of the driver. > > > > +{ > > > + const struct sun6i_csi_capture_format *format; > > > + > > > + if (fsize->index > 0) > > > + return -EINVAL; > > > + > > > + format = sun6i_csi_capture_format_find(fsize->pixel_format); > > > + if (!format) > > > + return -EINVAL; > > > + > > > + fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; > > > + fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN; > > > + fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX; > > > + fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN; > > > + fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX; > > > + fsize->stepwise.step_width = 1; > > > + fsize->stepwise.step_height = 1; > > > + > > > + return 0; > > > +} > > > + > > > static int sun6i_csi_capture_enum_input(struct file *file, void *private, > > > struct v4l2_input *input) > > > { > > > @@ -775,6 +798,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = { > > > .vidioc_g_fmt_vid_cap = sun6i_csi_capture_g_fmt, > > > .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, -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] suns6-csi changes to support libcamera 2023-01-06 19:40 [PATCH 0/3] suns6-csi changes to support libcamera adam ` (2 preceding siblings ...) 2023-01-06 19:40 ` [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes adam @ 2023-01-06 23:10 ` piggz1 2023-01-07 8:43 ` Adam Pigg 3 siblings, 1 reply; 18+ messages in thread From: piggz1 @ 2023-01-06 23:10 UTC (permalink / raw) To: adam, linux-media Cc: mchehab, linux-sunxi, paul.kocialkowski, laurent.pinchart, adam, yong.deng Apologies, accidentally sent this set twice. please ignore this set. Adam On Friday, 6 January 2023, adam@piggz.co.uk wrote: > From: Adam Pigg <adam@piggz.co.uk> > > This 3 patch set adds V4L2_CAP_IO_MC and vidioc_enum_framesizes support > to the sun6i-csi driver, allowing the Pinephone rear camera (ov5640) to > be supported. > > These have been developed with the guidance of Laurent Pinchart, and > been tested by capturing frames from the libcamera cam utility. > > In addition to these, a patch to change the v4l gain mode in the ov5640 > driver, and the 2 format propagation patches all by Laurnet are > required. > > Adam Pigg (3): > media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match > media: sun6i-csi: implement V4L2_CAP_IO_MC > media: sun6i-csi: implement vidioc_enum_framesizes > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 215 +++++++++--------- > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > 2 files changed, 103 insertions(+), 118 deletions(-) > > -- > 2.39.0 > > -- Sent from my Sailfish device ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] suns6-csi changes to support libcamera 2023-01-06 23:10 ` [PATCH 0/3] suns6-csi changes to support libcamera piggz1 @ 2023-01-07 8:43 ` Adam Pigg 0 siblings, 0 replies; 18+ messages in thread From: Adam Pigg @ 2023-01-07 8:43 UTC (permalink / raw) To: linux-media, piggz1 Cc: mchehab, linux-sunxi, paul.kocialkowski, laurent.pinchart, yong.deng [-- Attachment #1: Type: text/plain, Size: 1513 bytes --] On Friday, 6 January 2023 23:10:46 GMT piggz1@gmail.com wrote: > Apologies, accidentally sent this set twice. please ignore this set. > Ignore this, the first set I sent myself, and I didnt actually send it twice to the list! Also, if you see 2 email addresses, adam@piggz.co.uk is a forwarding address to GMail. I could see how to make git-send-email set the preferred sender address, only the from address. > Adam > > On Friday, 6 January 2023, adam@piggz.co.uk wrote: > > > From: Adam Pigg <adam@piggz.co.uk> > > > > This 3 patch set adds V4L2_CAP_IO_MC and vidioc_enum_framesizes support > > to the sun6i-csi driver, allowing the Pinephone rear camera (ov5640) to > > be supported. > > > > These have been developed with the guidance of Laurent Pinchart, and > > been tested by capturing frames from the libcamera cam utility. > > > > In addition to these, a patch to change the v4l gain mode in the ov5640 > > driver, and the 2 format propagation patches all by Laurnet are > > required. > > > > Adam Pigg (3): > > > > media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match > > media: sun6i-csi: implement V4L2_CAP_IO_MC > > media: sun6i-csi: implement vidioc_enum_framesizes > > > > > > > > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 215 +++++++++--------- > > .../sunxi/sun6i-csi/sun6i_csi_capture.h | 6 +- > > 2 files changed, 103 insertions(+), 118 deletions(-) > > > > > > -- > > 2.39.0 > > > > > > > > > -- > Sent from my Sailfish device [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-03-25 23:17 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-06 19:40 [PATCH 0/3] suns6-csi changes to support libcamera adam 2023-01-06 19:40 ` [PATCH 1/3] media: sun6i-csi: merge sun6i_csi_formats and sun6i_csi_formats_match adam 2023-01-06 23:31 ` Laurent Pinchart 2023-01-08 18:23 ` Adam Pigg 2023-01-08 20:26 ` Laurent Pinchart 2023-02-17 18:20 ` Laurent Pinchart 2023-02-22 10:39 ` Paul Kocialkowski 2023-03-21 17:50 ` Adam Pigg 2023-03-23 12:55 ` Paul Kocialkowski 2023-03-25 23:17 ` Laurent Pinchart 2023-01-06 19:40 ` [PATCH 2/3] media: sun6i-csi: implement V4L2_CAP_IO_MC adam 2023-01-06 23:34 ` Laurent Pinchart 2023-01-06 19:40 ` [PATCH 3/3] media: sun6i-csi: implement vidioc_enum_framesizes adam 2023-01-06 23:35 ` Laurent Pinchart 2023-02-22 10:43 ` Paul Kocialkowski 2023-03-25 23:12 ` Laurent Pinchart 2023-01-06 23:10 ` [PATCH 0/3] suns6-csi changes to support libcamera piggz1 2023-01-07 8:43 ` Adam Pigg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox