| Message ID | 20230324151228.2778112-6-paul.kocialkowski@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: sun6i-csi/isp: Implement MC I/O support | expand |
Dne petek, 24. marec 2023 ob 16:12:24 CET je Paul Kocialkowski napisal(a): > Introduce a new sun6i_csi_capture_format_check helper to indicate > whether a set of pixel format and mbus code are compatible. > Most of the logic is taken from sun6i_csi_capture_link_validate, > with extra checks added along the way. > > Note that v4l2_format_info is now used for all pixel formats > since they should all be listed in the helper at this point. > > The motivation behind this change is to pave the way for supporting > the mc-style enum_fmt. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
Hi Paul, Thank you for the patch. On Fri, Mar 24, 2023 at 04:12:24PM +0100, Paul Kocialkowski wrote: > Introduce a new sun6i_csi_capture_format_check helper to indicate > whether a set of pixel format and mbus code are compatible. > Most of the logic is taken from sun6i_csi_capture_link_validate, > with extra checks added along the way. > > Note that v4l2_format_info is now used for all pixel formats > since they should all be listed in the helper at this point. > > The motivation behind this change is to pave the way for supporting > the mc-style enum_fmt. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 95 ++++++++++--------- > 1 file changed, 49 insertions(+), 46 deletions(-) > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > index cf6aadbc130b..6ce7f1d3ed57 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > @@ -327,6 +327,52 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code) > return false; > } > > +static bool sun6i_csi_capture_format_check(u32 pixelformat, u32 mbus_code) > +{ > + const struct sun6i_csi_capture_format *capture_format; > + const struct sun6i_csi_bridge_format *bridge_format; I think I would have named those output_format and input_format respectively, as "capture" and "bridge" take a bit more mental processing when reading the code. That may be because I'm not familiar with the driver though, so feel free to ignore this. > + const struct v4l2_format_info *format_info; > + > + format_info = v4l2_format_info(pixelformat); > + if (WARN_ON(!format_info)) > + return false; > + > + capture_format = sun6i_csi_capture_format_find(pixelformat); > + if (WARN_ON(!capture_format)) > + return false; > + > + bridge_format = sun6i_csi_bridge_format_find(mbus_code); > + if (WARN_ON(!bridge_format)) > + return false; > + > + /* Raw input is required for non-YUV formats. */ > + if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW && > + (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER || > + format_info->pixel_enc == V4L2_PIXEL_ENC_RGB || > + format_info->pixel_enc == V4L2_PIXEL_ENC_COMPRESSED)) Unless I'm mistaken, the compressed format check is new. You could split it to a separate patch, or at least mention it in the commit message. > + return false; > + > + if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) { > + /* YUV input is required for YUV pixels. */ > + if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 && > + bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422) > + return false; > + > + /* YUV420 input can't produce (upsampled) YUV422 output. */ > + if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 && > + format_info->vdiv == 1) > + return false; > + } > + > + /* Raw input requires a 1:1 match between input and output. */ > + if ((bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW || > + capture_format->input_format_raw) && > + !sun6i_csi_capture_format_match(pixelformat, mbus_code)) > + return false; Wrong indentation. With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + return true; > +} > + > /* Capture */ > > static void > @@ -890,28 +936,16 @@ static int sun6i_csi_capture_link_validate(struct media_link *link) > media_entity_to_video_device(link->sink->entity); > struct sun6i_csi_device *csi_dev = video_get_drvdata(video_dev); > struct v4l2_device *v4l2_dev = csi_dev->v4l2_dev; > - const struct sun6i_csi_capture_format *capture_format; > - const struct sun6i_csi_bridge_format *bridge_format; > unsigned int capture_width, capture_height; > unsigned int bridge_width, bridge_height; > - const struct v4l2_format_info *format_info; > u32 pixelformat, capture_field; > u32 mbus_code, bridge_field; > - bool match; > > sun6i_csi_capture_dimensions(csi_dev, &capture_width, &capture_height); > - > sun6i_csi_capture_format(csi_dev, &pixelformat, &capture_field); > - capture_format = sun6i_csi_capture_format_find(pixelformat); > - if (WARN_ON(!capture_format)) > - return -EINVAL; > > sun6i_csi_bridge_dimensions(csi_dev, &bridge_width, &bridge_height); > - > sun6i_csi_bridge_format(csi_dev, &mbus_code, &bridge_field); > - bridge_format = sun6i_csi_bridge_format_find(mbus_code); > - if (WARN_ON(!bridge_format)) > - return -EINVAL; > > /* No cropping/scaling is supported. */ > if (capture_width != bridge_width || capture_height != bridge_height) { > @@ -922,43 +956,12 @@ static int sun6i_csi_capture_link_validate(struct media_link *link) > return -EINVAL; > } > > - format_info = v4l2_format_info(pixelformat); > - /* Some formats are not listed. */ > - if (!format_info) > - return 0; > - > - if (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER && > - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW) > - goto invalid; > - > - if (format_info->pixel_enc == V4L2_PIXEL_ENC_RGB && > - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW) > - goto invalid; > - > - if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) { > - if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 && > - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422) > - goto invalid; > - > - /* YUV420 input can't produce YUV422 output. */ > - if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 && > - format_info->vdiv == 1) > - goto invalid; > - } > - > - /* With raw input mode, we need a 1:1 match between input and output. */ > - if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW || > - capture_format->input_format_raw) { > - match = sun6i_csi_capture_format_match(pixelformat, mbus_code); > - if (!match) > - goto invalid; > + if (!sun6i_csi_capture_format_check(pixelformat, mbus_code)) { > + v4l2_err(v4l2_dev, "invalid input/output format combination\n"); > + return -EINVAL; > } > > return 0; > - > -invalid: > - v4l2_err(v4l2_dev, "invalid input/output format combination\n"); > - return -EINVAL; > } > > static const struct media_entity_operations sun6i_csi_capture_media_ops = {
diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c index cf6aadbc130b..6ce7f1d3ed57 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c @@ -327,6 +327,52 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code) return false; } +static bool sun6i_csi_capture_format_check(u32 pixelformat, u32 mbus_code) +{ + const struct sun6i_csi_capture_format *capture_format; + const struct sun6i_csi_bridge_format *bridge_format; + const struct v4l2_format_info *format_info; + + format_info = v4l2_format_info(pixelformat); + if (WARN_ON(!format_info)) + return false; + + capture_format = sun6i_csi_capture_format_find(pixelformat); + if (WARN_ON(!capture_format)) + return false; + + bridge_format = sun6i_csi_bridge_format_find(mbus_code); + if (WARN_ON(!bridge_format)) + return false; + + /* Raw input is required for non-YUV formats. */ + if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW && + (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER || + format_info->pixel_enc == V4L2_PIXEL_ENC_RGB || + format_info->pixel_enc == V4L2_PIXEL_ENC_COMPRESSED)) + return false; + + if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) { + /* YUV input is required for YUV pixels. */ + if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 && + bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422) + return false; + + /* YUV420 input can't produce (upsampled) YUV422 output. */ + if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 && + format_info->vdiv == 1) + return false; + } + + /* Raw input requires a 1:1 match between input and output. */ + if ((bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW || + capture_format->input_format_raw) && + !sun6i_csi_capture_format_match(pixelformat, mbus_code)) + return false; + + return true; +} + /* Capture */ static void @@ -890,28 +936,16 @@ static int sun6i_csi_capture_link_validate(struct media_link *link) media_entity_to_video_device(link->sink->entity); struct sun6i_csi_device *csi_dev = video_get_drvdata(video_dev); struct v4l2_device *v4l2_dev = csi_dev->v4l2_dev; - const struct sun6i_csi_capture_format *capture_format; - const struct sun6i_csi_bridge_format *bridge_format; unsigned int capture_width, capture_height; unsigned int bridge_width, bridge_height; - const struct v4l2_format_info *format_info; u32 pixelformat, capture_field; u32 mbus_code, bridge_field; - bool match; sun6i_csi_capture_dimensions(csi_dev, &capture_width, &capture_height); - sun6i_csi_capture_format(csi_dev, &pixelformat, &capture_field); - capture_format = sun6i_csi_capture_format_find(pixelformat); - if (WARN_ON(!capture_format)) - return -EINVAL; sun6i_csi_bridge_dimensions(csi_dev, &bridge_width, &bridge_height); - sun6i_csi_bridge_format(csi_dev, &mbus_code, &bridge_field); - bridge_format = sun6i_csi_bridge_format_find(mbus_code); - if (WARN_ON(!bridge_format)) - return -EINVAL; /* No cropping/scaling is supported. */ if (capture_width != bridge_width || capture_height != bridge_height) { @@ -922,43 +956,12 @@ static int sun6i_csi_capture_link_validate(struct media_link *link) return -EINVAL; } - format_info = v4l2_format_info(pixelformat); - /* Some formats are not listed. */ - if (!format_info) - return 0; - - if (format_info->pixel_enc == V4L2_PIXEL_ENC_BAYER && - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW) - goto invalid; - - if (format_info->pixel_enc == V4L2_PIXEL_ENC_RGB && - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_RAW) - goto invalid; - - if (format_info->pixel_enc == V4L2_PIXEL_ENC_YUV) { - if (bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV420 && - bridge_format->input_format != SUN6I_CSI_INPUT_FMT_YUV422) - goto invalid; - - /* YUV420 input can't produce YUV422 output. */ - if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_YUV420 && - format_info->vdiv == 1) - goto invalid; - } - - /* With raw input mode, we need a 1:1 match between input and output. */ - if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW || - capture_format->input_format_raw) { - match = sun6i_csi_capture_format_match(pixelformat, mbus_code); - if (!match) - goto invalid; + if (!sun6i_csi_capture_format_check(pixelformat, mbus_code)) { + v4l2_err(v4l2_dev, "invalid input/output format combination\n"); + return -EINVAL; } return 0; - -invalid: - v4l2_err(v4l2_dev, "invalid input/output format combination\n"); - return -EINVAL; } static const struct media_entity_operations sun6i_csi_capture_media_ops = {
Introduce a new sun6i_csi_capture_format_check helper to indicate whether a set of pixel format and mbus code are compatible. Most of the logic is taken from sun6i_csi_capture_link_validate, with extra checks added along the way. Note that v4l2_format_info is now used for all pixel formats since they should all be listed in the helper at this point. The motivation behind this change is to pave the way for supporting the mc-style enum_fmt. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> --- .../sunxi/sun6i-csi/sun6i_csi_capture.c | 95 ++++++++++--------- 1 file changed, 49 insertions(+), 46 deletions(-)