| Message ID | 20230324151228.2778112-1-paul.kocialkowski@bootlin.com |
|---|---|
| Headers | show |
| Series | media: sun6i-csi/isp: Implement MC I/O support | expand |
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 = {
Hi Paul, Thank you for the patch. On Fri, Mar 24, 2023 at 04:12:26PM +0100, Paul Kocialkowski wrote: > Report available frame sizes as a continuous range between the > hardware min/max limits. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > Co-authored-by: Adam Pigg <adam@piggz.co.uk> > Signed-off-by: Adam Pigg <adam@piggz.co.uk> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > .../sunxi/sun6i-csi/sun6i_csi_capture.c | 26 +++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > index 9627030ff060..31ba83014086 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c > @@ -847,6 +847,30 @@ static int sun6i_csi_capture_try_fmt(struct file *file, void *private, > return 0; > } > > +static int > +sun6i_csi_capture_enum_framesizes(struct file *file, void *fh, > + struct v4l2_frmsizeenum *frmsizeenum) > +{ > + const struct sun6i_csi_capture_format *format; > + > + if (frmsizeenum->index > 0) > + return -EINVAL; > + > + format = sun6i_csi_capture_format_find(frmsizeenum->pixel_format); > + if (!format) > + return -EINVAL; > + > + frmsizeenum->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; > + frmsizeenum->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN; > + frmsizeenum->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX; > + frmsizeenum->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN; > + frmsizeenum->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX; > + frmsizeenum->stepwise.step_width = 1; > + frmsizeenum->stepwise.step_height = 1; > + > + return 0; > +} > + > static int sun6i_csi_capture_enum_input(struct file *file, void *private, > struct v4l2_input *input) > { > @@ -884,6 +908,8 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = { > .vidioc_s_fmt_vid_cap = sun6i_csi_capture_s_fmt, > .vidioc_try_fmt_vid_cap = sun6i_csi_capture_try_fmt, > > + .vidioc_enum_framesizes = sun6i_csi_capture_enum_framesizes, > + > .vidioc_enum_input = sun6i_csi_capture_enum_input, > .vidioc_g_input = sun6i_csi_capture_g_input, > .vidioc_s_input = sun6i_csi_capture_s_input,