cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Jun 4 04:00:50 CEST 2014 git branch: test git hash: 5ea878796f0a1d9649fe43a6a09df53d3915c0ef gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: v0.5.0-11-g38d1124 host hardware: x86_64 host os:3.14-4.slh.4-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-i686: OK linux-3.13-i686: OK linux-3.14-i686: OK linux-3.15-rc1-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-x86_64: OK linux-3.13-x86_64: OK linux-3.14-x86_64: OK linux-3.15-rc1-x86_64: OK apps: OK spec-git: OK sparse version: v0.5.0-11-g38d1124 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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
Trouble scanning either of my cards, error "dvb_fe_set_parms failed (Invalid argument)"
Hi, So, I have two different tv tuner cards that I'm trying to get working. The kernel modules seem to be loading, no errors there. The /dev/dvb files are showing up. Everything has the right permissions. (Well, and I'm doing my testing as root too, just to get it working in the first place.) There are more specific boards I can go for help with each card, but they seem to be having a common error. One card is pcHDTV HD-5500, and it uses the cx8800 kernel module. And the other is Hauppauge WinTV-HVR-1250, and uses the cx23885 kernel module. http://pastebin.com/TzNt7mTn http://pastebin.com/m6cdLNEV Anyone have any ideas? -- 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: Poll and empty queues
Hi Nicolas, On Tuesday 03 June 2014 13:39:54 Nicolas Dufresne wrote: > Le mardi 03 juin 2014 à 18:11 +0200, Laurent Pinchart a écrit : > > On Tuesday 03 June 2014 10:37:50 Nicolas Dufresne wrote: > >> Le mardi 03 juin 2014 à 08:38 +0200, Hans Verkuil a écrit : > >>> On 06/02/2014 09:47 PM, Nicolas Dufresne wrote: > Hi everyone, > > Recently in GStreamer we notice that we where not handling the > POLLERR flag at all. Though we found that what the code do, and what > the doc says is slightly ambiguous. > > "When the application did not call VIDIOC_QBUF or > VIDIOC_STREAMON yet the poll() function succeeds, but sets > the POLLERR flag in the revents field." > > In our case, we first seen this error with a capture device. How > things worked is that we first en-queue all allocated buffers. Our > interpretation was that this would avoid not calling "VIDIOC_QBUF > [...] yet", and only then we would call VIDIOC_STREAMON. This way, > in our interpretation we would never get that error. > > Though, this is not what the code does. Looking at videobuf2, if > simply return this error when the queue is empty. > > /* > * There is nothing to wait for if no buffers have already been > queued. > */ > if (list_empty(&q->queued_list)) > return res | POLLERR; > > So basically, we endup in this situation where as soon as all > existing buffers has been dequeued, we can't rely on the driver to > wait for a buffer to be queued and then filled again. This basically > forces us into adding a new user-space mechanism, to wait for buffer > to come back. We are wandering if this is a bug. If not, maybe it > would be nice to improve the documentation. > >>> > >>> Just for my understanding: I assume that gstreamer polls in one > >>> process or thread and does the queuing/dequeuing in a different > >>> process/thread, is that correct? > >> > >> Yes, in this particular case (polling with queues/thread downstream), > >> the streaming thread do the polling, and then push the buffers. The > >> buffer reach a queue element, which will queued and return. Polling > >> restart at this point. The queue will later pass it downstream from the > >> next streaming thread, and eventually the buffer will be released. For > >> capture device, QBUF will be called upon release. > >> > >> It is assumed that this call to QBUF should take a finite amount of > >> time. Though, libv4l2 makes this assumption wrong by inter-locking DQBUF > >> and QBUF, clearly a bug, but not strictly related to this thread. Also, > >> as we try and avoid blocking in the DQBUF ioctl, it should not be a > >> problem for us. > >> > >>> If it was all in one process, then it would make no sense polling for > >>> a buffer to become available if you never queued one. > >> > >> Not exactly true, the driver may take some time before the buffer we > >> have queued back is filled and available again. The poll/select FD set > >> also have a control pipe, so we can stop the process at any moment. Not > >> polling would mean blocking on an ioctl() which cannot be canceled. > >> > >> But, without downstream queues (thread), the size of the queue will be > >> negotiated so that the buffer will be released before we go back > >> polling. The queue will never be empty in this case. > >> > >>> That is probably the reasoning behind what poll does today. That said, > >>> I've always thought it was wrong and that it should be replaced by > >>> something like: > >>> > >>> if (!vb2_is_streaming(q)) > >>> return res | POLLERR; > >>> > >>> I.e.: only return an error if we're not streaming. > >> > >> I think it would be easier for user space and closer to what the doc > >> says. > > > > I tend to agree, and I'd like to raise a different but related issue. > > > > I've recently run into a problem with a V4L2 device (OMAP4 ISS if you want > > details) that sometimes crashes during video capture. When this occurs the > > device is rendered completely unusable, and userspace need to stop the > > video stream and close the video device node in order to reset the > > device. That's not ideal, but until I pinpoint the root cause that's what > > we have to live with. > > > > When the OMAP4 ISS driver detects the error it immediately completes all > > queued buffers with the V4L2_BUF_FLAG_ERROR flag set, and returns -EIO > > from all subsequent VIDIOC_QBUF calls. The next few VIDIOC_DQBUF calls > > will return buffers with the V4L2_BUF_FLAG_ERROR flag set, after which the > > next VIDIOC_DQBUF call will block in __vb2_wait_for_done_vb() on > > > > ret = wait_event_interruptible(q->done_wq, > > !list_empty(&q->done_list) || !q->streaming); > > > > as the queue is still streaming and the done list stays empty. > > > > (Disclaimer : I'm using gstreamer 0.10 for this
Re: [PATCH 1/3] media-ctl: libv4l2subdev: Add DV timings support
Hi Laurent,
Laurent Pinchart wrote:
Hi Sakari,
On Tuesday 03 June 2014 15:32:24 Sakari Ailus wrote:
On Mon, Jun 02, 2014 at 05:10:02PM +0200, Laurent Pinchart wrote:
Expose the pad-level get caps, query, get and set DV timings ioctls.
Signed-off-by: Laurent Pinchart
---
utils/media-ctl/libv4l2subdev.c | 72
utils/media-ctl/v4l2subdev.h| 53 ++
2 files changed, 125 insertions(+)
diff --git a/utils/media-ctl/libv4l2subdev.c
b/utils/media-ctl/libv4l2subdev.c index 14daffa..8015330 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -189,6 +189,78 @@ int v4l2_subdev_set_selection(struct media_entity
*entity,
return 0;
}
+int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
+ struct v4l2_dv_timings_cap *caps)
+{
+ unsigned int pad = caps->pad;
+ int ret;
+
+ ret = v4l2_subdev_open(entity);
+ if (ret < 0)
+ return ret;
In every function v4l2_subdev_open() is called before the ioctl command. How
about implementing a wrapper which does both, and using that before this
patch?
I'm not too fond of the current subdev API. Subdevs are opened implicitly but
must be closed explicitly. Implicit open is of course easy, but the unbalanced
API makes me feel uneasy. I'm open to ideas for better alternatives :-)
I had a patch to reference count v4l2_subdev_open() (and close()), but
it didn't get applied back in 2012. And... apparently the reason for
that was that it didn't, well, apply. :-)
http://www.spinics.net/lists/linux-media/msg52258.html>
I'll resend it.
--
Sakari Ailus
sakari.ai...@iki.fi
--
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: Poll and empty queues
Le mardi 03 juin 2014 à 18:11 +0200, Laurent Pinchart a écrit : > Hi Nicolas, > > On Tuesday 03 June 2014 10:37:50 Nicolas Dufresne wrote: > > Le mardi 03 juin 2014 à 08:38 +0200, Hans Verkuil a écrit : > > > On 06/02/2014 09:47 PM, Nicolas Dufresne wrote: > > > > Hi everyone, > > > > > > > > Recently in GStreamer we notice that we where not handling the POLLERR > > > > flag at all. Though we found that what the code do, and what the doc > > > > says is slightly ambiguous. > > > > > > > > "When the application did not call VIDIOC_QBUF or > > > > VIDIOC_STREAMON yet the poll() function succeeds, but sets the > > > > POLLERR flag in the revents field." > > > > > > > > In our case, we first seen this error with a capture device. How things > > > > worked is that we first en-queue all allocated buffers. Our > > > > interpretation was that this would avoid not calling "VIDIOC_QBUF [...] > > > > yet", and only then we would call VIDIOC_STREAMON. This way, in our > > > > interpretation we would never get that error. > > > > > > > > Though, this is not what the code does. Looking at videobuf2, if simply > > > > return this error when the queue is empty. > > > > > > > > /* > > > > * There is nothing to wait for if no buffers have already been queued. > > > > */ > > > > if (list_empty(&q->queued_list)) > > > > return res | POLLERR; > > > > > > > > So basically, we endup in this situation where as soon as all existing > > > > buffers has been dequeued, we can't rely on the driver to wait for a > > > > buffer to be queued and then filled again. This basically forces us into > > > > adding a new user-space mechanism, to wait for buffer to come back. We > > > > are wandering if this is a bug. If not, maybe it would be nice to > > > > improve the documentation. > > > > > > Just for my understanding: I assume that gstreamer polls in one process or > > > thread and does the queuing/dequeuing in a different process/thread, is > > > that correct? > > > > Yes, in this particular case (polling with queues/thread downstream), > > the streaming thread do the polling, and then push the buffers. The > > buffer reach a queue element, which will queued and return. Polling > > restart at this point. The queue will later pass it downstream from the > > next streaming thread, and eventually the buffer will be released. For > > capture device, QBUF will be called upon release. > > > > It is assumed that this call to QBUF should take a finite amount of > > time. Though, libv4l2 makes this assumption wrong by inter-locking DQBUF > > and QBUF, clearly a bug, but not strictly related to this thread. Also, > > as we try and avoid blocking in the DQBUF ioctl, it should not be a > > problem for us. > > > > > If it was all in one process, then it would make no sense polling for a > > > buffer to become available if you never queued one. > > > > Not exactly true, the driver may take some time before the buffer we > > have queued back is filled and available again. The poll/select FD set > > also have a control pipe, so we can stop the process at any moment. Not > > polling would mean blocking on an ioctl() which cannot be canceled. > > > > But, without downstream queues (thread), the size of the queue will be > > negotiated so that the buffer will be released before we go back > > polling. The queue will never be empty in this case. > > > > > That is probably the reasoning behind what poll does today. That said, > > > I've always thought it was wrong and that it should be replaced by > > > something like: > > > > > > if (!vb2_is_streaming(q)) > > > return res | POLLERR; > > > > > > I.e.: only return an error if we're not streaming. > > > > I think it would be easier for user space and closer to what the doc says. > > I tend to agree, and I'd like to raise a different but related issue. > > I've recently run into a problem with a V4L2 device (OMAP4 ISS if you want > details) that sometimes crashes during video capture. When this occurs the > device is rendered completely unusable, and userspace need to stop the video > stream and close the video device node in order to reset the device. That's > not ideal, but until I pinpoint the root cause that's what we have to live > with. > > When the OMAP4 ISS driver detects the error it immediately completes all > queued buffers with the V4L2_BUF_FLAG_ERROR flag set, and returns -EIO from > all subsequent VIDIOC_QBUF calls. The next few VIDIOC_DQBUF calls will return > buffers with the V4L2_BUF_FLAG_ERROR flag set, after which the next > VIDIOC_DQBUF call will block in __vb2_wait_for_done_vb() on > > ret = wait_event_interruptible(q->done_wq, > !list_empty(&q->done_list) || !q->streaming); > > as the queue is still streaming and the done list stays empty. > > (Disclaimer : I'm using gstreamer 0.10 for this project due to TI shipping > the > OMAP4 H.264 codec for this version only) Nod, nothing I
[PATCH] dw2102: Geniatech T220 init fixed
Geniatech T220 init fixed - reset cmd from windows driver and fixed TS bus
config for cxd2820r.
Signed-off-by: Evgeny Plehov
---
drivers/media/usb/dvb-usb/dw2102.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dw2102.c
b/drivers/media/usb/dvb-usb/dw2102.c
index ae0f56a..7135a3e 100644
--- a/drivers/media/usb/dvb-usb/dw2102.c
+++ b/drivers/media/usb/dvb-usb/dw2102.c
@@ -1109,6 +1109,7 @@ static struct ds3000_config su3000_ds3000_config = {
static struct cxd2820r_config cxd2820r_config = {
.i2c_address = 0x6c, /* (0xd8 >> 1) */
.ts_mode = 0x38,
+ .ts_clock_inv = 1,
};
static struct tda18271_config tda18271_config = {
@@ -1387,20 +1388,27 @@ static int su3000_frontend_attach(struct
dvb_usb_adapter *d)
static int t220_frontend_attach(struct dvb_usb_adapter *d)
{
- u8 obuf[3] = { 0xe, 0x80, 0 };
+ u8 obuf[3] = { 0xe, 0x87, 0 };
u8 ibuf[] = { 0 };
if (dvb_usb_generic_rw(d->dev, obuf, 3, ibuf, 1, 0) < 0)
err("command 0x0e transfer failed.");
obuf[0] = 0xe;
- obuf[1] = 0x83;
+ obuf[1] = 0x86;
+ obuf[2] = 1;
+
+ if (dvb_usb_generic_rw(d->dev, obuf, 3, ibuf, 1, 0) < 0)
+ err("command 0x0e transfer failed.");
+
+ obuf[0] = 0xe;
+ obuf[1] = 0x80;
obuf[2] = 0;
if (dvb_usb_generic_rw(d->dev, obuf, 3, ibuf, 1, 0) < 0)
err("command 0x0e transfer failed.");
- msleep(100);
+ msleep(50);
obuf[0] = 0xe;
obuf[1] = 0x80;
--
1.9.1
--
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
[PATCH]cxd2820r: TS clock inversion in config
TS clock inversion in config.
Signed-off-by: Evgeny Plehov
---
drivers/media/dvb-frontends/cxd2820r.h| 6 ++
drivers/media/dvb-frontends/cxd2820r_c.c | 1 +
drivers/media/dvb-frontends/cxd2820r_t.c | 1 +
drivers/media/dvb-frontends/cxd2820r_t2.c | 1 +
4 files changed, 9 insertions(+)
diff --git a/drivers/media/dvb-frontends/cxd2820r.h
b/drivers/media/dvb-frontends/cxd2820r.h
index 82b3d93..6095dbc 100644
--- a/drivers/media/dvb-frontends/cxd2820r.h
+++ b/drivers/media/dvb-frontends/cxd2820r.h
@@ -52,6 +52,12 @@ struct cxd2820r_config {
*/
u8 ts_mode;
+ /* TS clock inverted.
+* Default: 0
+* Values: 0, 1
+*/
+ bool ts_clock_inv;
+
/* IF AGC polarity.
* Default: 0
* Values: 0, 1
diff --git a/drivers/media/dvb-frontends/cxd2820r_c.c
b/drivers/media/dvb-frontends/cxd2820r_c.c
index 5c6ab49..0f4657e 100644
--- a/drivers/media/dvb-frontends/cxd2820r_c.c
+++ b/drivers/media/dvb-frontends/cxd2820r_c.c
@@ -45,6 +45,7 @@ int cxd2820r_set_frontend_c(struct dvb_frontend *fe)
{ 0x1008b, 0x07, 0xff },
{ 0x1001f, priv->cfg.if_agc_polarity << 7, 0x80 },
{ 0x10070, priv->cfg.ts_mode, 0xff },
+ { 0x10071, !priv->cfg.ts_clock_inv << 4, 0x10 },
};
dev_dbg(&priv->i2c->dev, "%s: frequency=%d symbol_rate=%d\n", __func__,
diff --git a/drivers/media/dvb-frontends/cxd2820r_t.c
b/drivers/media/dvb-frontends/cxd2820r_t.c
index fa184ca..9b5a45b 100644
--- a/drivers/media/dvb-frontends/cxd2820r_t.c
+++ b/drivers/media/dvb-frontends/cxd2820r_t.c
@@ -46,6 +46,7 @@ int cxd2820r_set_frontend_t(struct dvb_frontend *fe)
{ 0x00088, 0x01, 0xff },
{ 0x00070, priv->cfg.ts_mode, 0xff },
+ { 0x00071, !priv->cfg.ts_clock_inv << 4, 0x10 },
{ 0x000cb, priv->cfg.if_agc_polarity << 6, 0x40 },
{ 0x000a5, 0x00, 0x01 },
{ 0x00082, 0x20, 0x60 },
diff --git a/drivers/media/dvb-frontends/cxd2820r_t2.c
b/drivers/media/dvb-frontends/cxd2820r_t2.c
index 2ba130e..9c0c4f4 100644
--- a/drivers/media/dvb-frontends/cxd2820r_t2.c
+++ b/drivers/media/dvb-frontends/cxd2820r_t2.c
@@ -47,6 +47,7 @@ int cxd2820r_set_frontend_t2(struct dvb_frontend *fe)
{ 0x02083, 0x0a, 0xff },
{ 0x020cb, priv->cfg.if_agc_polarity << 6, 0x40 },
{ 0x02070, priv->cfg.ts_mode, 0xff },
+ { 0x02071, !priv->cfg.ts_clock_inv << 6, 0x40 },
{ 0x020b5, priv->cfg.spec_inv << 4, 0x10 },
{ 0x02567, 0x07, 0x0f },
{ 0x02569, 0x03, 0x03 },
--
1.9.1
--
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 1/1] v4l: subdev: Unify argument validation across IOCTLs
Hi Sakari,
Thank you for the patch.
On Tuesday 03 June 2014 12:25:16 Sakari Ailus wrote:
> Separate validation of different argument types. There's no reason to do
> this separately for every IOCTL.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 119 ++-
> 1 file changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> b/drivers/media/v4l2-core/v4l2-subdev.c index 058c1a6..496f9bc 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -126,6 +126,55 @@ static int subdev_close(struct file *file)
> return 0;
> }
>
> +static int check_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_format *format)
> +{
> + if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> + format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (format->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop
> *crop) +{
> + if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> + crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (crop->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_selection(struct v4l2_subdev *sd,
> +struct v4l2_subdev_selection *sel)
> +{
> + if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
> + sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (sel->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
> *edid)
> +{
> + if (edid->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + if (edid->blocks && edid->edid == NULL)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> {
> struct video_device *vdev = video_devdata(file);
> @@ -202,26 +251,20 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> case VIDIOC_SUBDEV_G_FMT: {
> struct v4l2_subdev_format *format = arg;
> + int rval = check_format(sd, format);
How about declaring the variable once only at the beginning of the function ?
Apart from that,
Acked-by: Laurent Pinchart
>
> - if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> - format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (format->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh, format);
> }
>
> case VIDIOC_SUBDEV_S_FMT: {
> struct v4l2_subdev_format *format = arg;
> + int rval = check_format(sd, format);
>
> - if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> - format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (format->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> }
> @@ -229,14 +272,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) case VIDIOC_SUBDEV_G_CROP: {
> struct v4l2_subdev_crop *crop = arg;
> struct v4l2_subdev_selection sel;
> - int rval;
> -
> - if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> - crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> + int rval = check_crop(sd, crop);
>
> - if (crop->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> rval = v4l2_subdev_call(sd, pad, get_crop, subdev_fh, crop);
> if (rval != -ENOIOCTLCMD)
> @@ -258,14 +297,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) case VIDIOC_SUBDEV_S_CROP: {
> struct v4l2_subdev_crop *crop = arg;
> struct v4l2_subdev_selection sel;
> - int rval;
> -
> - if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> - crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> + int rval = check_crop(sd, crop);
>
> - if (crop->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
>
Re: [PATCH 1/3] media-ctl: libv4l2subdev: Add DV timings support
Hi Sakari,
On Tuesday 03 June 2014 15:32:24 Sakari Ailus wrote:
> On Mon, Jun 02, 2014 at 05:10:02PM +0200, Laurent Pinchart wrote:
> > Expose the pad-level get caps, query, get and set DV timings ioctls.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > utils/media-ctl/libv4l2subdev.c | 72
> > utils/media-ctl/v4l2subdev.h| 53 ++
> > 2 files changed, 125 insertions(+)
> >
> > diff --git a/utils/media-ctl/libv4l2subdev.c
> > b/utils/media-ctl/libv4l2subdev.c index 14daffa..8015330 100644
> > --- a/utils/media-ctl/libv4l2subdev.c
> > +++ b/utils/media-ctl/libv4l2subdev.c
> > @@ -189,6 +189,78 @@ int v4l2_subdev_set_selection(struct media_entity
> > *entity,
> > return 0;
> > }
> >
> > +int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
> > + struct v4l2_dv_timings_cap *caps)
> > +{
> > + unsigned int pad = caps->pad;
> > + int ret;
> > +
> > + ret = v4l2_subdev_open(entity);
> > + if (ret < 0)
> > + return ret;
>
> In every function v4l2_subdev_open() is called before the ioctl command. How
> about implementing a wrapper which does both, and using that before this
> patch?
I'm not too fond of the current subdev API. Subdevs are opened implicitly but
must be closed explicitly. Implicit open is of course easy, but the unbalanced
API makes me feel uneasy. I'm open to ideas for better alternatives :-)
--
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: Poll and empty queues
Hi Nicolas, On Tuesday 03 June 2014 10:37:50 Nicolas Dufresne wrote: > Le mardi 03 juin 2014 à 08:38 +0200, Hans Verkuil a écrit : > > On 06/02/2014 09:47 PM, Nicolas Dufresne wrote: > > > Hi everyone, > > > > > > Recently in GStreamer we notice that we where not handling the POLLERR > > > flag at all. Though we found that what the code do, and what the doc > > > says is slightly ambiguous. > > > > > > "When the application did not call VIDIOC_QBUF or > > > VIDIOC_STREAMON yet the poll() function succeeds, but sets the > > > POLLERR flag in the revents field." > > > > > > In our case, we first seen this error with a capture device. How things > > > worked is that we first en-queue all allocated buffers. Our > > > interpretation was that this would avoid not calling "VIDIOC_QBUF [...] > > > yet", and only then we would call VIDIOC_STREAMON. This way, in our > > > interpretation we would never get that error. > > > > > > Though, this is not what the code does. Looking at videobuf2, if simply > > > return this error when the queue is empty. > > > > > > /* > > > * There is nothing to wait for if no buffers have already been queued. > > > */ > > > if (list_empty(&q->queued_list)) > > > return res | POLLERR; > > > > > > So basically, we endup in this situation where as soon as all existing > > > buffers has been dequeued, we can't rely on the driver to wait for a > > > buffer to be queued and then filled again. This basically forces us into > > > adding a new user-space mechanism, to wait for buffer to come back. We > > > are wandering if this is a bug. If not, maybe it would be nice to > > > improve the documentation. > > > > Just for my understanding: I assume that gstreamer polls in one process or > > thread and does the queuing/dequeuing in a different process/thread, is > > that correct? > > Yes, in this particular case (polling with queues/thread downstream), > the streaming thread do the polling, and then push the buffers. The > buffer reach a queue element, which will queued and return. Polling > restart at this point. The queue will later pass it downstream from the > next streaming thread, and eventually the buffer will be released. For > capture device, QBUF will be called upon release. > > It is assumed that this call to QBUF should take a finite amount of > time. Though, libv4l2 makes this assumption wrong by inter-locking DQBUF > and QBUF, clearly a bug, but not strictly related to this thread. Also, > as we try and avoid blocking in the DQBUF ioctl, it should not be a > problem for us. > > > If it was all in one process, then it would make no sense polling for a > > buffer to become available if you never queued one. > > Not exactly true, the driver may take some time before the buffer we > have queued back is filled and available again. The poll/select FD set > also have a control pipe, so we can stop the process at any moment. Not > polling would mean blocking on an ioctl() which cannot be canceled. > > But, without downstream queues (thread), the size of the queue will be > negotiated so that the buffer will be released before we go back > polling. The queue will never be empty in this case. > > > That is probably the reasoning behind what poll does today. That said, > > I've always thought it was wrong and that it should be replaced by > > something like: > > > > if (!vb2_is_streaming(q)) > > return res | POLLERR; > > > > I.e.: only return an error if we're not streaming. > > I think it would be easier for user space and closer to what the doc says. I tend to agree, and I'd like to raise a different but related issue. I've recently run into a problem with a V4L2 device (OMAP4 ISS if you want details) that sometimes crashes during video capture. When this occurs the device is rendered completely unusable, and userspace need to stop the video stream and close the video device node in order to reset the device. That's not ideal, but until I pinpoint the root cause that's what we have to live with. When the OMAP4 ISS driver detects the error it immediately completes all queued buffers with the V4L2_BUF_FLAG_ERROR flag set, and returns -EIO from all subsequent VIDIOC_QBUF calls. The next few VIDIOC_DQBUF calls will return buffers with the V4L2_BUF_FLAG_ERROR flag set, after which the next VIDIOC_DQBUF call will block in __vb2_wait_for_done_vb() on ret = wait_event_interruptible(q->done_wq, !list_empty(&q->done_list) || !q->streaming); as the queue is still streaming and the done list stays empty. (Disclaimer : I'm using gstreamer 0.10 for this project due to TI shipping the OMAP4 H.264 codec for this version only) As gstreamer doesn't handle POLLERR in v4l2src the gst_poll_wait() call in gst_v4l2src_grab_frame() will return success, and the function then proceeds to call gst_v4l2_buffer_pool_dqbuf() which will block. Trying to stop the pipeline at that point just hangs forever on the VID
Re: Poll and empty queues
Le mardi 03 juin 2014 à 08:38 +0200, Hans Verkuil a écrit : > Hi Nicholas, > > On 06/02/2014 09:47 PM, Nicolas Dufresne wrote: > > Hi everyone, > > > > Recently in GStreamer we notice that we where not handling the POLLERR > > flag at all. Though we found that what the code do, and what the doc > > says is slightly ambiguous. > > > > "When the application did not call VIDIOC_QBUF or > > VIDIOC_STREAMON yet the poll() function succeeds, but sets the > > POLLERR flag in the revents field." > > > > In our case, we first seen this error with a capture device. How things > > worked is that we first en-queue all allocated buffers. Our > > interpretation was that this would avoid not calling "VIDIOC_QBUF [...] > > yet", and only then we would call VIDIOC_STREAMON. This way, in our > > interpretation we would never get that error. > > > > Though, this is not what the code does. Looking at videobuf2, if simply > > return this error when the queue is empty. > > > > /* > > * There is nothing to wait for if no buffers have already been queued. > > */ > > if (list_empty(&q->queued_list)) > > return res | POLLERR; > > > > So basically, we endup in this situation where as soon as all existing > > buffers has been dequeued, we can't rely on the driver to wait for a > > buffer to be queued and then filled again. This basically forces us into > > adding a new user-space mechanism, to wait for buffer to come back. We > > are wandering if this is a bug. If not, maybe it would be nice to > > improve the documentation. > > Just for my understanding: I assume that gstreamer polls in one process or > thread and does the queuing/dequeuing in a different process/thread, is that > correct? Yes, in this particular case (polling with queues/thread downstream), the streaming thread do the polling, and then push the buffers. The buffer reach a queue element, which will queued and return. Polling restart at this point. The queue will later pass it downstream from the next streaming thread, and eventually the buffer will be released. For capture device, QBUF will be called upon release. It is assumed that this call to QBUF should take a finite amount of time. Though, libv4l2 makes this assumption wrong by inter-locking DQBUF and QBUF, clearly a bug, but not strictly related to this thread. Also, as we try and avoid blocking in the DQBUF ioctl, it should not be a problem for us. > > If it was all in one process, then it would make no sense polling for a > buffer to become available if you never queued one. Not exactly true, the driver may take some time before the buffer we have queued back is filled and available again. The poll/select FD set also have a control pipe, so we can stop the process at any moment. Not polling would mean blocking on an ioctl() which cannot be canceled. But, without downstream queues (thread), the size of the queue will be negotiated so that the buffer will be released before we go back polling. The queue will never be empty in this case. > > That is probably the reasoning behind what poll does today. That said, I've > always thought it was wrong and that it should be replaced by something like: > > if (!vb2_is_streaming(q)) > return res | POLLERR; > > I.e.: only return an error if we're not streaming. I think it would be easier for user space and closer to what the doc says. Though, it's not just about changing that check, there is some more work involved from what I've seen. cheers, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH] libv4lconvert: Fix a regression when converting from Y10B
On Tue, 3 Jun 2014 15:48:46 +0200
Antonio Ospite wrote:
> Fix a regression introduced in commit
> efc29f1764a30808ebf7b3e1d9bfa27b909bf641 (libv4lconvert: Reject too
> short source buffer before accessing it).
>
> The old code:
>
> case V4L2_PIX_FMT_Y10BPACK:
> ...
> if (result == 0 && src_size < (width * height * 10 / 8)) {
> V4LCONVERT_ERR("short y10b data frame\n");
> errno = EPIPE;
> result = -1;
> }
> ...
>
> meant to say "If the conversion was *successful* _but_ the frame size
> was invalid, then take the error path", but in
> efc29f1764a30808ebf7b3e1d9bfa27b909bf641 this (maybe weird) logic was
> misunderstood and the v4lconvert_convert_pixfmt() was made to return an
^^^
Dear committer, you can remove this "the", if you feel like it :)
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
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
[PATCH] libv4lconvert: Fix a regression when converting from Y10B
Fix a regression introduced in commit
efc29f1764a30808ebf7b3e1d9bfa27b909bf641 (libv4lconvert: Reject too
short source buffer before accessing it).
The old code:
case V4L2_PIX_FMT_Y10BPACK:
...
if (result == 0 && src_size < (width * height * 10 / 8)) {
V4LCONVERT_ERR("short y10b data frame\n");
errno = EPIPE;
result = -1;
}
...
meant to say "If the conversion was *successful* _but_ the frame size
was invalid, then take the error path", but in
efc29f1764a30808ebf7b3e1d9bfa27b909bf641 this (maybe weird) logic was
misunderstood and the v4lconvert_convert_pixfmt() was made to return an
error even in the case of a successful conversion from Y10B.
Fix the check, and now print only the message letting the errno and the
result from the conversion routines to be propagated to the caller.
Signed-off-by: Antonio Ospite
Cc: Gregor Jasny
---
Hi,
the regression affects only the users of the gspca-kinect driver when using
the IR stream.
I think this should be cherry-picked in stable-1.0 too.
BTW Gregor, in efc29f1764a30808ebf7b3e1d9bfa27b909bf641 you say "Reject too
short source buffer before accessing it" but the code only does "result = -1"
and then still calls the conversion routines, so it's not actually *rejecting*
the frames, am I missing anything?
Thanks,
Antonio
lib/libv4lconvert/libv4lconvert.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index c49d30d..50d6906 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -1052,11 +1052,8 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
width, height);
break;
}
- if (result == 0) {
+ if (result != 0)
V4LCONVERT_ERR("y10b conversion failed\n");
- errno = EPIPE;
- result = -1;
- }
break;
case V4L2_PIX_FMT_RGB565:
--
2.0.0
--
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 v3 1/3] v4l: Add test pattern colour component controls
On 06/03/14 14:21, Laurent Pinchart wrote: > Hi Hans, > > On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: >> On 05/29/2014 05:01 PM, Laurent Pinchart wrote: >>> On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: Laurent Pinchart wrote: > On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: >> In many cases the test pattern has selectable values for each colour >> component. Implement controls for raw bayer components. Additional >> controls should be defined for colour components that are not covered >> by these controls. >> >> Signed-off-by: Sakari Ailus >> --- >> >> Documentation/DocBook/media/v4l/controls.xml | 34 >> drivers/media/v4l2-core/v4l2-ctrls.c | 4 >> include/uapi/linux/v4l2-controls.h | 4 >> 3 files changed, 42 insertions(+) >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml >> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 >> 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -4677,6 +4677,40 @@ interface and may change in the future. >> conversion. >> >> >> + >> +> spanname="id">V4L2_CID_TEST_PATTERN_RED >> + integer >> + >> + >> +Test pattern red colour component. >> + >> + >> + >> +> spanname="id">V4L2_CID_TEST_PATTERN_GREENR >> +integer >> + >> + >> +Test pattern green (next to red) >> +colour component. > > What about non-Bayer RGB sensors ? Should they use the GREENR or the > GREENB control for the green component ? Or a different control ? A different one. It should be simply green. I could add it to the same patch if you wish. > I'm wondering whether we shouldn't have a single test pattern color > control and create a color type using Hans' complex controls API. A raw bayer four-pixel value, you mean? >>> >>> Yes. I'll let Hans comment on that. >> >> Why would you need the complex control API for that? It would fit in a s32, >> and certainly in a s64. > > It wouldn't fit in a s32 when using more than 8 bits per component. s64 would > be an option, until we reach 16 bits per component (or more than 4 > components). > >> We have done something similar to this in the past (V4L2_CID_BG_COLOR). >> >> The main problem is that the interpretation of the s32 value has to be >> clearly defined. And if different sensors might have different min/max >> values for each component, then it becomes messy to use a single control. > > The interpretation would depend on both the sensor and the color format. > >> My feeling is that it is better to go with separate controls, one for each >> component. > > What bothers me is that we'll need to add lots of controls, for each > component. There's 4 controls for Bayer, one additional green control for > RGB, > 3 controls for YUV, ... That's already 8 controls to support the common > Bayer/RGB/YUV formats. As colors can be used for different purposes (test > pattern with possibly more than one color, background color, ...) that would > increase the complexity beyond my comfort zone. Why would this be any better with compound type controls? In fact, I think it is worse: simple controls will show up in qv4l2 and v4l2-ctl and are easy to set from the GUI/command line. To set compound controls you will need to write custom code for them. You also only see the controls that the driver actually needs. So while there may be X component controls, a driver for a Bayer sensor will only use four of them. If you want to support a driver that has highly specialized test pattern support, then I'm leaning towards a custom ioctl for that driver. Regards, Hans -- 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 1/3] media-ctl: libv4l2subdev: Add DV timings support
Hi Laurent,
On Mon, Jun 02, 2014 at 05:10:02PM +0200, Laurent Pinchart wrote:
> Expose the pad-level get caps, query, get and set DV timings ioctls.
>
> Signed-off-by: Laurent Pinchart
> ---
> utils/media-ctl/libv4l2subdev.c | 72
> +
> utils/media-ctl/v4l2subdev.h| 53 ++
> 2 files changed, 125 insertions(+)
>
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 14daffa..8015330 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -189,6 +189,78 @@ int v4l2_subdev_set_selection(struct media_entity
> *entity,
> return 0;
> }
>
> +int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
> + struct v4l2_dv_timings_cap *caps)
> +{
> + unsigned int pad = caps->pad;
> + int ret;
> +
> + ret = v4l2_subdev_open(entity);
> + if (ret < 0)
> + return ret;
In every function v4l2_subdev_open() is called before the ioctl command. How
about implementing a wrapper which does both, and using that before this
patch?
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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 v3 1/3] v4l: Add test pattern colour component controls
Hi Laurent, Laurent Pinchart wrote: Hi Hans, On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: On 05/29/2014 05:01 PM, Laurent Pinchart wrote: On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: Laurent Pinchart wrote: On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: In many cases the test pattern has selectable values for each colour component. Implement controls for raw bayer components. Additional controls should be defined for colour components that are not covered by these controls. Signed-off-by: Sakari Ailus --- Documentation/DocBook/media/v4l/controls.xml | 34 drivers/media/v4l2-core/v4l2-ctrls.c | 4 include/uapi/linux/v4l2-controls.h | 4 3 files changed, 42 insertions(+) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -4677,6 +4677,40 @@ interface and may change in the future. conversion. + + V4L2_CID_TEST_PATTERN_RED + integer + + + Test pattern red colour component. + + + + V4L2_CID_TEST_PATTERN_GREENR + integer + + + Test pattern green (next to red) + colour component. What about non-Bayer RGB sensors ? Should they use the GREENR or the GREENB control for the green component ? Or a different control ? A different one. It should be simply green. I could add it to the same patch if you wish. I'm wondering whether we shouldn't have a single test pattern color control and create a color type using Hans' complex controls API. A raw bayer four-pixel value, you mean? Yes. I'll let Hans comment on that. Why would you need the complex control API for that? It would fit in a s32, and certainly in a s64. It wouldn't fit in a s32 when using more than 8 bits per component. s64 would be an option, until we reach 16 bits per component (or more than 4 components). We have done something similar to this in the past (V4L2_CID_BG_COLOR). The main problem is that the interpretation of the s32 value has to be clearly defined. And if different sensors might have different min/max values for each component, then it becomes messy to use a single control. The interpretation would depend on both the sensor and the color format. My feeling is that it is better to go with separate controls, one for each component. What bothers me is that we'll need to add lots of controls, for each component. There's 4 controls for Bayer, one additional green control for RGB, 3 controls for YUV, ... That's already 8 controls to support the common Bayer/RGB/YUV formats. As colors can be used for different purposes (test Three if you make colour controls, and three more control types. Something to consider as well is that these controls are commonly used by test programs and you'd need to implement support for each new colour specific control type in the test programs such as yavta. I'm not sure if implementing three new control types for that purpose only makes sense. Do you expect to see much use for such control types outside the test patterns? pattern with possibly more than one color, background color, ...) that would increase the complexity beyond my comfort zone. I think that where they exist, such, likely device specific, controls wouldn't make it to the list of standard controls anyway. The test patterns typically are quite simple after all. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com -- 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 v3 1/3] v4l: Add test pattern colour component controls
Hi Hans, On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: > On 05/29/2014 05:01 PM, Laurent Pinchart wrote: > > On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: > >> Laurent Pinchart wrote: > >>> On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: > In many cases the test pattern has selectable values for each colour > component. Implement controls for raw bayer components. Additional > controls should be defined for colour components that are not covered > by these controls. > > Signed-off-by: Sakari Ailus > --- > > Documentation/DocBook/media/v4l/controls.xml | 34 > drivers/media/v4l2-core/v4l2-ctrls.c | 4 > include/uapi/linux/v4l2-controls.h | 4 > 3 files changed, 42 insertions(+) > > diff --git a/Documentation/DocBook/media/v4l/controls.xml > b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 > 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -4677,6 +4677,40 @@ interface and may change in the future. > conversion. > > > + > + spanname="id">V4L2_CID_TEST_PATTERN_RED > + integer > + > + > +Test pattern red colour component. > + > + > + > + spanname="id">V4L2_CID_TEST_PATTERN_GREENR > +integer > + > + > +Test pattern green (next to red) > +colour component. > >>> > >>> What about non-Bayer RGB sensors ? Should they use the GREENR or the > >>> GREENB control for the green component ? Or a different control ? > >> > >> A different one. It should be simply green. I could add it to the same > >> patch if you wish. > >> > >>> I'm wondering whether we shouldn't have a single test pattern color > >>> control and create a color type using Hans' complex controls API. > >> > >> A raw bayer four-pixel value, you mean? > > > > Yes. I'll let Hans comment on that. > > Why would you need the complex control API for that? It would fit in a s32, > and certainly in a s64. It wouldn't fit in a s32 when using more than 8 bits per component. s64 would be an option, until we reach 16 bits per component (or more than 4 components). > We have done something similar to this in the past (V4L2_CID_BG_COLOR). > > The main problem is that the interpretation of the s32 value has to be > clearly defined. And if different sensors might have different min/max > values for each component, then it becomes messy to use a single control. The interpretation would depend on both the sensor and the color format. > My feeling is that it is better to go with separate controls, one for each > component. What bothers me is that we'll need to add lots of controls, for each component. There's 4 controls for Bayer, one additional green control for RGB, 3 controls for YUV, ... That's already 8 controls to support the common Bayer/RGB/YUV formats. As colors can be used for different purposes (test pattern with possibly more than one color, background color, ...) that would increase the complexity beyond my comfort zone. -- 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 0/2] v4l-utils: Add missing v4l2-mediabus.h header
Hi Hans, On Tuesday 03 June 2014 11:48:14 Hans Verkuil wrote: > On 06/03/14 11:46, Laurent Pinchart wrote: > > On Tuesday 03 June 2014 08:52:29 Hans Verkuil wrote: > >> On 06/03/2014 02:44 AM, Laurent Pinchart wrote: > >>> Hello, > >>> > >>> This patch set adds the missing v4l2-mediabus.h header, required by > >>> media-ctl. Please see individual patches for details, they're pretty > >>> straightforward. > >> > >> Nack. > >> > >> The kernel headers used in v4l-utils are installed via 'make > >> sync-with-kernel'. So these headers shouldn't be edited, instead > >> Makefile.am should be updated. In particular, that's where the missing > >> header should be added. > > > > I had seen mentions of sync-with-kernel and for some reason thought it was > > a script. As I couldn't find it in the repository I decided to sync the > > headers manually :-/ > > > > Thanks for fixing the problem. By the way, what would you think about > > modifying sync-with-kernel to use installed kernel headers ? > > Patches are welcome! > > :-) Patch sent :-) -- 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
[PATCH] Update sync-with-kernel to use installed kernel headers
Kernel headers exported to userspace can contain kernel-specific statements (such as __user annotations) that are removed when installing the headers with 'make headers_install' in the kernel sources. Only those headers must be used by userspace, raw headers are private to the kernel. Update the sync-with-kernel make target to use the installed headers. The user must install the kernel headers by running make headers_install in KERNEL_DIR prior to run sync-with-kernel. Signed-off-by: Laurent Pinchart --- Makefile.am | 45 ++- contrib/freebsd/Makefile.am | 2 +- contrib/freebsd/bsdify.sh | 2 +- contrib/freebsd/patches/dvb-dmx-header.diff | 8 ++--- contrib/freebsd/patches/dvb-osd-header.diff | 2 +- contrib/freebsd/patches/dvb-video-header.diff | 8 ++--- contrib/freebsd/patches/input-header.diff | 8 ++--- contrib/freebsd/patches/ivtv-header.diff | 5 ++- contrib/freebsd/patches/uinput-header.diff| 8 ++--- contrib/freebsd/patches/videodev2-header.diff | 13 lib/libdvbv5/Makefile.am | 2 +- lib/libdvbv5/gen_dvb_structs.pl | 2 +- utils/keytable/Makefile.am| 12 +++ 13 files changed, 55 insertions(+), 62 deletions(-) diff --git a/Makefile.am b/Makefile.am index 11baed1..35d0030 100644 --- a/Makefile.am +++ b/Makefile.am @@ -12,31 +12,32 @@ EXTRA_DIST = include COPYING.libv4l README.libv4l README.lib-multi-threading # custom targets sync-with-kernel: - @if [ ! -f $(KERNEL_DIR)/include/uapi/linux/videodev2.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/fb.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/v4l2-controls.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/v4l2-common.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/v4l2-subdev.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/v4l2-mediabus.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/ivtv.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/dvb/frontend.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/dvb/dmx.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/dvb/audio.h -o \ - ! -f $(KERNEL_DIR)/include/uapi/linux/dvb/video.h ]; then \ + @if [ ! -f $(KERNEL_DIR)/usr/include/linux/videodev2.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/fb.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/v4l2-controls.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/v4l2-common.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/v4l2-subdev.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/v4l2-mediabus.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/ivtv.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/dvb/frontend.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/dvb/dmx.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/dvb/audio.h -o \ + ! -f $(KERNEL_DIR)/usr/include/linux/dvb/video.h ]; then \ echo "Error you must set KERNEL_DIR to point to an extracted kernel source dir"; \ + echo "and run 'make headers_install' in \$$KERNEL_DIR."; \ exit 1; \ fi - cp -a $(KERNEL_DIR)/include/uapi/linux/videodev2.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/fb.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/v4l2-controls.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/v4l2-common.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/v4l2-subdev.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/v4l2-mediabus.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/ivtv.h $(top_srcdir)/include/linux - cp -a $(KERNEL_DIR)/include/uapi/linux/dvb/frontend.h $(top_srcdir)/include/linux/dvb - cp -a $(KERNEL_DIR)/include/uapi/linux/dvb/dmx.h $(top_srcdir)/include/linux/dvb - cp -a $(KERNEL_DIR)/include/uapi/linux/dvb/audio.h $(top_srcdir)/include/linux/dvb - cp -a $(KERNEL_DIR)/include/uapi/linux/dvb/video.h $(top_srcdir)/include/linux/dvb + cp -a $(KERNEL_DIR)/usr/include/linux/videodev2.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/fb.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/v4l2-controls.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/v4l2-common.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/v4l2-subdev.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/v4l2-mediabus.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/ivtv.h $(top_srcdir)/include/linux + cp -a $(KERNEL_DIR)/usr/include/linux/dvb/frontend.h $(top_srcdir)/include/linux/dvb + cp -a $(KE
Re: [PATCH 0/2] v4l-utils: Add missing v4l2-mediabus.h header
On 06/03/14 11:46, Laurent Pinchart wrote: > Hi Hans, > > On Tuesday 03 June 2014 08:52:29 Hans Verkuil wrote: >> On 06/03/2014 02:44 AM, Laurent Pinchart wrote: >>> Hello, >>> >>> This patch set adds the missing v4l2-mediabus.h header, required by >>> media-ctl. Please see individual patches for details, they're pretty >>> straightforward. >> >> Nack. >> >> The kernel headers used in v4l-utils are installed via 'make >> sync-with-kernel'. So these headers shouldn't be edited, instead >> Makefile.am should be updated. In particular, that's where the missing >> header should be added. > > I had seen mentions of sync-with-kernel and for some reason thought it was a > script. As I couldn't find it in the repository I decided to sync the headers > manually :-/ > > Thanks for fixing the problem. By the way, what would you think about > modifying sync-with-kernel to use installed kernel headers ? Patches are welcome! :-) Regards, Hans -- 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 0/2] v4l-utils: Add missing v4l2-mediabus.h header
Hi Hans, On Tuesday 03 June 2014 08:52:29 Hans Verkuil wrote: > On 06/03/2014 02:44 AM, Laurent Pinchart wrote: > > Hello, > > > > This patch set adds the missing v4l2-mediabus.h header, required by > > media-ctl. Please see individual patches for details, they're pretty > > straightforward. > > Nack. > > The kernel headers used in v4l-utils are installed via 'make > sync-with-kernel'. So these headers shouldn't be edited, instead > Makefile.am should be updated. In particular, that's where the missing > header should be added. I had seen mentions of sync-with-kernel and for some reason thought it was a script. As I couldn't find it in the repository I decided to sync the headers manually :-/ Thanks for fixing the problem. By the way, what would you think about modifying sync-with-kernel to use installed kernel headers ? > > Laurent Pinchart (2): > > Use installed kernel headers instead of raw kernel headers > > Add the missing v4l2-mediabus.h kernel header > > > > include/linux/dvb/dmx.h | 8 +-- > > include/linux/dvb/frontend.h | 4 -- > > include/linux/dvb/video.h | 12 ++-- > > include/linux/fb.h| 8 +-- > > include/linux/ivtv.h | 6 +- > > include/linux/v4l2-mediabus.h | 147 + > > include/linux/videodev2.h | 16 ++--- > > 7 files changed, 168 insertions(+), 33 deletions(-) > > create mode 100644 include/linux/v4l2-mediabus.h -- 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
[PATCH v2 5/5] [media] mt9v032: use regmap
This switches all register accesses to use regmap. It allows to
use the regmap cache, tracing, and debug register dump facilities,
and removes the need to open code read-modify-writes.
Signed-off-by: Philipp Zabel
---
This patch was not included before.
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/mt9v032.c | 112 ++--
2 files changed, 46 insertions(+), 67 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 441053b..f40b4cf 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -551,6 +551,7 @@ config VIDEO_MT9V032
tristate "Micron MT9V032 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
+ select REGMAP_I2C
---help---
This is a Video4Linux2 sensor-level driver for the Micron
MT9V032 752x480 CMOS sensor.
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index cb7c6df..e756d50 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -245,6 +246,7 @@ struct mt9v032 {
struct mutex power_lock;
int power_count;
+ struct regmap *regmap;
struct clk *clk;
struct mt9v032_platform_data *pdata;
@@ -252,7 +254,6 @@ struct mt9v032 {
const struct mt9v032_model_version *version;
u32 sysclk;
- u16 chip_control;
u16 aec_agc;
u16 hblank;
struct {
@@ -266,40 +267,10 @@ static struct mt9v032 *to_mt9v032(struct v4l2_subdev *sd)
return container_of(sd, struct mt9v032, subdev);
}
-static int mt9v032_read(struct i2c_client *client, const u8 reg)
-{
- s32 data = i2c_smbus_read_word_swapped(client, reg);
- dev_dbg(&client->dev, "%s: read 0x%04x from 0x%02x\n", __func__,
- data, reg);
- return data;
-}
-
-static int mt9v032_write(struct i2c_client *client, const u8 reg,
-const u16 data)
-{
- dev_dbg(&client->dev, "%s: writing 0x%04x to 0x%02x\n", __func__,
- data, reg);
- return i2c_smbus_write_word_swapped(client, reg, data);
-}
-
-static int mt9v032_set_chip_control(struct mt9v032 *mt9v032, u16 clear, u16
set)
-{
- struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev);
- u16 value = (mt9v032->chip_control & ~clear) | set;
- int ret;
-
- ret = mt9v032_write(client, MT9V032_CHIP_CONTROL, value);
- if (ret < 0)
- return ret;
-
- mt9v032->chip_control = value;
- return 0;
-}
-
static int
mt9v032_update_aec_agc(struct mt9v032 *mt9v032, u16 which, int enable)
{
- struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev);
+ struct regmap *map = mt9v032->regmap;
u16 value = mt9v032->aec_agc;
int ret;
@@ -308,7 +279,7 @@ mt9v032_update_aec_agc(struct mt9v032 *mt9v032, u16 which,
int enable)
else
value &= ~which;
- ret = mt9v032_write(client, MT9V032_AEC_AGC_ENABLE, value);
+ ret = regmap_write(map, MT9V032_AEC_AGC_ENABLE, value);
if (ret < 0)
return ret;
@@ -319,7 +290,6 @@ mt9v032_update_aec_agc(struct mt9v032 *mt9v032, u16 which,
int enable)
static int
mt9v032_update_hblank(struct mt9v032 *mt9v032)
{
- struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev);
struct v4l2_rect *crop = &mt9v032->crop;
unsigned int min_hblank = mt9v032->model->data->min_hblank;
unsigned int hblank;
@@ -330,12 +300,13 @@ mt9v032_update_hblank(struct mt9v032 *mt9v032)
min_hblank);
hblank = max_t(unsigned int, mt9v032->hblank, min_hblank);
- return mt9v032_write(client, MT9V032_HORIZONTAL_BLANKING, hblank);
+ return regmap_write(mt9v032->regmap, MT9V032_HORIZONTAL_BLANKING,
+ hblank);
}
static int mt9v032_power_on(struct mt9v032 *mt9v032)
{
- struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev);
+ struct regmap *map = mt9v032->regmap;
unsigned long rate;
int ret;
@@ -350,7 +321,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032)
udelay(1);
/* Reset the chip and stop data read out */
- ret = mt9v032_write(client, MT9V032_RESET, 1);
+ ret = regmap_write(map, MT9V032_RESET, 1);
if (ret < 0)
return ret;
@@ -358,7 +329,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032)
rate = clk_get_rate(mt9v032->clk);
ndelay(DIV_ROUND_UP(15 * 12500, rate >> 3));
- return mt9v032_write(client, MT9V032_CHIP_CONTROL, 0);
+ return regmap_write(map, MT9V032_CHIP_CONTROL, 0);
}
static void mt9v032_power_off(struct mt9v032 *mt9v032)
@@ -368,7 +339,7 @@ static void mt9v032_power_off(struct mt9v032 *mt9v032)
static
[PATCH v2 0/5] mt9v032: add support for v4l2-async, mt9v02x, and regmap
Hi, This series contains the fixed up mt9v032 patches I recently sent, plus an additional patch to use regmap for the i2c register access. regards Philipp Philipp Zabel (5): [media] mt9v032: reset is self clearing [media] mt9v032: register v4l2 asynchronous subdevice [media] mt9v032: do not clear reserved bits in read mode register [media] mt9v032: add support for mt9v022 and mt9v024 [media] mt9v032: use regmap drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/mt9v032.c | 168 +--- 2 files changed, 95 insertions(+), 74 deletions(-) -- 2.0.0.rc2 -- 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
[PATCH v2 3/5] [media] mt9v032: do not clear reserved bits in read mode register
The read mode register bits 8 and 9 are set and marked as reserved. Don't clear them. Signed-off-by: Philipp Zabel --- Changes since v1: - Add MT9V032_READ_MODE_RESERVED #define --- drivers/media/i2c/mt9v032.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c index 83ae8ca6d..d969663 100644 --- a/drivers/media/i2c/mt9v032.c +++ b/drivers/media/i2c/mt9v032.c @@ -87,6 +87,7 @@ #defineMT9V032_READ_MODE_COLUMN_FLIP (1 << 5) #defineMT9V032_READ_MODE_DARK_COLUMNS (1 << 6) #defineMT9V032_READ_MODE_DARK_ROWS (1 << 7) +#defineMT9V032_READ_MODE_RESERVED 0x0300 #define MT9V032_PIXEL_OPERATION_MODE 0x0f #defineMT9V034_PIXEL_OPERATION_MODE_HDR(1 << 0) #defineMT9V034_PIXEL_OPERATION_MODE_COLOR (1 << 1) @@ -415,6 +416,7 @@ static int mt9v032_s_stream(struct v4l2_subdev *subdev, int enable) struct i2c_client *client = v4l2_get_subdevdata(subdev); struct mt9v032 *mt9v032 = to_mt9v032(subdev); struct v4l2_rect *crop = &mt9v032->crop; + unsigned int read_mode; unsigned int hbin; unsigned int vbin; int ret; @@ -425,9 +427,13 @@ static int mt9v032_s_stream(struct v4l2_subdev *subdev, int enable) /* Configure the window size and row/column bin */ hbin = fls(mt9v032->hratio) - 1; vbin = fls(mt9v032->vratio) - 1; - ret = mt9v032_write(client, MT9V032_READ_MODE, - hbin << MT9V032_READ_MODE_COLUMN_BIN_SHIFT | - vbin << MT9V032_READ_MODE_ROW_BIN_SHIFT); + read_mode = mt9v032_read(client, MT9V032_READ_MODE); + if (read_mode < 0) + return read_mode; + read_mode &= MT9V032_READ_MODE_RESERVED; + read_mode |= hbin << MT9V032_READ_MODE_COLUMN_BIN_SHIFT | +vbin << MT9V032_READ_MODE_ROW_BIN_SHIFT; + ret = mt9v032_write(client, MT9V032_READ_MODE, read_mode); if (ret < 0) return ret; -- 2.0.0.rc2 -- 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
[PATCH v2 4/5] [media] mt9v032: add support for mt9v022 and mt9v024
>From the looks of it, mt9v022 and mt9v032 are very similar,
as are mt9v024 and mt9v034. With minimal changes it is possible
to support mt9v02[24] with the same driver.
Signed-off-by: Philipp Zabel
---
Changes since v1:
- Removed code changes for fault pixel correction bit on mt9v02x
---
drivers/media/i2c/mt9v032.c | 40
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index d969663..cb7c6df 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -1,5 +1,5 @@
/*
- * Driver for MT9V032 CMOS Image Sensor from Micron
+ * Driver for MT9V022, MT9V024, MT9V032, and MT9V034 CMOS Image Sensors
*
* Copyright (C) 2010, Laurent Pinchart
*
@@ -134,8 +134,12 @@
#define MT9V032_THERMAL_INFO 0xc1
enum mt9v032_model {
- MT9V032_MODEL_V032_COLOR,
- MT9V032_MODEL_V032_MONO,
+ MT9V032_MODEL_V022_COLOR, /* MT9V022IX7ATC */
+ MT9V032_MODEL_V022_MONO,/* MT9V022IX7ATM */
+ MT9V032_MODEL_V024_COLOR, /* MT9V024IA7XTC */
+ MT9V032_MODEL_V024_MONO,/* MT9V024IA7XTM */
+ MT9V032_MODEL_V032_COLOR, /* MT9V032C12STM */
+ MT9V032_MODEL_V032_MONO,/* MT9V032C12STC */
MT9V032_MODEL_V034_COLOR,
MT9V032_MODEL_V034_MONO,
};
@@ -161,14 +165,14 @@ struct mt9v032_model_info {
};
static const struct mt9v032_model_version mt9v032_versions[] = {
- { MT9V032_CHIP_ID_REV1, "MT9V032 rev1/2" },
- { MT9V032_CHIP_ID_REV3, "MT9V032 rev3" },
- { MT9V034_CHIP_ID_REV1, "MT9V034 rev1" },
+ { MT9V032_CHIP_ID_REV1, "MT9V022/MT9V032 rev1/2" },
+ { MT9V032_CHIP_ID_REV3, "MT9V022/MT9V032 rev3" },
+ { MT9V034_CHIP_ID_REV1, "MT9V024/MT9V034 rev1" },
};
static const struct mt9v032_model_data mt9v032_model_data[] = {
{
- /* MT9V032 revisions 1/2/3 */
+ /* MT9V022, MT9V032 revisions 1/2/3 */
.min_row_time = 660,
.min_hblank = MT9V032_HORIZONTAL_BLANKING_MIN,
.min_vblank = MT9V032_VERTICAL_BLANKING_MIN,
@@ -177,7 +181,7 @@ static const struct mt9v032_model_data mt9v032_model_data[]
= {
.max_shutter = MT9V032_TOTAL_SHUTTER_WIDTH_MAX,
.pclk_reg = MT9V032_PIXEL_CLOCK,
}, {
- /* MT9V034 */
+ /* MT9V024, MT9V034 */
.min_row_time = 690,
.min_hblank = MT9V034_HORIZONTAL_BLANKING_MIN,
.min_vblank = MT9V034_VERTICAL_BLANKING_MIN,
@@ -189,6 +193,22 @@ static const struct mt9v032_model_data
mt9v032_model_data[] = {
};
static const struct mt9v032_model_info mt9v032_models[] = {
+ [MT9V032_MODEL_V022_COLOR] = {
+ .data = &mt9v032_model_data[0],
+ .color = true,
+ },
+ [MT9V032_MODEL_V022_MONO] = {
+ .data = &mt9v032_model_data[0],
+ .color = false,
+ },
+ [MT9V032_MODEL_V024_COLOR] = {
+ .data = &mt9v032_model_data[1],
+ .color = true,
+ },
+ [MT9V032_MODEL_V024_MONO] = {
+ .data = &mt9v032_model_data[1],
+ .color = false,
+ },
[MT9V032_MODEL_V032_COLOR] = {
.data = &mt9v032_model_data[0],
.color = true,
@@ -1022,6 +1042,10 @@ static int mt9v032_remove(struct i2c_client *client)
}
static const struct i2c_device_id mt9v032_id[] = {
+ { "mt9v022", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V022_COLOR]
},
+ { "mt9v022m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V022_MONO]
},
+ { "mt9v024", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V024_COLOR]
},
+ { "mt9v024m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V024_MONO]
},
{ "mt9v032", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V032_COLOR]
},
{ "mt9v032m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V032_MONO]
},
{ "mt9v034", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V034_COLOR]
},
--
2.0.0.rc2
--
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
[PATCH v2 2/5] [media] mt9v032: register v4l2 asynchronous subdevice
Add support for registering the sensor subdevice using the v4l2-async API. Signed-off-by: Philipp Zabel --- Changes since v1: - Fixed cleanup and error handling --- drivers/media/i2c/mt9v032.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c index 29d8d8f..83ae8ca6d 100644 --- a/drivers/media/i2c/mt9v032.c +++ b/drivers/media/i2c/mt9v032.c @@ -985,10 +985,20 @@ static int mt9v032_probe(struct i2c_client *client, mt9v032->pad.flags = MEDIA_PAD_FL_SOURCE; ret = media_entity_init(&mt9v032->subdev.entity, 1, &mt9v032->pad, 0); + if (ret < 0) + goto err_entity; + mt9v032->subdev.dev = &client->dev; + ret = v4l2_async_register_subdev(&mt9v032->subdev); if (ret < 0) - v4l2_ctrl_handler_free(&mt9v032->ctrls); + goto err_async; + + return 0; +err_async: + media_entity_cleanup(&mt9v032->subdev.entity); +err_entity: + v4l2_ctrl_handler_free(&mt9v032->ctrls); return ret; } @@ -997,6 +1007,7 @@ static int mt9v032_remove(struct i2c_client *client) struct v4l2_subdev *subdev = i2c_get_clientdata(client); struct mt9v032 *mt9v032 = to_mt9v032(subdev); + v4l2_async_unregister_subdev(subdev); v4l2_ctrl_handler_free(&mt9v032->ctrls); v4l2_device_unregister_subdev(subdev); media_entity_cleanup(&subdev->entity); -- 2.0.0.rc2 -- 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
[PATCH v2 1/5] [media] mt9v032: reset is self clearing
According to the publicly available MT9V032 data sheet, the reset bits are self
clearing and the reset register always reads 0. The reset will be asserted for
15 SYSCLK cycles. Instead of writing 0 to the register, wait using ndelay.
Signed-off-by: Philipp Zabel
---
drivers/media/i2c/mt9v032.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index f04d0bb..29d8d8f 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -315,6 +315,7 @@ mt9v032_update_hblank(struct mt9v032 *mt9v032)
static int mt9v032_power_on(struct mt9v032 *mt9v032)
{
struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev);
+ unsigned long rate;
int ret;
ret = clk_set_rate(mt9v032->clk, mt9v032->sysclk);
@@ -332,9 +333,9 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032)
if (ret < 0)
return ret;
- ret = mt9v032_write(client, MT9V032_RESET, 0);
- if (ret < 0)
- return ret;
+ /* Wait 15 SYSCLK cycles, 564 ns @ 26.6 MHz */
+ rate = clk_get_rate(mt9v032->clk);
+ ndelay(DIV_ROUND_UP(15 * 12500, rate >> 3));
return mt9v032_write(client, MT9V032_CHIP_CONTROL, 0);
}
--
2.0.0.rc2
--
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: [RFC PATCH] [media] mt9v032: Add support for mt9v022 and mt9v024
Hi Laurent, Guennadi, Am Mittwoch, den 28.05.2014, 16:44 +0200 schrieb Laurent Pinchart: > If you had submitted an entirely new driver for a sensor already supported by > an soc-camera sensor driver, I would have told you to fix the problem on soc- > camera side. As you're only expanding hardware support for an existing > driver, > it's hard to nack your patch in all fairness :-) I will thus not veto option > 2, even though I would prefer if we fixed the problem once and for all. > > > > > I'm ok with either 1 or 3, whereas 3 is > > > > more difficult than 1. > > > > > > This topic has been discussed over and over. It indeed "just" requires > > > someone to do it, although it might be more complex than that sounds. > > > > > > We need to fix the infrastructure to make sensor drivers completely > > > unaware of soc-camera. This isn't about extending the mt9v022 driver to > > > work with non soc-camera hosts, it's about fixing soc-camera not to > > > require any change to sensor drivers. Philipp, if you have time to work > > > on that, we can discuss what needs to be done. What steps would need to be taken to make soc_camera work with the non-soc_camera drivers in drivers/media/i2c? I don't have any soc_camera platform at hand, although I could try to revive a PXA270 board. > > I don't have a use case for soc_camera. Instead of trying to fix it to > > use generic sensor drivers, I'd rather use that time to prepare > > non-soc_camera capture host support. > > Which host would that be, if you can tell ? Yes, i.MX6. > > > On the sensor side, we should have a single driver for the mt9v022, 024 > > > and 032 sensors. I would vote for merging the two drivers into > > > drivers/media/i2c/mt9v032.c, as that one is closer to the goal of not > > > being soc-camera specific. regards Philipp -- 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
[PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs
Separate validation of different argument types. There's no reason to do
this separately for every IOCTL.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-subdev.c | 119 +-
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 058c1a6..496f9bc 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -126,6 +126,55 @@ static int subdev_close(struct file *file)
return 0;
}
+static int check_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_format *format)
+{
+ if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
+ format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (format->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
+{
+ if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
+ crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (crop->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_selection *sel)
+{
+ if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
+ sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (sel->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
+{
+ if (edid->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ if (edid->blocks && edid->edid == NULL)
+ return -EINVAL;
+
+ return 0;
+}
+
static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
@@ -202,26 +251,20 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
case VIDIOC_SUBDEV_G_FMT: {
struct v4l2_subdev_format *format = arg;
+ int rval = check_format(sd, format);
- if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
- format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (format->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh, format);
}
case VIDIOC_SUBDEV_S_FMT: {
struct v4l2_subdev_format *format = arg;
+ int rval = check_format(sd, format);
- if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
- format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (format->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
}
@@ -229,14 +272,10 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case VIDIOC_SUBDEV_G_CROP: {
struct v4l2_subdev_crop *crop = arg;
struct v4l2_subdev_selection sel;
- int rval;
-
- if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
- crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
+ int rval = check_crop(sd, crop);
- if (crop->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
rval = v4l2_subdev_call(sd, pad, get_crop, subdev_fh, crop);
if (rval != -ENOIOCTLCMD)
@@ -258,14 +297,10 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case VIDIOC_SUBDEV_S_CROP: {
struct v4l2_subdev_crop *crop = arg;
struct v4l2_subdev_selection sel;
- int rval;
-
- if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
- crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
+ int rval = check_crop(sd, crop);
- if (crop->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
rval = v4l2_subdev_call(sd, pad, set_crop, subdev_fh, crop);
if (rval != -ENOIOCTLCMD)
@@ -335,13 +370,10 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case VIDIOC_SUBDEV_G_SELECTION: {
struct v4l2_subdev_selection *
