Hi Adam, Thank you for the patch. On Fri, Jan 06, 2023 at 07:40:36PM +0000, adam@xxxxxxxxxxx wrote: > From: Adam Pigg <adam@xxxxxxxxxxx> > > 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@xxxxxxxxxxx> > --- > .../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
