Re: [PATCH 08/16] media: vimc: cap: Add handler for multiplanar fmt ioctls

[Helen Koike <helen.koike@collabora.com>]

On 3/15/19 1:43 PM, André Almeida wrote:
> Add functions to handle multiplanar format ioctls, calling
> the generic format ioctls functions when possible.
> 
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vimc/vimc-capture.c | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index 47acf50f1ad2..09a8fd618b12 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -114,6 +114,10 @@ static void vimc_cap_get_format(struct vimc_ent_device *ved,
>  	*fmt = vcap->format.fmt.pix;
>  }
>  
> +/*
> + * Functions to handle both single- and multi-planar VIDIOC FMT
> + */
> +
>  static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
>  				  struct v4l2_format *f)
>  {
> @@ -237,6 +241,71 @@ static bool vimc_cap_is_pixfmt_supported(u32 pixelformat)
>  			return true;
>  	return false;
>  }
> +/*
> + * VIDIOC handlers for multi-planar formats
> + */
> +
> +static int vimc_cap_g_fmt_vid_cap_mp(struct file *file, void *priv,
> +				  struct v4l2_format *f)

Please align this second line with the parameters of the function (this
also applies to other places).

Regards,
Helen

> +{
> +	if (!multiplanar)
> +		return -EINVAL;
> +
> +	return vimc_cap_g_fmt_vid_cap(file, priv, f);
> +}
> +
> +static int vimc_cap_try_fmt_vid_cap_mp(struct file *file, void *priv,
> +				  struct v4l2_format *f)
> +{
> +	if (!multiplanar)
> +		return -EINVAL;
> +
> +	return vimc_cap_try_fmt_vid_cap(file, priv, f);
> +}
> +
> +static int vimc_cap_s_fmt_vid_cap_mp(struct file *file, void *priv,
> +				  struct v4l2_format *f)
> +{
> +	struct vimc_cap_device *vcap = video_drvdata(file);
> +
> +	if (!multiplanar)
> +		return -EINVAL;
> +
> +	/* Do not change the format while stream is on */
> +	if (vb2_is_busy(&vcap->queue))
> +		return -EBUSY;
> +
> +	vimc_cap_try_fmt_vid_cap(file, priv, f);
> +
> +	dev_dbg(vcap->dev, "%s: format update: "
> +		"old:%dx%d (0x%x, %d, %d, %d, %d) "
> +		"new:%dx%d (0x%x, %d, %d, %d, %d)\n", vcap->vdev.name,
> +		/* old */
> +		vcap->format.fmt.pix_mp.width, vcap->format.fmt.pix_mp.height,
> +		vcap->format.fmt.pix_mp.pixelformat,
> +		vcap->format.fmt.pix_mp.colorspace,
> +		vcap->format.fmt.pix_mp.quantization,
> +		vcap->format.fmt.pix_mp.xfer_func,
> +		vcap->format.fmt.pix_mp.ycbcr_enc,
> +		/* new */
> +		f->fmt.pix_mp.width, f->fmt.pix_mp.height,
> +		f->fmt.pix_mp.pixelformat, f->fmt.pix_mp.colorspace,
> +		f->fmt.pix_mp.quantization, f->fmt.pix_mp.xfer_func,
> +		f->fmt.pix_mp.ycbcr_enc);
> +
> +	vcap->format = *f;
> +
> +	return 0;
> +}
> +
> +static int vimc_cap_enum_fmt_vid_cap_mp(struct file *file, void *priv,
> +				     struct v4l2_fmtdesc *f)
> +{
> +	if (!multiplanar)
> +		return -EINVAL;
> +
> +	return vimc_cap_enum_fmt_vid_cap(file, priv, f);
> +}
>  
>  static int vimc_cap_enum_framesizes(struct file *file, void *fh,
>  				    struct v4l2_frmsizeenum *fsize)
> @@ -268,14 +337,24 @@ static const struct v4l2_file_operations vimc_cap_fops = {
>  	.mmap           = vb2_fop_mmap,
>  };
>  
> +
>  static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
>  	.vidioc_querycap = vimc_cap_querycap,
>  
> +	/**
> +	 * The vidioc_*_vid_cap* functions acts as a front end to
> +	 * vimc_*_vid_cap, dealing with the single- and multi-planar
> +	 */
>  	.vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap_sp,
>  	.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_sp,
>  
> +	.vidioc_g_fmt_vid_cap_mplane = vimc_cap_g_fmt_vid_cap_mp,
> +	.vidioc_s_fmt_vid_cap_mplane = vimc_cap_s_fmt_vid_cap_mp,
> +	.vidioc_try_fmt_vid_cap_mplane = vimc_cap_try_fmt_vid_cap_mp,
> +	.vidioc_enum_fmt_vid_cap_mplane = vimc_cap_enum_fmt_vid_cap_mp,
> +
>  	.vidioc_enum_framesizes = vimc_cap_enum_framesizes,
>  
>  	.vidioc_reqbufs = vb2_ioctl_reqbufs,
>