On 4/24/19 10:56 AM, André Almeida wrote: > Since multiplanar is a superset of single planar formats, instead of > having different implementations for them, treat all formats as > multiplanar. If we need to work with single planar formats, convert them to > multiplanar (with num_planes = 1), re-use the multiplanar code, and > transform them back to single planar. This is implemented with > v4l2_fmt_sp2mp_func(). > > Signed-off-by: André Almeida <andrealmeid@xxxxxxxxxxxxx> > --- > Changes in v3: > - Commit title renamed > - Get rid of `if (IS_MULTIPLANAR(vcap))` checks and functions to do this > check > > Changes in v2: > - Make more clear that the generic function _vimc_cap_try_fmt_vid_cap_mp > expects a multiplanar format as input > > drivers/media/platform/vimc/vimc-capture.c | 51 +++++++++++++--------- > 1 file changed, 30 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c > index ae703c52d3f8..fbe51004aba8 100644 > --- a/drivers/media/platform/vimc/vimc-capture.c > +++ b/drivers/media/platform/vimc/vimc-capture.c > @@ -128,10 +128,10 @@ static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv, > return 0; > } > > -static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv, > - struct v4l2_format *f) > +static int vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv, > + struct v4l2_format *f) > { > - struct v4l2_pix_format *format = &f->fmt.pix; > + struct v4l2_pix_format_mplane *format = &f->fmt.pix_mp; > > format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH, > VIMC_FRAME_MAX_WIDTH) & ~1; > @@ -149,12 +149,32 @@ static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv, > if (!v4l2_format_info(format->pixelformat)) > format->pixelformat = fmt_default.fmt.pix.pixelformat; > > - return v4l2_fill_pixfmt(format, format->pixelformat, > - format->width, format->height); > + return v4l2_fill_pixfmt_mp(format, format->pixelformat, > + format->width, format->height); > } > > -static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv, > - struct v4l2_format *f) > +static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv, > + struct v4l2_fmtdesc *f) > +{ > + if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt)) > + return -EINVAL; > + > + f->pixelformat = vimc_cap_supported_pixfmt[f->index]; > + > + return 0; > +} > + > +/* > + * VIDIOC FMT handlers for single-planar > + */ > +static int vimc_cap_try_fmt_vid_cap_sp(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + return v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp); > +} > + > +static int vimc_cap_s_fmt_vid_cap_sp(struct file *file, void *priv, > + struct v4l2_format *f) > { > struct vimc_cap_device *vcap = video_drvdata(file); > > @@ -162,7 +182,7 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv, > if (vb2_is_busy(&vcap->queue)) > return -EBUSY; > > - vimc_cap_try_fmt_vid_cap(file, priv, f); > + v4l2_fmt_sp2mp_func(file, priv, f, vimc_cap_try_fmt_vid_cap_mp); I know the original code wasn't checking for errors in this func, but if you could add a check here it would be great. Helen > > dev_dbg(vcap->dev, "%s: format update: " > "old:%dx%d (0x%x, %d, %d, %d, %d) " > @@ -185,17 +205,6 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv, > return 0; > } > > -static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv, > - struct v4l2_fmtdesc *f) > -{ > - if (f->index >= ARRAY_SIZE(vimc_cap_supported_pixfmt)) > - return -EINVAL; > - > - f->pixelformat = vimc_cap_supported_pixfmt[f->index]; > - > - return 0; > -} > - > static bool vimc_cap_is_pixfmt_supported(u32 pixelformat) > { > unsigned int i; > @@ -240,8 +249,8 @@ static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = { > .vidioc_querycap = vimc_cap_querycap, > > .vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap, > - .vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap, > - .vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap, > + .vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap_sp, > + .vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap_sp, > .vidioc_enum_fmt_vid_cap = vimc_cap_enum_fmt_vid_cap, > .vidioc_enum_framesizes = vimc_cap_enum_framesizes, > >