* subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE @ 2016-03-15 10:14 Philippe De Muyter 2016-03-15 11:06 ` Sakari Ailus 0 siblings, 1 reply; 9+ messages in thread From: Philippe De Muyter @ 2016-03-15 10:14 UTC (permalink / raw) To: linux-media Hi, Sorry if you read the following twice, but the subject of my previous post was not precise enough :( I am in the process of converting a sensor driver compatible with the imx6 freescale linux kernel, to a subdev driver compatible with a current kernel and Steve Longerbeam's work. My sensor can work at any fps (even fractional) up to 60 fps with its default frame size or even higher when using crop or "binning'. That fact is reflected in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. This seem not possible anymore because of the lack of the needed fields in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible anymore either. Has that been replaced by something else or is that functionality not considered relevant anymorea ? Best regards Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE 2016-03-15 10:14 subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE Philippe De Muyter @ 2016-03-15 11:06 ` Sakari Ailus 2016-03-15 11:54 ` Hans Verkuil 0 siblings, 1 reply; 9+ messages in thread From: Sakari Ailus @ 2016-03-15 11:06 UTC (permalink / raw) To: Philippe De Muyter; +Cc: linux-media, hverkuil Hi Philippe, On Tue, Mar 15, 2016 at 11:14:17AM +0100, Philippe De Muyter wrote: > Hi, > > Sorry if you read the following twice, but the subject of my previous post > was not precise enough :( > > I am in the process of converting a sensor driver compatible with the imx6 > freescale linux kernel, to a subdev driver compatible with a current kernel > and Steve Longerbeam's work. > > My sensor can work at any fps (even fractional) up to 60 fps with its default > frame size or even higher when using crop or "binning'. That fact is reflected > in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered > with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. > > This seem not possible anymore because of the lack of the needed fields > in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question > to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible > anymore either. Has that been replaced by something else or is that > functionality not considered relevant anymorea ? I think the issue was that the CONTINUOUS and STEPWISE were considered too clumsy for applications and practically no application was using them, or at least the need for these was not seen to be there. They were not added to the V4L2 sub-device implementation of the interface as a result. Cc Hans. The smiapp driver uses horizontal and vertical blanking controls for changing the frame rate. That's a bit lower level interface than most drivers use, but then you have to provide enough other information to the user space as well, including the pixel rate. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE 2016-03-15 11:06 ` Sakari Ailus @ 2016-03-15 11:54 ` Hans Verkuil 2016-03-15 11:58 ` Philippe De Muyter 2016-03-15 12:40 ` subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE Sakari Ailus 0 siblings, 2 replies; 9+ messages in thread From: Hans Verkuil @ 2016-03-15 11:54 UTC (permalink / raw) To: Sakari Ailus, Philippe De Muyter; +Cc: linux-media On 03/15/16 12:06, Sakari Ailus wrote: > Hi Philippe, > > On Tue, Mar 15, 2016 at 11:14:17AM +0100, Philippe De Muyter wrote: >> Hi, >> >> Sorry if you read the following twice, but the subject of my previous post >> was not precise enough :( >> >> I am in the process of converting a sensor driver compatible with the imx6 >> freescale linux kernel, to a subdev driver compatible with a current kernel >> and Steve Longerbeam's work. >> >> My sensor can work at any fps (even fractional) up to 60 fps with its default >> frame size or even higher when using crop or "binning'. That fact is reflected >> in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered >> with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. >> >> This seem not possible anymore because of the lack of the needed fields >> in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question >> to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible >> anymore either. Has that been replaced by something else or is that >> functionality not considered relevant anymorea ? > > I think the issue was that the CONTINUOUS and STEPWISE were considered too > clumsy for applications and practically no application was using them, or at > least the need for these was not seen to be there. They were not added to > the V4L2 sub-device implementation of the interface as a result. It never made sense to me why the two APIs weren't kept the same. My suggestion would be to add step_width and step_height fields to struct v4l2_subdev_frame_size_enum, that way you have the same functionality back. I.e. for discrete formats the min values equal the max values, for continuous formats the step values are both 1 (or 0 for compability's sake) and the remainder maps to a stepwise range. Regards, Hans > > Cc Hans. > > The smiapp driver uses horizontal and vertical blanking controls for > changing the frame rate. That's a bit lower level interface than most > drivers use, but then you have to provide enough other information to the > user space as well, including the pixel rate. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE 2016-03-15 11:54 ` Hans Verkuil @ 2016-03-15 11:58 ` Philippe De Muyter 2016-03-15 12:11 ` Hans Verkuil 2016-03-15 12:40 ` subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE Sakari Ailus 1 sibling, 1 reply; 9+ messages in thread From: Philippe De Muyter @ 2016-03-15 11:58 UTC (permalink / raw) To: Hans Verkuil; +Cc: Sakari Ailus, linux-media My post is about framerate, not framesize :) I agree the macro's names are misleading. On Tue, Mar 15, 2016 at 12:54:22PM +0100, Hans Verkuil wrote: > On 03/15/16 12:06, Sakari Ailus wrote: > > Hi Philippe, > > > > On Tue, Mar 15, 2016 at 11:14:17AM +0100, Philippe De Muyter wrote: > >> Hi, > >> > >> Sorry if you read the following twice, but the subject of my previous post > >> was not precise enough :( > >> > >> I am in the process of converting a sensor driver compatible with the imx6 > >> freescale linux kernel, to a subdev driver compatible with a current kernel > >> and Steve Longerbeam's work. > >> > >> My sensor can work at any fps (even fractional) up to 60 fps with its default > >> frame size or even higher when using crop or "binning'. That fact is reflected > >> in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered > >> with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. > >> > >> This seem not possible anymore because of the lack of the needed fields > >> in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question > >> to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible > >> anymore either. Has that been replaced by something else or is that > >> functionality not considered relevant anymorea ? > > > > I think the issue was that the CONTINUOUS and STEPWISE were considered too > > clumsy for applications and practically no application was using them, or at > > least the need for these was not seen to be there. They were not added to > > the V4L2 sub-device implementation of the interface as a result. > > It never made sense to me why the two APIs weren't kept the same. I agree completely with that. > > My suggestion would be to add step_width and step_height fields to > struct v4l2_subdev_frame_size_enum, that way you have the same functionality > back. > > I.e. for discrete formats the min values equal the max values, for continuous > formats the step values are both 1 (or 0 for compability's sake) and the > remainder maps to a stepwise range. > > Regards, > > Hans > > > > > Cc Hans. > > > > The smiapp driver uses horizontal and vertical blanking controls for > > changing the frame rate. That's a bit lower level interface than most > > drivers use, but then you have to provide enough other information to the > > user space as well, including the pixel rate. > > Philippe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE 2016-03-15 11:58 ` Philippe De Muyter @ 2016-03-15 12:11 ` Hans Verkuil 2016-06-11 22:29 ` [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE Philippe De Muyter [not found] ` <1465683659-12221-1-git-send-email-phdm@macqel.be> 0 siblings, 2 replies; 9+ messages in thread From: Hans Verkuil @ 2016-03-15 12:11 UTC (permalink / raw) To: Philippe De Muyter; +Cc: Sakari Ailus, linux-media On 03/15/16 12:58, Philippe De Muyter wrote: > My post is about framerate, not framesize :) I agree the macro's > names are misleading. Oops. Well, the solution would be similar (i.e. adding max_interval and step_interval fields to struct v4l2_subdev_frame_interval_enum. It takes away 4 fields from the reserved array, but this should have been done right from the start :-( Regards, Hans > > On Tue, Mar 15, 2016 at 12:54:22PM +0100, Hans Verkuil wrote: >> On 03/15/16 12:06, Sakari Ailus wrote: >>> Hi Philippe, >>> >>> On Tue, Mar 15, 2016 at 11:14:17AM +0100, Philippe De Muyter wrote: >>>> Hi, >>>> >>>> Sorry if you read the following twice, but the subject of my previous post >>>> was not precise enough :( >>>> >>>> I am in the process of converting a sensor driver compatible with the imx6 >>>> freescale linux kernel, to a subdev driver compatible with a current kernel >>>> and Steve Longerbeam's work. >>>> >>>> My sensor can work at any fps (even fractional) up to 60 fps with its default >>>> frame size or even higher when using crop or "binning'. That fact is reflected >>>> in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered >>>> with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. >>>> >>>> This seem not possible anymore because of the lack of the needed fields >>>> in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question >>>> to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible >>>> anymore either. Has that been replaced by something else or is that >>>> functionality not considered relevant anymorea ? >>> >>> I think the issue was that the CONTINUOUS and STEPWISE were considered too >>> clumsy for applications and practically no application was using them, or at >>> least the need for these was not seen to be there. They were not added to >>> the V4L2 sub-device implementation of the interface as a result. >> >> It never made sense to me why the two APIs weren't kept the same. > > I agree completely with that. > >> >> My suggestion would be to add step_width and step_height fields to >> struct v4l2_subdev_frame_size_enum, that way you have the same functionality >> back. >> >> I.e. for discrete formats the min values equal the max values, for continuous >> formats the step values are both 1 (or 0 for compability's sake) and the >> remainder maps to a stepwise range. >> >> Regards, >> >> Hans >> >>> >>> Cc Hans. >>> >>> The smiapp driver uses horizontal and vertical blanking controls for >>> changing the frame rate. That's a bit lower level interface than most >>> drivers use, but then you have to provide enough other information to the >>> user space as well, including the pixel rate. >>> > > Philippe > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE 2016-03-15 12:11 ` Hans Verkuil @ 2016-06-11 22:29 ` Philippe De Muyter 2016-06-11 22:31 ` [PATCH 2/2] media: mx6-camif: add V4L2_FRMIVAL_TYPE_STEPWISE & _CONTINUOUS Philippe De Muyter [not found] ` <1465683659-12221-1-git-send-email-phdm@macqel.be> 1 sibling, 1 reply; 9+ messages in thread From: Philippe De Muyter @ 2016-06-11 22:29 UTC (permalink / raw) To: hverkuil, linux-media, steve_longerbeam; +Cc: Philippe De Muyter add max_interval and step_interval to struct v4l2_subdev_frame_interval_enum. When filled correctly by the sensor driver, those fields must be used as follows by the intermediate level : struct v4l2_frmivalenum *fival; struct v4l2_subdev_frame_interval_enum fie; if (fie.max_interval.numerator == 0) { fival->type = V4L2_FRMIVAL_TYPE_DISCRETE; fival->discrete = fie.interval; } else if (fie.step_interval.numerator == 0) { fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; fival->stepwise.min = fie.interval; fival->stepwise.max = fie.max_interval; } else { fival->type = V4L2_FRMIVAL_TYPE_STEPWISE; fival->stepwise.min = fie.interval; fival->stepwise.max = fie.max_interval; fival->stepwise.step = fie.step_interval; } Signed-off-by: Philippe De Muyter <phdm@macqel.be> --- include/uapi/linux/v4l2-subdev.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index dbce2b554..846dd36 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -127,7 +127,9 @@ struct v4l2_subdev_frame_interval_enum { __u32 height; struct v4l2_fract interval; __u32 which; - __u32 reserved[8]; + struct v4l2_fract max_interval; + struct v4l2_fract step_interval; + __u32 reserved[4]; }; /** -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] media: mx6-camif: add V4L2_FRMIVAL_TYPE_STEPWISE & _CONTINUOUS 2016-06-11 22:29 ` [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE Philippe De Muyter @ 2016-06-11 22:31 ` Philippe De Muyter 0 siblings, 0 replies; 9+ messages in thread From: Philippe De Muyter @ 2016-06-11 22:31 UTC (permalink / raw) To: linux-media, steve_longerbeam; +Cc: Philippe De Muyter Signed-off-by: Philippe De Muyter <phdm@macqel.be> --- drivers/staging/media/imx6/capture/mx6-camif.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/imx6/capture/mx6-camif.c b/drivers/staging/media/imx6/capture/mx6-camif.c index b36f9d1..137733e 100644 --- a/drivers/staging/media/imx6/capture/mx6-camif.c +++ b/drivers/staging/media/imx6/capture/mx6-camif.c @@ -1344,8 +1344,19 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv, if (ret) return ret; - fival->type = V4L2_FRMIVAL_TYPE_DISCRETE; - fival->discrete = fie.interval; + if (fie.max_interval.numerator == 0) { + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE; + fival->discrete = fie.interval; + } else if (fie.step_interval.numerator == 0) { + fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; + fival->stepwise.min = fie.interval; + fival->stepwise.max = fie.max_interval; + } else { + fival->type = V4L2_FRMIVAL_TYPE_STEPWISE; + fival->stepwise.min = fie.interval; + fival->stepwise.max = fie.max_interval; + fival->stepwise.step = fie.step_interval; + } return 0; } -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <1465683659-12221-1-git-send-email-phdm@macqel.be>]
* Re: [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE [not found] ` <1465683659-12221-1-git-send-email-phdm@macqel.be> @ 2016-07-13 15:23 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 9+ messages in thread From: Mauro Carvalho Chehab @ 2016-07-13 15:23 UTC (permalink / raw) To: Philippe De Muyter; +Cc: linux-media, hverkuil Em Sun, 12 Jun 2016 00:20:59 +0200 Philippe De Muyter <phdm@macqel.be> escreveu: > add max_interval and step_interval to struct > v4l2_subdev_frame_interval_enum. > > When filled correctly by the sensor driver, those fields must be > used as follows by the intermediate level : > > struct v4l2_frmivalenum *fival; > struct v4l2_subdev_frame_interval_enum fie; > > if (fie.max_interval.numerator == 0) { > fival->type = V4L2_FRMIVAL_TYPE_DISCRETE; > fival->discrete = fie.interval; > } else if (fie.step_interval.numerator == 0) { > fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; > fival->stepwise.min = fie.interval; > fival->stepwise.max = fie.max_interval; > } else { > fival->type = V4L2_FRMIVAL_TYPE_STEPWISE; > fival->stepwise.min = fie.interval; > fival->stepwise.max = fie.max_interval; > fival->stepwise.step = fie.step_interval; > } > > Signed-off-by: Philippe De Muyter <phdm@macqel.be> > --- > include/uapi/linux/v4l2-subdev.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > index dbce2b554..846dd36 100644 > --- a/include/uapi/linux/v4l2-subdev.h > +++ b/include/uapi/linux/v4l2-subdev.h > @@ -127,7 +127,9 @@ struct v4l2_subdev_frame_interval_enum { > __u32 height; > struct v4l2_fract interval; > __u32 which; > - __u32 reserved[8]; > + struct v4l2_fract max_interval; > + struct v4l2_fract step_interval; > + __u32 reserved[4]; As this changes the userspace API, you need to also patch the Documentation/media to reflect such change and explain the meaning for the new fields. Please notice that the rst documentation is under the "docs-next" branch at the media_tree.git. You should either patch against that or wait for the end for 4.8-rc1, where the documentation will be merged back on the master branch. > }; > > /** -- Thanks, Mauro ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE 2016-03-15 11:54 ` Hans Verkuil 2016-03-15 11:58 ` Philippe De Muyter @ 2016-03-15 12:40 ` Sakari Ailus 1 sibling, 0 replies; 9+ messages in thread From: Sakari Ailus @ 2016-03-15 12:40 UTC (permalink / raw) To: Hans Verkuil; +Cc: Philippe De Muyter, linux-media, laurent.pinchart Hi Hans, On Tue, Mar 15, 2016 at 12:54:22PM +0100, Hans Verkuil wrote: > On 03/15/16 12:06, Sakari Ailus wrote: > > Hi Philippe, > > > > On Tue, Mar 15, 2016 at 11:14:17AM +0100, Philippe De Muyter wrote: > >> Hi, > >> > >> Sorry if you read the following twice, but the subject of my previous post > >> was not precise enough :( > >> > >> I am in the process of converting a sensor driver compatible with the imx6 > >> freescale linux kernel, to a subdev driver compatible with a current kernel > >> and Steve Longerbeam's work. > >> > >> My sensor can work at any fps (even fractional) up to 60 fps with its default > >> frame size or even higher when using crop or "binning'. That fact is reflected > >> in my previous implemetation of VIDIOC_ENUM_FRAMEINTERVALS, which answered > >> with a V4L2_FRMIVAL_TYPE_CONTINUOUS-type reply. > >> > >> This seem not possible anymore because of the lack of the needed fields > >> in the 'struct v4l2_subdev_frame_interval_enum' used to delegate the question > >> to the subdev driver. V4L2_FRMIVAL_TYPE_STEPWISE does not seem possible > >> anymore either. Has that been replaced by something else or is that > >> functionality not considered relevant anymorea ? > > > > I think the issue was that the CONTINUOUS and STEPWISE were considered too > > clumsy for applications and practically no application was using them, or at > > least the need for these was not seen to be there. They were not added to > > the V4L2 sub-device implementation of the interface as a result. > > It never made sense to me why the two APIs weren't kept the same. > > My suggestion would be to add step_width and step_height fields to > struct v4l2_subdev_frame_size_enum, that way you have the same functionality > back. > > I.e. for discrete formats the min values equal the max values, for continuous > formats the step values are both 1 (or 0 for compability's sake) and the > remainder maps to a stepwise range. On frame intervals... I'm not against changing it if there's a need to. Cc'ing Laurent who I believe wrote the original API. I probably acked it. We just needed to add the type field (snatch one from the reserved array), DISCRETE translates to zero so there are on compatibility issues either. struct v4l2_frmival_stepwise will consume all remaining reserved entries with the exception of one, but there is still room for it. Documentation is needed as well. I don't think it'd be very pretty but I guess there's no way around that at this point. We should have had the which field before the interval field. -- Regards, Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-13 15:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-15 10:14 subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE Philippe De Muyter 2016-03-15 11:06 ` Sakari Ailus 2016-03-15 11:54 ` Hans Verkuil 2016-03-15 11:58 ` Philippe De Muyter 2016-03-15 12:11 ` Hans Verkuil 2016-06-11 22:29 ` [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE Philippe De Muyter 2016-06-11 22:31 ` [PATCH 2/2] media: mx6-camif: add V4L2_FRMIVAL_TYPE_STEPWISE & _CONTINUOUS Philippe De Muyter [not found] ` <1465683659-12221-1-git-send-email-phdm@macqel.be> 2016-07-13 15:23 ` [PATCH 1/2] [media] v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE Mauro Carvalho Chehab 2016-03-15 12:40 ` subdev sensor driver and V4L2_FRMIVAL_TYPE_CONTINUOUS/V4L2_FRMIVAL_TYPE_STEPWISE Sakari Ailus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.