Re: [PATCH 6/7] v4l2-subdev: remove enum_framesizes/intervals
On 02/13/15 12:30, Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com
Replace the video ops enum_framesizes and enum_frameintervals by the pad
ops enum_frame_size and enum_frame_interval.
The video and pad ops are duplicates, so get rid of the more limited video op.
The whole point of the subdev API is to allow reuse of subdev drivers by
bridge drivers. Having duplicate ops makes that much harder. We should never
have allowed duplicate ops in the first place. A lesson for the future.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
For ov7670 and marvell-ccic:
Tested-by: Hans Verkuil hans.verk...@cisco.com
It took blood, sweat and a bucket full of tears, but I finally managed to
get it to run on a OLPC XO-1 laptop. I plan to test more using v4l2-compliance,
but first I need to get the OS installed on an SD card. I think I finally found
good instructions for that. I'm certain this driver fails big time on the
compliance tests.
Regards,
Hans
Cc: Jonathan Corbet cor...@lwn.net
---
drivers/media/i2c/ov7670.c | 37 +++
drivers/media/platform/marvell-ccic/mcam-core.c | 48
++---
drivers/media/platform/soc_camera/soc_camera.c | 30 +++-
drivers/media/platform/via-camera.c | 15 ++--
include/media/v4l2-subdev.h | 2 --
5 files changed, 101 insertions(+), 31 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 957927f..b984752 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1069,29 +1069,35 @@ static int ov7670_s_parm(struct v4l2_subdev *sd,
struct v4l2_streamparm *parms)
static int ov7670_frame_rates[] = { 30, 15, 10, 5, 1 };
-static int ov7670_enum_frameintervals(struct v4l2_subdev *sd,
- struct v4l2_frmivalenum *interval)
+static int ov7670_enum_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_interval_enum
*fie)
{
- if (interval-index = ARRAY_SIZE(ov7670_frame_rates))
+ if (fie-pad)
return -EINVAL;
- interval-type = V4L2_FRMIVAL_TYPE_DISCRETE;
- interval-discrete.numerator = 1;
- interval-discrete.denominator = ov7670_frame_rates[interval-index];
+ if (fie-index = ARRAY_SIZE(ov7670_frame_rates))
+ return -EINVAL;
+ fie-interval.numerator = 1;
+ fie-interval.denominator = ov7670_frame_rates[fie-index];
return 0;
}
/*
* Frame size enumeration
*/
-static int ov7670_enum_framesizes(struct v4l2_subdev *sd,
- struct v4l2_frmsizeenum *fsize)
+static int ov7670_enum_frame_size(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum *fse)
{
struct ov7670_info *info = to_state(sd);
int i;
int num_valid = -1;
- __u32 index = fsize-index;
+ __u32 index = fse-index;
unsigned int n_win_sizes = info-devtype-n_win_sizes;
+ if (fse-pad)
+ return -EINVAL;
+
/*
* If a minimum width/height was requested, filter out the capture
* windows that fall outside that.
@@ -1103,9 +1109,8 @@ static int ov7670_enum_framesizes(struct v4l2_subdev
*sd,
if (info-min_height win-height info-min_height)
continue;
if (index == ++num_valid) {
- fsize-type = V4L2_FRMSIZE_TYPE_DISCRETE;
- fsize-discrete.width = win-width;
- fsize-discrete.height = win-height;
+ fse-min_width = fse-max_width = win-width;
+ fse-min_height = fse-max_height = win-height;
return 0;
}
}
@@ -1485,13 +1490,17 @@ static const struct v4l2_subdev_video_ops
ov7670_video_ops = {
.s_mbus_fmt = ov7670_s_mbus_fmt,
.s_parm = ov7670_s_parm,
.g_parm = ov7670_g_parm,
- .enum_frameintervals = ov7670_enum_frameintervals,
- .enum_framesizes = ov7670_enum_framesizes,
+};
+
+static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
+ .enum_frame_interval = ov7670_enum_frame_interval,
+ .enum_frame_size = ov7670_enum_frame_size,
};
static const struct v4l2_subdev_ops ov7670_ops = {
.core = ov7670_core_ops,
.video = ov7670_video_ops,
+ .pad = ov7670_pad_ops,
};
/* --- */
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index dd5b141..9c64b5d 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1568,24 +1568,64 @@ static int
Re: [PATCH 6/7] v4l2-subdev: remove enum_framesizes/intervals
On Wed, 04 Mar 2015 09:47:47 +0100 Hans Verkuil hverk...@xs4all.nl wrote: It took blood, sweat and a bucket full of tears, but I finally managed to get it to run on a OLPC XO-1 laptop. Sorry you had to do that. I've been trying to find a moment to employ my own bucket, but life remains rather unsettled in these parts. Feel free to add my ack, thanks for doing this. jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] v4l2-subdev: remove enum_framesizes/intervals
Hi Hans,
On Tuesday 24 February 2015 09:08:09 Hans Verkuil wrote:
On 02/13/2015 12:30 PM, Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com
Replace the video ops enum_framesizes and enum_frameintervals by the pad
ops enum_frame_size and enum_frame_interval.
The video and pad ops are duplicates, so get rid of the more limited video
op.
The whole point of the subdev API is to allow reuse of subdev drivers by
bridge drivers. Having duplicate ops makes that much harder. We should
never have allowed duplicate ops in the first place. A lesson for the
future.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Cc: Jonathan Corbet cor...@lwn.net
Laurent, can you Ack the v4l2 core change?
For the core,
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Jon, can you take a look at the changes to your drivers?
Thanks!
Hans
---
drivers/media/i2c/ov7670.c | 37 +++
drivers/media/platform/marvell-ccic/mcam-core.c | 48 +---
drivers/media/platform/soc_camera/soc_camera.c | 30 +++-
drivers/media/platform/via-camera.c | 15 ++--
include/media/v4l2-subdev.h | 2 --
5 files changed, 101 insertions(+), 31 deletions(-)
[snip]
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 6192f66..933f767 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -332,8 +332,6 @@ struct v4l2_subdev_video_ops {
struct v4l2_subdev_frame_interval *interval);
int (*s_frame_interval)(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_interval *interval);
- int (*enum_framesizes)(struct v4l2_subdev *sd, struct
v4l2_frmsizeenum *fsize);
- int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
v4l2_frmivalenum *fival);
int (*s_dv_timings)(struct v4l2_subdev *sd,
struct v4l2_dv_timings *timings);
int (*g_dv_timings)(struct v4l2_subdev *sd,
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] v4l2-subdev: remove enum_framesizes/intervals
On 02/13/2015 12:30 PM, Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com
Replace the video ops enum_framesizes and enum_frameintervals by the pad
ops enum_frame_size and enum_frame_interval.
The video and pad ops are duplicates, so get rid of the more limited video op.
The whole point of the subdev API is to allow reuse of subdev drivers by
bridge drivers. Having duplicate ops makes that much harder. We should never
have allowed duplicate ops in the first place. A lesson for the future.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Cc: Jonathan Corbet cor...@lwn.net
Laurent, can you Ack the v4l2 core change?
Jon, can you take a look at the changes to your drivers?
Thanks!
Hans
---
drivers/media/i2c/ov7670.c | 37 +++
drivers/media/platform/marvell-ccic/mcam-core.c | 48
++---
drivers/media/platform/soc_camera/soc_camera.c | 30 +++-
drivers/media/platform/via-camera.c | 15 ++--
include/media/v4l2-subdev.h | 2 --
5 files changed, 101 insertions(+), 31 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 957927f..b984752 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1069,29 +1069,35 @@ static int ov7670_s_parm(struct v4l2_subdev *sd,
struct v4l2_streamparm *parms)
static int ov7670_frame_rates[] = { 30, 15, 10, 5, 1 };
-static int ov7670_enum_frameintervals(struct v4l2_subdev *sd,
- struct v4l2_frmivalenum *interval)
+static int ov7670_enum_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_interval_enum
*fie)
{
- if (interval-index = ARRAY_SIZE(ov7670_frame_rates))
+ if (fie-pad)
return -EINVAL;
- interval-type = V4L2_FRMIVAL_TYPE_DISCRETE;
- interval-discrete.numerator = 1;
- interval-discrete.denominator = ov7670_frame_rates[interval-index];
+ if (fie-index = ARRAY_SIZE(ov7670_frame_rates))
+ return -EINVAL;
+ fie-interval.numerator = 1;
+ fie-interval.denominator = ov7670_frame_rates[fie-index];
return 0;
}
/*
* Frame size enumeration
*/
-static int ov7670_enum_framesizes(struct v4l2_subdev *sd,
- struct v4l2_frmsizeenum *fsize)
+static int ov7670_enum_frame_size(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum *fse)
{
struct ov7670_info *info = to_state(sd);
int i;
int num_valid = -1;
- __u32 index = fsize-index;
+ __u32 index = fse-index;
unsigned int n_win_sizes = info-devtype-n_win_sizes;
+ if (fse-pad)
+ return -EINVAL;
+
/*
* If a minimum width/height was requested, filter out the capture
* windows that fall outside that.
@@ -1103,9 +1109,8 @@ static int ov7670_enum_framesizes(struct v4l2_subdev
*sd,
if (info-min_height win-height info-min_height)
continue;
if (index == ++num_valid) {
- fsize-type = V4L2_FRMSIZE_TYPE_DISCRETE;
- fsize-discrete.width = win-width;
- fsize-discrete.height = win-height;
+ fse-min_width = fse-max_width = win-width;
+ fse-min_height = fse-max_height = win-height;
return 0;
}
}
@@ -1485,13 +1490,17 @@ static const struct v4l2_subdev_video_ops
ov7670_video_ops = {
.s_mbus_fmt = ov7670_s_mbus_fmt,
.s_parm = ov7670_s_parm,
.g_parm = ov7670_g_parm,
- .enum_frameintervals = ov7670_enum_frameintervals,
- .enum_framesizes = ov7670_enum_framesizes,
+};
+
+static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
+ .enum_frame_interval = ov7670_enum_frame_interval,
+ .enum_frame_size = ov7670_enum_frame_size,
};
static const struct v4l2_subdev_ops ov7670_ops = {
.core = ov7670_core_ops,
.video = ov7670_video_ops,
+ .pad = ov7670_pad_ops,
};
/* --- */
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index dd5b141..9c64b5d 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1568,24 +1568,64 @@ static int mcam_vidioc_enum_framesizes(struct file
*filp, void *priv,
struct v4l2_frmsizeenum *sizes)
{
struct mcam_camera *cam = priv;
+ struct mcam_format_struct *f;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .index = sizes-index,
+ .which =
