On 4/26/19 9:51 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@xxxxxxxxxxxxx>
> Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
> ---
> Change in v5: none
>
> Change in v4:
> - Split from multiplanar parameter commit
> - Use `IS_MULTIPLANAR` instead of `multiplanar`
> - Move "Functions to handle..." to commit 5
>
> Change in v3:
> - Remove functions that only did the `IS_MULTIPLANAR(vcap)` check
> - Squash with multiplanar parameter commit
>
> Change in v2:
> - Fix alignment
>
> drivers/media/platform/vimc/vimc-capture.c | 63 +++++++++++++++++++---
> 1 file changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
> index fa0d75eb06e8..652d3d08bc50 100644
> --- a/drivers/media/platform/vimc/vimc-capture.c
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -211,6 +211,44 @@ static int vimc_cap_s_fmt_vid_cap_sp(struct file *file, void *priv,
> return 0;
> }
>
> +/*
> + * VIDIOC handlers for multi-planar formats
> + */
> +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);
> + int ret;
> +
> + /* Do not change the format while stream is on */
> + if (vb2_is_busy(&vcap->queue))
> + return -EBUSY;
> +
> + ret = vimc_cap_try_fmt_vid_cap_mp(file, priv, f);
> + if (ret)
> + return ret;
> +
> + 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 bool vimc_cap_is_pixfmt_supported(u32 pixelformat)
> {
> unsigned int i;
> @@ -251,13 +289,9 @@ static const struct v4l2_file_operations vimc_cap_fops = {
> .mmap = vb2_fop_mmap,
> };
>
> -static const struct v4l2_ioctl_ops vimc_cap_ioctl_ops = {
> +static 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_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,
>
> .vidioc_reqbufs = vb2_ioctl_reqbufs,
> @@ -455,6 +489,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
> {
> struct v4l2_device *v4l2_dev = master_data;
> struct vimc_platform_data *pdata = comp->platform_data;
> + struct v4l2_ioctl_ops *ioctl_ops = &vimc_cap_ioctl_ops;
> struct vimc_cap_device *vcap;
> struct video_device *vdev;
> struct vb2_queue *q;
> @@ -527,7 +562,23 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master,
> vdev->entity.ops = &vimc_cap_mops;
> vdev->release = vimc_cap_release;
> vdev->fops = &vimc_cap_fops;
> - vdev->ioctl_ops = &vimc_cap_ioctl_ops;
> +
> + if (IS_MULTIPLANAR(vcap)) {
> + ioctl_ops->vidioc_g_fmt_vid_cap_mplane = vimc_cap_g_fmt_vid_cap;
> + ioctl_ops->vidioc_s_fmt_vid_cap_mplane =
> + vimc_cap_s_fmt_vid_cap_mp;
> + ioctl_ops->vidioc_try_fmt_vid_cap_mplane =
> + vimc_cap_try_fmt_vid_cap_mp;
> + ioctl_ops->vidioc_enum_fmt_vid_cap_mplane =
> + vimc_cap_enum_fmt_vid_cap;
> + } else {
> + ioctl_ops->vidioc_g_fmt_vid_cap = vimc_cap_g_fmt_vid_cap;
> + ioctl_ops->vidioc_s_fmt_vid_cap = vimc_cap_s_fmt_vid_cap_sp;
> + ioctl_ops->vidioc_try_fmt_vid_cap = vimc_cap_try_fmt_vid_cap_sp;
> + ioctl_ops->vidioc_enum_fmt_vid_cap = vimc_cap_enum_fmt_vid_cap;
> + }
I don't really like this approach. It is good practice to keep vimc_cap_ioctl_ops
a const struct.
Instead do the same thing that vivid does: see e.g. vidioc_s_fmt_vid_cap and
vidioc_s_fmt_vid_cap_mplane in vivid-vid-cap.c.
Regards,
Hans
> +
> + vdev->ioctl_ops = ioctl_ops;
> vdev->lock = &vcap->lock;
> vdev->queue = q;
> vdev->v4l2_dev = v4l2_dev;
>
