* [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval
@ 2018-01-22 10:18 Hans Verkuil
2018-01-22 10:18 ` [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
` (8 more replies)
0 siblings, 9 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi
From: Hans Verkuil <hans.verkuil@cisco.com>
There are currently two subdev ops variants to get/set the frame interval:
g/s_parm and g/s_frame_interval.
This patch series replaces all g/s_parm calls by g/s_frame_interval.
The first patch adds helper functions that can be used by bridge drivers.
Only em28xx can't use it and it needs custom code (it uses v4l2_device_call()
to try all subdevs instead of calling a specific subdev).
The next patch converts all non-staging drivers, then come Sakari's
atomisp staging fixes.
The v4l2-subdev.h patch removes the now obsolete g/s_parm ops and the
final patch clarifies the documentation a bit (the core allows for
_MPLANE to be used as well).
I would really like to take the next step and introduce two new ioctls
VIDIOC_G/S_FRAME_INTERVAL (just like the SUBDEV variants that already
exist) and convert all bridge drivers to use that and just have helper
functions in the core for VIDIOC_G/S_PARM.
I hate that ioctl and it always confuses driver developers. It would
also prevent the type of abuse that was present in the atomisp driver.
But that's for later, let's simplify the subdev drivers first.
Regards,
Hans
Hans Verkuil (4):
v4l2-common: create v4l2_g/s_parm_cap helpers
media: convert g/s_parm to g/s_frame_interval in subdevs
v4l2-subdev.h: remove obsolete g/s_parm
vidioc-g-parm.rst: also allow _MPLANE buffer types
Sakari Ailus (5):
staging: atomisp: Kill subdev s_parm abuse
staging: atomisp: i2c: Disable non-preview configurations
staging: atomisp: i2c: Drop g_parm support in sensor drivers
staging: atomisp: mt9m114: Drop empty s_parm callback
staging: atomisp: Drop g_parm and s_parm subdev ops use
Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +-
drivers/media/i2c/mt9v011.c | 29 +++++----
drivers/media/i2c/ov6650.c | 33 +++++------
drivers/media/i2c/ov7670.c | 26 +++++----
drivers/media/i2c/ov7740.c | 29 ++++-----
drivers/media/i2c/tvp514x.c | 37 +++++-------
drivers/media/i2c/vs6624.c | 29 +++++----
drivers/media/platform/atmel/atmel-isc.c | 10 +---
drivers/media/platform/atmel/atmel-isi.c | 12 +---
drivers/media/platform/blackfin/bfin_capture.c | 14 ++---
drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++--
drivers/media/platform/soc_camera/soc_camera.c | 10 ++--
drivers/media/platform/via-camera.c | 4 +-
drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++--
drivers/media/v4l2-core/v4l2-common.c | 49 ++++++++++++++++
drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 53 -----------------
drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 53 -----------------
.../staging/media/atomisp/i2c/atomisp-mt9m114.c | 6 --
drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 56 ------------------
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 53 -----------------
drivers/staging/media/atomisp/i2c/gc0310.h | 43 --------------
drivers/staging/media/atomisp/i2c/gc2235.h | 3 +-
drivers/staging/media/atomisp/i2c/ov2680.h | 68 ----------------------
drivers/staging/media/atomisp/i2c/ov2722.h | 2 +
.../media/atomisp/i2c/ov5693/atomisp-ov5693.c | 54 -----------------
drivers/staging/media/atomisp/i2c/ov5693/ov5693.h | 2 +
.../media/atomisp/pci/atomisp2/atomisp_cmd.c | 9 +--
.../media/atomisp/pci/atomisp2/atomisp_file.c | 16 -----
.../media/atomisp/pci/atomisp2/atomisp_subdev.c | 12 +---
.../media/atomisp/pci/atomisp2/atomisp_tpg.c | 14 -----
include/media/v4l2-common.h | 26 +++++++++
include/media/v4l2-subdev.h | 6 --
32 files changed, 224 insertions(+), 588 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:28 ` Sakari Ailus
2018-01-22 10:18 ` [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Create helpers to handle VIDIOC_G/S_PARM by querying the
g/s_frame_interval v4l2_subdev ops.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/v4l2-common.c | 49 +++++++++++++++++++++++++++++++++++
include/media/v4l2-common.h | 26 +++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 8650ad92b64d..4e371ae4aed7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
}
EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
+
+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;
+
+ if (vdev->device_caps & V4L2_CAP_READWRITE)
+ a->parm.capture.readbuffers = 2;
+ if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+ ret = v4l2_subdev_call(sd, video, g_frame_interval, &ival);
+ if (!ret)
+ a->parm.capture.timeperframe = ival.interval;
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
+
+int v4l2_s_parm_cap(struct video_device *vdev,
+ struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+ struct v4l2_subdev_frame_interval ival = {
+ 0,
+ a->parm.capture.timeperframe
+ };
+ int ret;
+
+ if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+ a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+ return -EINVAL;
+
+ memset(&a->parm, 0, sizeof(a->parm));
+ if (vdev->device_caps & V4L2_CAP_READWRITE)
+ a->parm.capture.readbuffers = 2;
+ else
+ a->parm.capture.readbuffers = 0;
+
+ if (v4l2_subdev_has_op(sd, video, g_frame_interval))
+ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+ ret = v4l2_subdev_call(sd, video, s_frame_interval, &ival);
+ if (!ret)
+ a->parm.capture.timeperframe = ival.interval;
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index e0d95a7c5d48..f3aa1d728c0b 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -341,4 +341,30 @@ v4l2_find_nearest_format(const struct v4l2_frmsize_discrete *sizes,
*/
void v4l2_get_timestamp(struct timeval *tv);
+/**
+ * v4l2_g_parm_cap - helper routine for vidioc_g_parm to fill this in by
+ * calling the g_frame_interval op of the given subdev. It only works
+ * for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ * function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_G_PARM argument.
+ */
+int v4l2_g_parm_cap(struct video_device *vdev,
+ struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
+/**
+ * v4l2_s_parm_cap - helper routine for vidioc_s_parm to fill this in by
+ * calling the s_frame_interval op of the given subdev. It only works
+ * for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
+ * function name.
+ *
+ * @vdev: the struct video_device pointer. Used to determine the device caps.
+ * @sd: the sub-device pointer.
+ * @a: the VIDIOC_S_PARM argument.
+ */
+int v4l2_s_parm_cap(struct video_device *vdev,
+ struct v4l2_subdev *sd, struct v4l2_streamparm *a);
+
#endif /* V4L2_COMMON_H_ */
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
2018-01-22 10:18 ` [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:26 ` Sakari Ailus
2018-01-22 14:45 ` jacopo mondi
2018-01-22 10:18 ` [PATCH 3/9] staging: atomisp: Kill subdev s_parm abuse Hans Verkuil
` (6 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Convert all g/s_parm calls to g/s_frame_interval. This allows us
to remove the g/s_parm ops since those are a duplicate of
g/s_frame_interval.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/i2c/mt9v011.c | 29 +++++++++----------
drivers/media/i2c/ov6650.c | 33 ++++++++++------------
drivers/media/i2c/ov7670.c | 26 +++++++++--------
drivers/media/i2c/ov7740.c | 29 ++++++++-----------
drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
drivers/media/i2c/vs6624.c | 29 ++++++++++---------
drivers/media/platform/atmel/atmel-isc.c | 10 ++-----
drivers/media/platform/atmel/atmel-isi.c | 12 ++------
drivers/media/platform/blackfin/bfin_capture.c | 14 +++-------
drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++----
drivers/media/platform/soc_camera/soc_camera.c | 10 ++++---
drivers/media/platform/via-camera.c | 4 +--
drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++++++++++++----
13 files changed, 137 insertions(+), 144 deletions(-)
diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
index 5e29064fae91..0e0bcc8b67ca 100644
--- a/drivers/media/i2c/mt9v011.c
+++ b/drivers/media/i2c/mt9v011.c
@@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
return 0;
}
-static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
-
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- memset(cp, 0, sizeof(struct v4l2_captureparm));
- cp->capability = V4L2_CAP_TIMEPERFRAME;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
calc_fps(sd,
- &cp->timeperframe.numerator,
- &cp->timeperframe.denominator);
+ &ival->interval.numerator,
+ &ival->interval.denominator);
return 0;
}
-static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int mt9v011_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
u16 speed;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
- if (cp->extendedmode != 0)
+ if (ival->pad)
return -EINVAL;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
speed = calc_speed(sd, tpf->numerator, tpf->denominator);
mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed);
@@ -469,8 +466,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops = {
};
static const struct v4l2_subdev_video_ops mt9v011_video_ops = {
- .g_parm = mt9v011_g_parm,
- .s_parm = mt9v011_s_parm,
+ .g_frame_interval = mt9v011_g_frame_interval,
+ .s_frame_interval = mt9v011_s_frame_interval,
};
static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = {
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 8975d16b2b24..86fb75d3df5b 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -201,7 +201,7 @@ struct ov6650 {
struct v4l2_rect rect; /* sensor cropping window */
unsigned long pclk_limit; /* from host */
unsigned long pclk_max; /* from resolution and format */
- struct v4l2_fract tpf; /* as requested with s_parm */
+ struct v4l2_fract tpf; /* as requested with s_frame_interval */
u32 code;
enum v4l2_colorspace colorspace;
};
@@ -723,42 +723,39 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
}
-static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov6650_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov6650 *priv = to_ov6650(client);
- struct v4l2_captureparm *cp = &parms->parm.capture;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- memset(cp, 0, sizeof(*cp));
- cp->capability = V4L2_CAP_TIMEPERFRAME;
- cp->timeperframe.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf,
+ memset(ival->reserved, 0, sizeof(ival->reserved));
+ ival->interval.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf,
priv->pclk_limit, priv->pclk_max));
- cp->timeperframe.denominator = FRAME_RATE_MAX;
+ ival->interval.denominator = FRAME_RATE_MAX;
dev_dbg(&client->dev, "Frame interval: %u/%u s\n",
- cp->timeperframe.numerator, cp->timeperframe.denominator);
+ ival->interval.numerator, ival->interval.denominator);
return 0;
}
-static int ov6650_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov6650_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov6650 *priv = to_ov6650(client);
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
int div, ret;
u8 clkrc;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
-
- if (cp->extendedmode != 0)
+ if (ival->pad)
return -EINVAL;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
if (tpf->numerator == 0 || tpf->denominator == 0)
div = 1; /* Reset to full rate */
else
@@ -921,8 +918,8 @@ static int ov6650_s_mbus_config(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops ov6650_video_ops = {
.s_stream = ov6650_s_stream,
- .g_parm = ov6650_g_parm,
- .s_parm = ov6650_s_parm,
+ .g_frame_interval = ov6650_g_frame_interval,
+ .s_frame_interval = ov6650_s_frame_interval,
.g_mbus_config = ov6650_g_mbus_config,
.s_mbus_config = ov6650_s_mbus_config,
};
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index fd229bc8a0e5..e843bf9584df 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1100,30 +1100,32 @@ static int ov7670_get_fmt(struct v4l2_subdev *sd,
* Implement G/S_PARM. There is a "high quality" mode we could try
* to do someday; for now, we just do the frame rate tweak.
*/
-static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov7670_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
struct ov7670_info *info = to_state(sd);
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- cp->capability = V4L2_CAP_TIMEPERFRAME;
- info->devtype->get_framerate(sd, &cp->timeperframe);
+ memset(ival->reserved, 0, sizeof(ival->reserved));
+
+ info->devtype->get_framerate(sd, &ival->interval);
return 0;
}
-static int ov7670_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int ov7670_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
struct ov7670_info *info = to_state(sd);
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- cp->capability = V4L2_CAP_TIMEPERFRAME;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
+
return info->devtype->set_framerate(sd, tpf);
}
@@ -1636,8 +1638,8 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops = {
};
static const struct v4l2_subdev_video_ops ov7670_video_ops = {
- .s_parm = ov7670_s_parm,
- .g_parm = ov7670_g_parm,
+ .s_frame_interval = ov7670_s_frame_interval,
+ .g_frame_interval = ov7670_g_frame_interval,
};
static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 0308ba437bbb..cf60dcb8129d 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -624,17 +624,15 @@ static int ov7740_set_stream(struct v4l2_subdev *sd, int enable)
return ret;
}
-static int ov7740_get_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *parms)
+static int ov7740_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- memset(cp, 0, sizeof(struct v4l2_captureparm));
- cp->capability = V4L2_CAP_TIMEPERFRAME;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
tpf->numerator = 1;
tpf->denominator = 60;
@@ -642,18 +640,15 @@ static int ov7740_get_parm(struct v4l2_subdev *sd,
return 0;
}
-static int ov7740_set_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *parms)
+static int ov7740_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
- if (cp->extendedmode != 0)
+ if (ival->pad)
return -EINVAL;
- cp->capability = V4L2_CAP_TIMEPERFRAME;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
tpf->numerator = 1;
tpf->denominator = 60;
@@ -663,8 +658,8 @@ static int ov7740_set_parm(struct v4l2_subdev *sd,
static struct v4l2_subdev_video_ops ov7740_subdev_video_ops = {
.s_stream = ov7740_set_stream,
- .s_parm = ov7740_set_parm,
- .g_parm = ov7740_get_parm,
+ .s_frame_interval = ov7740_s_frame_interval,
+ .g_frame_interval = ov7740_g_frame_interval,
};
static const struct reg_sequence ov7740_format_yuyv[] = {
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index d575b3e7e835..f99328a0092f 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -747,60 +747,55 @@ static int tvp514x_s_ctrl(struct v4l2_ctrl *ctrl)
}
/**
- * tvp514x_g_parm() - V4L2 decoder interface handler for g_parm
+ * tvp514x_g_frame_interval() - V4L2 decoder interface handler
* @sd: pointer to standard V4L2 sub-device structure
- * @a: pointer to standard V4L2 VIDIOC_G_PARM ioctl structure
+ * @a: pointer to a v4l2_subdev_frame_interval structure
*
* Returns the decoder's video CAPTURE parameters.
*/
static int
-tvp514x_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+tvp514x_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct tvp514x_decoder *decoder = to_decoder(sd);
- struct v4l2_captureparm *cparm;
enum tvp514x_std current_std;
- if (a == NULL)
+ if (ival->pad)
return -EINVAL;
- if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- /* only capture is supported */
- return -EINVAL;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
/* get the current standard */
current_std = decoder->current_std;
- cparm = &a->parm.capture;
- cparm->capability = V4L2_CAP_TIMEPERFRAME;
- cparm->timeperframe =
+ ival->interval =
decoder->std_list[current_std].standard.frameperiod;
return 0;
}
/**
- * tvp514x_s_parm() - V4L2 decoder interface handler for s_parm
+ * tvp514x_s_frame_interval() - V4L2 decoder interface handler
* @sd: pointer to standard V4L2 sub-device structure
- * @a: pointer to standard V4L2 VIDIOC_S_PARM ioctl structure
+ * @a: pointer to a v4l2_subdev_frame_interval structure
*
* Configures the decoder to use the input parameters, if possible. If
* not possible, returns the appropriate error code.
*/
static int
-tvp514x_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+tvp514x_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct tvp514x_decoder *decoder = to_decoder(sd);
struct v4l2_fract *timeperframe;
enum tvp514x_std current_std;
- if (a == NULL)
+ if (ival->pad)
return -EINVAL;
- if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- /* only capture is supported */
- return -EINVAL;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
- timeperframe = &a->parm.capture.timeperframe;
+ timeperframe = &ival->interval;
/* get the current standard */
current_std = decoder->current_std;
@@ -961,8 +956,8 @@ static const struct v4l2_subdev_video_ops tvp514x_video_ops = {
.s_std = tvp514x_s_std,
.s_routing = tvp514x_s_routing,
.querystd = tvp514x_querystd,
- .g_parm = tvp514x_g_parm,
- .s_parm = tvp514x_s_parm,
+ .g_frame_interval = tvp514x_g_frame_interval,
+ .s_frame_interval = tvp514x_s_frame_interval,
.s_stream = tvp514x_s_stream,
};
diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c
index 560738213c00..729695b6f455 100644
--- a/drivers/media/i2c/vs6624.c
+++ b/drivers/media/i2c/vs6624.c
@@ -657,32 +657,31 @@ static int vs6624_get_fmt(struct v4l2_subdev *sd,
return 0;
}
-static int vs6624_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int vs6624_g_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct vs6624 *sensor = to_vs6624(sd);
- struct v4l2_captureparm *cp = &parms->parm.capture;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (ival->pad)
return -EINVAL;
- memset(cp, 0, sizeof(*cp));
- cp->capability = V4L2_CAP_TIMEPERFRAME;
- cp->timeperframe.numerator = sensor->frame_rate.denominator;
- cp->timeperframe.denominator = sensor->frame_rate.numerator;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
+ ival->interval.numerator = sensor->frame_rate.denominator;
+ ival->interval.denominator = sensor->frame_rate.numerator;
return 0;
}
-static int vs6624_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
+static int vs6624_s_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_frame_interval *ival)
{
struct vs6624 *sensor = to_vs6624(sd);
- struct v4l2_captureparm *cp = &parms->parm.capture;
- struct v4l2_fract *tpf = &cp->timeperframe;
+ struct v4l2_fract *tpf = &ival->interval;
- if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
- if (cp->extendedmode != 0)
+ if (ival->pad)
return -EINVAL;
+ memset(ival->reserved, 0, sizeof(ival->reserved));
+
if (tpf->numerator == 0 || tpf->denominator == 0
|| (tpf->denominator > tpf->numerator * MAX_FRAME_RATE)) {
/* reset to max frame rate */
@@ -738,8 +737,8 @@ static const struct v4l2_subdev_core_ops vs6624_core_ops = {
};
static const struct v4l2_subdev_video_ops vs6624_video_ops = {
- .s_parm = vs6624_s_parm,
- .g_parm = vs6624_g_parm,
+ .s_frame_interval = vs6624_s_frame_interval,
+ .g_frame_interval = vs6624_g_frame_interval,
.s_stream = vs6624_s_stream,
};
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 34676409ca08..92d695b29fa9 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
{
struct isc_device *isc = video_drvdata(file);
- if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
-
- return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
+ return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
}
static int isc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
{
struct isc_device *isc = video_drvdata(file);
- if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
- return -EINVAL;
-
- return v4l2_subdev_call(isc->current_subdev->sd, video, s_parm, a);
+ return v4l2_s_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
}
static int isc_enum_framesizes(struct file *file, void *fh,
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 9958918e2449..e5be21a31640 100644
--- 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;
-
- 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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/9] staging: atomisp: Kill subdev s_parm abuse
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
2018-01-22 10:18 ` [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
2018-01-22 10:18 ` [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 4/9] staging: atomisp: i2c: Disable non-preview configurations Hans Verkuil
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Remove sensor driver's interface that made use of use case specific
knowledge of platform capabilities.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 26 ---------
drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 26 ---------
drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 29 ---------
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 26 ---------
drivers/staging/media/atomisp/i2c/gc0310.h | 43 --------------
drivers/staging/media/atomisp/i2c/gc2235.h | 1 -
drivers/staging/media/atomisp/i2c/ov2680.h | 68 ----------------------
.../media/atomisp/i2c/ov5693/atomisp-ov5693.c | 27 ---------
.../media/atomisp/pci/atomisp2/atomisp_cmd.c | 9 +--
.../media/atomisp/pci/atomisp2/atomisp_subdev.c | 12 +---
10 files changed, 3 insertions(+), 264 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 61b7598469eb..572c9127c24d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1224,37 +1224,12 @@ static int gc0310_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc0310_res[dev->fmt_idx].fps;
}
return 0;
}
-static int gc0310_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct gc0310_device *dev = to_gc0310_sensor(sd);
- dev->run_mode = param->parm.capture.capturemode;
-
- mutex_lock(&dev->input_lock);
- switch (dev->run_mode) {
- case CI_MODE_VIDEO:
- gc0310_res = gc0310_res_video;
- N_RES = N_RES_VIDEO;
- break;
- case CI_MODE_STILL_CAPTURE:
- gc0310_res = gc0310_res_still;
- N_RES = N_RES_STILL;
- break;
- default:
- gc0310_res = gc0310_res_preview;
- N_RES = N_RES_PREVIEW;
- }
- mutex_unlock(&dev->input_lock);
- return 0;
-}
-
static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1314,7 +1289,6 @@ static const struct v4l2_subdev_sensor_ops gc0310_sensor_ops = {
static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
.g_parm = gc0310_g_parm,
- .s_parm = gc0310_s_parm,
.g_frame_interval = gc0310_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d8de46da64ae..2bc179f3afe5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -964,37 +964,12 @@ static int gc2235_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
gc2235_res[dev->fmt_idx].fps;
}
return 0;
}
-static int gc2235_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct gc2235_device *dev = to_gc2235_sensor(sd);
- dev->run_mode = param->parm.capture.capturemode;
-
- mutex_lock(&dev->input_lock);
- switch (dev->run_mode) {
- case CI_MODE_VIDEO:
- gc2235_res = gc2235_res_video;
- N_RES = N_RES_VIDEO;
- break;
- case CI_MODE_STILL_CAPTURE:
- gc2235_res = gc2235_res_still;
- N_RES = N_RES_STILL;
- break;
- default:
- gc2235_res = gc2235_res_preview;
- N_RES = N_RES_PREVIEW;
- }
- mutex_unlock(&dev->input_lock);
- return 0;
-}
-
static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1053,7 +1028,6 @@ static const struct v4l2_subdev_sensor_ops gc2235_sensor_ops = {
static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
.g_parm = gc2235_g_parm,
- .s_parm = gc2235_s_parm,
.g_frame_interval = gc2235_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 84f8d33ce2d1..e3e0fdd0c816 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -1300,40 +1300,12 @@ static int ov2680_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
ov2680_res[dev->fmt_idx].fps;
}
return 0;
}
-static int ov2680_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov2680_device *dev = to_ov2680_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- dev->run_mode = param->parm.capture.capturemode;
-
- v4l2_info(client, "\n%s:run_mode :%x\n", __func__, dev->run_mode);
-
- mutex_lock(&dev->input_lock);
- switch (dev->run_mode) {
- case CI_MODE_VIDEO:
- ov2680_res = ov2680_res_video;
- N_RES = N_RES_VIDEO;
- break;
- case CI_MODE_STILL_CAPTURE:
- ov2680_res = ov2680_res_still;
- N_RES = N_RES_STILL;
- break;
- default:
- ov2680_res = ov2680_res_preview;
- N_RES = N_RES_PREVIEW;
- }
- mutex_unlock(&dev->input_lock);
- return 0;
-}
-
static int ov2680_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1388,7 +1360,6 @@ static int ov2680_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
static const struct v4l2_subdev_video_ops ov2680_video_ops = {
.s_stream = ov2680_s_stream,
.g_parm = ov2680_g_parm,
- .s_parm = ov2680_s_parm,
.g_frame_interval = ov2680_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 2b6ae0faf972..cd9f6433cd42 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -1103,37 +1103,12 @@ static int ov2722_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
ov2722_res[dev->fmt_idx].fps;
}
return 0;
}
-static int ov2722_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov2722_device *dev = to_ov2722_sensor(sd);
- dev->run_mode = param->parm.capture.capturemode;
-
- mutex_lock(&dev->input_lock);
- switch (dev->run_mode) {
- case CI_MODE_VIDEO:
- ov2722_res = ov2722_res_video;
- N_RES = N_RES_VIDEO;
- break;
- case CI_MODE_STILL_CAPTURE:
- ov2722_res = ov2722_res_still;
- N_RES = N_RES_STILL;
- break;
- default:
- ov2722_res = ov2722_res_preview;
- N_RES = N_RES_PREVIEW;
- }
- mutex_unlock(&dev->input_lock);
- return 0;
-}
-
static int ov2722_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1193,7 +1168,6 @@ static const struct v4l2_subdev_sensor_ops ov2722_sensor_ops = {
static const struct v4l2_subdev_video_ops ov2722_video_ops = {
.s_stream = ov2722_s_stream,
.g_parm = ov2722_g_parm,
- .s_parm = ov2722_s_parm,
.g_frame_interval = ov2722_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/gc0310.h b/drivers/staging/media/atomisp/i2c/gc0310.h
index c422d0398fc7..af6b11f6e5e7 100644
--- a/drivers/staging/media/atomisp/i2c/gc0310.h
+++ b/drivers/staging/media/atomisp/i2c/gc0310.h
@@ -150,7 +150,6 @@ struct gc0310_device {
struct camera_sensor_platform_data *platform_data;
int vt_pix_clk_freq_mhz;
int fmt_idx;
- int run_mode;
u8 res;
u8 type;
};
@@ -400,48 +399,6 @@ struct gc0310_resolution gc0310_res_preview[] = {
};
#define N_RES_PREVIEW (ARRAY_SIZE(gc0310_res_preview))
-struct gc0310_resolution gc0310_res_still[] = {
- {
- .desc = "gc0310_VGA_30fps",
- .width = 656, // 648,
- .height = 496, // 488,
- .fps = 30,
- //.pix_clk_freq = 73,
- .used = 0,
-#if 0
- .pixels_per_line = 0x0314,
- .lines_per_frame = 0x0213,
-#endif
- .bin_factor_x = 1,
- .bin_factor_y = 1,
- .bin_mode = 0,
- .skip_frames = 2,
- .regs = gc0310_VGA_30fps,
- },
-};
-#define N_RES_STILL (ARRAY_SIZE(gc0310_res_still))
-
-struct gc0310_resolution gc0310_res_video[] = {
- {
- .desc = "gc0310_VGA_30fps",
- .width = 656, // 648,
- .height = 496, // 488,
- .fps = 30,
- //.pix_clk_freq = 73,
- .used = 0,
-#if 0
- .pixels_per_line = 0x0314,
- .lines_per_frame = 0x0213,
-#endif
- .bin_factor_x = 1,
- .bin_factor_y = 1,
- .bin_mode = 0,
- .skip_frames = 2,
- .regs = gc0310_VGA_30fps,
- },
-};
-#define N_RES_VIDEO (ARRAY_SIZE(gc0310_res_video))
-
static struct gc0310_resolution *gc0310_res = gc0310_res_preview;
static unsigned long N_RES = N_RES_PREVIEW;
#endif
diff --git a/drivers/staging/media/atomisp/i2c/gc2235.h b/drivers/staging/media/atomisp/i2c/gc2235.h
index 3c30a05c3991..45a54fea5466 100644
--- a/drivers/staging/media/atomisp/i2c/gc2235.h
+++ b/drivers/staging/media/atomisp/i2c/gc2235.h
@@ -156,7 +156,6 @@ struct gc2235_device {
struct camera_sensor_platform_data *platform_data;
int vt_pix_clk_freq_mhz;
int fmt_idx;
- int run_mode;
u8 res;
u8 type;
};
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
index 03f75dd80f87..cb38e6e79409 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.h
+++ b/drivers/staging/media/atomisp/i2c/ov2680.h
@@ -850,74 +850,6 @@ struct ov2680_format {
};
#define N_RES_PREVIEW (ARRAY_SIZE(ov2680_res_preview))
-static struct ov2680_resolution ov2680_res_still[] = {
- {
- .desc = "ov2680_1616x1216_30fps",
- .width = 1616,
- .height = 1216,
- .pix_clk_freq = 66,
- .fps = 30,
- .used = 0,
- .pixels_per_line = 1698,//1704,
- .lines_per_frame = 1294,
- .bin_factor_x = 0,
- .bin_factor_y = 0,
- .bin_mode = 0,
- .skip_frames = 3,
- .regs = ov2680_1616x1216_30fps,
- },
- {
- .desc = "ov2680_1616x916_30fps",
- .width = 1616,
- .height = 916,
- .fps = 30,
- .pix_clk_freq = 66,
- .used = 0,
- .pixels_per_line = 1698,//1704,
- .lines_per_frame = 1294,
- .bin_factor_x = 0,
- .bin_factor_y = 0,
- .bin_mode = 0,
- .skip_frames = 3,
- .regs = ov2680_1616x916_30fps,
- },
-};
-#define N_RES_STILL (ARRAY_SIZE(ov2680_res_still))
-
-static struct ov2680_resolution ov2680_res_video[] = {
- {
- .desc = "ov2680_1616x1216_30fps",
- .width = 1616,
- .height = 1216,
- .pix_clk_freq = 66,
- .fps = 30,
- .used = 0,
- .pixels_per_line = 1698,//1704,
- .lines_per_frame = 1294,
- .bin_factor_x = 0,
- .bin_factor_y = 0,
- .bin_mode = 0,
- .skip_frames = 3,
- .regs = ov2680_1616x1216_30fps,
- },
- {
- .desc = "ov2680_720p_30fps",
- .width = 1616,
- .height = 916,
- .fps = 30,
- .pix_clk_freq = 66,
- .used = 0,
- .pixels_per_line = 1698,//1704,
- .lines_per_frame = 1294,
- .bin_factor_x = 0,
- .bin_factor_y = 0,
- .bin_mode = 0,
- .skip_frames = 3,
- .regs = ov2680_1616x916_30fps,
- },
-};
-#define N_RES_VIDEO (ARRAY_SIZE(ov2680_res_video))
-
static struct ov2680_resolution *ov2680_res = ov2680_res_preview;
static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
index 40d01bf4bf28..7f594c7de76e 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
+++ b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
@@ -1825,38 +1825,12 @@ static int ov5693_g_parm(struct v4l2_subdev *sd,
if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.capturemode = dev->run_mode;
param->parm.capture.timeperframe.denominator =
ov5693_res[dev->fmt_idx].fps;
}
return 0;
}
-static int ov5693_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov5693_device *dev = to_ov5693_sensor(sd);
-
- dev->run_mode = param->parm.capture.capturemode;
-
- mutex_lock(&dev->input_lock);
- switch (dev->run_mode) {
- case CI_MODE_VIDEO:
- ov5693_res = ov5693_res_video;
- N_RES = N_RES_VIDEO;
- break;
- case CI_MODE_STILL_CAPTURE:
- ov5693_res = ov5693_res_still;
- N_RES = N_RES_STILL;
- break;
- default:
- ov5693_res = ov5693_res_preview;
- N_RES = N_RES_PREVIEW;
- }
- mutex_unlock(&dev->input_lock);
- return 0;
-}
-
static int ov5693_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1900,7 +1874,6 @@ static int ov5693_enum_frame_size(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops ov5693_video_ops = {
.s_stream = ov5693_s_stream,
.g_parm = ov5693_g_parm,
- .s_parm = ov5693_s_parm,
.g_frame_interval = ov5693_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index debf0e3853ff..3410a7fb1fcf 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2091,7 +2091,7 @@ int atomisp_set_sensor_runmode(struct atomisp_sub_device *asd,
struct atomisp_device *isp = asd->isp;
struct v4l2_ctrl *c;
struct v4l2_streamparm p = {0};
- int ret;
+ int ret = 0;
int modes[] = { CI_MODE_NONE,
CI_MODE_VIDEO,
CI_MODE_STILL_CAPTURE,
@@ -2105,13 +2105,8 @@ int atomisp_set_sensor_runmode(struct atomisp_sub_device *asd,
c = v4l2_ctrl_find(isp->inputs[asd->input_curr].camera->ctrl_handler,
V4L2_CID_RUN_MODE);
- if (c) {
+ if (c)
ret = v4l2_ctrl_s_ctrl(c, runmode->mode);
- } else {
- p.parm.capture.capturemode = modes[runmode->mode];
- ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
- video, s_parm, &p);
- }
mutex_unlock(asd->ctrl_handler.lock);
return ret;
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
index f3e18d627b0a..b78276ac22da 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c
@@ -819,12 +819,6 @@ static int __atomisp_update_run_mode(struct atomisp_sub_device *asd)
struct atomisp_device *isp = asd->isp;
struct v4l2_ctrl *ctrl = asd->run_mode;
struct v4l2_ctrl *c;
- struct v4l2_streamparm p = {0};
- int modes[] = { CI_MODE_NONE,
- CI_MODE_VIDEO,
- CI_MODE_STILL_CAPTURE,
- CI_MODE_CONTINUOUS,
- CI_MODE_PREVIEW };
s32 mode;
if (ctrl->val != ATOMISP_RUN_MODE_VIDEO &&
@@ -840,11 +834,7 @@ static int __atomisp_update_run_mode(struct atomisp_sub_device *asd)
if (c)
return v4l2_ctrl_s_ctrl(c, mode);
- /* Fall back to obsolete s_parm */
- p.parm.capture.capturemode = modes[mode];
-
- return v4l2_subdev_call(
- isp->inputs[asd->input_curr].camera, video, s_parm, &p);
+ return 0;
}
int atomisp_update_run_mode(struct atomisp_sub_device *asd)
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/9] staging: atomisp: i2c: Disable non-preview configurations
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (2 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 3/9] staging: atomisp: Kill subdev s_parm abuse Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers Hans Verkuil
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Disable configurations for non-preview modes until configuration selection
is improved.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/staging/media/atomisp/i2c/gc2235.h | 2 ++
drivers/staging/media/atomisp/i2c/ov2722.h | 2 ++
drivers/staging/media/atomisp/i2c/ov5693/ov5693.h | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/staging/media/atomisp/i2c/gc2235.h b/drivers/staging/media/atomisp/i2c/gc2235.h
index 45a54fea5466..817c0068c1d3 100644
--- a/drivers/staging/media/atomisp/i2c/gc2235.h
+++ b/drivers/staging/media/atomisp/i2c/gc2235.h
@@ -574,6 +574,7 @@ static struct gc2235_resolution gc2235_res_preview[] = {
};
#define N_RES_PREVIEW (ARRAY_SIZE(gc2235_res_preview))
+#if 0 /* Disable non-previes configurations for now */
static struct gc2235_resolution gc2235_res_still[] = {
{
.desc = "gc2235_1600_900_30fps",
@@ -658,6 +659,7 @@ static struct gc2235_resolution gc2235_res_video[] = {
};
#define N_RES_VIDEO (ARRAY_SIZE(gc2235_res_video))
+#endif
static struct gc2235_resolution *gc2235_res = gc2235_res_preview;
static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov2722.h b/drivers/staging/media/atomisp/i2c/ov2722.h
index d8a973d71699..f133439adfd5 100644
--- a/drivers/staging/media/atomisp/i2c/ov2722.h
+++ b/drivers/staging/media/atomisp/i2c/ov2722.h
@@ -1148,6 +1148,7 @@ struct ov2722_resolution ov2722_res_preview[] = {
};
#define N_RES_PREVIEW (ARRAY_SIZE(ov2722_res_preview))
+#if 0 /* Disable non-previes configurations for now */
struct ov2722_resolution ov2722_res_still[] = {
{
.desc = "ov2722_480P_30fps",
@@ -1250,6 +1251,7 @@ struct ov2722_resolution ov2722_res_video[] = {
},
};
#define N_RES_VIDEO (ARRAY_SIZE(ov2722_res_video))
+#endif
static struct ov2722_resolution *ov2722_res = ov2722_res_preview;
static unsigned long N_RES = N_RES_PREVIEW;
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
index 68cfcb4a6c3c..15a33dcd2d59 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
+++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h
@@ -1147,6 +1147,7 @@ struct ov5693_resolution ov5693_res_preview[] = {
};
#define N_RES_PREVIEW (ARRAY_SIZE(ov5693_res_preview))
+#if 0 /* Disable non-previes configurations for now */
struct ov5693_resolution ov5693_res_still[] = {
{
.desc = "ov5693_736x496_30fps",
@@ -1364,6 +1365,7 @@ struct ov5693_resolution ov5693_res_video[] = {
},
};
#define N_RES_VIDEO (ARRAY_SIZE(ov5693_res_video))
+#endif
static struct ov5693_resolution *ov5693_res = ov5693_res_preview;
static unsigned long N_RES = N_RES_PREVIEW;
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (3 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 4/9] staging: atomisp: i2c: Disable non-preview configurations Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback Hans Verkuil
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Sakari Ailus <sakari.ailus@linux.intel.com>
These drivers already support g_frame_interval. Therefore just dropping
g_parm is enough.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 27 ----------------------
drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 27 ----------------------
drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 27 ----------------------
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 27 ----------------------
.../media/atomisp/i2c/ov5693/atomisp-ov5693.c | 27 ----------------------
5 files changed, 135 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 572c9127c24d..93753cb96180 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1204,32 +1204,6 @@ static int gc0310_s_config(struct v4l2_subdev *sd,
return ret;
}
-static int gc0310_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct gc0310_device *dev = to_gc0310_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
-
- if (!param)
- return -EINVAL;
-
- if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- dev_err(&client->dev, "unsupported buffer type.\n");
- return -EINVAL;
- }
-
- memset(param, 0, sizeof(*param));
- param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
- if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
- param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.timeperframe.denominator =
- gc0310_res[dev->fmt_idx].fps;
- }
- return 0;
-}
-
static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1288,7 +1262,6 @@ static const struct v4l2_subdev_sensor_ops gc0310_sensor_ops = {
static const struct v4l2_subdev_video_ops gc0310_video_ops = {
.s_stream = gc0310_s_stream,
- .g_parm = gc0310_g_parm,
.g_frame_interval = gc0310_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 2bc179f3afe5..93f9c618f3d8 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -944,32 +944,6 @@ static int gc2235_s_config(struct v4l2_subdev *sd,
return ret;
}
-static int gc2235_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct gc2235_device *dev = to_gc2235_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
-
- if (!param)
- return -EINVAL;
-
- if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- dev_err(&client->dev, "unsupported buffer type.\n");
- return -EINVAL;
- }
-
- memset(param, 0, sizeof(*param));
- param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
- if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
- param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.timeperframe.denominator =
- gc2235_res[dev->fmt_idx].fps;
- }
- return 0;
-}
-
static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1027,7 +1001,6 @@ static const struct v4l2_subdev_sensor_ops gc2235_sensor_ops = {
static const struct v4l2_subdev_video_ops gc2235_video_ops = {
.s_stream = gc2235_s_stream,
- .g_parm = gc2235_g_parm,
.g_frame_interval = gc2235_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index e3e0fdd0c816..11412061c40e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -1280,32 +1280,6 @@ static int ov2680_s_config(struct v4l2_subdev *sd,
return ret;
}
-static int ov2680_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov2680_device *dev = to_ov2680_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
-
- if (!param)
- return -EINVAL;
-
- if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- dev_err(&client->dev, "unsupported buffer type.\n");
- return -EINVAL;
- }
-
- memset(param, 0, sizeof(*param));
- param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
- if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
- param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.timeperframe.denominator =
- ov2680_res[dev->fmt_idx].fps;
- }
- return 0;
-}
-
static int ov2680_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1359,7 +1333,6 @@ static int ov2680_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
static const struct v4l2_subdev_video_ops ov2680_video_ops = {
.s_stream = ov2680_s_stream,
- .g_parm = ov2680_g_parm,
.g_frame_interval = ov2680_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index cd9f6433cd42..e59358ac89ce 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -1083,32 +1083,6 @@ static int ov2722_s_config(struct v4l2_subdev *sd,
return ret;
}
-static int ov2722_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov2722_device *dev = to_ov2722_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
-
- if (!param)
- return -EINVAL;
-
- if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- dev_err(&client->dev, "unsupported buffer type.\n");
- return -EINVAL;
- }
-
- memset(param, 0, sizeof(*param));
- param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
- if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
- param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.timeperframe.denominator =
- ov2722_res[dev->fmt_idx].fps;
- }
- return 0;
-}
-
static int ov2722_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1167,7 +1141,6 @@ static const struct v4l2_subdev_sensor_ops ov2722_sensor_ops = {
static const struct v4l2_subdev_video_ops ov2722_video_ops = {
.s_stream = ov2722_s_stream,
- .g_parm = ov2722_g_parm,
.g_frame_interval = ov2722_g_frame_interval,
};
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
index 7f594c7de76e..56f3cd0d8c23 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
+++ b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
@@ -1805,32 +1805,6 @@ static int ov5693_s_config(struct v4l2_subdev *sd,
return ret;
}
-static int ov5693_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- struct ov5693_device *dev = to_ov5693_sensor(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
-
- if (!param)
- return -EINVAL;
-
- if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- dev_err(&client->dev, "unsupported buffer type.\n");
- return -EINVAL;
- }
-
- memset(param, 0, sizeof(*param));
- param->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
- if (dev->fmt_idx >= 0 && dev->fmt_idx < N_RES) {
- param->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- param->parm.capture.timeperframe.numerator = 1;
- param->parm.capture.timeperframe.denominator =
- ov5693_res[dev->fmt_idx].fps;
- }
- return 0;
-}
-
static int ov5693_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
{
@@ -1873,7 +1847,6 @@ static int ov5693_enum_frame_size(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops ov5693_video_ops = {
.s_stream = ov5693_s_stream,
- .g_parm = ov5693_g_parm,
.g_frame_interval = ov5693_g_frame_interval,
};
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (4 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use Hans Verkuil
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Sakari Ailus <sakari.ailus@linux.intel.com>
The s_parm callback in mt9m114 driver did nothing, remove it.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index df253a557c76..834fba8c4fa0 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -1684,11 +1684,6 @@ static int mt9m114_t_vflip(struct v4l2_subdev *sd, int value)
return !!err;
}
-static int mt9m114_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- return 0;
-}
static int mt9m114_g_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval)
@@ -1781,7 +1776,6 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
}
static const struct v4l2_subdev_video_ops mt9m114_video_ops = {
- .s_parm = mt9m114_s_parm,
.s_stream = mt9m114_s_stream,
.g_frame_interval = mt9m114_g_frame_interval,
};
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (5 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 8/9] v4l2-subdev.h: remove obsolete g/s_parm Hans Verkuil
2018-01-22 10:18 ` [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types Hans Verkuil
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Sakari Ailus <sakari.ailus@linux.intel.com>
The s_parm and g_parm did nothing. Remove them.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
.../staging/media/atomisp/pci/atomisp2/atomisp_file.c | 16 ----------------
drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c | 14 --------------
2 files changed, 30 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
index 377ec2a9fa6d..c6d96987561d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_file.c
@@ -77,20 +77,6 @@ static int file_input_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-static int file_input_g_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- /*to fake*/
- return 0;
-}
-
-static int file_input_s_parm(struct v4l2_subdev *sd,
- struct v4l2_streamparm *param)
-{
- /*to fake*/
- return 0;
-}
-
static int file_input_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
@@ -166,8 +152,6 @@ static int file_input_enum_frame_ival(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops file_input_video_ops = {
.s_stream = file_input_s_stream,
- .g_parm = file_input_g_parm,
- .s_parm = file_input_s_parm,
};
static const struct v4l2_subdev_core_ops file_input_core_ops = {
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
index b71cc7bcdbab..adc900272f6f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
@@ -27,18 +27,6 @@ static int tpg_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-static int tpg_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
- /*to fake*/
- return 0;
-}
-
-static int tpg_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *param)
-{
- /*to fake*/
- return 0;
-}
-
static int tpg_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
@@ -101,8 +89,6 @@ static int tpg_enum_frame_ival(struct v4l2_subdev *sd,
static const struct v4l2_subdev_video_ops tpg_video_ops = {
.s_stream = tpg_s_stream,
- .g_parm = tpg_g_parm,
- .s_parm = tpg_s_parm,
};
static const struct v4l2_subdev_core_ops tpg_core_ops = {
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 8/9] v4l2-subdev.h: remove obsolete g/s_parm
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (6 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types Hans Verkuil
8 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Hans Verkuil <hansverk@cisco.com>
Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
include/media/v4l2-subdev.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 980a86c08fce..457917e9237f 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -393,10 +393,6 @@ struct v4l2_mbus_frame_desc {
*
* @g_pixelaspect: callback to return the pixelaspect ratio.
*
- * @g_parm: callback for VIDIOC_G_PARM() ioctl handler code.
- *
- * @s_parm: callback for VIDIOC_S_PARM() ioctl handler code.
- *
* @g_frame_interval: callback for VIDIOC_SUBDEV_G_FRAME_INTERVAL()
* ioctl handler code.
*
@@ -434,8 +430,6 @@ struct v4l2_subdev_video_ops {
int (*g_input_status)(struct v4l2_subdev *sd, u32 *status);
int (*s_stream)(struct v4l2_subdev *sd, int enable);
int (*g_pixelaspect)(struct v4l2_subdev *sd, struct v4l2_fract *aspect);
- int (*g_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
- int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
int (*g_frame_interval)(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval);
int (*s_frame_interval)(struct v4l2_subdev *sd,
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
` (7 preceding siblings ...)
2018-01-22 10:18 ` [PATCH 8/9] v4l2-subdev.h: remove obsolete g/s_parm Hans Verkuil
@ 2018-01-22 10:18 ` Hans Verkuil
2018-01-22 10:26 ` Sakari Ailus
8 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:18 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Jacopo Mondi, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
but the v4l2 core implementation also allows the _MPLANE variant.
Document this.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
index 616a5ea3f8fa..a941526cfeb4 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
@@ -66,7 +66,7 @@ union holding separate parameters for input and output devices.
-
- The buffer (stream) type, same as struct
:c:type:`v4l2_format` ``type``, set by the
- application. See :c:type:`v4l2_buf_type`
+ application. See :c:type:`v4l2_buf_type`.
* - union
- ``parm``
-
@@ -75,12 +75,12 @@ union holding separate parameters for input and output devices.
- struct :c:type:`v4l2_captureparm`
- ``capture``
- Parameters for capture devices, used when ``type`` is
- ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
+ ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
* -
- struct :c:type:`v4l2_outputparm`
- ``output``
- Parameters for output devices, used when ``type`` is
- ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
+ ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
* -
- __u8
- ``raw_data``\ [200]
--
2.15.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:18 ` [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
@ 2018-01-22 10:26 ` Sakari Ailus
2018-01-22 10:33 ` Hans Verkuil
2018-01-22 14:45 ` jacopo mondi
1 sibling, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2018-01-22 10:26 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
Hi Hans,
On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Convert all g/s_parm calls to g/s_frame_interval. This allows us
> to remove the g/s_parm ops since those are a duplicate of
> g/s_frame_interval.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> drivers/media/i2c/mt9v011.c | 29 +++++++++----------
> drivers/media/i2c/ov6650.c | 33 ++++++++++------------
> drivers/media/i2c/ov7670.c | 26 +++++++++--------
> drivers/media/i2c/ov7740.c | 29 ++++++++-----------
> drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
> drivers/media/i2c/vs6624.c | 29 ++++++++++---------
> drivers/media/platform/atmel/atmel-isc.c | 10 ++-----
> drivers/media/platform/atmel/atmel-isi.c | 12 ++------
> drivers/media/platform/blackfin/bfin_capture.c | 14 +++-------
> drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++----
> drivers/media/platform/soc_camera/soc_camera.c | 10 ++++---
> drivers/media/platform/via-camera.c | 4 +--
> drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++++++++++++----
> 13 files changed, 137 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
> index 5e29064fae91..0e0bcc8b67ca 100644
> --- a/drivers/media/i2c/mt9v011.c
> +++ b/drivers/media/i2c/mt9v011.c
> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> -
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
The pad number checks are already present in v4l2-subdev.c. Do you think
we'll need them in drivers as well?
It's true that another driver could mis-use this interface. In that case
I'd introduce a wrapper to the op rather than introduce the check in every
driver.
>
> - memset(cp, 0, sizeof(struct v4l2_captureparm));
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> calc_fps(sd,
> - &cp->timeperframe.numerator,
> - &cp->timeperframe.denominator);
> + &ival->interval.numerator,
> + &ival->interval.denominator);
>
> return 0;
> }
>
> -static int mt9v011_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int mt9v011_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
> u16 speed;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - if (cp->extendedmode != 0)
> + if (ival->pad)
> return -EINVAL;
>
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> speed = calc_speed(sd, tpf->numerator, tpf->denominator);
>
> mt9v011_write(sd, R0A_MT9V011_CLK_SPEED, speed);
> @@ -469,8 +466,8 @@ static const struct v4l2_subdev_core_ops mt9v011_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops mt9v011_video_ops = {
> - .g_parm = mt9v011_g_parm,
> - .s_parm = mt9v011_s_parm,
> + .g_frame_interval = mt9v011_g_frame_interval,
> + .s_frame_interval = mt9v011_s_frame_interval,
> };
>
> static const struct v4l2_subdev_pad_ops mt9v011_pad_ops = {
> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> index 8975d16b2b24..86fb75d3df5b 100644
> --- a/drivers/media/i2c/ov6650.c
> +++ b/drivers/media/i2c/ov6650.c
> @@ -201,7 +201,7 @@ struct ov6650 {
> struct v4l2_rect rect; /* sensor cropping window */
> unsigned long pclk_limit; /* from host */
> unsigned long pclk_max; /* from resolution and format */
> - struct v4l2_fract tpf; /* as requested with s_parm */
> + struct v4l2_fract tpf; /* as requested with s_frame_interval */
> u32 code;
> enum v4l2_colorspace colorspace;
> };
> @@ -723,42 +723,39 @@ static int ov6650_enum_mbus_code(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int ov6650_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int ov6650_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct ov6650 *priv = to_ov6650(client);
> - struct v4l2_captureparm *cp = &parms->parm.capture;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
>
> - memset(cp, 0, sizeof(*cp));
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> - cp->timeperframe.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf,
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> + ival->interval.numerator = GET_CLKRC_DIV(to_clkrc(&priv->tpf,
> priv->pclk_limit, priv->pclk_max));
> - cp->timeperframe.denominator = FRAME_RATE_MAX;
> + ival->interval.denominator = FRAME_RATE_MAX;
>
> dev_dbg(&client->dev, "Frame interval: %u/%u s\n",
> - cp->timeperframe.numerator, cp->timeperframe.denominator);
> + ival->interval.numerator, ival->interval.denominator);
>
> return 0;
> }
>
> -static int ov6650_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int ov6650_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct ov6650 *priv = to_ov6650(client);
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
> int div, ret;
> u8 clkrc;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - if (cp->extendedmode != 0)
> + if (ival->pad)
> return -EINVAL;
>
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> if (tpf->numerator == 0 || tpf->denominator == 0)
> div = 1; /* Reset to full rate */
> else
> @@ -921,8 +918,8 @@ static int ov6650_s_mbus_config(struct v4l2_subdev *sd,
>
> static const struct v4l2_subdev_video_ops ov6650_video_ops = {
> .s_stream = ov6650_s_stream,
> - .g_parm = ov6650_g_parm,
> - .s_parm = ov6650_s_parm,
> + .g_frame_interval = ov6650_g_frame_interval,
> + .s_frame_interval = ov6650_s_frame_interval,
> .g_mbus_config = ov6650_g_mbus_config,
> .s_mbus_config = ov6650_s_mbus_config,
> };
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index fd229bc8a0e5..e843bf9584df 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -1100,30 +1100,32 @@ static int ov7670_get_fmt(struct v4l2_subdev *sd,
> * Implement G/S_PARM. There is a "high quality" mode we could try
> * to do someday; for now, we just do the frame rate tweak.
> */
> -static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int ov7670_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> struct ov7670_info *info = to_state(sd);
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
>
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> - info->devtype->get_framerate(sd, &cp->timeperframe);
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> +
> + info->devtype->get_framerate(sd, &ival->interval);
>
> return 0;
> }
>
> -static int ov7670_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int ov7670_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
> struct ov7670_info *info = to_state(sd);
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
>
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> +
> return info->devtype->set_framerate(sd, tpf);
> }
>
> @@ -1636,8 +1638,8 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops ov7670_video_ops = {
> - .s_parm = ov7670_s_parm,
> - .g_parm = ov7670_g_parm,
> + .s_frame_interval = ov7670_s_frame_interval,
> + .g_frame_interval = ov7670_g_frame_interval,
> };
>
> static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 0308ba437bbb..cf60dcb8129d 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -624,17 +624,15 @@ static int ov7740_set_stream(struct v4l2_subdev *sd, int enable)
> return ret;
> }
>
> -static int ov7740_get_parm(struct v4l2_subdev *sd,
> - struct v4l2_streamparm *parms)
> +static int ov7740_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
>
> - memset(cp, 0, sizeof(struct v4l2_captureparm));
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>
> tpf->numerator = 1;
> tpf->denominator = 60;
> @@ -642,18 +640,15 @@ static int ov7740_get_parm(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int ov7740_set_parm(struct v4l2_subdev *sd,
> - struct v4l2_streamparm *parms)
> +static int ov7740_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - if (cp->extendedmode != 0)
> + if (ival->pad)
> return -EINVAL;
>
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>
> tpf->numerator = 1;
> tpf->denominator = 60;
> @@ -663,8 +658,8 @@ static int ov7740_set_parm(struct v4l2_subdev *sd,
>
> static struct v4l2_subdev_video_ops ov7740_subdev_video_ops = {
> .s_stream = ov7740_set_stream,
> - .s_parm = ov7740_set_parm,
> - .g_parm = ov7740_get_parm,
> + .s_frame_interval = ov7740_s_frame_interval,
> + .g_frame_interval = ov7740_g_frame_interval,
> };
>
> static const struct reg_sequence ov7740_format_yuyv[] = {
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index d575b3e7e835..f99328a0092f 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -747,60 +747,55 @@ static int tvp514x_s_ctrl(struct v4l2_ctrl *ctrl)
> }
>
> /**
> - * tvp514x_g_parm() - V4L2 decoder interface handler for g_parm
> + * tvp514x_g_frame_interval() - V4L2 decoder interface handler
> * @sd: pointer to standard V4L2 sub-device structure
> - * @a: pointer to standard V4L2 VIDIOC_G_PARM ioctl structure
> + * @a: pointer to a v4l2_subdev_frame_interval structure
> *
> * Returns the decoder's video CAPTURE parameters.
> */
> static int
> -tvp514x_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +tvp514x_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct tvp514x_decoder *decoder = to_decoder(sd);
> - struct v4l2_captureparm *cparm;
> enum tvp514x_std current_std;
>
> - if (a == NULL)
> + if (ival->pad)
> return -EINVAL;
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - /* only capture is supported */
> - return -EINVAL;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>
> /* get the current standard */
> current_std = decoder->current_std;
>
> - cparm = &a->parm.capture;
> - cparm->capability = V4L2_CAP_TIMEPERFRAME;
> - cparm->timeperframe =
> + ival->interval =
> decoder->std_list[current_std].standard.frameperiod;
>
> return 0;
> }
>
> /**
> - * tvp514x_s_parm() - V4L2 decoder interface handler for s_parm
> + * tvp514x_s_frame_interval() - V4L2 decoder interface handler
> * @sd: pointer to standard V4L2 sub-device structure
> - * @a: pointer to standard V4L2 VIDIOC_S_PARM ioctl structure
> + * @a: pointer to a v4l2_subdev_frame_interval structure
> *
> * Configures the decoder to use the input parameters, if possible. If
> * not possible, returns the appropriate error code.
> */
> static int
> -tvp514x_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +tvp514x_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct tvp514x_decoder *decoder = to_decoder(sd);
> struct v4l2_fract *timeperframe;
> enum tvp514x_std current_std;
>
> - if (a == NULL)
> + if (ival->pad)
> return -EINVAL;
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - /* only capture is supported */
> - return -EINVAL;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
>
> - timeperframe = &a->parm.capture.timeperframe;
> + timeperframe = &ival->interval;
>
> /* get the current standard */
> current_std = decoder->current_std;
> @@ -961,8 +956,8 @@ static const struct v4l2_subdev_video_ops tvp514x_video_ops = {
> .s_std = tvp514x_s_std,
> .s_routing = tvp514x_s_routing,
> .querystd = tvp514x_querystd,
> - .g_parm = tvp514x_g_parm,
> - .s_parm = tvp514x_s_parm,
> + .g_frame_interval = tvp514x_g_frame_interval,
> + .s_frame_interval = tvp514x_s_frame_interval,
> .s_stream = tvp514x_s_stream,
> };
>
> diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c
> index 560738213c00..729695b6f455 100644
> --- a/drivers/media/i2c/vs6624.c
> +++ b/drivers/media/i2c/vs6624.c
> @@ -657,32 +657,31 @@ static int vs6624_get_fmt(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int vs6624_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int vs6624_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct vs6624 *sensor = to_vs6624(sd);
> - struct v4l2_captureparm *cp = &parms->parm.capture;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (ival->pad)
> return -EINVAL;
>
> - memset(cp, 0, sizeof(*cp));
> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> - cp->timeperframe.numerator = sensor->frame_rate.denominator;
> - cp->timeperframe.denominator = sensor->frame_rate.numerator;
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> + ival->interval.numerator = sensor->frame_rate.denominator;
> + ival->interval.denominator = sensor->frame_rate.numerator;
> return 0;
> }
>
> -static int vs6624_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +static int vs6624_s_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *ival)
> {
> struct vs6624 *sensor = to_vs6624(sd);
> - struct v4l2_captureparm *cp = &parms->parm.capture;
> - struct v4l2_fract *tpf = &cp->timeperframe;
> + struct v4l2_fract *tpf = &ival->interval;
>
> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> - if (cp->extendedmode != 0)
> + if (ival->pad)
> return -EINVAL;
>
> + memset(ival->reserved, 0, sizeof(ival->reserved));
> +
> if (tpf->numerator == 0 || tpf->denominator == 0
> || (tpf->denominator > tpf->numerator * MAX_FRAME_RATE)) {
> /* reset to max frame rate */
> @@ -738,8 +737,8 @@ static const struct v4l2_subdev_core_ops vs6624_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops vs6624_video_ops = {
> - .s_parm = vs6624_s_parm,
> - .g_parm = vs6624_g_parm,
> + .s_frame_interval = vs6624_s_frame_interval,
> + .g_frame_interval = vs6624_g_frame_interval,
> .s_stream = vs6624_s_stream,
> };
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index 34676409ca08..92d695b29fa9 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct isc_device *isc = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
> + return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
> }
>
> static int isc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct isc_device *isc = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - return v4l2_subdev_call(isc->current_subdev->sd, video, s_parm, a);
> + return v4l2_s_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
> }
>
> static int isc_enum_framesizes(struct file *file, void *fh,
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index 9958918e2449..e5be21a31640 100644
> --- 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;
> -
> - 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,
--
Kind regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types
2018-01-22 10:18 ` [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types Hans Verkuil
@ 2018-01-22 10:26 ` Sakari Ailus
2018-01-22 10:37 ` Hans Verkuil
0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2018-01-22 10:26 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
Hi Hans,
On Mon, Jan 22, 2018 at 11:18:57AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
> but the v4l2 core implementation also allows the _MPLANE variant.
>
> Document this.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> index 616a5ea3f8fa..a941526cfeb4 100644
> --- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
> @@ -66,7 +66,7 @@ union holding separate parameters for input and output devices.
> -
> - The buffer (stream) type, same as struct
> :c:type:`v4l2_format` ``type``, set by the
> - application. See :c:type:`v4l2_buf_type`
> + application. See :c:type:`v4l2_buf_type`.
> * - union
> - ``parm``
> -
> @@ -75,12 +75,12 @@ union holding separate parameters for input and output devices.
> - struct :c:type:`v4l2_captureparm`
> - ``capture``
> - Parameters for capture devices, used when ``type`` is
> - ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
Wrap this one? It's over 80...
> * -
> - struct :c:type:`v4l2_outputparm`
> - ``output``
> - Parameters for output devices, used when ``type`` is
> - ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
> + ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
> * -
> - __u8
> - ``raw_data``\ [200]
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers
2018-01-22 10:18 ` [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
@ 2018-01-22 10:28 ` Sakari Ailus
2018-01-22 10:34 ` Hans Verkuil
0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2018-01-22 10:28 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
On Mon, Jan 22, 2018 at 11:18:49AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Create helpers to handle VIDIOC_G/S_PARM by querying the
> g/s_frame_interval v4l2_subdev ops.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 49 +++++++++++++++++++++++++++++++++++
> include/media/v4l2-common.h | 26 +++++++++++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 8650ad92b64d..4e371ae4aed7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
> tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
> }
> EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
> +
> +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;
> +
> + if (vdev->device_caps & V4L2_CAP_READWRITE)
> + a->parm.capture.readbuffers = 2;
> + if (v4l2_subdev_has_op(sd, video, g_frame_interval))
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + ret = v4l2_subdev_call(sd, video, g_frame_interval, &ival);
> + if (!ret)
> + a->parm.capture.timeperframe = ival.interval;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
> +
> +int v4l2_s_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +{
> + struct v4l2_subdev_frame_interval ival = {
> + 0,
> + a->parm.capture.timeperframe
I'd explicitly specify which members are assigned here. It looks cleaner
and does not depend on field order in the struct definition. It won't be
changed as it's part of uAPI but then again people take examples from
existing code and could do this where it's not safe.
> + };
> + int ret;
> +
> + if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return -EINVAL;
> +
> + memset(&a->parm, 0, sizeof(a->parm));
> + if (vdev->device_caps & V4L2_CAP_READWRITE)
> + a->parm.capture.readbuffers = 2;
> + else
> + a->parm.capture.readbuffers = 0;
> +
> + if (v4l2_subdev_has_op(sd, video, g_frame_interval))
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + ret = v4l2_subdev_call(sd, video, s_frame_interval, &ival);
> + if (!ret)
> + a->parm.capture.timeperframe = ival.interval;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_s_parm_cap);
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index e0d95a7c5d48..f3aa1d728c0b 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -341,4 +341,30 @@ v4l2_find_nearest_format(const struct v4l2_frmsize_discrete *sizes,
> */
> void v4l2_get_timestamp(struct timeval *tv);
>
> +/**
> + * v4l2_g_parm_cap - helper routine for vidioc_g_parm to fill this in by
> + * calling the g_frame_interval op of the given subdev. It only works
> + * for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
> + * function name.
> + *
> + * @vdev: the struct video_device pointer. Used to determine the device caps.
> + * @sd: the sub-device pointer.
> + * @a: the VIDIOC_G_PARM argument.
> + */
> +int v4l2_g_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a);
> +
> +/**
> + * v4l2_s_parm_cap - helper routine for vidioc_s_parm to fill this in by
> + * calling the s_frame_interval op of the given subdev. It only works
> + * for V4L2_BUF_TYPE_VIDEO_CAPTURE(_MPLANE), hence the _cap in the
> + * function name.
> + *
> + * @vdev: the struct video_device pointer. Used to determine the device caps.
> + * @sd: the sub-device pointer.
> + * @a: the VIDIOC_S_PARM argument.
> + */
> +int v4l2_s_parm_cap(struct video_device *vdev,
> + struct v4l2_subdev *sd, struct v4l2_streamparm *a);
> +
> #endif /* V4L2_COMMON_H_ */
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:26 ` Sakari Ailus
@ 2018-01-22 10:33 ` Hans Verkuil
2018-01-22 10:59 ` Sakari Ailus
0 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:33 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
On 22/01/18 11:26, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Convert all g/s_parm calls to g/s_frame_interval. This allows us
>> to remove the g/s_parm ops since those are a duplicate of
>> g/s_frame_interval.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>> drivers/media/i2c/mt9v011.c | 29 +++++++++----------
>> drivers/media/i2c/ov6650.c | 33 ++++++++++------------
>> drivers/media/i2c/ov7670.c | 26 +++++++++--------
>> drivers/media/i2c/ov7740.c | 29 ++++++++-----------
>> drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
>> drivers/media/i2c/vs6624.c | 29 ++++++++++---------
>> drivers/media/platform/atmel/atmel-isc.c | 10 ++-----
>> drivers/media/platform/atmel/atmel-isi.c | 12 ++------
>> drivers/media/platform/blackfin/bfin_capture.c | 14 +++-------
>> drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++----
>> drivers/media/platform/soc_camera/soc_camera.c | 10 ++++---
>> drivers/media/platform/via-camera.c | 4 +--
>> drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++++++++++++----
>> 13 files changed, 137 insertions(+), 144 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
>> index 5e29064fae91..0e0bcc8b67ca 100644
>> --- a/drivers/media/i2c/mt9v011.c
>> +++ b/drivers/media/i2c/mt9v011.c
>> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>> return 0;
>> }
>>
>> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_frame_interval *ival)
>> {
>> - struct v4l2_captureparm *cp = &parms->parm.capture;
>> -
>> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + if (ival->pad)
>> return -EINVAL;
>
> The pad number checks are already present in v4l2-subdev.c. Do you think
> we'll need them in drivers as well?
>
> It's true that another driver could mis-use this interface. In that case
> I'd introduce a wrapper to the op rather than introduce the check in every
> driver.
I'm not that keen on introducing wrappers for an op. I wouldn't actually know
how to implement that cleanly. Since the pad check is subdev driver specific,
and the overhead of a wrapper is almost certainly higher than just doing this
check I feel it is OK to do this.
>
>>
>> - memset(cp, 0, sizeof(struct v4l2_captureparm));
>> - cp->capability = V4L2_CAP_TIMEPERFRAME;
>> + memset(ival->reserved, 0, sizeof(ival->reserved));
>> calc_fps(sd,
>> - &cp->timeperframe.numerator,
>> - &cp->timeperframe.denominator);
>> + &ival->interval.numerator,
>> + &ival->interval.denominator);
>>
>> return 0;
>> }
Regards,
Hans
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers
2018-01-22 10:28 ` Sakari Ailus
@ 2018-01-22 10:34 ` Hans Verkuil
0 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:34 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
On 22/01/18 11:28, Sakari Ailus wrote:
> On Mon, Jan 22, 2018 at 11:18:49AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Create helpers to handle VIDIOC_G/S_PARM by querying the
>> g/s_frame_interval v4l2_subdev ops.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>> drivers/media/v4l2-core/v4l2-common.c | 49 +++++++++++++++++++++++++++++++++++
>> include/media/v4l2-common.h | 26 +++++++++++++++++++
>> 2 files changed, 75 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index 8650ad92b64d..4e371ae4aed7 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -392,3 +392,52 @@ void v4l2_get_timestamp(struct timeval *tv)
>> tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>> }
>> EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
>> +
>> +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;
>> +
>> + if (vdev->device_caps & V4L2_CAP_READWRITE)
>> + a->parm.capture.readbuffers = 2;
>> + if (v4l2_subdev_has_op(sd, video, g_frame_interval))
>> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
>> + ret = v4l2_subdev_call(sd, video, g_frame_interval, &ival);
>> + if (!ret)
>> + a->parm.capture.timeperframe = ival.interval;
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_g_parm_cap);
>> +
>> +int v4l2_s_parm_cap(struct video_device *vdev,
>> + struct v4l2_subdev *sd, struct v4l2_streamparm *a)
>> +{
>> + struct v4l2_subdev_frame_interval ival = {
>> + 0,
>> + a->parm.capture.timeperframe
>
> I'd explicitly specify which members are assigned here. It looks cleaner
> and does not depend on field order in the struct definition. It won't be
> changed as it's part of uAPI but then again people take examples from
> existing code and could do this where it's not safe.
True. I'll change this.
Regards,
Hans
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types
2018-01-22 10:26 ` Sakari Ailus
@ 2018-01-22 10:37 ` Hans Verkuil
0 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 10:37 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
On 22/01/18 11:26, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Jan 22, 2018 at 11:18:57AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The specification mentions that type can be V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> but the v4l2 core implementation also allows the _MPLANE variant.
>>
>> Document this.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>> Documentation/media/uapi/v4l/vidioc-g-parm.rst | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-g-parm.rst b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> index 616a5ea3f8fa..a941526cfeb4 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-g-parm.rst
>> @@ -66,7 +66,7 @@ union holding separate parameters for input and output devices.
>> -
>> - The buffer (stream) type, same as struct
>> :c:type:`v4l2_format` ``type``, set by the
>> - application. See :c:type:`v4l2_buf_type`
>> + application. See :c:type:`v4l2_buf_type`.
>> * - union
>> - ``parm``
>> -
>> @@ -75,12 +75,12 @@ union holding separate parameters for input and output devices.
>> - struct :c:type:`v4l2_captureparm`
>> - ``capture``
>> - Parameters for capture devices, used when ``type`` is
>> - ``V4L2_BUF_TYPE_VIDEO_CAPTURE``.
> ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` or ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``.
>
> Wrap this one? It's over 80...
OK, done.
Hans
>
>> * -
>> - struct :c:type:`v4l2_outputparm`
>> - ``output``
>> - Parameters for output devices, used when ``type`` is
>> - ``V4L2_BUF_TYPE_VIDEO_OUTPUT``.
>> + ``V4L2_BUF_TYPE_VIDEO_OUTPUT`` or ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE``.
>> * -
>> - __u8
>> - ``raw_data``\ [200]
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:33 ` Hans Verkuil
@ 2018-01-22 10:59 ` Sakari Ailus
2018-01-22 12:19 ` Hans Verkuil
0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2018-01-22 10:59 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
Hi Hans,
On Mon, Jan 22, 2018 at 11:33:28AM +0100, Hans Verkuil wrote:
> On 22/01/18 11:26, Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >> Convert all g/s_parm calls to g/s_frame_interval. This allows us
> >> to remove the g/s_parm ops since those are a duplicate of
> >> g/s_frame_interval.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >> drivers/media/i2c/mt9v011.c | 29 +++++++++----------
> >> drivers/media/i2c/ov6650.c | 33 ++++++++++------------
> >> drivers/media/i2c/ov7670.c | 26 +++++++++--------
> >> drivers/media/i2c/ov7740.c | 29 ++++++++-----------
> >> drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
> >> drivers/media/i2c/vs6624.c | 29 ++++++++++---------
> >> drivers/media/platform/atmel/atmel-isc.c | 10 ++-----
> >> drivers/media/platform/atmel/atmel-isi.c | 12 ++------
> >> drivers/media/platform/blackfin/bfin_capture.c | 14 +++-------
> >> drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++----
> >> drivers/media/platform/soc_camera/soc_camera.c | 10 ++++---
> >> drivers/media/platform/via-camera.c | 4 +--
> >> drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++++++++++++----
> >> 13 files changed, 137 insertions(+), 144 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
> >> index 5e29064fae91..0e0bcc8b67ca 100644
> >> --- a/drivers/media/i2c/mt9v011.c
> >> +++ b/drivers/media/i2c/mt9v011.c
> >> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
> >> return 0;
> >> }
> >>
> >> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_frame_interval *ival)
> >> {
> >> - struct v4l2_captureparm *cp = &parms->parm.capture;
> >> -
> >> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> + if (ival->pad)
> >> return -EINVAL;
> >
> > The pad number checks are already present in v4l2-subdev.c. Do you think
> > we'll need them in drivers as well?
> >
> > It's true that another driver could mis-use this interface. In that case
> > I'd introduce a wrapper to the op rather than introduce the check in every
> > driver.
>
> I'm not that keen on introducing wrappers for an op. I wouldn't actually know
> how to implement that cleanly. Since the pad check is subdev driver specific,
> and the overhead of a wrapper is almost certainly higher than just doing this
> check I feel it is OK to do this.
For a majority of drivers, the check is that the pad must be a valid
pad in an entity. The case where the overhead could matter (it's just a
single if) is when called through an IOCTL from the user space. And the
subdevices' IOCTL handler already performs this check. The wrapper would
simply extend the check to other drivers calling the op.
It's easy to miss such checks in drivers. We've had quite a few such cases
in the past. Performing the check universally would make the framework more
robust.
What comes to this patchset, I'd omit the pad number check as other drivers
don't do such checks either --- unless the check is more strict than what
the interface allows.
>
> >
> >>
> >> - memset(cp, 0, sizeof(struct v4l2_captureparm));
> >> - cp->capability = V4L2_CAP_TIMEPERFRAME;
> >> + memset(ival->reserved, 0, sizeof(ival->reserved));
> >> calc_fps(sd,
> >> - &cp->timeperframe.numerator,
> >> - &cp->timeperframe.denominator);
> >> + &ival->interval.numerator,
> >> + &ival->interval.denominator);
> >>
> >> return 0;
> >> }
>
> Regards,
>
> Hans
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:59 ` Sakari Ailus
@ 2018-01-22 12:19 ` Hans Verkuil
0 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 12:19 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Jacopo Mondi, Hans Verkuil
On 22/01/18 11:59, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Jan 22, 2018 at 11:33:28AM +0100, Hans Verkuil wrote:
>> On 22/01/18 11:26, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Convert all g/s_parm calls to g/s_frame_interval. This allows us
>>>> to remove the g/s_parm ops since those are a duplicate of
>>>> g/s_frame_interval.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> ---
>>>> drivers/media/i2c/mt9v011.c | 29 +++++++++----------
>>>> drivers/media/i2c/ov6650.c | 33 ++++++++++------------
>>>> drivers/media/i2c/ov7670.c | 26 +++++++++--------
>>>> drivers/media/i2c/ov7740.c | 29 ++++++++-----------
>>>> drivers/media/i2c/tvp514x.c | 37 +++++++++++--------------
>>>> drivers/media/i2c/vs6624.c | 29 ++++++++++---------
>>>> drivers/media/platform/atmel/atmel-isc.c | 10 ++-----
>>>> drivers/media/platform/atmel/atmel-isi.c | 12 ++------
>>>> drivers/media/platform/blackfin/bfin_capture.c | 14 +++-------
>>>> drivers/media/platform/marvell-ccic/mcam-core.c | 12 ++++----
>>>> drivers/media/platform/soc_camera/soc_camera.c | 10 ++++---
>>>> drivers/media/platform/via-camera.c | 4 +--
>>>> drivers/media/usb/em28xx/em28xx-video.c | 36 ++++++++++++++++++++----
>>>> 13 files changed, 137 insertions(+), 144 deletions(-)
>>>>
>>>> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
>>>> index 5e29064fae91..0e0bcc8b67ca 100644
>>>> --- a/drivers/media/i2c/mt9v011.c
>>>> +++ b/drivers/media/i2c/mt9v011.c
>>>> @@ -364,33 +364,30 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>>>> return 0;
>>>> }
>>>>
>>>> -static int mt9v011_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>>>> +static int mt9v011_g_frame_interval(struct v4l2_subdev *sd,
>>>> + struct v4l2_subdev_frame_interval *ival)
>>>> {
>>>> - struct v4l2_captureparm *cp = &parms->parm.capture;
>>>> -
>>>> - if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>>> + if (ival->pad)
>>>> return -EINVAL;
>>>
>>> The pad number checks are already present in v4l2-subdev.c. Do you think
>>> we'll need them in drivers as well?
>>>
>>> It's true that another driver could mis-use this interface. In that case
>>> I'd introduce a wrapper to the op rather than introduce the check in every
>>> driver.
>>
>> I'm not that keen on introducing wrappers for an op. I wouldn't actually know
>> how to implement that cleanly. Since the pad check is subdev driver specific,
>> and the overhead of a wrapper is almost certainly higher than just doing this
>> check I feel it is OK to do this.
>
> For a majority of drivers, the check is that the pad must be a valid
> pad in an entity. The case where the overhead could matter (it's just a
> single if) is when called through an IOCTL from the user space. And the
> subdevices' IOCTL handler already performs this check. The wrapper would
> simply extend the check to other drivers calling the op.
>
> It's easy to miss such checks in drivers. We've had quite a few such cases
> in the past. Performing the check universally would make the framework more
> robust.
>
> What comes to this patchset, I'd omit the pad number check as other drivers
> don't do such checks either --- unless the check is more strict than what
> the interface allows.
I'll remove the check.
Hans
>
>>
>>>
>>>>
>>>> - memset(cp, 0, sizeof(struct v4l2_captureparm));
>>>> - cp->capability = V4L2_CAP_TIMEPERFRAME;
>>>> + memset(ival->reserved, 0, sizeof(ival->reserved));
>>>> calc_fps(sd,
>>>> - &cp->timeperframe.numerator,
>>>> - &cp->timeperframe.denominator);
>>>> + &ival->interval.numerator,
>>>> + &ival->interval.denominator);
>>>>
>>>> return 0;
>>>> }
>>
>> Regards,
>>
>> Hans
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 10:18 ` [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
2018-01-22 10:26 ` Sakari Ailus
@ 2018-01-22 14:45 ` jacopo mondi
2018-01-22 14:48 ` Hans Verkuil
1 sibling, 1 reply; 21+ messages in thread
From: jacopo mondi @ 2018-01-22 14:45 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Sakari Ailus, Jacopo Mondi, Hans Verkuil
Hi Hans,
On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
>
[snip]
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index 34676409ca08..92d695b29fa9 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
> struct isc_device *isc = video_drvdata(file);
>
> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return -EINVAL;
> -
> - return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
> + return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
> }
As I've reported in a comment to the CEU patch series, after having
re-based my in-review driver on this series, I noticed I need to set
a->parm.capture.readbuffers to 0, as my driver does not support
CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
that's the case.
As atmel-isc (and I suspect other drivers modified by this patch) does
not support CAP_READWRITE, don't you need to set capturebuffers to 0
as well in order not to v4l2-compliance unhappy?
(I would have tested myself, but I don't have any of these platforms)
Thanks
j
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 14:45 ` jacopo mondi
@ 2018-01-22 14:48 ` Hans Verkuil
2018-01-22 14:53 ` jacopo mondi
0 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2018-01-22 14:48 UTC (permalink / raw)
To: jacopo mondi; +Cc: linux-media, Sakari Ailus, Jacopo Mondi, Hans Verkuil
On 22/01/18 15:45, jacopo mondi wrote:
> Hi Hans,
>
> On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>>
>
> [snip]
>
>> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
>> index 34676409ca08..92d695b29fa9 100644
>> --- a/drivers/media/platform/atmel/atmel-isc.c
>> +++ b/drivers/media/platform/atmel/atmel-isc.c
>> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
>> {
>> struct isc_device *isc = video_drvdata(file);
>>
>> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> - return -EINVAL;
>> -
>> - return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
>> + return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
>> }
>
> As I've reported in a comment to the CEU patch series, after having
> re-based my in-review driver on this series, I noticed I need to set
> a->parm.capture.readbuffers to 0, as my driver does not support
> CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
> that's the case.
>
> As atmel-isc (and I suspect other drivers modified by this patch) does
> not support CAP_READWRITE, don't you need to set capturebuffers to 0
> as well in order not to v4l2-compliance unhappy?
See the v4l2_g_parm_cap code in the first patch: I test if CAP_READWRITE is
set and if not, then readbuffers is initialized to 0.
Regards,
Hans
>
> (I would have tested myself, but I don't have any of these platforms)
>
> Thanks
> j
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs
2018-01-22 14:48 ` Hans Verkuil
@ 2018-01-22 14:53 ` jacopo mondi
0 siblings, 0 replies; 21+ messages in thread
From: jacopo mondi @ 2018-01-22 14:53 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Sakari Ailus, Jacopo Mondi, Hans Verkuil
Hi Hans,
On Mon, Jan 22, 2018 at 03:48:17PM +0100, Hans Verkuil wrote:
> On 22/01/18 15:45, jacopo mondi wrote:
> > Hi Hans,
> >
> > On Mon, Jan 22, 2018 at 11:18:50AM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >>
> >
> > [snip]
> >
> >> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> >> index 34676409ca08..92d695b29fa9 100644
> >> --- a/drivers/media/platform/atmel/atmel-isc.c
> >> +++ b/drivers/media/platform/atmel/atmel-isc.c
> >> @@ -1417,20 +1417,14 @@ static int isc_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> >> {
> >> struct isc_device *isc = video_drvdata(file);
> >>
> >> - if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> - return -EINVAL;
> >> -
> >> - return v4l2_subdev_call(isc->current_subdev->sd, video, g_parm, a);
> >> + return v4l2_g_parm_cap(video_devdata(file), isc->current_subdev->sd, a);
> >> }
> >
> > As I've reported in a comment to the CEU patch series, after having
> > re-based my in-review driver on this series, I noticed I need to set
> > a->parm.capture.readbuffers to 0, as my driver does not support
> > CAP_READWRITE, and v4l2-compliance checks for (readbuffers == 0) if
> > that's the case.
> >
> > As atmel-isc (and I suspect other drivers modified by this patch) does
> > not support CAP_READWRITE, don't you need to set capturebuffers to 0
> > as well in order not to v4l2-compliance unhappy?
>
> See the v4l2_g_parm_cap code in the first patch: I test if CAP_READWRITE is
> set and if not, then readbuffers is initialized to 0.
>
Uh, I'm sorry, that's different from the series that was on patchwork
yesterday! I'll rebase and test with CEU.
Thanks
j
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-01-22 14:53 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-22 10:18 [PATCH 0/9] media: replace g/s_parm by g/s_frame_interval Hans Verkuil
2018-01-22 10:18 ` [PATCH 1/9] v4l2-common: create v4l2_g/s_parm_cap helpers Hans Verkuil
2018-01-22 10:28 ` Sakari Ailus
2018-01-22 10:34 ` Hans Verkuil
2018-01-22 10:18 ` [PATCH 2/9] media: convert g/s_parm to g/s_frame_interval in subdevs Hans Verkuil
2018-01-22 10:26 ` Sakari Ailus
2018-01-22 10:33 ` Hans Verkuil
2018-01-22 10:59 ` Sakari Ailus
2018-01-22 12:19 ` Hans Verkuil
2018-01-22 14:45 ` jacopo mondi
2018-01-22 14:48 ` Hans Verkuil
2018-01-22 14:53 ` jacopo mondi
2018-01-22 10:18 ` [PATCH 3/9] staging: atomisp: Kill subdev s_parm abuse Hans Verkuil
2018-01-22 10:18 ` [PATCH 4/9] staging: atomisp: i2c: Disable non-preview configurations Hans Verkuil
2018-01-22 10:18 ` [PATCH 5/9] staging: atomisp: i2c: Drop g_parm support in sensor drivers Hans Verkuil
2018-01-22 10:18 ` [PATCH 6/9] staging: atomisp: mt9m114: Drop empty s_parm callback Hans Verkuil
2018-01-22 10:18 ` [PATCH 7/9] staging: atomisp: Drop g_parm and s_parm subdev ops use Hans Verkuil
2018-01-22 10:18 ` [PATCH 8/9] v4l2-subdev.h: remove obsolete g/s_parm Hans Verkuil
2018-01-22 10:18 ` [PATCH 9/9] vidioc-g-parm.rst: also allow _MPLANE buffer types Hans Verkuil
2018-01-22 10:26 ` Sakari Ailus
2018-01-22 10:37 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).