Re: [PATCH take 2] V4L: videobuf-core.c VIDIOC_QBUF should return video buffer flags
On Tuesday 01 September 2009 11:56:19 ext Laurent Pinchart wrote: > On Monday 31 August 2009 13:58:54 Tuukka.O Toivonen wrote: > > When user space queues a buffer using VIDIOC_QBUF, the kernel > > should set flags in struct v4l2_buffer as specified in the V4L2 > > documentation. > > You forgot your SoB line. My bad. > > + __u32 buffer_flags = b->flags; > > Is that safe ? What if userspace sets bogus flags ? What else userspace should expect than garbage in, garbage out? Now this sets and clears exactly what V4L2 spec says it should, should we clear all other flags also? What about other fields that are not mentioned in the VIDIOC_QBUF documentation for input buffers, like bytesused, timestamp, etc.? Should VIDIOC_QBUF clear those also? - Tuukka -- 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 take 2] V4L: videobuf-core.c VIDIOC_QBUF should return video buffer flags
When user space queues a buffer using VIDIOC_QBUF, the kernel
should set flags in struct v4l2_buffer as specified in the V4L2
documentation.
---
drivers/media/video/videobuf-core.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/videobuf-core.c
b/drivers/media/video/videobuf-core.c
index b7b0584..1322056 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -477,6 +477,7 @@ int videobuf_qbuf(struct videobuf_queue *q,
struct videobuf_buffer *buf;
enum v4l2_field field;
unsigned long flags = 0;
+ __u32 buffer_flags = b->flags;
int retval;
MAGIC_CHECK(q->int_ops->magic, MAGIC_QTYPE_OPS);
@@ -531,6 +532,8 @@ int videobuf_qbuf(struct videobuf_queue *q,
"but buffer addr is zero!\n");
goto done;
}
+ buffer_flags |= V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED;
+ buffer_flags &= ~V4L2_BUF_FLAG_DONE;
break;
case V4L2_MEMORY_USERPTR:
if (b->length < buf->bsize) {
@@ -541,6 +544,8 @@ int videobuf_qbuf(struct videobuf_queue *q,
buf->baddr != b->m.userptr)
q->ops->buf_release(q, buf);
buf->baddr = b->m.userptr;
+ buffer_flags |= V4L2_BUF_FLAG_QUEUED;
+ buffer_flags &= ~(V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_DONE);
break;
case V4L2_MEMORY_OVERLAY:
buf->boff = b->m.offset;
@@ -564,6 +569,8 @@ int videobuf_qbuf(struct videobuf_queue *q,
q->ops->buf_queue(q, buf);
spin_unlock_irqrestore(q->irqlock, flags);
}
+
+ b->flags = buffer_flags;
dprintk(1, "qbuf: succeded\n");
retval = 0;
wake_up_interruptible_sync(&q->wait);
--
1.5.4.3
--
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
Units in V4L2 controls
Hi, quoting V4L2 spec: http://v4l2spec.bytesex.org/spec/r13317.htm "__s32 step [...] Generally drivers should not scale hardware control values. It may be necessary for example when the name or id imply a particular unit and the hardware actually accepts only multiples of said unit. If so, drivers must take care values are properly rounded when scaling, such that errors will not accumulate on repeated read-write cycles." I'm wondering what that "particular unit" means. Is it OK to name V4L2_CID_EXPOSURE to "Exposure time [us]" and then use microseconds for exposure time, even if HW supports only image row granularity (rolling shutter)? If not, how should the driver report to user program the actual exposure time (necessary eg. for 50 Hz/60 Hz flicker elimination). What about flash timeout, we have here a circuit which supports only 50, 100, 200, 400, etc. milliseconds. I report "step" to be 50 ms and then round the user setting to the closest value available. User program could query the actual value used with VIDIOC_G_CTRL. The same problem holds for other controls, at least we'd like to use exposure value (EV) units for gain, etc. - Tuukka -- 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] V4L: videobuf-core.c VIDIOC_QBUF should return video buffer flags
When user space queues a buffer using VIDIOC_QBUF, the kernel should set flags to V4L2_BUF_FLAG_QUEUED in struct v4l2_buffer. videobuf_qbuf() was missing a call to videobuf_status() which does that. This patch adds the proper function call. Signed-off-by: Tuukka Toivonen --- drivers/media/video/videobuf-core.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c index b7b0584..d212710 100644 --- a/drivers/media/video/videobuf-core.c +++ b/drivers/media/video/videobuf-core.c @@ -565,6 +565,8 @@ int videobuf_qbuf(struct videobuf_queue *q, spin_unlock_irqrestore(q->irqlock, flags); } dprintk(1, "qbuf: succeded\n"); + memset(b, 0, sizeof(*b)); + videobuf_status(q, b, buf, q->type); retval = 0; wake_up_interruptible_sync(&q->wait); -- 1.5.4.3 -- 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: OMAP3 ISP and camera drivers (update 2)
On Monday 22 June 2009 17:00:53 ext Dongsoo Kim wrote: > OK, what I'm afraid is that even though the device could be opened and > recognized as a v4l2 device but has no capability should be weird. > Actually I'm not sure about this case is spec-in or not. > In my opinion it should be better when the camera interface (or ISP) > has no int device (or subdev) attahced on it, no device node mounted > in /dev or returning ENODEV. But before that, I'm very curious about > why you made in that way. We had to be able to use other slave devices (eg. flash) before attaching the actual camera module. - Tuukka -- 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: OMAP3 ISP and camera drivers (update 2)
On Saturday 20 June 2009 12:05:13 ext Dongsoo, Nathaniel Kim wrote: > Following patch. > http://www.gitorious.org/omap3camera/mainline/commit/d92c96406296310a977b00f45b209523929b15b5 > What happens to the capability when the int device is dummy? (does it > mean that there is no int device?) Yes, when the int device is dummy, there is no such a device. For example, when vdev->vdev_sensor == v4l2_int_device_dummy() it means that the device has no sensor. In that case, obviously, the device is not capable of capturing or streaming. > And one more thing. If I want to test how the "ISP" driver is working, > is there any target board that I can buy also a sensor device already > attached on it? I think that TI probably has some boards for sale, you could take a look at their web pages. - Tuukka -- 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: ISP Configuration for RAW Bayer sensor
On Wednesday 18 March 2009 18:17:56 ext Suresh Rao wrote:
> I am working with MT9V023 RAW sensor. The data format from the sensor is
>
> B G B G B G B G ...
> G R G R G R G R ...
> B G B G B G B G ...
> G R G R G R G R [ Format 1]
[...]
> I want to use the ISP on the OMAP for doing interpolation and format
> conversion to UYVY. I am able to capture the images from the sensor,
> however I notice that the color information is missing. I dug the
> sources and found that in the RAW capture mode ISP is getting
> configured to input format
>
> G R G R G R G R ...
> B G B G B G B G ...
> G R G R G R G R ...
> B G B G B G B G ... [Format 2]
>
> Has anyone tried sensors with BGGR ( Format 1) on OMAP?
>
> Can anyone give me some pointers or information on how to configure
> ISP for BGGR (Format 1)
If you can live with losing a few pixels (maybe sensor has a few extra)
I recommend to configure ISP to crop away the topmost line.
Here's couple of old _example_ patches how to configure the cropping.
Just gives an idea where to start...
- Tuukka
diff --git a/drivers/media/video/isp/ispccdc.c
b/drivers/media/video/isp/ispccdc.c
index 2288bc9..87870f1 100644
--- a/drivers/media/video/isp/ispccdc.c
+++ b/drivers/media/video/isp/ispccdc.c
@@ -1189,13 +1189,13 @@ int ispccdc_config_size(u32 input_w, u32 input_h, u32
output_w, u32 output_h)
ISPCCDC_HORZ_INFO);
} else {
if (ispccdc_obj.ccdc_inpfmt == CCDC_RAW) {
- omap_writel(1 << ISPCCDC_HORZ_INFO_SPH_SHIFT
- | ((ispccdc_obj.ccdcout_w - 1)
+ omap_writel(0 << ISPCCDC_HORZ_INFO_SPH_SHIFT
+ | (ispccdc_obj.ccdcout_w
<< ISPCCDC_HORZ_INFO_NPH_SHIFT),
ISPCCDC_HORZ_INFO);
} else {
omap_writel(0 << ISPCCDC_HORZ_INFO_SPH_SHIFT
- | ((ispccdc_obj.ccdcout_w - 1)
+ | (ispccdc_obj.ccdcout_w
<< ISPCCDC_HORZ_INFO_NPH_SHIFT),
ISPCCDC_HORZ_INFO);
}
@@ -1227,7 +1227,7 @@ int ispccdc_config_size(u32 input_w, u32 input_h, u32
output_w, u32 output_h)
ISPCCDC_VP_OUT_VERT_NUM_SHIFT),
ISPCCDC_VP_OUT);
omap_writel(0 << ISPCCDC_HORZ_INFO_SPH_SHIFT |
- ((ispccdc_obj.ccdcout_w - 1) <<
+ (ispccdc_obj.ccdcout_w <<
ISPCCDC_HORZ_INFO_NPH_SHIFT),
ISPCCDC_HORZ_INFO);
omap_writel(0 << ISPCCDC_VERT_START_SLV0_SHIFT,
diff --git a/drivers/media/video/isp/ispccdc.c
b/drivers/media/video/isp/ispccdc.c
index f5957b2..6afaabf 100644
--- a/drivers/media/video/isp/ispccdc.c
+++ b/drivers/media/video/isp/ispccdc.c
@@ -478,7 +478,7 @@ EXPORT_SYMBOL(ispccdc_enable_lsc);
**/
void ispccdc_config_crop(u32 left, u32 top, u32 height, u32 width)
{
- ispccdc_obj.ccdcin_woffset = left + ((left + 1) % 2);
+ ispccdc_obj.ccdcin_woffset = left + (left % 2);
ispccdc_obj.ccdcin_hoffset = top + (top % 2);
ispccdc_obj.crop_w = width - (width % 16);
@@ -1166,7 +1166,7 @@ int ispccdc_config_size(u32 input_w, u32 input_h,
u32 output_w, u32 output_h)
ISPCCDC_FMT_VERT);
omap_writel((ispccdc_obj.ccdcout_w <<
ISPCCDC_VP_OUT_HORZ_NUM_SHIFT) |
- (ispccdc_obj.ccdcout_h <<
+ (ispccdc_obj.ccdcout_h - 1 <<
ISPCCDC_VP_OUT_VERT_NUM_SHIFT),
ISPCCDC_VP_OUT);
omap_writelispccdc_obj.ccdcout_h - 25) &
--
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 3/5] OV3640: Add driver
On Monday 09 March 2009 23:29:27 ext Menon, Nishanth wrote: > Further, we have multiple sensors following CCI[1] - why not have a driver > for the same, it will simplify the entire process - ov3640, mt9p012 both > follow the spec at least.. dependency would be sensor -> cci dev->i2c > framework. Sakari has written smiaregs.c pretty much exactly for this purpose. You should check it out. - Tuukka -- 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: identifying camera sensor
On Wednesday 04 March 2009 16:52:20 ext Hans Verkuil wrote: > > Alternatively, VIDIOC_QUERYCAP could be used to identify the sensor. > > Would it make more sense if it would return something like > > capability.card: `omap3/smia-sensor-12-1234-5678//' > > where 12 would be manufacturer_id, 1234 model_id, and > > 5678 revision_number? > > Yuck :-) Agreed :) Also, if there are many slaves, the length of the capability.card field is not sufficient. From: Trent Piepho > You could always try to decode the manufacturer name and maybe even the > model name. After all, pretty much every other driver does this. That would be possible, but the driver would then need a device name table which would need to be modified whenever a new chip comes up :( On Wednesday 04 March 2009 16:52:20 ext Hans Verkuil wrote: > G_CHIP_IDENT is probably the way to go, provided you are aware of the > limitations of this ioctl. Should this be a problem, then we need to think > of a better solution. Could you tell me what limitations? I thought about that ioctl initially, but then read that it is going to be removed, that's why abandoned it. http://n2.nabble.com/-REVIEW--v4l2-debugging:-match-drivers-by-name-instead-of-the-deprecated-ID-td1681635.html But if you say it's a good way, then I'll go that way. The intention is to get the SMIA driver included into official kernel, so I'd prefer a method which allows that :-) - Tuukka -- 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
identifying camera sensor
Hi, I am writing a generic driver for SMIA-compatible sensors. SMIA-sensors have registers containing: u16 model_id u16 revision_number u8 manufacturer_id which could be used to detect the sensor. However, since the driver is generic, it is not interested of these values. Nevertheless, in some cases user space applications want to know the exact chip. For example, to get the highest possible image quality, user space application might capture an image and postprocess it using sensor-specific filtering algorithms (which don't belong into kernel driver). I am planning to export the chip identification information to user space using VIDIOC_DBG_G_CHIP_IDENT. Here's a sketch: #define V4L2_IDENT_SMIA_BASE (0x53 << 24) then in sensor driver's VIDIOC_DBG_G_CHIP_IDENT ioctl handler: struct v4l2_dbg_chip_ident id; id.ident = V4L2_IDENT_SMIA_BASE | (manufacturer_id << 16) | model_id; id.revision = revision_number; Do you think this is acceptable? Alternatively, VIDIOC_QUERYCAP could be used to identify the sensor. Would it make more sense if it would return something like capability.card: `omap3/smia-sensor-12-1234-5678//' where 12 would be manufacturer_id, 1234 model_id, and 5678 revision_number? I'll start writing a patch as soon as you let me know which would be the best alternative. Thanks! - Tuukka -- 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: [REVIEW PATCH 11/14] OMAP34XXCAM: Add driver
On Wednesday 04 March 2009 09:39:48 ext Hans Verkuil wrote: > BTW, do I understand correctly that e.g. lens drivers also get their > own /dev/videoX node? Please tell me I'm mistaken! Since that would be so > very wrong. You're mistaken :) With the v4l2-int-interface/omap34xxcam camera driver one device node consists of all slaves (sensor, lens, flash, ...) making up the complete camera device. > I hope that the conversion to v4l2_subdev will take place soon. You are > basically stuck in a technological dead-end :-( Ok :( - Tuukka -- 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
