From: jacopo mondi <jacopo@jmondi.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: linux-media@vger.kernel.org,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Jacopo Mondi <jacopo+renesas@jmondi.org>,
Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCHv2 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
Date: Tue, 23 Jan 2018 15:47:42 +0100 [thread overview]
Message-ID: <20180123144742.GA17416@w540> (raw)
In-Reply-To: <20180122123125.24709-3-hverkuil@xs4all.nl>
Hi Hans,
On Mon, Jan 22, 2018 at 01:31:18PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -689,22 +689,14 @@ static int isi_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct atmel_isi *isi = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - a->parm.capture.readbuffers = 2;
> - return v4l2_subdev_call(isi->entity.subdev, video, g_parm, a);
> + return v4l2_g_parm_cap(video_devdata(file), isi->entity.subdev, a);
> }
>
> static int isi_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct atmel_isi *isi = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
I've now tested the right version with CEU, and v4l2-compliance
reports:
fail: v4l2-test-formats.cpp(1218): Video Capture cap not set, but
G/S_PARM worked
To get rid of this error I had to refuse V4L2_BUF_TYPE_VIDEO_CAPTURE
type in g/s_parm as my driver only supports VIDEO_CAPTURE_MPLANE type.
static int ceu_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
{
struct ceu_device *ceudev = video_drvdata(file);
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
return -EINVAL;
return v4l2_s_parm_cap(video_devdata(file), ceudev->sd->v4l2_sd, a);
}
To make sure the same error does not happen for atmel-isi/isc and other
platform drivers this patch modifies, I would keep the checks on
a->type you have removed here.
I now wonder if the following test in the newly added v4l2_g/s_parm_cap()
still makes sense if platform drivers have to check for the correct
buffer type anyhow (the same for the _cap name suffix)
int v4l2_g_parm_cap(struct video_device *vdev,
struct v4l2_subdev *sd, struct v4l2_streamparm *a)
{
struct v4l2_subdev_frame_interval ival = { 0 };
int ret;
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
return -EINVAL;
Thanks
j
> -
> - a->parm.capture.readbuffers = 2;
> - return v4l2_subdev_call(isi->entity.subdev, video, s_parm, a);
> + return v4l2_s_parm_cap(video_devdata(file), isi->entity.subdev, a);
> }
>
> static int isi_enum_framesizes(struct file *file, void *fh,
> diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c
> index 41f179117fb0..b7660b1000fd 100644
> --- a/drivers/media/platform/blackfin/bfin_capture.c
> +++ b/drivers/media/platform/blackfin/bfin_capture.c
> @@ -712,24 +712,18 @@ static int bcap_querycap(struct file *file, void *priv,
> return 0;
> }
>
> -static int bcap_g_parm(struct file *file, void *fh,
> - struct v4l2_streamparm *a)
> +static int bcap_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct bcap_device *bcap_dev = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - return v4l2_subdev_call(bcap_dev->sd, video, g_parm, a);
> + return v4l2_g_parm_cap(video_devdata(file), bcap_dev->sd, a);
> }
>
> -static int bcap_s_parm(struct file *file, void *fh,
> - struct v4l2_streamparm *a)
> +static int bcap_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct bcap_device *bcap_dev = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - return v4l2_subdev_call(bcap_dev->sd, video, s_parm, a);
> + return v4l2_s_parm_cap(video_devdata(file), bcap_dev->sd, a);
> }
>
> static int bcap_log_status(struct file *file, void *priv)
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
> index 7b7250b1cff8..80670eeee142 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
> @@ -1443,24 +1443,24 @@ static int mcam_vidioc_s_input(struct file *filp, void *priv, unsigned int i)
> * the level which controls the number of read buffers.
> */
> static int mcam_vidioc_g_parm(struct file *filp, void *priv,
> - struct v4l2_streamparm *parms)
> + struct v4l2_streamparm *a)
> {
> struct mcam_camera *cam = video_drvdata(filp);
> int ret;
>
> - ret = sensor_call(cam, video, g_parm, parms);
> - parms->parm.capture.readbuffers = n_dma_bufs;
> + ret = v4l2_g_parm_cap(video_devdata(filp), cam->sensor, a);
> + a->parm.capture.readbuffers = n_dma_bufs;
> return ret;
> }
>
> static int mcam_vidioc_s_parm(struct file *filp, void *priv,
> - struct v4l2_streamparm *parms)
> + struct v4l2_streamparm *a)
> {
> struct mcam_camera *cam = video_drvdata(filp);
> int ret;
>
> - ret = sensor_call(cam, video, s_parm, parms);
> - parms->parm.capture.readbuffers = n_dma_bufs;
> + ret = v4l2_s_parm_cap(video_devdata(filp), cam->sensor, a);
> + a->parm.capture.readbuffers = n_dma_bufs;
> return ret;
> }
>
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index d13e2c5fb06f..1a9d4610045f 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1788,17 +1788,19 @@ static int default_s_selection(struct soc_camera_device *icd,
> }
>
> static int default_g_parm(struct soc_camera_device *icd,
> - struct v4l2_streamparm *parm)
> + struct v4l2_streamparm *a)
> {
> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> - return v4l2_subdev_call(sd, video, g_parm, parm);
> +
> + return v4l2_g_parm_cap(icd->vdev, sd, a);
> }
>
> static int default_s_parm(struct soc_camera_device *icd,
> - struct v4l2_streamparm *parm)
> + struct v4l2_streamparm *a)
> {
> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> - return v4l2_subdev_call(sd, video, s_parm, parm);
> +
> + return v4l2_s_parm_cap(icd->vdev, sd, a);
> }
>
> static int default_enum_framesizes(struct soc_camera_device *icd,
> diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c
> index 805d4a8fc17e..c632279a4209 100644
> --- a/drivers/media/platform/via-camera.c
> +++ b/drivers/media/platform/via-camera.c
> @@ -1112,7 +1112,7 @@ static int viacam_g_parm(struct file *filp, void *priv,
> int ret;
>
> mutex_lock(&cam->lock);
> - ret = sensor_call(cam, video, g_parm, parm);
> + ret = v4l2_g_parm_cap(video_devdata(filp), cam->sensor, parm);
> mutex_unlock(&cam->lock);
> parm->parm.capture.readbuffers = cam->n_cap_bufs;
> return ret;
> @@ -1125,7 +1125,7 @@ static int viacam_s_parm(struct file *filp, void *priv,
> int ret;
>
> mutex_lock(&cam->lock);
> - ret = sensor_call(cam, video, s_parm, parm);
> + ret = v4l2_s_parm_cap(video_devdata(filp), cam->sensor, parm);
> mutex_unlock(&cam->lock);
> parm->parm.capture.readbuffers = cam->n_cap_bufs;
> return ret;
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index a2ba2d905952..2724e3b99af2 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -1582,17 +1582,26 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
> static int vidioc_g_parm(struct file *file, void *priv,
> struct v4l2_streamparm *p)
> {
> + struct v4l2_subdev_frame_interval ival = { 0 };
> struct em28xx *dev = video_drvdata(file);
> struct em28xx_v4l2 *v4l2 = dev->v4l2;
> int rc = 0;
>
> + if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return -EINVAL;
> +
> p->parm.capture.readbuffers = EM28XX_MIN_BUF;
> - if (dev->board.is_webcam)
> + p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + if (dev->board.is_webcam) {
> rc = v4l2_device_call_until_err(&v4l2->v4l2_dev, 0,
> - video, g_parm, p);
> - else
> + video, g_frame_interval, &ival);
> + if (!rc)
> + p->parm.capture.timeperframe = ival.interval;
> + } else {
> v4l2_video_std_frame_period(v4l2->norm,
> &p->parm.capture.timeperframe);
> + }
>
> return rc;
> }
> @@ -1601,10 +1610,27 @@ static int vidioc_s_parm(struct file *file, void *priv,
> struct v4l2_streamparm *p)
> {
> struct em28xx *dev = video_drvdata(file);
> + struct v4l2_subdev_frame_interval ival = {
> + 0,
> + p->parm.capture.timeperframe
> + };
> + int rc = 0;
> +
> + if (!dev->board.is_webcam)
> + return -ENOTTY;
>
> + if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return -EINVAL;
> +
> + memset(&p->parm, 0, sizeof(p->parm));
> p->parm.capture.readbuffers = EM28XX_MIN_BUF;
> - return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,
> - 0, video, s_parm, p);
> + p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
> + video, s_frame_interval, &ival);
> + if (!rc)
> + p->parm.capture.timeperframe = ival.interval;
> + return rc;
> }
>
> static int vidioc_enum_input(struct file *file, void *priv,
> --
> 2.15.1
>
next prev parent reply other threads:[~2018-01-23 14:48 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-22 12:31 [PATCHv2 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
2018-02-14 15:50 ` Mauro Carvalho Chehab
2018-02-14 16:23 ` Hans Verkuil
2018-02-14 16:35 ` Mauro Carvalho Chehab
2018-02-14 16:47 ` Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
2018-01-23 14:47 ` jacopo mondi [this message]
2018-01-23 14:52 ` Hans Verkuil
2018-02-14 16:03 ` Mauro Carvalho Chehab
2018-02-14 16:34 ` Hans Verkuil
2018-02-14 17:02 ` Mauro Carvalho Chehab
2018-02-14 17:16 ` Hans Verkuil
2018-02-14 17:30 ` Mauro Carvalho Chehab
2018-01-22 12:31 ` [PATCHv2 3/9] staging: atomisp: Kill subdev s_parm abuse Hans Verkuil
2018-02-14 16:14 ` Mauro Carvalho Chehab
2018-02-16 9:04 ` Sakari Ailus
2018-02-16 11:58 ` Mauro Carvalho Chehab
2018-02-19 11:01 ` Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 4/9] staging: atomisp: i2c: Disable non-preview configurations Hans Verkuil
2018-02-14 16:20 ` Mauro Carvalho Chehab
2018-02-16 10:12 ` Sakari Ailus
2018-02-16 12:04 ` Mauro Carvalho Chehab
2018-02-19 12:24 ` Sakari Ailus
2018-02-19 13:27 ` [PATCH v2.1 " Sakari Ailus
2018-02-19 13:30 ` Hans Verkuil
2018-02-19 13:48 ` Sakari Ailus
2018-02-19 13:58 ` Hans Verkuil
2018-02-19 14:05 ` Sakari Ailus
2018-01-22 12:31 ` [PATCHv2 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 8/9] v4l2-subdev.h: remove obsolete g/s_parm Hans Verkuil
2018-01-22 12:31 ` [PATCHv2 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types Hans Verkuil
2018-01-22 12:38 ` [PATCHv2 0/9] media: replace g/s_parm by g/s_frame_interval Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180123144742.GA17416@w540 \
--to=jacopo@jmondi.org \
--cc=hans.verkuil@cisco.com \
--cc=hverkuil@xs4all.nl \
--cc=jacopo+renesas@jmondi.org \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.