double-buffering with the omap3 parallel interface
Hi Laurent & co., I'd like to look at what the maximum possible frame rates are for a sensor connected to the OMAP3 ISP CCDC via the parallel interface, writing frames directly to memory. I understand that there is some minimum amount of time required between frames to pass on the finished frame and set up the address to be written to for the next frame. From the manual it looks like a double buffering scheme would've been available on a different sensor interface, but isn't on the parallel one. Do I see that right? Is it impossible to use double buffering of any sort while using the parallel interface to memory? I'm still using an older version of the driver, but I've browsed the current state of the code, too. What behavior do you expect if the time between frames is too short for the buffer management? Can it be recovered from? Has this behavior changed in recent versions? I see from the ISP block diagram that the "circular buffer" is between the SBL and the MMU. Could this maybe be used to help the situation? It seems to currently not be used at all along this path. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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
omap3isp: timeout in ccdc_disable()
Hi Laurent, I am looking at a case where the sensor may stop delivering data, at which point I want to do a STREAMOFF. In this case, the STREAMOFF takes 2s because of the wait_event_timeout() in ccdc_disable(). This wait waits for ccdc->stopping to be CCDC_STOP_FINISHED, which happens in two stages (only 2 because LSC is always LSC_STATE_STOPPED for me), 1. in VD1 because CCDC_STOP_REQUEST has been set by ccdc_disable() 2. in VD0 after CCDC_STOP_REQUEST had happened in VD1. But because the data has already stopped coming from the sensor, when I do STREAMOFF, I'm no longer getting VD1/0, so ccdc->stopping will never become CCDC_STOP_FINISHED, and the wait_event_timeout() has to run its course of 2 seconds. This doesn't change the control flow in ccdc_disable(), except to print a warning "CCDC stop timeout!" and return -ETIMEDOUT to ccdc_set_stream(), which in turn returns that as its return value. But this value is ignored by its caller, isp_pipeline_disable(). It looks to me, then, like the only difference between timing out and not timing out is getting the warning message. omap3isp_sbl_disble() is called the same in both cases. Q: Is there another reason for the wait & timeout? Is there some functional difference between timing out and not timing out? 2 seconds sounds fairly arbitrary, are there constraints on that, or could I at least lower it to speed up STREAMOFF? I know that normally the data wouldn't stop coming from the sensor until after the CCDC is disabled, when the sensor's s_stream(0) is called. But in this case the sensor is being driven externally, and I'm trying to react to that. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: Patches submitted via linux-media ML that are at patchwork.linuxtv.org
On 08/14/2012 05:21 PM, Laurent Pinchart wrote:
Hi Mauro,
On Tuesday 14 August 2012 11:28:05 Mauro Carvalho Chehab wrote:
Em 14-08-2012 10:46, Hans Verkuil escreveu:
That would work if the others would be doing the same. Unfortunately, other
usual developers don't do that: they send all patches under discussions as
"PATCH", making really hard to track what's ready for maintainer's review
and what isn't. As not-so-frequent contributors (trivial fixes people; users
submitting their bug fix patches; first time contributors) send their patch
as "PATCH". Those patches aren't typically picked by driver maintainers, so
the task of reviewing them is, unfortunately, typically done only by me.
So if I post a [PATCH] as opposed to an [RFC PATCH], then that means that
I believe that the [PATCH] is ready for merging. If I should no longer
do that, but make a pull request instead, then that needs to be stated
very explicitly by you. Otherwise things will get very confusing.
Yes, please post them as [RFC PATCH].
Maybe the patches that are about to be sent though a pull request could
use something like [RFC FINAL] or [PATCH FINAL], but maybe doing that
would be just overkill.
I post patches that I believe to be ready to be merged as "[PATCH]", even if I
plan to push them through my tree later. "RFC" usually has a different
meaning, I understand it as a work in progress on which comments would be
appreciated.
As new developers just post patches as "[PATCH]" (probably because that's
git's default) we can't really change the meaning of that tag. We could ask
developers who maintain their own git tree to use a different tag (something
like "[PATCH FOR REVIEW]" for instance), but that won't work well if we need
to cross-post to other mailing lists that follow a different standard.
As one of the "not-so-frequent" contributors, it's obvious to me why we
(incorrectly?) use just [PATCH] for initial submissions. Partly because
it's git's default. Partly because Documentation/SubmittingPatches
describes this. The LinuxTV Wiki says to [1] ("RFC" is nowhere on this
page). There are many parts of protocol here that may just be obvious to
the regulars, but documentation-by-mailing-list is a frustrating and
error-prone way to have to glean the guidelines before submission. If
this thread leads to new agreed-upon guidelines, could someone please
update [1] to reflect whatever the consensus is? It would be
appropriate to at least mention 'git send-email' there, too.
-Michael
[1] http://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] omap3-isp G_FMT & ENUM_FMT
Hi Laurent, On 07/27/2012 11:35 AM, Laurent Pinchart wrote: On Friday 27 July 2012 11:07:50 Michael Jones wrote: Hi Laurent, On 07/27/2012 01:32 AM, Laurent Pinchart wrote: Hi Michael, [snip] OK, so this sounds like the same behavior I'd like to add before CREATE_BUFS and PREPARE_BUFS support is in. My other question was if this is the case, can we use my approach until your planned changes are in? We can't, as it would break the use case of preallocating buffers without providing any alternative solution. That's why I haven't fixed the G_FMT/S_FMT/ENUM_FMT issue yet. OK, now I understand. Thanks for clarifying. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] omap3-isp G_FMT & ENUM_FMT
Hi Laurent, On 07/27/2012 01:32 AM, Laurent Pinchart wrote: Hi Michael, On Thursday 26 July 2012 16:22:17 Michael Jones wrote: On 07/26/2012 04:05 PM, Laurent Pinchart wrote: On Thursday 26 July 2012 13:59:54 Michael Jones wrote: Hello, I would like to (re)submit a couple of patches to support V4L2 behavior at the V4L2 device nodes of the omap3-isp driver, but I'm guessing they require some discussion first. Indeed. The main reason why the OMAP3 ISP driver implements G_FMT/S_FMT as it does today is to hack around a restriction in the V4L2 API. We needed a way to preallocate and possibly prequeue buffers for snapshot, which wasn't possible in a standard-compliant way back then. The situation has since changed, and we now have the VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctls. My plan is to - port the OMAP3 ISP driver to videobuf2 - implement support for CREATE_BUFS and PREPARE_BUF - fix the G_FMT/S_FMT/ENUM_FMT behaviour What will the G_FMT/S_FMT/ENUM_FMT behavior be then? Can you contrast it with the behavior of my patches? If the behavior will be the same for user space, and your proposed changes won't be in very soon, can we use my patches until you make your changes? At the moment the driver accepts any format you give it in a S_FMT call, regardless of the format of the connected pad. The reason for that is to allow VIDIOC_REQBUFS to allocate buffers for an arbitrary size. With CREATE_BUFS and PREPARE_BUFS support, G_FMT, S_FMT and ENUM_FMT should return the format of the connected pad. OK, so this sounds like the same behavior I'd like to add before CREATE_BUFS and PREPARE_BUFS support is in. My other question was if this is the case, can we use my approach until your planned changes are in? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] omap3isp: #include videodev2.h in omap3isp.h
include/linux/omap3isp.h uses BASE_VIDIOC_PRIVATE from include/linux/videodev2.h but didn't include this file. Signed-off-by: Michael Jones --- include/linux/omap3isp.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/include/linux/omap3isp.h b/include/linux/omap3isp.h index c73a34c..e7a79db 100644 --- a/include/linux/omap3isp.h +++ b/include/linux/omap3isp.h @@ -28,6 +28,7 @@ #define OMAP3_ISP_USER_H #include +#include /* * Private IOCTLs -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] v4l2: typos
Signed-off-by: Michael Jones --- Documentation/video4linux/v4l2-framework.txt |2 +- drivers/media/video/omap3isp/ispqueue.c |2 +- drivers/media/video/omap3isp/ispresizer.c|6 +++--- drivers/media/video/v4l2-common.c|2 +- include/media/v4l2-common.h |4 ++-- include/media/v4l2-subdev.h |2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt index 1f59052..fa01440 100644 --- a/Documentation/video4linux/v4l2-framework.txt +++ b/Documentation/video4linux/v4l2-framework.txt @@ -666,7 +666,7 @@ can take a long time you may want to do your own locking for the buffer queuing ioctls. If you want still finer-grained locking then you have to set mutex_lock to NULL -and do you own locking completely. +and do your own locking completely. It is up to the driver developer to decide which method to use. However, if your driver has high-latency operations (for example, changing the exposure diff --git a/drivers/media/video/omap3isp/ispqueue.c b/drivers/media/video/omap3isp/ispqueue.c index 9bebb1e..61fc87d 100644 --- a/drivers/media/video/omap3isp/ispqueue.c +++ b/drivers/media/video/omap3isp/ispqueue.c @@ -647,7 +647,7 @@ static int isp_video_queue_alloc(struct isp_video_queue *queue, if (ret < 0) return ret; - /* Bail out of no buffers should be allocated. */ + /* Bail out if no buffers should be allocated. */ if (nbuffers == 0) return 0; diff --git a/drivers/media/video/omap3isp/ispresizer.c b/drivers/media/video/omap3isp/ispresizer.c index 14041c9..46ac6df 100644 --- a/drivers/media/video/omap3isp/ispresizer.c +++ b/drivers/media/video/omap3isp/ispresizer.c @@ -690,7 +690,7 @@ static void resizer_print_status(struct isp_res_device *res) } /* - * resizer_calc_ratios - Helper function for calculate resizer ratios + * resizer_calc_ratios - Helper function for calculating resizer ratios * @res: pointer to resizer private data structure * @input: input frame size * @output: output frame size @@ -734,7 +734,7 @@ static void resizer_print_status(struct isp_res_device *res) * value will still satisfy the original inequality, as b will disappear when * the expression will be shifted right by 8. * - * The reverted the equations thus become + * The reverted equations thus become * * - 8-phase, 4-tap mode * hrsz = ((iw - 7) * 256 + 255 - 16 - 32 * sph) / (ow - 1) @@ -759,7 +759,7 @@ static void resizer_print_status(struct isp_res_device *res) * loop', the smallest of the ratio values will be used, never exceeding the * requested input size. * - * We first clamp the output size according to the hardware capabilitie to avoid + * We first clamp the output size according to the hardware capability to avoid * auto-cropping the input more than required to satisfy the TRM equations. The * minimum output size is achieved with a scaling factor of 1024. It is thus * computed using the 7-tap equations. diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c index 1baec83..105f88c 100644 --- a/drivers/media/video/v4l2-common.c +++ b/drivers/media/video/v4l2-common.c @@ -418,7 +418,7 @@ EXPORT_SYMBOL_GPL(v4l2_i2c_tuner_addrs); #if defined(CONFIG_SPI) -/* Load a spi sub-device. */ +/* Load an spi sub-device. */ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi, const struct v4l2_subdev_ops *ops) diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h index a298ec4..4404829 100644 --- a/include/media/v4l2-common.h +++ b/include/media/v4l2-common.h @@ -133,7 +133,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev, struct i2c_adapter *adapter, struct i2c_board_info *info, const unsigned short *probe_addrs); -/* Initialize an v4l2_subdev with data from an i2c_client struct */ +/* Initialize a v4l2_subdev with data from an i2c_client struct */ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client, const struct v4l2_subdev_ops *ops); /* Return i2c client address of v4l2_subdev. */ @@ -166,7 +166,7 @@ struct spi_device; struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev, struct spi_master *master, struct spi_board_info *info); -/* Initialize an v4l2_subdev with data from an spi_device struct */ +/* Initialize a v4l2_subdev with data from an spi_device struct */ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi, const struct v4l2_subdev_ops *ops); #endif diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index c35a354..4cc1652 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -120,7 +120,7 @@ struct v4l2_subdev_io_pi
Re: [RFC] omap3-isp G_FMT & ENUM_FMT
Hi Laurent, Thanks for the reply. On 07/26/2012 04:05 PM, Laurent Pinchart wrote: Hi Michael, On Thursday 26 July 2012 13:59:54 Michael Jones wrote: Hello, I would like to (re)submit a couple of patches to support V4L2 behavior at the V4L2 device nodes of the omap3-isp driver, but I'm guessing they require some discussion first. Indeed. The main reason why the OMAP3 ISP driver implements G_FMT/S_FMT as it does today is to hack around a restriction in the V4L2 API. We needed a way to preallocate and possibly prequeue buffers for snapshot, which wasn't possible in a standard-compliant way back then. The situation has since changed, and we now have the VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctls. My plan is to - port the OMAP3 ISP driver to videobuf2 - implement support for CREATE_BUFS and PREPARE_BUF - fix the G_FMT/S_FMT/ENUM_FMT behaviour What will the G_FMT/S_FMT/ENUM_FMT behavior be then? Can you contrast it with the behavior of my patches? If the behavior will be the same for user space, and your proposed changes won't be in very soon, can we use my patches until you make your changes? MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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 2/2] [media] omap3isp: support G_FMT
This allows a V4L2 application which has no knowledge of the media
controller to open a video device node of the already-configured ISP
and query what it will deliver. Previously, G_FMT only worked after a
S_FMT had already been done.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3isp/ispvideo.c | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c
index d1d2c14..955211b 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -1244,6 +1244,7 @@ static int isp_video_open(struct file *file)
{
struct isp_video *video = video_drvdata(file);
struct isp_video_fh *handle;
+ struct media_pad *src_pad;
int ret = 0;
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
@@ -1273,6 +1274,32 @@ static int isp_video_open(struct file *file)
handle->format.type = video->type;
handle->timeperframe.denominator = 1;
+ src_pad = media_entity_remote_source(&video->pad);
+
+ if (src_pad) { /* it's on an active link */
+ struct v4l2_subdev_format srcfmt = {
+ .pad = src_pad->index,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct v4l2_subdev *src_subdev =
+ isp_video_remote_subdev(video, NULL);
+ pr_debug("%s src_subdev=\"%s\"\n", __func__, src_subdev->name);
+
+ ret = v4l2_subdev_call(src_subdev, pad, get_fmt, NULL, &srcfmt);
+ if (ret)
+ goto done;
+ pr_debug("%s MBUS format %dx%d code:%x\n", __func__,
+ srcfmt.format.width, srcfmt.format.height,
+ srcfmt.format.code);
+
+ isp_video_mbus_to_pix(video, &srcfmt.format,
+ &handle->format.fmt.pix);
+ pr_debug("%s V4L format %dx%d 4CC:%x\n", __func__,
+ handle->format.fmt.pix.width,
+ handle->format.fmt.pix.height,
+ handle->format.fmt.pix.pixelformat);
+ }
+
handle->video = video;
file->private_data = &handle->vfh;
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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/2] [media] omap3isp: implement ENUM_FMT
ENUM_FMT will not enumerate all formats that the ISP is capable of,
it will only return the format which has been previously configured
using the media controller, because this is the only format available
to a V4L2 application which is unaware of the media controller.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3isp/ispvideo.c | 23 +++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c
index b37379d..d1d2c14 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -678,6 +678,28 @@ isp_video_get_format(struct file *file, void *fh, struct
v4l2_format *format)
}
static int
+isp_video_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc
*fmtdesc)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+
+ if (fmtdesc->index)
+ return -EINVAL;
+
+ if (fmtdesc->type != video->type)
+ return -EINVAL;
+
+ fmtdesc->flags = 0;
+ fmtdesc->description[0] = 0;
+
+ mutex_lock(&video->mutex);
+ fmtdesc->pixelformat = vfh->format.fmt.pix.pixelformat;
+ mutex_unlock(&video->mutex);
+
+ return 0;
+}
+
+static int
isp_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
{
struct isp_video_fh *vfh = to_isp_video_fh(fh);
@@ -1191,6 +1213,7 @@ isp_video_s_input(struct file *file, void *fh, unsigned
int input)
static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
.vidioc_querycap= isp_video_querycap,
+ .vidioc_enum_fmt_vid_cap= isp_video_enum_format,
.vidioc_g_fmt_vid_cap = isp_video_get_format,
.vidioc_s_fmt_vid_cap = isp_video_set_format,
.vidioc_try_fmt_vid_cap = isp_video_try_format,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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
[RFC] omap3-isp G_FMT & ENUM_FMT
Hello, I would like to (re)submit a couple of patches to support V4L2 behavior at the V4L2 device nodes of the omap3-isp driver, but I'm guessing they require some discussion first. I've previously submitted one of them here [1] to support ENUM_FMT for the omap3-isp. This sparked some discussion, the result of which was that my patch probably made sense. Later [2], Laurent mentioned that there was some discussion/decision about adding "profiles" to the V4L2 specification, and the OMAP3 ISP would probably implement the "streaming" profile. That was 7 months ago and I haven't seen any discussion of such profiles. Can somebody bring me up to speed on this? The purpose of these two patches is for the V4L2 device nodes to support mandatory V4L2 ioctls G_FMT and ENUM_FMT, such that a pure V4L2 application, ignorant of the media controller, can still stream the images from the video nodes. This presumes that the media controller would have been pre-configured. This approach is often seen on the mailing list using 'media-ctl' to configure the ISP, then 'yavta' to retrieve images. Currently this works because yavta doesn't require ENUM_FMT (unlike Gstreamer), and only as long as one sets the same format with yavta as had already been set with media-ctl. I think yavta should be able to just do G_FMT to see what is configured. To be clear (as discussed in [1]), ENUM_FMT does not behave according to its original intent, because it cannot enumerate possible formats the ISP can deliver. It will enumerate only one format: the one configured with the media controller. In a sense this complies with the specification, because S_FMT wouldn't be able to change the format to anything else. I have tested these patches on v3.4, but I have rebased them to v3.5 here. I would remove the pr_debug() calls before submitting upstream, but they're useful for testing. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg29640.html [2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg40618.html Michael Jones (2): [media] omap3isp: implement ENUM_FMT [media] omap3isp: support G_FMT drivers/media/video/omap3isp/ispvideo.c | 50 +++ 1 files changed, 50 insertions(+), 0 deletions(-) -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] Documentation: DocBook DRM framework documentation
Hi Laurent, I've used "behavior" when copying sections from the existing documentation. I'll unify that. Does kernel documentation favour one of the spellings ? Looking at v3.5, the American spelling is more common, but looking at how you spell favour, I think I know which one you favor :) linux-git$ grep -ri behaviour Documentation/ | wc -l 150 linux-git$ grep -ri behavior Documentation/ | wc -l 269 -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] Documentation: DocBook DRM framework documentation
Hi Laurent, At a quick glance I noticed a couple of things: On 07/13/2012 02:00 AM, Laurent Pinchart wrote: [snip] + + The drm_driver structure contains static + information that describe the driver and features it supports, and s/describe/describes/ + pointers to methods that the DRM core will call to implement the DRM API. + We will first go through the drm_driver static + information fields, and will then describe individual operations in + details as they get used in later sections. - - Driver private & performance counters - - The driver private hangs off the main drm_device structure and - can be used for tracking various device-specific bits of - information, like register offsets, command buffer status, - register state for suspend/resume, etc. At load time, a - driver may simply allocate one and set drm_device.dev_priv - appropriately; it should be freed and drm_device.dev_priv set - to NULL when the driver is unloaded. - + Driver Information + +Driver Features + + Drivers inform the DRM core about their requirements and supported + features by setting appropriate flags in the + driver_features field. Since those flags + influence the DRM core behaviour since registration time, most of them Elsewhere you use the American spelling "behavior". [snip] + +Major, Minor and Patchlevel +int major; + int minor; + int patchlevel; In my browser, "int minor" and "int patchlevel" look indented, whereas "int major" does not. Looks like they _should_ be indented identically. Don't know how you fix this or if you even see the same problem. + + The DRM core identifies driver versions by a major, minor and patch + level triplet. The information is printed to the kernel log at + initialization time and passed to userspace through the + DRM_IOCTL_VERSION ioctl. + + + The major and minor numbers are also used to verify the requested driver + API version passed to DRM_IOCTL_SET_VERSION. When the driver API changes + between minor versions, applications can call DRM_IOCTL_SET_VERSION to + select a specific version of the API. If the requested major isn't equal + to the driver major, or the requested minor is larger than the driver + minor, the DRM_IOCTL_SET_VERSION call will return an error. Otherwise + the driver's set_version() method will be called with the requested + version. + + + +Name, Description and Date +char *name; + char *desc; + char *date; Same indentation issue here. [snip] + +The mode_fixup operation should reject the +mode if it can't reasonably use it. The definition of "reasonable" +is currently fuzzy in this context. One possible behaviour would be maybe s/behaviour/behavior/ again MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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
capture_mem limitations in OMAP ISP
Hi Laurent & co., I'm looking at the memory limitations in the omap3isp driver. 'struct isp_video' contains member 'capture_mem', which is set separately for each of our v4l2 video device nodes. The CCDC, for example, has capture_mem = 4096 * 4096 * 3 = 48MB, while the previewer and resizer each have twice that. Where do these numbers come from? Is the CCDC incapable of DMA'ing more than 48MB into memory? I know that ISP_VIDEO_MAX_BUFFERS also limits the # of buffers, but I assume this is basically an arbitrary number so we can have a finite array of isp_video_buffer's. The 48MB, on the other hand, looks like it might have a good reason. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] [media] omap3isp: fix dqbuf description comment
Signed-off-by: Michael Jones --- drivers/media/video/omap3isp/ispqueue.c |9 ++--- 1 files changed, 2 insertions(+), 7 deletions(-) This comment looks like it was a copy-paste from the description of qbuf. diff --git a/drivers/media/video/omap3isp/ispqueue.c b/drivers/media/video/omap3isp/ispqueue.c index 5fda5d0..23915ce 100644 --- a/drivers/media/video/omap3isp/ispqueue.c +++ b/drivers/media/video/omap3isp/ispqueue.c @@ -908,13 +908,8 @@ done: * * This function is intended to be used as a VIDIOC_DQBUF ioctl handler. * - * The v4l2_buffer structure passed from userspace is first sanity tested. If - * sane, the buffer is then processed and added to the main queue and, if the - * queue is streaming, to the IRQ queue. - * - * Before being enqueued, USERPTR buffers are checked for address changes. If - * the buffer has a different userspace address, the old memory area is unlocked - * and the new memory area is locked. + * if nonblocking=1, returns -EAGAIN if no buffer is available. + * if nonblocking=0, waits on IRQ queue until a buffer becomes available. */ int omap3isp_video_queue_dqbuf(struct isp_video_queue *queue, struct v4l2_buffer *vbuf, int nonblocking) -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: reading config parameters of omap3-isp subdevs
Hi Laurent, On 03/20/2012 12:22 AM, Laurent Pinchart wrote: Hi Michael, On Friday 16 March 2012 15:06:15 Michael Jones wrote: [snip] Adding a R/W bit to the flag argument should indeed work. However, I'm wondering what your use case for reading parameters back is. The simplest use case in my mind is that after the user has fiddled around with config parameters, they should be able to set them back to their original state. For that, they need to know what the original state was. > The preview engine parameter structures seem pretty-much self-contained to me, I'm not sure it would make sense to only modify one of the parameters. Why doesn't it make sense to write to only e.g. 'COEF3' in the PRV_WBGAIN register? Especially considering the sparse documentation of many of these registers, I would like to be able to tweak the existing parameters from their defaults, rather than start from scratch. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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
reading config parameters of omap3-isp subdevs
Hi all, I am playing around with some parameters in the previewer on the ISP. With ioctl VIDIOC_OMAP3ISP_PRV_CFG I am able to write the various parameters but what I'm missing is a way to read them. For example, I have no way to adjust only coef2 in 'struct omap3isp_prev_wbal' while leaving the others unchanged. If I could first read the whole omap3isp_prev_wbal structure, then I could change just the things I want to change. This seems like it would be common functionality for such ioctls. I didn't find any previous discussion related to this. I could imagine either adding a r/w flag to 'struct omap3isp_prev_update_config' or adding a new ioctl entirely. I think I would prefer the r/w flag. Feedback? I noticed that other ISP subdevs have similar ioctls. Perhaps a similar thing would be useful there, but right now I'm only looking at the previewer. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: Lockup on second streamon with omap3-isp
Hi Laurent, On 03/09/2012 11:42 AM, Laurent Pinchart wrote: Hi Jean-Philippe, [snip] From my experience, the ISP doesn't handle free-running sensors very well. There are other things it doesn't handle well, such as sensors stopping in the middle of the frame. I would consider this as limitations. Considering choking on sensors which stop in the middle of the frame- is this just a limitation of the driver, or is it really a limitation of the ISP hardware itself? It is at least a limitation of the driver because we rely on the VD1 and VD0 interrupts, so we'll of course have problems if we never get to the last line. But isn't it conceivable to use HS_VS to do our end-of-frame stuff instead of VD0? Maybe then the ISP would be OK with frames that ended early, as long as they had reached VD1. Then of course, you could move VD1 to an even earlier line, even to the first line. Do you think that's possible? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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 v5 09/35] v4l: Add subdev selections documentation
Hi Sakari, Hopefully it's not too late to make a few minor suggestions. On 03/06/2012 05:32 PM, Sakari Ailus wrote: Add documentation for V4L2 subdev selection API. This changes also experimental V4L2 subdev API so that scaling now works through selection API only. Signed-off-by: Sakari Ailus [snip] + +On sink pads, cropping is applied relatively to the s/relatively/relative/ + current pad format. The pad format represents the image size as + received by the sub-device from the previous block in the + pipeline, and the crop rectangle represents the sub-image that + will be transmitted further inside the sub-device for + processing. [snip] +On source pads, cropping is similar to sink pads, with the + exception that the source size from which the cropping is + performed, is the COMPOSE rectangle on the sink pad. In both + sink and source pads, the crop rectangle must be entirely + containted inside the source image size for the crop s/containted/contained/ + operation. + +The drivers should always use the closest possible + rectangle the user requests on all selection targets, unless + specificly told otherwise. s/specificly/specifically/ +V4L2_SUBDEV_SEL_FLAG_SIZE_GE and +V4L2_SUBDEV_SEL_FLAG_SIZE_LE flags may be + used to round the image size either up or down. + [snip] +V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG flag. This + flag causes that no propagation of the changes are allowed in + any circumstances. This may also cause the accessed rectangle to "This flag causes that" sounds ungrammatical. I suggest: "This flag causes no propagation of the changes to be allowed under any circumstances." [snip] Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: Why is the Y12 support 12-bit grey formats at the CCDC input (Y12) is truncated to Y10 at the CCDC output?
Hi James, On 12/15/2011 10:49 AM, James wrote: Hi Michael, On Thu, Dec 15, 2011 at 3:58 PM, Michael Jones wrote: Hi James, On 12/15/2011 08:14 AM, James wrote: Hi all, I'm using an OMAP3530 board and a monochrome 12-bit grey sensor. Can anyone enlighten me why is the 12-bit grey formats at the CCDC input (Y12) is truncated to Y10 at the CCDC output? There are 2 CCDC outputs: CCDC_PAD_SOURCE_OF and CCDC_PAD_SOURCE_VP. Only the VP (video port) truncates data to 10 bits, and it does that because the subdevs it feeds can only handle 10 bits max. Thank you for the clarification. I need to read the entire RAW 12-bit grey value from the CCDC to memory and the data does not pass through other OMAP3ISP sub-devices. I intend to use Laurent's yavta to capture the data to file to verify its operation for the moment. Can this 12-bit (Y12) raw capture be done? Yes. If you are writing the 12-bit gray value directly into memory, you will use SOURCE_OF and can write the full 12-bits into memory. You need to set up your media pipeline to do sensor->CCDC->OMAP3 ISP CCDC output. Is there further modification needed to apply to the OMAP3ISP to achieve this? Do you have an application to test the pipeline for this setting to simple display? Let's establish where you're coming from. Are you familiar with the media controller? Laurent has a program 'media-ctl' to set up the pipeline (see http://git.ideasonboard.org/?p=media-ctl.git). You will find many examples of its usage in the archives of this mailing list. It will look something like: media-ctl -r media-ctl -l '"OMAP3 ISP CCDC":1 -> "OMAP3 ISP CCDC output":0 [1]' media-ctl -l '"your-sensor-name":0 -> "OMAP3 ISP CCDC":0 [1]' you will also need to set the formats through the pipeline with 'media-ctl --set-format'. After you use media-ctl to set up the pipeline, you can use yavta to capture the data from the CCDC output (for me, this is /dev/video2). -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: Why is the Y12 support 12-bit grey formats at the CCDC input (Y12) is truncated to Y10 at the CCDC output?
Hi James, On 12/15/2011 08:14 AM, James wrote: Hi all, I'm using an OMAP3530 board and a monochrome 12-bit grey sensor. Can anyone enlighten me why is the 12-bit grey formats at the CCDC input (Y12) is truncated to Y10 at the CCDC output? There are 2 CCDC outputs: CCDC_PAD_SOURCE_OF and CCDC_PAD_SOURCE_VP. Only the VP (video port) truncates data to 10 bits, and it does that because the subdevs it feeds can only handle 10 bits max. I need to read the entire RAW 12-bit grey value from the CCDC to memory and the data does not pass through other OMAP3ISP sub-devices. I intend to use Laurent's yavta to capture the data to file to verify its operation for the moment. Can this 12-bit (Y12) raw capture be done? Yes. If you are writing the 12-bit gray value directly into memory, you will use SOURCE_OF and can write the full 12-bits into memory. You need to set up your media pipeline to do sensor->CCDC->OMAP3 ISP CCDC output. Thank you in adv. -- Regards, James -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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 + Gstreamer v4l2src
Hi Adam, On 12/07/2011 11:34 AM, Laurent Pinchart wrote: Hi Adam, On Wednesday 07 December 2011 09:02:42 Adam Pledger wrote: Hi Laurent, Firstly, please accept my apologies, for what is very probably a naive question. I'm new to V4L2 and am just getting to grips with how things work. No worries. I'm using a tvp5151 in bt656 mode with the Omap3 ISP, Please note that BT.656 support is still experimental, so issues are not unexpected. as described in this thread (Your YUV support tree + some patches for bt656, based on 2.6.39): http://comments.gmane.org/gmane.linux.drivers.video-input- infrastructure/39539 I am able to capture some frames using yavta, using the media-ctl configuration as follows: media-ctl -v -r -l '"tvp5150 3-005d":0->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP CCDC":1->"OMAP3 ISP CCDC output":0[1]' media-ctl -v --set-format '"tvp5150 3-005d":0 [UYVY2X8 720x625]' media-ctl -v --set-format '"OMAP3 ISP CCDC":0 [UYVY2X8 720x625]' media-ctl -v --set-format '"OMAP3 ISP CCDC":1 [UYVY2X8 720x625]' This yields this: [snip] Looks good. The following works nicely: yavta -f UYVY -s 720x625 -n 4 --capture=4 -F /dev/video2 The problem comes when I try to use gstreamer to capture from /dev/video2, using the following: gst-launch v4l2src device="/dev/video2" ! 'video/x-raw-yuv,width=720,height=625' ! filesink location=sample.yuv This fails with: ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed getting controls attributes on device '/dev/video2'. Additional debug info: v4l2_calls.c(267): gst_v4l2_fill_lists (): /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed querying control 9963776 on device '/dev/video2'. (25 - Inappropriate ioctl for device) My question is, should this "just work"? It was my understanding that once the pipeline was configured with media-ctl then the CCDC output pad should behave like a standard V4L2 device node. That's more or less correct. There have been a passionate debate regarding what a "standard V4L2 device node" is. Not all V4L2 ioctls are mandatory, and no driver implements them all. The OMAP3 ISP driver implements a very small subset of the V4L2 API, and it wasn't clear whether that still qualified as V4L2. After discussions we decided that the V4L2 specification will document profiles, with a set of required ioctls for each of them. The OMAP3 ISP implements the future video streaming profile. I'm not sure what ioctls v4l2src consider as mandatory. The above error related to a CTRL ioctl (possibly VIDIOC_QUERYCTRL), which isn't implemented by the OMAP3 ISP driver and will likely never be. I don't think that should be considered as mandatory. I think that v4l2src requires the VIDIOC_ENUMFMT ioctl, which isn't implemented in the OMAP3 ISP driver. That might change in the future, but I'm not sure yet whether it will. In any case, you might have to modify v4l2src and/or the OMAP3 ISP driver for now. Some patches have been posted a while ago to this mailing list. Here was my submission for ENUM_FMT support: http://www.mail-archive.com/linux-media@vger.kernel.org/msg29640.html I submitted this in order to be able to use the omap3-isp with GStreamer. I missed the discussion about V4L2 "profiles", but when I submitted that patch we discussed whether ENUM_FMT was mandatory. After I pointed out that the V4L2 spec states plainly that it _is_ mandatory, I thought Laurent basically agreed that it was reasonable. Laurent, what do you think about adding ENUM_FMT support now? I realise that this might be something borked with my build dependencies (although I'm pretty certain that v4l2src is being built against the latest libv42) or gstreamer. Before I start digging through the code to work out what is going on with the ioctl handling, I thought I would check to see whether this should work, or whether I am doing something fundamentally silly. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: [beagleboard] Re: [PATCH v7 1/2] Add driver for Aptina (Micron) mt9p031 sensor.
Hi Chris, On 08/22/2011 11:41 AM, Laurent Pinchart wrote: > > Hi Chris, > > On Monday 22 August 2011 06:12:41 CJ wrote: >> On 19/08/11 22:12, Laurent Pinchart wrote: I am trying to get the mt9p031 working from nand with a ubifs file system and I am having a few problems. /dev/media0 is not present unless I run: #mknod /dev/media0 c 251 0 #chown root:video /dev/media0 #media-ctl -p Enumerating entities media_open: Unable to enumerate entities for device /dev/media0 (Inappropriate ioctl for device) With the same rig/files it works fine running from EXT4 on an SD card. Any idea why this does not work on nand with ubifs? >>> >>> Is the OMAP3 ISP driver loaded ? Has it probed the device successfully ? >>> Check the kernel log for OMAP3 ISP-related messages. >> >> Here is the version running from SD card: >> # dmesg | grep isp >> [0.265502] omap-iommu omap-iommu.0: isp registered >> [2.986541] omap3isp omap3isp: Revision 2.0 found >> [2.991577] omap-iommu omap-iommu.0: isp: version 1.1 >> [2.997406] omap3isp omap3isp: hist: DMA channel = 0 >> [3.006256] omap3isp omap3isp: isp_set_xclk(): cam_xclka set to >> 2160 Hz >> [3.011932] omap3isp omap3isp: isp_set_xclk(): cam_xclka set to 0 Hz >> >> From NAND using UBIFS: >> # dmesg | grep isp >> [3.457061] omap3isp omap3isp: Revision 2.0 found >> [3.462036] omap-iommu omap-iommu.0: isp: version 1.1 >> [3.467620] omap3isp omap3isp: hist: DMA channel = 0 >> [3.472564] omap3isp omap3isp: isp_set_xclk(): cam_xclka set to >> 2160 Hz >> [3.478027] omap3isp omap3isp: isp_set_xclk(): cam_xclka set to 0 Hz >> >> Seems to be missing: >> omap-iommu omap-iommu.0: isp registered >> >> Is that the issue? Why would this not work when running from NAND? I'm not sure, either, but I had a similar problem before using Laurent's patch below. IIRC, usually udev would create /dev/media0 from a cached list of /dev/*. Later modutils would come along and load the modules in the proper order (iommu, then omap3-isp) and everybody was happy. Occasionally, udev would fail to use the cached version of /dev/, and look through /sys/devices to re-create the devices in /dev/. When media0 was found, omap3-isp.ko would be loaded, but iommu had not yet been, presumably because it doesn't have an entry in /sys/devices/. So maybe udev is behaving differently for you on NAND than it did on the card? Either way, as I said, using Laurent's patch below did the job for me. -Michael > > I'm not sure why it doesn't work from NAND, but the iommu2 module needs to be > loaded before the omap3-isp module. Alternatively you can compile the iommu2 > module in the kernel with > > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig > index 49a4c75..3c87644 100644 > --- a/arch/arm/plat-omap/Kconfig > +++ b/arch/arm/plat-omap/Kconfig > @@ -132,7 +132,7 @@ config OMAP_MBOX_KFIFO_SIZE > module parameter). > > config OMAP_IOMMU > - tristate > + bool > > config OMAP_IOMMU_DEBUG > tristate "Export OMAP IOMMU internals in DebugFS" > MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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
omap3isp buffer alignment
Hi Laurent, If I understood your discussion with Russell [1] correctly, user pointer buffers are required to be page-aligned because of the IOMMU API, and it's desirable to keep the IOMMU driver that way for other subsystems which may use it. So we're stuck with user buffers needing to be page-aligned. There's a check in ispvideo.c:isp_video_buffer_prepare() that the buffer address is 32-byte aligned. Isn't this superfluous considering the page-aligned restriction? -Michael [1] http://www.mail-archive.com/linux-omap%40vger.kernel.org/msg50611.html MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] [media] omap3isp: queue: fail QBUF if user buffer is too small
Add buffer length check to sanity checks for USERPTR QBUF
Signed-off-by: Michael Jones
---
Changes for v2:
- only check when V4L2_MEMORY_USERPTR
drivers/media/video/omap3isp/ispqueue.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispqueue.c
b/drivers/media/video/omap3isp/ispqueue.c
index 9c31714..9bebb1e 100644
--- a/drivers/media/video/omap3isp/ispqueue.c
+++ b/drivers/media/video/omap3isp/ispqueue.c
@@ -868,6 +868,10 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue
*queue,
goto done;
if (vbuf->memory == V4L2_MEMORY_USERPTR &&
+ vbuf->length < buf->vbuf.length)
+ goto done;
+
+ if (vbuf->memory == V4L2_MEMORY_USERPTR &&
vbuf->m.userptr != buf->vbuf.m.userptr) {
isp_video_buffer_cleanup(buf);
buf->vbuf.m.userptr = vbuf->m.userptr;
--
1.7.6
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] [media] omap3isp: queue: fail QBUF if buffer is too small
Hi Laurent,
On 08/08/2011 12:08 PM, Laurent Pinchart wrote:
>
> Hi Michael,
>
> On Friday 05 August 2011 13:41:54 Michael Jones wrote:
>> On 08/05/2011 10:59 AM, Laurent Pinchart wrote:
>>> Hi Michael,
>>>
>>> Thanks for the patch.
>>>
>>> On Thursday 04 August 2011 17:40:37 Michael Jones wrote:
>>>> Add buffer length to sanity checks for QBUF.
>>>>
>>>> Signed-off-by: Michael Jones
>>>> ---
>>>>
>>>> drivers/media/video/omap3isp/ispqueue.c |3 +++
>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/media/video/omap3isp/ispqueue.c
>>>> b/drivers/media/video/omap3isp/ispqueue.c index 9c31714..4f6876f 100644
>>>> --- a/drivers/media/video/omap3isp/ispqueue.c
>>>> +++ b/drivers/media/video/omap3isp/ispqueue.c
>>>> @@ -867,6 +867,9 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue
>>>> *queue, if (buf->state != ISP_BUF_STATE_IDLE)
>>>>
>>>>goto done;
>>>>
>>>> + if (vbuf->length < buf->vbuf.length)
>>>> + goto done;
>>>> +
>>>
>>> The vbuf->length value passed from userspace isn't used by the driver, so
>>> I'm not sure if verifying it is really useful. We verify the memory
>>> itself instead, to make sure that enough pages can be accessed. The
>>> application can always lie about the length, so we can't rely on it
>>> anyway.
>>
>> According to the spec, it's expected that the application set 'length':
>> "To enqueue a user pointer buffer applications set [...] length to its
>> size." (Now that I say that, I realize I should only do this length
>> check for USERPTR buffers.) If we don't at least sanity check it for the
>> application, then it has no purpose at all on QBUF. If this is
>> desirable, I would propose changing the spec.
>>
>> This patch was born of a mistake when my application set 624x480, which
>> resulted in sizeimage=640x480=307200 but it used width & height to
>> calculate the buffer size rather than sizeimage or even to take
>> bytesperline into account. It was then honest with QBUF, confessing that
>> it wasn't providing enough space, but QBUF just went ahead. What
>> followed were random crashes while data was DMA'd into memory not set
>> aside for the buffer, while I assumed that the buffer size was OK
>> because QBUF had succeeded and was looking elsewhere in the program for
>> the culprit. I think it makes sense to give the app an error on QBUF in
>> this situation.
>
> Right. This will help catching application errors without any drawback on the
> kernel side.
>
> Do you want to resubmit the patch with an additional USERPTR check, or should
> I write one ?
Thanks for the review. I will resubmit the patch with the USERPTR check.
>
>>>>if (vbuf->memory == V4L2_MEMORY_USERPTR &&
>>>>vbuf->m.userptr != buf->vbuf.m.userptr) {
>>>>isp_video_buffer_cleanup(buf);
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] [media] omap3isp: queue: fail QBUF if buffer is too small
Hi Laurent,
On 08/05/2011 10:59 AM, Laurent Pinchart wrote:
>
> Hi Michael,
>
> Thanks for the patch.
>
> On Thursday 04 August 2011 17:40:37 Michael Jones wrote:
>> Add buffer length to sanity checks for QBUF.
>>
>> Signed-off-by: Michael Jones
>> ---
>> drivers/media/video/omap3isp/ispqueue.c |3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/video/omap3isp/ispqueue.c
>> b/drivers/media/video/omap3isp/ispqueue.c index 9c31714..4f6876f 100644
>> --- a/drivers/media/video/omap3isp/ispqueue.c
>> +++ b/drivers/media/video/omap3isp/ispqueue.c
>> @@ -867,6 +867,9 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue
>> *queue, if (buf->state != ISP_BUF_STATE_IDLE)
>> goto done;
>>
>> +if (vbuf->length < buf->vbuf.length)
>> +goto done;
>> +
>
> The vbuf->length value passed from userspace isn't used by the driver, so I'm
> not sure if verifying it is really useful. We verify the memory itself
> instead, to make sure that enough pages can be accessed. The application can
> always lie about the length, so we can't rely on it anyway.
According to the spec, it's expected that the application set 'length':
"To enqueue a user pointer buffer applications set [...] length to its
size." (Now that I say that, I realize I should only do this length
check for USERPTR buffers.) If we don't at least sanity check it for the
application, then it has no purpose at all on QBUF. If this is
desirable, I would propose changing the spec.
This patch was born of a mistake when my application set 624x480, which
resulted in sizeimage=640x480=307200 but it used width & height to
calculate the buffer size rather than sizeimage or even to take
bytesperline into account. It was then honest with QBUF, confessing that
it wasn't providing enough space, but QBUF just went ahead. What
followed were random crashes while data was DMA'd into memory not set
aside for the buffer, while I assumed that the buffer size was OK
because QBUF had succeeded and was looking elsewhere in the program for
the culprit. I think it makes sense to give the app an error on QBUF in
this situation.
>
>> if (vbuf->memory == V4L2_MEMORY_USERPTR &&
>> vbuf->m.userptr != buf->vbuf.m.userptr) {
>> isp_video_buffer_cleanup(buf);
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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 CCDC freeze-up on STREAMON
Hi Laurent, On 07/20/2011 10:47 AM, Laurent Pinchart wrote: > > Hi Michael, > > Sorry for the late reply. Likewise :) > > On Thursday 30 June 2011 10:31:52 Michael Jones wrote: >> Hi Laurent, >> >> I'm observing a system freeze-up with the ISP when writing data to memory >> directly from the ccdc. >> >> Here's the sequence I'm using: >> >> 0. apply the patch I'm sending separate in this thread. >> >> 1. configure the ISP pipeline for the CCDC to deliver V4L2_PIX_FMT_GREY >> directly from the sensor to memory. >> >> 2. yavta -c10 /dev/video2 >> >> The patch is pretty self-explanatory. It introduces a loop (with ugly >> indenting to keep the patch simple) with 100 iterations leaving the device >> open between them. My system usually hangs up within the first 30 >> iterations. I've never made it to 100 successfully. I see the same >> behavior with user pointers and with mmap, but I don't see it when using >> data from the previewer. >> >> Can you please try this out with your setup? Even if you can't get 8-bit >> gray data from your sensor, hopefully you could observe it with any other >> format directly from the CCDC. >> >> I'll postpone further discussion until you confirm that you can reproduce >> the behavior. As the patch illustrates, it looks like it is hanging up in >> STREAMON. > > I've tested this with a serial CSI-2 sensor and a parallel sensor (MT9V032, > in > both 8-bit and 10-bit modes, albeit with SGRBG8 instead of GREY for the 8-bit > mode), and I can't reproduce the issue. > > I thought I've asked you already but can't find this in my mailbox, so I > apologize if I have, but could you try increasing vertical blanking and see > if > it helps ? > I think that was the first time you suggested that. Indeed, if I stretch out the time between frames, the problem goes away. I haven't tested it precisely to see how long it needs to be to work correctly. But what does this tell me? This isn't a very appealing fix as 1) I would have to fish around for a minimum vertical blank time that works and 2) this would slow down the frame rate for the normal case, when frames are just being streamed uninterrupted. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] [media] omap3isp: queue: fail QBUF if buffer is too small
Add buffer length to sanity checks for QBUF.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3isp/ispqueue.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispqueue.c
b/drivers/media/video/omap3isp/ispqueue.c
index 9c31714..4f6876f 100644
--- a/drivers/media/video/omap3isp/ispqueue.c
+++ b/drivers/media/video/omap3isp/ispqueue.c
@@ -867,6 +867,9 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue *queue,
if (buf->state != ISP_BUF_STATE_IDLE)
goto done;
+ if (vbuf->length < buf->vbuf.length)
+ goto done;
+
if (vbuf->memory == V4L2_MEMORY_USERPTR &&
vbuf->m.userptr != buf->vbuf.m.userptr) {
isp_video_buffer_cleanup(buf);
--
1.7.6
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] capture-example: don't use bytesperline when allocating buffers
This removes "buggy driver paranoia", which set sizeimage equal to
at least width * height * 2. This was a false assumption when the
pixel format only required 1 byte per pixel. Originally, the
paranoia was in place to handle drivers which incorrectly set
sizeimage=0, but these seem to have been fixed.
Signed-off-by: Michael Jones
---
contrib/test/capture-example.c |8
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/contrib/test/capture-example.c b/contrib/test/capture-example.c
index 2f77cbf..417615a 100644
--- a/contrib/test/capture-example.c
+++ b/contrib/test/capture-example.c
@@ -498,14 +498,6 @@ static void init_device(void)
errno_exit("VIDIOC_G_FMT");
}
- /* Buggy driver paranoia. */
- min = fmt.fmt.pix.width * 2;
- if (fmt.fmt.pix.bytesperline < min)
- fmt.fmt.pix.bytesperline = min;
- min = fmt.fmt.pix.bytesperline * fmt.fmt.pix.height;
- if (fmt.fmt.pix.sizeimage < min)
- fmt.fmt.pix.sizeimage = min;
-
switch (io) {
case IO_METHOD_READ:
init_read(fmt.fmt.pix.sizeimage);
--
1.7.5.4
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] capture-example: allow V4L2_PIX_FMT_GREY with USERPTR
Hi Mauro,
On 07/14/2011 12:35 AM, Mauro Carvalho Chehab wrote:
> Em 28-06-2011 11:23, Michael Jones escreveu:
>> There is an assumption that the format coming from the device
>> needs 2 bytes per pixel, which is not the case when the device
>> delivers e.g. V4L2_PIX_FMT_GREY. This doesn't manifest itself with
>> IO_METHOD_MMAP because init_mmap() (the default) doesn't take
>> sizeimage as an argument.
>>
>> Signed-off-by: Michael Jones
>> ---
>>
>> This same issue would apply to other formats which have 1 byte per pixel,
>> this patch only fixes it for GREY. Is this OK for now, or does somebody
>> have a better suggestion for supporting other formats as well?
>
> Well, just rely on the bytesperline provided by the driver should be enough.
> Devices should be returning it on a consistent way.
>
> Regards,
> Mauro
So you would rather remove the "Buggy driver paranoia" altogether and
just trust the bytesperline from the driver? That's fine with me, but I
presumed the paranoia was there for a reason. Would you accept a patch
then that just removes the 7 lines which fiddle with bytesperline?
-Michael
>
>>
>> contrib/test/capture-example.c |4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/test/capture-example.c b/contrib/test/capture-example.c
>> index 3852c58..0eb5235 100644
>> --- a/contrib/test/capture-example.c
>> +++ b/contrib/test/capture-example.c
>> @@ -416,6 +416,7 @@ static void init_device(void)
>> struct v4l2_crop crop;
>> struct v4l2_format fmt;
>> unsigned int min;
>> +unsigned int bytes_per_pixel;
>>
>> if (-1 == xioctl(fd, VIDIOC_QUERYCAP, &cap)) {
>> if (EINVAL == errno) {
>> @@ -519,7 +520,8 @@ static void init_device(void)
>> }
>>
>> /* Buggy driver paranoia. */
>> -min = fmt.fmt.pix.width * 2;
>> +bytes_per_pixel = fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_GREY ? 1 : 2;
>> +min = fmt.fmt.pix.width * bytes_per_pixel;
>> if (fmt.fmt.pix.bytesperline < min)
>> fmt.fmt.pix.bytesperline = min;
>> min = fmt.fmt.pix.bytesperline * fmt.fmt.pix.height;
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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] v4l: Don't register media entities for subdev device nodes
On 04/11/2011 04:26 PM, Laurent Pinchart wrote:
> Subdevs already have their own entity, don't register as second one when
> registering the subdev device node.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/media/video/v4l2-dev.c | 15 ++-
> 1 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 498e674..6dc7196 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -389,7 +389,8 @@ static int v4l2_open(struct inode *inode, struct file
> *filp)
> video_get(vdev);
> mutex_unlock(&videodev_lock);
> #if defined(CONFIG_MEDIA_CONTROLLER)
> - if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev &&
> + vdev->vfl_type != VFL_TYPE_SUBDEV) {
> entity = media_entity_get(&vdev->entity);
> if (!entity) {
> ret = -EBUSY;
> @@ -415,7 +416,8 @@ err:
> /* decrease the refcount in case of an error */
> if (ret) {
> #if defined(CONFIG_MEDIA_CONTROLLER)
> - if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev &&
> + vdev->vfl_type != VFL_TYPE_SUBDEV)
> media_entity_put(entity);
> #endif
> video_put(vdev);
> @@ -437,7 +439,8 @@ static int v4l2_release(struct inode *inode, struct file
> *filp)
> mutex_unlock(vdev->lock);
> }
> #if defined(CONFIG_MEDIA_CONTROLLER)
> - if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev &&
> + vdev->vfl_type != VFL_TYPE_SUBDEV)
> media_entity_put(&vdev->entity);
> #endif
> /* decrease the refcount unconditionally since the release()
> @@ -686,7 +689,8 @@ int __video_register_device(struct video_device *vdev,
> int type, int nr,
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> /* Part 5: Register the entity. */
> - if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev &&
> + vdev->vfl_type != VFL_TYPE_SUBDEV) {
> vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
> vdev->entity.name = vdev->name;
> vdev->entity.v4l.major = VIDEO_MAJOR;
> @@ -733,7 +737,8 @@ void video_unregister_device(struct video_device *vdev)
> return;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> - if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
> + if (vdev->v4l2_dev && vdev->v4l2_dev->mdev &&
> + vdev->vfl_type != VFL_TYPE_SUBDEV)
> media_device_unregister_entity(&vdev->entity);
> #endif
>
Hi Laurent,
If v4l2_subdev has a 'struct media_entity' inside of its 'struct
video_device' member, why does it need a media_entity of its own?
Shouldn't we eliminate v4l2_subdev.entity and always just use
v4l2_subdev.devnode.entity where it is needed? Or do they have 2
different purposes?
-Michael
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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
prompt ISP CCDC freeze-up on STREAMON
---
yavta.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/yavta.c b/yavta.c
index 2a166c6..95976b4 100644
--- a/yavta.c
+++ b/yavta.c
@@ -485,7 +485,9 @@ static int video_enable(struct device *dev, int enable)
int type = dev->type;
int ret;
+ printf("+%s\n", enable ? "STREAMON" : "STREAMOFF");
ret = ioctl(dev->fd, enable ? VIDIOC_STREAMON : VIDIOC_STREAMOFF,
&type);
+ printf("-%s\n", enable ? "STREAMON" : "STREAMOFF");
if (ret < 0) {
printf("Unable to %s streaming: %d.\n", enable ? "start" :
"stop",
errno);
@@ -1063,6 +1065,7 @@ int main(int argc, char *argv[])
{
struct device dev;
int ret;
+ int i;
/* Options parsings */
int do_file = 0, do_capture = 0, do_pause = 0;
@@ -1259,6 +1262,9 @@ int main(int argc, char *argv[])
video_enum_inputs(&dev);
}
+ for (i=0; i<100; i++) {
+ printf(" %d \n", i);
+
if (do_set_input) {
video_set_input(&dev, input);
ret = video_get_input(&dev);
@@ -1313,6 +1319,8 @@ int main(int argc, char *argv[])
return 1;
}
+ }
+
video_close(&dev);
return 0;
}
--
1.7.5.4
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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
ISP CCDC freeze-up on STREAMON
Hi Laurent, I'm observing a system freeze-up with the ISP when writing data to memory directly from the ccdc. Here's the sequence I'm using: 0. apply the patch I'm sending separate in this thread. 1. configure the ISP pipeline for the CCDC to deliver V4L2_PIX_FMT_GREY directly from the sensor to memory. 2. yavta -c10 /dev/video2 The patch is pretty self-explanatory. It introduces a loop (with ugly indenting to keep the patch simple) with 100 iterations leaving the device open between them. My system usually hangs up within the first 30 iterations. I've never made it to 100 successfully. I see the same behavior with user pointers and with mmap, but I don't see it when using data from the previewer. Can you please try this out with your setup? Even if you can't get 8-bit gray data from your sensor, hopefully you could observe it with any other format directly from the CCDC. I'll postpone further discussion until you confirm that you can reproduce the behavior. As the patch illustrates, it looks like it is hanging up in STREAMON. -Michael Michael Jones (1): prompt ISP CCDC freeze-up on STREAMON yavta.c |8 1 files changed, 8 insertions(+), 0 deletions(-) -- 1.7.5.4 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
auto-loading omap3-isp
I am trying to get omap3-isp.ko to be loaded upon bootup. The problem is that iommu2.ko needs to be loaded first, which can't just be compiled into the kernel. Udev will see '/sys/devices/platform/omap3isp' and load omap3-isp.ko, which fails because iommu2.ko hasn't been loaded yet. iommu2 doesn't have a counterpart in /sys/devices/, so I don't know how to get udev to load it first. I can think of a few ways to accomplish this, but they all amount to hacking the init sequence (e.g. the udev init script). I'm looking for a better way. How are others doing this? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
[RFC PATCH] capture-example: allow V4L2_PIX_FMT_GREY with USERPTR
There is an assumption that the format coming from the device
needs 2 bytes per pixel, which is not the case when the device
delivers e.g. V4L2_PIX_FMT_GREY. This doesn't manifest itself with
IO_METHOD_MMAP because init_mmap() (the default) doesn't take
sizeimage as an argument.
Signed-off-by: Michael Jones
---
This same issue would apply to other formats which have 1 byte per pixel,
this patch only fixes it for GREY. Is this OK for now, or does somebody
have a better suggestion for supporting other formats as well?
contrib/test/capture-example.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/contrib/test/capture-example.c b/contrib/test/capture-example.c
index 3852c58..0eb5235 100644
--- a/contrib/test/capture-example.c
+++ b/contrib/test/capture-example.c
@@ -416,6 +416,7 @@ static void init_device(void)
struct v4l2_crop crop;
struct v4l2_format fmt;
unsigned int min;
+ unsigned int bytes_per_pixel;
if (-1 == xioctl(fd, VIDIOC_QUERYCAP, &cap)) {
if (EINVAL == errno) {
@@ -519,7 +520,8 @@ static void init_device(void)
}
/* Buggy driver paranoia. */
- min = fmt.fmt.pix.width * 2;
+ bytes_per_pixel = fmt.fmt.pix.pixelformat == V4L2_PIX_FMT_GREY ? 1 : 2;
+ min = fmt.fmt.pix.width * bytes_per_pixel;
if (fmt.fmt.pix.bytesperline < min)
fmt.fmt.pix.bytesperline = min;
min = fmt.fmt.pix.bytesperline * fmt.fmt.pix.height;
--
1.7.5.4
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 2/2] allow using autoconf 2.61+
Autoconf v2.61 seems to work just fine. Allow it. Signed-off-by: Michael Jones --- configure.in |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.in b/configure.in index 3f4f35b..fd4c70c 100644 --- a/configure.in +++ b/configure.in @@ -1,4 +1,4 @@ -AC_PREREQ([2.65]) +AC_PREREQ([2.61]) AC_INIT([media-ctl], [0.0.1], [laurent.pinch...@ideasonboard.com]) AC_CONFIG_SRCDIR([src/main.c]) AC_CONFIG_AUX_DIR([config]) -- 1.7.5.4 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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/2] add Y10, Y12 formats
Signed-off-by: Michael Jones
---
I added these when playing around with the shifter.
src/main.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/main.c b/src/main.c
index 35c34a2..b9b9150 100644
--- a/src/main.c
+++ b/src/main.c
@@ -50,6 +50,8 @@ static struct {
enum v4l2_mbus_pixelcode code;
} mbus_formats[] = {
{ "Y8", V4L2_MBUS_FMT_Y8_1X8},
+ { "Y10", V4L2_MBUS_FMT_Y10_1X10 },
+ { "Y12", V4L2_MBUS_FMT_Y12_1X12 },
{ "YUYV", V4L2_MBUS_FMT_YUYV8_1X16 },
{ "UYVY", V4L2_MBUS_FMT_UYVY8_1X16 },
{ "SBGGR8", V4L2_MBUS_FMT_SBGGR8_1X8 },
--
1.7.5.4
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 0/2] media-ctl: minor changes
Hi Laurent, These are a couple of commits that I have locally on top of your media-ctl head which I would like to see in your rep. Michael Jones (2): add Y10, Y12 formats try using autoconf 2.61 configure.in |2 +- src/main.c |2 ++ 2 files changed, 3 insertions(+), 1 deletions(-) -- 1.7.5.4 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
buffer index when streaming user-ptr buffers
In the V4L2 spec, the description for v4l2_buffer.index says "This field is only used for memory mapping I/O..." However, in v4l-utils/contrib/capture-example.c, even user-pointer buffers are indeed given a buf.index before being passed to VIDIOC_QBUF. And at least in the OMAP ISP driver, this information is relied upon in QBUF regardless of V4L2_MEMORY_MMAP/USERPTR. videobuf-core also uses v4l2_buffer->index even if b->memory == V4L2_MEMORY_USERPTR. Is this a bug in the OMAP driver and videobuf-core, and an unnecessary assignment in capture-example? Or is the V4L2 spec out of touch/ out of date? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 1/2] v4l: Add generic board subdev registration function
On 05/20/2011 09:29 AM, Laurent Pinchart wrote: [snip] >> I had an issue when tried to call request_module, to register subdev of >> platform device type, in probe() of other platform device. Driver's >> probe() for devices belonging same bus type cannot be nested as the bus >> lock is taken by the driver core before entering probe(), so this would >> lead to a deadlock. >> That exactly happens in __driver_attach(). >> >> For the same reason v4l2_new_subdev_board could not be called from probe() >> of devices belonging to I2C or SPI bus, as request_module is called inside >> of it. I'm not sure how to solve it, yet:) > > Ouch. I wasn't aware of that issue. Looks like it's indeed time to fix the > subdev registration issue, including the module load race condition. Michael, > you said you have a patch to add platform subdev support, how have you > avoided > the race condition ? I spoke too soon. This deadlock is staring me in the face right now, too. Ouch, indeed. [snip] MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: FW: OMAP 3 ISP
On 05/19/2011 03:56 PM, Laurent Pinchart wrote:
> Hi Michael,
>
> On Thursday 19 May 2011 15:44:18 Michael Jones wrote:
>> On 05/19/2011 03:02 PM, Laurent Pinchart wrote:
>>> On Thursday 19 May 2011 14:51:16 Alex Gershgorin wrote:
>>>> Thanks Laurent,
>>>>
>>>> My video source is not the video camera and performs many other
>>>> functions. For this purpose I have RS232 port.
>>>> As for the video, it runs continuously and is not subject to control
>>>> except for the power supply.
>>>
>>> As a quick hack, you can create an I2C driver for your video source that
>>> doesn't access the device and just returns fixed format and frame size.
>>>
>>> The correct fix is to implement support for platform subdevs in the V4L2
>>> core.
>>
>> I recently implemented support for platform V4L2 subdevs. Now that it
>> sounds like others would be interested in this, I will try to polish it
>> up and submit the patch for review in the next week or so.
>
> Great. This has been discussed during the V4L meeting in Warsaw, here are a
> couple of pointers, to make sure we're going in the same direction.
>
> Bridge drivers should not care whether the subdev sits on an I2C, SPI,
> platform or other bus. To achieve that, an abstraction layer must be provided
> by the V4L2 core. Here's what I got in one of my trees:
>
> /* V4L2 core */
>
> struct v4l2_subdev_i2c_board_info {
> struct i2c_board_info *board_info;
> int i2c_adapter_id;
> };
>
> enum v4l2_subdev_bus_type {
> V4L2_SUBDEV_BUS_TYPE_NONE,
> V4L2_SUBDEV_BUS_TYPE_I2C,
> V4L2_SUBDEV_BUS_TYPE_SPI,
> };
>
> struct v4l2_subdev_board_info {
> enum v4l2_subdev_bus_type type;
> union {
> struct v4l2_subdev_i2c_board_info i2c;
> struct spi_board_info *spi;
> } info;
> };
>
> /* OMAP3 ISP */
>
> struct isp_v4l2_subdevs_group {
> struct v4l2_subdev_board_info *subdevs;
> enum isp_interface_type interface;
> union {
> struct isp_parallel_platform_data parallel;
> struct isp_ccp2_platform_data ccp2;
> struct isp_csi2_platform_data csi2;
> } bus; /* gcc < 4.6.0 chokes on anonymous union initializers */
> };
>
> struct isp_platform_data {
> struct isp_v4l2_subdevs_group *subdevs;
> };
>
> The V4L2 core would need to provide a function to register a subdev based on
> a
> v4l2_subdev_board_info structure.
>
> Is that in line with what you've done ? I can provide a patch that implements
> this for I2C and SPI, and let you add platform subdevs if that can help you.
>
Hi Laurent,
Yes, that looks very similar to what I've done. I was going to submit
SPI support, too, which I also have, but it sounds like you've already
done that? I'm currently still using a 2.6.38 tree based on an older
media branch of yours, so I'm not familiar with any new changes there yet.
I just need to know what I should use as my baseline. I don't need to
step on toes and submit something you've already done, so maybe you want
to point me to a branch with the SPI stuff, and I'll just put the
platform stuff on top of it?
-Michael
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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: FW: OMAP 3 ISP
On 05/19/2011 03:02 PM, Laurent Pinchart wrote: > Hi Alex, > > On Thursday 19 May 2011 14:51:16 Alex Gershgorin wrote: >> Thanks Laurent, >> >> My video source is not the video camera and performs many other functions. >> For this purpose I have RS232 port. >> As for the video, it runs continuously and is not subject to control except >> for the power supply. > > As a quick hack, you can create an I2C driver for your video source that > doesn't access the device and just returns fixed format and frame size. > > The correct fix is to implement support for platform subdevs in the V4L2 core. > I recently implemented support for platform V4L2 subdevs. Now that it sounds like others would be interested in this, I will try to polish it up and submit the patch for review in the next week or so. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
locking in OMAP ISP subdevs
Hi Laurent, I can't find where the locking is handled for ISP subdev standard ioctls like ccdc_v4l2_pad_ops.set_fmt(). Using the CCDC as an example, it looks to me like the following sequence happens when e.g. format is set on CCDC pad 0: 1. # media-ctl --set-format '"OMAP3 ISP CCDC":0 [Y8 640x480]' 2. v4l2-dev.c:v4l2_ioctl() this calls vdev->fops->unlocked_ioctl, which was set to v4l2-subdev.c:subdev_ioctl() in "v4l2_subdev_fops" in v4l2-device.c:v4l2_device_register_subdev_nodes() 3. v4l2-subdev.c:subdev_ioctl() 4. video_usercopy() 5. v4l2-ioctl.c:__video_usercopy() 6. v4l2-subdev.c:subdev_do_ioctl() 7. ispccdc.c:ccdc_set_format() ccdc_set_format() sets ccdc->formats[pad], access to which should be serialized, but I don't see how this happens. In the call sequence above, the only opportunity I see is in (2), but only then if ccdc->subdev.devnode.lock is set, which doesn't seem to be done. Can you clarify this for me? What mutex is held during a call to ccdc_set_format()? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v4 0/4] omap3isp: lane shifter support
Hi Laurent, On 03/29/2011 09:40 PM, Laurent Pinchart wrote: > Hi Michael, > > Thanks for the patches. I've applied them to my tree > (http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media- > next-0001-omap3isp) with modified commit messages. thanks. > Could you please check them and let me know if they're ok ? The commit messages look fine to me. > > The next time you submit patches, please include a commit message body in > addition to the subject line. > Will do. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v4 4/4] omap3isp: lane shifter support
To use the lane shifter, set different pixel formats at each end of
the link at the CCDC input.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3isp/isp.c |7 ++-
drivers/media/video/omap3isp/isp.h |5 +-
drivers/media/video/omap3isp/ispccdc.c | 27 ++--
drivers/media/video/omap3isp/ispvideo.c | 108 +--
drivers/media/video/omap3isp/ispvideo.h |3 +
5 files changed, 120 insertions(+), 30 deletions(-)
diff --git a/drivers/media/video/omap3isp/isp.c
b/drivers/media/video/omap3isp/isp.c
index f46b481..5d97c58 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -285,7 +285,8 @@ static void isp_power_settings(struct isp_device *isp, int
idle)
*/
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata)
+ const struct isp_parallel_platform_data *pdata,
+ unsigned int shift)
{
u32 ispctrl_val;
@@ -298,9 +299,9 @@ void omap3isp_configure_bridge(struct isp_device *isp,
switch (input) {
case CCDC_INPUT_PARALLEL:
ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
- ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
+ shift += pdata->data_lane_shift*2;
break;
case CCDC_INPUT_CSI2A:
@@ -319,6 +320,8 @@ void omap3isp_configure_bridge(struct isp_device *isp,
return;
}
+ ispctrl_val |= ((shift/2) << ISPCTRL_SHIFT_SHIFT) & ISPCTRL_SHIFT_MASK;
+
ispctrl_val &= ~ISPCTRL_SYNC_DETECT_MASK;
ispctrl_val |= ISPCTRL_SYNC_DETECT_VSRISE;
diff --git a/drivers/media/video/omap3isp/isp.h
b/drivers/media/video/omap3isp/isp.h
index dcf5004..84d6442 100644
--- a/drivers/media/video/omap3isp/isp.h
+++ b/drivers/media/video/omap3isp/isp.h
@@ -133,7 +133,6 @@ struct isp_reg {
/**
* struct isp_parallel_platform_data - Parallel interface platform data
- * @width: Parallel bus width in bits (8, 10, 11 or 12)
* @data_lane_shift: Data lane shifter
* 0 - CAMEXT[13:0] -> CAM[13:0]
* 1 - CAMEXT[13:2] -> CAM[11:0]
@@ -147,7 +146,6 @@ struct isp_reg {
* ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
*/
struct isp_parallel_platform_data {
- unsigned int width;
unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
unsigned int bridge:4;
@@ -326,7 +324,8 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
enum isp_pipeline_stream_state state);
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata);
+ const struct isp_parallel_platform_data *pdata,
+ unsigned int shift);
#define ISP_XCLK_NONE -1
#define ISP_XCLK_A 0
diff --git a/drivers/media/video/omap3isp/ispccdc.c
b/drivers/media/video/omap3isp/ispccdc.c
index 651b47b..af20cd0 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1120,21 +1120,38 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
struct isp_parallel_platform_data *pdata = NULL;
struct v4l2_subdev *sensor;
struct v4l2_mbus_framefmt *format;
+ const struct isp_format_info *fmt_info;
+ struct v4l2_subdev_format fmt_src;
+ unsigned int depth_out = 0;
+ unsigned int depth_in = 0;
struct media_pad *pad;
unsigned long flags;
+ unsigned int shift;
u32 syn_mode;
u32 ccdc_pattern;
- if (ccdc->input == CCDC_INPUT_PARALLEL) {
- pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
- sensor = media_entity_to_v4l2_subdev(pad->entity);
+ pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
+ sensor = media_entity_to_v4l2_subdev(pad->entity);
+ if (ccdc->input == CCDC_INPUT_PARALLEL)
pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
->bus.parallel;
+
+ /* Compute shift value for lane shifter to configure the bridge. */
+ fmt_src.pad = pad->index;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
+ fmt_info = omap3isp_video_format_info(fmt_src.format.code);
+ depth_in = fmt_info->bpp;
}
- omap3isp_configure_bri
[PATCH v4 2/4] media: add 8-bit bayer formats and Y12
Signed-off-by: Michael Jones
---
Documentation/DocBook/v4l/subdev-formats.xml | 59 ++
include/linux/v4l2-mediabus.h|7 ++-
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/v4l/subdev-formats.xml
b/Documentation/DocBook/v4l/subdev-formats.xml
index b5376e2..70964a4 100644
--- a/Documentation/DocBook/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/v4l/subdev-formats.xml
@@ -456,6 +456,23 @@
b1
b0
+
+ V4L2_MBUS_FMT_SGBRG8_1X8
+ 0x3013
+
+ -
+ -
+ -
+ -
+ g7
+ g6
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+
V4L2_MBUS_FMT_SGRBG8_1X8
0x3002
@@ -473,6 +490,23 @@
g1
g0
+
+ V4L2_MBUS_FMT_SRGGB8_1X8
+ 0x3014
+
+ -
+ -
+ -
+ -
+ r7
+ r6
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8
0x300b
@@ -2159,6 +2193,31 @@
u1
u0
+
+ V4L2_MBUS_FMT_Y12_1X12
+ 0x2013
+
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ y11
+ y10
+ y9
+ y8
+ y7
+ y6
+ y5
+ y4
+ y3
+ y2
+ y1
+ y0
+
V4L2_MBUS_FMT_UYVY8_1X16
0x200f
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 7054a7a..de5c159 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -47,7 +47,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
- /* YUV (including grey) - next is 0x2013 */
+ /* YUV (including grey) - next is 0x2014 */
V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
@@ -60,6 +60,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_Y10_1X10 = 0x200a,
V4L2_MBUS_FMT_YUYV10_2X10 = 0x200b,
V4L2_MBUS_FMT_YVYU10_2X10 = 0x200c,
+ V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
V4L2_MBUS_FMT_UYVY8_1X16 = 0x200f,
V4L2_MBUS_FMT_VYUY8_1X16 = 0x2010,
V4L2_MBUS_FMT_YUYV8_1X16 = 0x2011,
@@ -67,9 +68,11 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
- /* Bayer - next is 0x3013 */
+ /* Bayer - next is 0x3015 */
V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001,
+ V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013,
V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002,
+ V4L2_MBUS_FMT_SRGGB8_1X8 = 0x3014,
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 = 0x300b,
V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c,
V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009,
--
1.7.4.2
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v4 3/4] omap3isp: ccdc: support Y10/12, 8-bit bayer fmts
Signed-off-by: Michael Jones
Acked-by: Laurent Pinchart
---
drivers/media/video/omap3isp/ispccdc.c |6 ++
drivers/media/video/omap3isp/ispvideo.c | 12
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispccdc.c
b/drivers/media/video/omap3isp/ispccdc.c
index 5c5219e..651b47b 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -43,6 +43,12 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct
v4l2_subdev_fh *fh,
static const unsigned int ccdc_fmts[] = {
V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12,
+ V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10,
V4L2_MBUS_FMT_SRGGB10_1X10,
V4L2_MBUS_FMT_SBGGR10_1X10,
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c
index 3db7bdd..e2ec9b0 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -48,6 +48,18 @@
static struct isp_format_info formats[] = {
{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
+ { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10, 10, },
+ { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_PIX_FMT_Y12, 12, },
+ { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8, },
+ { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8, },
+ { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8, },
+ { V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8, },
{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
--
1.7.4.2
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v4 1/4] v4l: add V4L2_PIX_FMT_Y12 format
Signed-off-by: Michael Jones
---
Documentation/DocBook/media-entities.tmpl |1 +
Documentation/DocBook/v4l/pixfmt-y12.xml | 79 +
Documentation/DocBook/v4l/pixfmt.xml |1 +
include/linux/videodev2.h |1 +
4 files changed, 82 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml
diff --git a/Documentation/DocBook/media-entities.tmpl
b/Documentation/DocBook/media-entities.tmpl
index 157d147..fbd5b1b 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -287,6 +287,7 @@
+
diff --git a/Documentation/DocBook/v4l/pixfmt-y12.xml
b/Documentation/DocBook/v4l/pixfmt-y12.xml
new file mode 100644
index 000..ff417b8
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y12.xml
@@ -0,0 +1,79 @@
+
+
+V4L2_PIX_FMT_Y12 ('Y12 ')
+&manvol;
+
+
+V4L2_PIX_FMT_Y12
+Grey-scale image
+
+
+Description
+
+This is a grey-scale image with a depth of 12 bits per pixel. Pixels
+are stored in 16-bit words with unused high bits padded with 0. The least
+significant byte is stored at lower memory addresses (little-endian).
+
+
+ V4L2_PIX_FMT_Y12 4 × 4
+pixel image
+
+
+ Byte Order.
+ Each cell is one byte.
+
+
+
+
+
+ start + 0:
+ Y'00low
+ Y'00high
+ Y'01low
+ Y'01high
+ Y'02low
+ Y'02high
+ Y'03low
+ Y'03high
+
+
+ start + 8:
+ Y'10low
+ Y'10high
+ Y'11low
+ Y'11high
+ Y'12low
+ Y'12high
+ Y'13low
+ Y'13high
+
+
+ start + 16:
+ Y'20low
+ Y'20high
+ Y'21low
+ Y'21high
+ Y'22low
+ Y'22high
+ Y'23low
+ Y'23high
+
+
+ start + 24:
+ Y'30low
+ Y'30high
+ Y'31low
+ Y'31high
+ Y'32low
+ Y'32high
+ Y'33low
+ Y'33high
+
+
+
+
+
+
+
+
+
diff --git a/Documentation/DocBook/v4l/pixfmt.xml
b/Documentation/DocBook/v4l/pixfmt.xml
index cfffc88..426bd27 100644
--- a/Documentation/DocBook/v4l/pixfmt.xml
+++ b/Documentation/DocBook/v4l/pixfmt.xml
@@ -592,6 +592,7 @@ information.
&sub-packed-yuv;
&sub-grey;
&sub-y10;
+&sub-y12;
&sub-y16;
&sub-yuyv;
&sub-uyvy;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 02da9e7..6fac463 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y12 v4l2_fourcc('Y', '1', '2', ' ') /* 12 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.4.2
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v4 0/4] omap3isp: lane shifter support
Add support for the ISP's lane shifter. To use the shifter, set different pixel formats at each end of the link at the CCDC input. This has only been tested shifting Y12 and SBGGR12 from a parallel sensor to Y8 and SBGGR8 (respectively) at the CCDC input. Support has also been added for other formats and other shifting values, but is untested. Shifting data coming from one of the serial sensor interfaces (CSI2a, etc) is also untested. As before, ccdc_try_format() does not check that the format at its input is compatible with the format coming from the sensor interface. This consistency check is first done when activating the pipeline. These patches apply to Laurent's media-2.6.38-0001-omap3isp branch, based on 2.6.38 Changes since v3: -applies to media-2.6.38-0001-omap3isp -added missing DocBook changes needed for Y12, e.g. pixfmt-y12.xml -omap3isp_configure_bridge() takes 'unsigned int' instead of 'int' shift -renamed link_has_shifter -> shifter_link Changes since v2: -new formats are also added to Documentation/DocBook/v4l/ -reintroduce .data_lane_shift for sensors whose LSB is not aligned with sensor interfaces's LSB. -style changes according to feedback Changes since v1: -added support for remaining 8-bit Bayer formats (SGBRG8_1X8 & SRGGB8_1X8) -moved omap3isp_is_shiftable() from isp.c to ispvideo.c and return bool -moved 'shift' calculation from omap3isp_configure_bridge() to ccdc_configure() -added 'shift' arg to omap3isp_configure_bridge() -misc minor changes according to feedback (removed unnecessary tests, etc.) Michael Jones (4): v4l: add V4L2_PIX_FMT_Y12 format media: add 8-bit bayer formats and Y12 omap3isp: ccdc: support Y10/12, 8-bit bayer fmts omap3isp: lane shifter support Documentation/DocBook/media-entities.tmpl|1 + Documentation/DocBook/v4l/pixfmt-y12.xml | 79 +++ Documentation/DocBook/v4l/pixfmt.xml |1 + Documentation/DocBook/v4l/subdev-formats.xml | 59 ++ drivers/media/video/omap3isp/isp.c |7 +- drivers/media/video/omap3isp/isp.h |5 +- drivers/media/video/omap3isp/ispccdc.c | 33 +++- drivers/media/video/omap3isp/ispvideo.c | 108 ++ drivers/media/video/omap3isp/ispvideo.h |3 + include/linux/v4l2-mediabus.h|7 +- include/linux/videodev2.h|1 + 11 files changed, 278 insertions(+), 26 deletions(-) create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml -- 1.7.4.2 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] omap3isp: implement ENUM_FMT
Hi Laurent, On 03/24/2011 11:36 AM, Laurent Pinchart wrote: > Hi, > [snip] > > Padding at end of line can be configured through S_FMT. Other than that, all > other options (width, height, pixelcode) are fixed for a given mbus format > *for the ISP driver*. Other drivers might support different pixel codes for a > given mbus code (with different padding and/or endianness). > > Application either need to be aware of the media controller framework, in > which case they will know how to deal with mbus formats and pixel formats, or > need to be run after an external application takes care of pipeline > configuration. In the second case I suppose it's reasonable to assume that no > application will touch the pipeline while the pure V4L2 runs. In that case I > think your implementation of ENUM_FMT makes sense. > It is this second case which I am currently using, and why I submitted this patch. I think supporting ENUM_FMT in some form at /dev/videoX makes sense unless this second case is deemed obsolete and unsupported. But then that seems like a bigger break from V4L. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] omap3isp: implement ENUM_FMT
Hi Sakari, On 03/24/2011 09:04 AM, Sakari Ailus wrote: > Michael Jones wrote: >> Hi Sakari, > > Hi Michael, > >> On 03/23/2011 10:52 AM, Sakari Ailus wrote: >>> Hi Michael, >>> >>> Thanks for the patch. >>> >>> Michael Jones wrote: >>>> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 >>>> From: Michael Jones >>>> Date: Tue, 22 Mar 2011 11:47:22 +0100 >>>> Subject: [PATCH] omap3isp: implement ENUM_FMT >>>> >>>> Whatever format is currently being delivered will be declared as the only >>>> possible format >>>> >>>> Signed-off-by: Michael Jones >>>> --- >>>> >>>> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. >>>> This patch doesn't enumerate all of the formats which could possibly be >>>> set (as is intended by ENUM_FMT), but at least it reports the one that >>>> is currently set. >>> >>> What would be the purpose of ENUM_FMT in this case? It provides no >>> additional information to user space, and the information it provides is >>> in fact incomplete. Using other formats is possible, but that requires >>> changes to the format configuration on links. >> >> The only purpose of it was to provide minimum functionality for apps to >> be able to fetch frames from the ISP after setting up the ISP pipeline >> with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had >> also recently asked Laurent about. > > Are you familiar with the mcsrc? No, I was not familiar with that, thank you. That looks very useful, I'll have to look into it. > > http://meego.gitorious.org/maemo-multimedia/gst-nokia-videosrc> > > I think it's > > Also, I think v4lsrc could be fixed to cope with lack of VIDIOC_ENUM_FMT. Yes, it could be. But ENUM_FMT is allowed and I think it's reasonable for a V4L2 app to require the use of ENUM_FMT. > > Besides what Laurent already mentioned, how the applications are > expected to select a video node from the 7 (or 8) that the ISP has is > not resolved yet. The requested pixelformat depends on the video node in > this case. > >>> As the relevant format configuration is done on the subdevs and not on >>> the video nodes, the format configuration on the video nodes is very >>> limited and much affected by the state of the formats on the subdev pads >>> (which I think is right). This is not limited to ENUM_FMT but all format >>> related IOCTLs on the OMAP 3 ISP driver. >>> >>> My view is that should a generic application want to change (or >>> enumerate) the format(s) on a video node, the application would need to >>> be using libv4l for that. >>> >>> A compatibility layer implemented in libv4l (plugin, not the main >>> library) needs to configure the links in the first place, so >>> implementing ENUM_FMT in the plugin would not be a big deal. It could >>> even provide useful information. The possible results of the ENUM_FMT >>> would also depend on what kind of pipeline configuration does the plugin >>> support, though. >> >> BTW, my GStreamer is using libv4l, although it looked like it's also >> possible to configure GStreamer to use ioctls directly. I can agree >> that it would be nice to implement ENUM_FMT and the like in a >> compatibility layer in libv4l. That would be in the true spirit of >> ENUM_FMT, where the app could actually see different formats it can set. >> >> But is there any work being done on such a compatibility layer? > > Yes. Yordan is working on that. > > There's a plugin interface patch for libv4l and a MC-aware plugin that > uses the interface. > > http://www.spinics.net/lists/linux-media/msg28925.html> > > I think so far only the plugin interface patch is on the list. > >> Is there a policy decision that in the future, apps will be required to >> use libv4l to get images from the ISP? Are we not intending to support >> using e.g. media-ctl + some v4l2 app, as I'm currently doing during >> development? > > If we speak about specific applications that are aware of the MC and the > OMAP 3 ISP itself, I'd guess those will continue to use the interfaces > provided by the driver directly. > > But generic applications would likely be better off using libv4l. This > way they could use whatever there is in the libv4l plugin for the > platform (automatic exposure and white balance, for example). > > I don't thi
[PATCH] ignore Documentation/DocBook/media/
It only contains generated files Signed-off-by: Michael Jones Acked-by: Laurent Pinchart --- Re-sending this because patchwork didn't catch it the first time. Documentation/DocBook/.gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Documentation/DocBook/.gitignore b/Documentation/DocBook/.gitignore index c6def35..679034c 100644 --- a/Documentation/DocBook/.gitignore +++ b/Documentation/DocBook/.gitignore @@ -8,3 +8,4 @@ *.dvi *.log *.out +media/ -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ignore Documentation/DocBook/media/
On 03/16/2011 03:22 PM, Michael Jones wrote: >>From 81a09633855b88d19f013d7e559e0c4f602ba711 Mon Sep 17 00:00:00 2001 > From: Michael Jones > Date: Thu, 10 Mar 2011 16:16:38 +0100 > Subject: [PATCH] ignore Documentation/DocBook/media/ > > It is created and populated by 'make htmldocs' > > > Signed-off-by: Michael Jones > --- > Documentation/DocBook/.gitignore |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Documentation/DocBook/.gitignore > b/Documentation/DocBook/.gitignore > index c6def35..679034c 100644 > --- a/Documentation/DocBook/.gitignore > +++ b/Documentation/DocBook/.gitignore > @@ -8,3 +8,4 @@ > *.dvi > *.log > *.out > +media/ In general, where do patches on this list land? On http://linuxtv.org/wiki/index.php/Developer_Section the link to "Current git log" is broken. The link to 'Git V4L-DVB development repository' is v4l-dvb.git which is suspiciously inactive: 2.6.37-rc8, from 2 months ago. Judging by the activity level, I guess that patches from this mailing list currently land in branch 'staging/for_v2.6.39' of 'http://git.linuxtv.org/media_tree.git'? And what's the destiny of this patch in particular? It doesn't seem to have even been picked up by patchwork. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] omap3isp: implement ENUM_FMT
Hi Laurent, On 03/23/2011 01:16 PM, Laurent Pinchart wrote: > Hi Michael, > [snip] >> >> Is there a policy decision that in the future, apps will be required to >> use libv4l to get images from the ISP? Are we not intending to support >> using e.g. media-ctl + some v4l2 app, as I'm currently doing during >> development? > > Apps should be able to use the V4L2 API directly. However, we can't implement > all that API, as most calls don't make sense for the OMA3 ISP driver. Which > calls need to be implemented is a grey area at the moment, as there's no > detailed semantics on how subdev-level configuration and video device > configuration should interact. > > Your implementation of ENUM_FMT looks correct to me, but the question is > whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a required > ioctl, so maybe v4l2src shouldn't depend on it. I'm interesting in getting > Hans' opinion on this. > I only implemented it after I saw that ENUM_FMT _was_ required by V4L2. From http://v4l2spec.bytesex.org/spec/x1859.htm#AEN1894 : "The VIDIOC_ENUM_FMT ioctl must be supported by all drivers exchanging image data with applications." -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] omap3isp: implement ENUM_FMT
Hi Sakari, On 03/23/2011 10:52 AM, Sakari Ailus wrote: > Hi Michael, > > Thanks for the patch. > > Michael Jones wrote: >> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 >> From: Michael Jones >> Date: Tue, 22 Mar 2011 11:47:22 +0100 >> Subject: [PATCH] omap3isp: implement ENUM_FMT >> >> Whatever format is currently being delivered will be declared as the only >> possible format >> >> Signed-off-by: Michael Jones >> --- >> >> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. >> This patch doesn't enumerate all of the formats which could possibly be >> set (as is intended by ENUM_FMT), but at least it reports the one that >> is currently set. > > What would be the purpose of ENUM_FMT in this case? It provides no > additional information to user space, and the information it provides is > in fact incomplete. Using other formats is possible, but that requires > changes to the format configuration on links. The only purpose of it was to provide minimum functionality for apps to be able to fetch frames from the ISP after setting up the ISP pipeline with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had also recently asked Laurent about. > > As the relevant format configuration is done on the subdevs and not on > the video nodes, the format configuration on the video nodes is very > limited and much affected by the state of the formats on the subdev pads > (which I think is right). This is not limited to ENUM_FMT but all format > related IOCTLs on the OMAP 3 ISP driver. > > My view is that should a generic application want to change (or > enumerate) the format(s) on a video node, the application would need to > be using libv4l for that. > > A compatibility layer implemented in libv4l (plugin, not the main > library) needs to configure the links in the first place, so > implementing ENUM_FMT in the plugin would not be a big deal. It could > even provide useful information. The possible results of the ENUM_FMT > would also depend on what kind of pipeline configuration does the plugin > support, though. BTW, my GStreamer is using libv4l, although it looked like it's also possible to configure GStreamer to use ioctls directly. I can agree that it would be nice to implement ENUM_FMT and the like in a compatibility layer in libv4l. That would be in the true spirit of ENUM_FMT, where the app could actually see different formats it can set. But is there any work being done on such a compatibility layer? Is there a policy decision that in the future, apps will be required to use libv4l to get images from the ISP? Are we not intending to support using e.g. media-ctl + some v4l2 app, as I'm currently doing during development? In the meantime, I will continue using this patch locally to enable getting a live image with Gstreamer, and it can at least serve as a help to Loïc if he's trying to do the same. > > (Cc Yordan and Hans.) > > I discussed this with Laurent initially and the conclusion was that more > discussion is required. :-) Hans: do you have an opinion on this? > > Best regards, > thanks for the discussion, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] omap3isp: implement ENUM_FMT
>From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones
Date: Tue, 22 Mar 2011 11:47:22 +0100
Subject: [PATCH] omap3isp: implement ENUM_FMT
Whatever format is currently being delivered will be declared as the only
possible format
Signed-off-by: Michael Jones
---
Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2.
This patch doesn't enumerate all of the formats which could possibly be
set (as is intended by ENUM_FMT), but at least it reports the one that
is currently set.
This patch applies to Laurent's 'media-2.6.38-0001-omap3isp' branch.
drivers/media/video/omap3isp/ispvideo.c | 23 +++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c
index a0bb5db..8e25f47 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -642,6 +642,28 @@ isp_video_get_format(struct file *file, void *fh, struct
v4l2_format *format)
}
static int
+isp_video_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc
*fmtdesc)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+
+ if (fmtdesc->index)
+ return -EINVAL;
+
+ if (fmtdesc->type != video->type)
+ return -EINVAL;
+
+ fmtdesc->flags = 0;
+ fmtdesc->description[0] = 0;
+
+ mutex_lock(&video->mutex);
+ fmtdesc->pixelformat = vfh->format.fmt.pix.pixelformat;
+ mutex_unlock(&video->mutex);
+
+ return 0;
+}
+
+static int
isp_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
{
struct isp_video_fh *vfh = to_isp_video_fh(fh);
@@ -1059,6 +1081,7 @@ isp_video_s_input(struct file *file, void *fh, unsigned
int input)
static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
.vidioc_querycap= isp_video_querycap,
+ .vidioc_enum_fmt_vid_cap= isp_video_enum_format,
.vidioc_g_fmt_vid_cap = isp_video_get_format,
.vidioc_s_fmt_vid_cap = isp_video_set_format,
.vidioc_try_fmt_vid_cap = isp_video_try_format,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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: Demande de support V4L2
On 03/21/2011 06:43 PM, Laurent Pinchart wrote: > Hi Loïc, > > On Friday 18 March 2011 16:43:40 Loïc Akue wrote: >> Hi, >> >> Do you know if Gstreamer or Mplayer are able to capture some vidéo from the >> CCDC output? >> I've been trying with the v4l2src plugin but Gstreamer "can negociate >> format". > > I haven't tried mplayer or v4l2src with the OAMP3 ISP, sorry. > Hi Loïc, GStreamer relies on ENUM_FMT to negotiate the format, which the current ISP driver doesn't provide. I have patched my kernel to do this and I have gotten GStreamer to display UYVY images from the previewer output, but I haven't spent much time trying to get it from the CCDC yet. I would be interested to hear your progress there. I will submit my ENUM_FMT patch to the list in a different thread. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 4/4] omap3isp: lane shifter support
On 03/17/2011 04:40 PM, Sakari Ailus wrote: > Hi Laurent and Michael, > > Laurent Pinchart wrote: >> On Thursday 17 March 2011 11:07:40 Michael Jones wrote: >>> On 03/16/2011 06:46 PM, Laurent Pinchart wrote: >>>> On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote: >>>>> Laurent Pinchart wrote: >>>>>> Hi Sakari, >>>>>> >>>>>>>> + return in_info->bpp - out_info->bpp + additional_shift <= 6; >>>>>>> >>>>>>> Currently there are no formats that would behave badly in this check? >>>>>>> Perhaps it'd be good idea to take that into consideration. The shift >>>>>>> that can be done is even. >>>>>> >>>>>> I've asked Michael to remove the check because we have no misbehaving >>>>>> formats >>>>>> >>>>>> :-) Do you think we need to add a check back ? >>>>> >>>>> I think it would be helpful in debugging if someone decides to attach a >>>>> sensor which supports a shift of non-even bits (8 and 9 bits, for >>>>> example). In any case an invalid configuration is possible in such case, >>>>> and I don't think that should be allowed, should it? >>>> >>>> I agree it shouldn't be allowed, but the ISP driver doesn't support >>>> non-even widths at the moment, so there's no big risk. There could be an >>>> issue when a non-even width is added to the driver if the developer >>>> forgets to update the shift code. Maybe a comment in ispvideo.c above >>>> the big formats array would help making sure this is not forgotten ? >>> >>> I think now that additional_shift is also being considered which comes >>> from the board file, it makes sense to reintroduce the check for an even >>> shift. As Sakari points out, this would be helpful for debugging if >>> someone tries using .data_lane_shift which is odd. >> >> How should we handle such a broken .data_lane_shift value ? Always refuse to >> start streaming (maybe with a kernel log message) ? Or should we catch it in >> isp_register_entities() instead ? > > If I understand correctly it's not possible to shift odd bits in any > case. It's a hardware limitation. > > I'd perhaps have just the appropriate register bits in the platform data > so that leaves no room for accidental misconfiguration, but this is > perhaps just too much work for not much gain. > Actually, that's the way .data_lane_shift was originally defined (0,1,2,3), and I left it that way to minimize confusion. I was mistaken above when I said that .data_lane_shift could sneak an odd shift to isp_video_is_shiftable(), because .data_lane_shift is multiplied by 2 before getting passed there. So I would like to leave this as is, and it sounds like we have a consensus on this. I'll submit v4 soon, then. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 4/4] omap3isp: lane shifter support
Hi Sakari,
Thanks for the review.
On 03/16/2011 06:46 PM, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote:
>> Laurent Pinchart wrote:
>>> Hi Sakari,
> + return in_info->bpp - out_info->bpp + additional_shift <= 6;
Currently there are no formats that would behave badly in this check?
Perhaps it'd be good idea to take that into consideration. The shift
that can be done is even.
>>>
>>> I've asked Michael to remove the check because we have no misbehaving
>>> formats
>>>
>>> :-) Do you think we need to add a check back ?
>>
>> I think it would be helpful in debugging if someone decides to attach a
>> sensor which supports a shift of non-even bits (8 and 9 bits, for
>> example). In any case an invalid configuration is possible in such case,
>> and I don't think that should be allowed, should it?
>
> I agree it shouldn't be allowed, but the ISP driver doesn't support non-even
> widths at the moment, so there's no big risk. There could be an issue when a
> non-even width is added to the driver if the developer forgets to update the
> shift code. Maybe a comment in ispvideo.c above the big formats array would
> help making sure this is not forgotten ?
I think now that additional_shift is also being considered which comes
from the board file, it makes sense to reintroduce the check for an even
shift. As Sakari points out, this would be helpful for debugging if
someone tries using .data_lane_shift which is odd.
>
> @@ -247,6 +296,7 @@ static int isp_video_validate_pipeline(struct
> isp_pipeline *pipe)
>
> return -EPIPE;
>
> while (1) {
>
> + unsigned int link_has_shifter;
link_has_shifter is only used in one place. Would it be cleaner to test
below if it's the CCDC? A comment there could be nice, too.
>>>
>>> I would like that better as well, but between the line where
>>> link_has_shifter is set and the line where it is checked, the subdev
>>> variable changes so we can't just check subdev == &isp->isp_ccdc.subdev
>>> there.
>>
>> That's definitely valid. I take my comment back. The variable could be
>> called is_ccdc, though, since only the CCDC has that feature. No need to
>> generalise. :-)
>
But this is not a feature of the CCDC, the lane shifter is outside of
the CCDC. Each 'while (1)' iteration handles 2 subdevs on each side of
one link, so I think it makes sense for a particular iteration to say
"this link has", especially when the subdev ptr changes values between
the assignment of this var and its usage. "is_ccdc" is vague as to
which side of the CCDC we're on. 'link_has_shifter' wasn't intended to
be general, it was supposed to mean 'this_is_the_link_with_the_shifter'.
If you want to be more specific where that is in the pipeline, maybe
'ccdc_sink_link'? If you just want it to sound less like "this is one
of the links with a shifter" and more like "We've found _the_ link with
_the_ shifter", it could just be 'shifter_link'.
After we iron these two things out, are you guys ready to see v4?
-Michael
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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] ignore Documentation/DocBook/media/
>From 81a09633855b88d19f013d7e559e0c4f602ba711 Mon Sep 17 00:00:00 2001 From: Michael Jones Date: Thu, 10 Mar 2011 16:16:38 +0100 Subject: [PATCH] ignore Documentation/DocBook/media/ It is created and populated by 'make htmldocs' Signed-off-by: Michael Jones --- Documentation/DocBook/.gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Documentation/DocBook/.gitignore b/Documentation/DocBook/.gitignore index c6def35..679034c 100644 --- a/Documentation/DocBook/.gitignore +++ b/Documentation/DocBook/.gitignore @@ -8,3 +8,4 @@ *.dvi *.log *.out +media/ -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: Where to find 8-bit sbggr patch for omap3-isp
Hi Bastian, On 03/16/2011 11:01 AM, Bastian Hecht wrote: > Hello dear omap-isp developers, > > I'm working with a OV5642 sensor with an 8-bit parallel bus. > > I'm referring to this patch: > http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/29876/match=sgrbg8 > > Michael, you say that the patch applies to media-0005-omap3isp from Laurent. > I cannot see it in the repo: > http://git.linuxtv.org/pinchartl/media.git?a=blob;f=drivers/media/video/omap3-isp/ispccdc.c;h=5ff9d14ce71099cc672e71e2bd1d7ca619bbcc98;hb=media-0005-omap3isp > > Hasn't the patch been merged into your tree yet, Laurent? > Or am I looking at the wrong spot? Laurent hasn't merged those patches into his tree yet, and when he does it will probably not show up on top of that media-0005-omap3isp branch, but rather be incorporated into a future branch. The most recent version I've submitted to the list is http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/30258 I'm waiting a bit to see if Laurent has any more comments on the patches before submitting v4. So if you want them you'll have to take them from the list for now. -Michael > > Thanks for help, > > Bastian Hecht MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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/4] v4l: add V4L2_PIX_FMT_Y12 format
On 03/11/2011 10:21 AM, Antonio Ospite wrote: > Hi Michael, > > are you going to release also Y12 conversion routines for libv4lconvert? > > Regards, >Antonio > Hi Antonio, As I am neither a user nor developer of libv4lconvert, I am not planning on adding Y12 conversion routines there. Hopefully somebody else will step up. Maybe you? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v3 4/4] omap3isp: lane shifter support
To use the lane shifter, set different pixel formats at each end of
the link at the CCDC input.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3-isp/isp.c |7 ++-
drivers/media/video/omap3-isp/isp.h |5 +-
drivers/media/video/omap3-isp/ispccdc.c | 27 ++--
drivers/media/video/omap3-isp/ispvideo.c | 108 --
drivers/media/video/omap3-isp/ispvideo.h |3 +
5 files changed, 120 insertions(+), 30 deletions(-)
diff --git a/drivers/media/video/omap3-isp/isp.c
b/drivers/media/video/omap3-isp/isp.c
index 08d90fe..866ce09 100644
--- a/drivers/media/video/omap3-isp/isp.c
+++ b/drivers/media/video/omap3-isp/isp.c
@@ -285,7 +285,8 @@ static void isp_power_settings(struct isp_device *isp, int
idle)
*/
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata)
+ const struct isp_parallel_platform_data *pdata,
+ int shift)
{
u32 ispctrl_val;
@@ -298,9 +299,9 @@ void omap3isp_configure_bridge(struct isp_device *isp,
switch (input) {
case CCDC_INPUT_PARALLEL:
ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
- ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
+ shift += pdata->data_lane_shift*2;
break;
case CCDC_INPUT_CSI2A:
@@ -319,6 +320,8 @@ void omap3isp_configure_bridge(struct isp_device *isp,
return;
}
+ ispctrl_val |= ((shift/2) << ISPCTRL_SHIFT_SHIFT) & ISPCTRL_SHIFT_MASK;
+
ispctrl_val &= ~ISPCTRL_SYNC_DETECT_MASK;
ispctrl_val |= ISPCTRL_SYNC_DETECT_VSRISE;
diff --git a/drivers/media/video/omap3-isp/isp.h
b/drivers/media/video/omap3-isp/isp.h
index 21fa88b..6f0ff1a 100644
--- a/drivers/media/video/omap3-isp/isp.h
+++ b/drivers/media/video/omap3-isp/isp.h
@@ -130,7 +130,6 @@ struct isp_reg {
/**
* struct isp_parallel_platform_data - Parallel interface platform data
- * @width: Parallel bus width in bits (8, 10, 11 or 12)
* @data_lane_shift: Data lane shifter
* 0 - CAMEXT[13:0] -> CAM[13:0]
* 1 - CAMEXT[13:2] -> CAM[11:0]
@@ -144,7 +143,6 @@ struct isp_reg {
* ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
*/
struct isp_parallel_platform_data {
- unsigned int width;
unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
unsigned int bridge:4;
@@ -322,7 +320,8 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
enum isp_pipeline_stream_state state);
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata);
+ const struct isp_parallel_platform_data *pdata,
+ int shift);
#define ISP_XCLK_NONE -1
#define ISP_XCLK_A 0
diff --git a/drivers/media/video/omap3-isp/ispccdc.c
b/drivers/media/video/omap3-isp/ispccdc.c
index 23000b6..bbcf619 100644
--- a/drivers/media/video/omap3-isp/ispccdc.c
+++ b/drivers/media/video/omap3-isp/ispccdc.c
@@ -1120,21 +1120,38 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
struct isp_parallel_platform_data *pdata = NULL;
struct v4l2_subdev *sensor;
struct v4l2_mbus_framefmt *format;
+ const struct isp_format_info *fmt_info;
+ struct v4l2_subdev_format fmt_src;
+ unsigned int depth_out = 0;
+ unsigned int depth_in = 0;
struct media_pad *pad;
unsigned long flags;
+ unsigned int shift;
u32 syn_mode;
u32 ccdc_pattern;
- if (ccdc->input == CCDC_INPUT_PARALLEL) {
- pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
- sensor = media_entity_to_v4l2_subdev(pad->entity);
+ pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
+ sensor = media_entity_to_v4l2_subdev(pad->entity);
+ if (ccdc->input == CCDC_INPUT_PARALLEL)
pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
->bus.parallel;
+
+ /* Compute shift value for lane shifter to configure the bridge. */
+ fmt_src.pad = pad->index;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
+ fmt_info = omap3isp_video_format_info(fmt_src.format.code);
+ depth_in = fmt_info->bpp;
}
- omap3isp_configure_bri
[PATCH v3 3/4] omap3isp: ccdc: support Y10/12, 8-bit bayer fmts
Signed-off-by: Michael Jones
Acked-by: Laurent Pinchart
---
drivers/media/video/omap3-isp/ispccdc.c |6 ++
drivers/media/video/omap3-isp/ispvideo.c | 12
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3-isp/ispccdc.c
b/drivers/media/video/omap3-isp/ispccdc.c
index e4d04ce..23000b6 100644
--- a/drivers/media/video/omap3-isp/ispccdc.c
+++ b/drivers/media/video/omap3-isp/ispccdc.c
@@ -43,6 +43,12 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct
v4l2_subdev_fh *fh,
static const unsigned int ccdc_fmts[] = {
V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12,
+ V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10,
V4L2_MBUS_FMT_SRGGB10_1X10,
V4L2_MBUS_FMT_SBGGR10_1X10,
diff --git a/drivers/media/video/omap3-isp/ispvideo.c
b/drivers/media/video/omap3-isp/ispvideo.c
index f16d787..3c3b3c4 100644
--- a/drivers/media/video/omap3-isp/ispvideo.c
+++ b/drivers/media/video/omap3-isp/ispvideo.c
@@ -48,6 +48,18 @@
static struct isp_format_info formats[] = {
{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
+ { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10, 10, },
+ { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_PIX_FMT_Y12, 12, },
+ { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8, },
+ { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8, },
+ { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8, },
+ { V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8, },
{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v3 2/4] media: add 8-bit bayer formats and Y12
Signed-off-by: Michael Jones
---
Documentation/DocBook/v4l/subdev-formats.xml | 59 ++
include/linux/v4l2-mediabus.h|7 ++-
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/v4l/subdev-formats.xml
b/Documentation/DocBook/v4l/subdev-formats.xml
index 2fed9be..37e5bc6 100644
--- a/Documentation/DocBook/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/v4l/subdev-formats.xml
@@ -456,6 +456,23 @@
b1
b0
+
+ V4L2_MBUS_FMT_SGBRG8_1X8
+ 0x3013
+
+ -
+ -
+ -
+ -
+ g7
+ g6
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+
V4L2_MBUS_FMT_SGRBG8_1X8
0x3002
@@ -473,6 +490,23 @@
g1
g0
+
+ V4L2_MBUS_FMT_SRGGB8_1X8
+ 0x3014
+
+ -
+ -
+ -
+ -
+ r7
+ r6
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8
0x300b
@@ -2159,6 +2193,31 @@
u1
u0
+
+ V4L2_MBUS_FMT_Y12_1X12
+ 0x2013
+
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ y11
+ y10
+ y9
+ y8
+ y7
+ y6
+ y5
+ y4
+ y3
+ y2
+ y1
+ y0
+
V4L2_MBUS_FMT_UYVY8_1X16
0x200f
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 7054a7a..de5c159 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -47,7 +47,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
- /* YUV (including grey) - next is 0x2013 */
+ /* YUV (including grey) - next is 0x2014 */
V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
@@ -60,6 +60,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_Y10_1X10 = 0x200a,
V4L2_MBUS_FMT_YUYV10_2X10 = 0x200b,
V4L2_MBUS_FMT_YVYU10_2X10 = 0x200c,
+ V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
V4L2_MBUS_FMT_UYVY8_1X16 = 0x200f,
V4L2_MBUS_FMT_VYUY8_1X16 = 0x2010,
V4L2_MBUS_FMT_YUYV8_1X16 = 0x2011,
@@ -67,9 +68,11 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
- /* Bayer - next is 0x3013 */
+ /* Bayer - next is 0x3015 */
V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001,
+ V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013,
V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002,
+ V4L2_MBUS_FMT_SRGGB8_1X8 = 0x3014,
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 = 0x300b,
V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c,
V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v3 1/4] v4l: add V4L2_PIX_FMT_Y12 format
Signed-off-by: Michael Jones
---
Documentation/DocBook/v4l/pixfmt-y12.xml | 79 ++
include/linux/videodev2.h|1 +
2 files changed, 80 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml
diff --git a/Documentation/DocBook/v4l/pixfmt-y12.xml
b/Documentation/DocBook/v4l/pixfmt-y12.xml
new file mode 100644
index 000..ff417b8
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y12.xml
@@ -0,0 +1,79 @@
+
+
+V4L2_PIX_FMT_Y12 ('Y12 ')
+&manvol;
+
+
+V4L2_PIX_FMT_Y12
+Grey-scale image
+
+
+Description
+
+This is a grey-scale image with a depth of 12 bits per pixel. Pixels
+are stored in 16-bit words with unused high bits padded with 0. The least
+significant byte is stored at lower memory addresses (little-endian).
+
+
+ V4L2_PIX_FMT_Y12 4 × 4
+pixel image
+
+
+ Byte Order.
+ Each cell is one byte.
+
+
+
+
+
+ start + 0:
+ Y'00low
+ Y'00high
+ Y'01low
+ Y'01high
+ Y'02low
+ Y'02high
+ Y'03low
+ Y'03high
+
+
+ start + 8:
+ Y'10low
+ Y'10high
+ Y'11low
+ Y'11high
+ Y'12low
+ Y'12high
+ Y'13low
+ Y'13high
+
+
+ start + 16:
+ Y'20low
+ Y'20high
+ Y'21low
+ Y'21high
+ Y'22low
+ Y'22high
+ Y'23low
+ Y'23high
+
+
+ start + 24:
+ Y'30low
+ Y'30high
+ Y'31low
+ Y'31high
+ Y'32low
+ Y'32high
+ Y'33low
+ Y'33high
+
+
+
+
+
+
+
+
+
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 02da9e7..6fac463 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y12 v4l2_fourcc('Y', '1', '2', ' ') /* 12 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 v3 0/4] OMAP3-ISP lane shifter support
Add support for the ISP's lane shifter. To use the shifter, set different pixel formats at each end of the link at the CCDC input. This has only been tested shifting Y12 and SBGGR12 from a parallel sensor to Y8 and SBGGR8 (respectively) at the CCDC input. Support has also been added for other formats and other shifting values, but is untested. Shifting data coming from one of the serial sensor interfaces (CSI2a, etc) is also untested. As before, ccdc_try_format() does not check that the format at its input is compatible with the format coming from the sensor interface. This consistency check is first done when activating the pipeline. These patches apply to Laurent's media-0005-omap3isp branch, based on 2.6.38-rc5 Changes since v2: -new formats are also added to Documentation/DocBook/v4l/ -reintroduce .data_lane_shift for sensors whose LSB is not aligned with sensor interfaces's LSB. -style changes according to feedback Changes since v1: -added support for remaining 8-bit Bayer formats (SGBRG8_1X8 & SRGGB8_1X8) -moved omap3isp_is_shiftable() from isp.c to ispvideo.c and return bool -moved 'shift' calculation from omap3isp_configure_bridge() to ccdc_configure() -added 'shift' arg to omap3isp_configure_bridge() -misc minor changes according to feedback (removed unnecessary tests, etc.) Michael Jones (4): v4l: add V4L2_PIX_FMT_Y12 format media: add 8-bit bayer formats and Y12 omap3isp: ccdc: support Y10/12, 8-bit bayer fmts omap3isp: lane shifter support Documentation/DocBook/v4l/pixfmt-y12.xml | 79 +++ Documentation/DocBook/v4l/subdev-formats.xml | 59 ++ drivers/media/video/omap3-isp/isp.c |7 +- drivers/media/video/omap3-isp/isp.h |5 +- drivers/media/video/omap3-isp/ispccdc.c | 33 +++- drivers/media/video/omap3-isp/ispvideo.c | 108 ++ drivers/media/video/omap3-isp/ispvideo.h |3 + include/linux/v4l2-mediabus.h|7 +- include/linux/videodev2.h|1 + 9 files changed, 276 insertions(+), 26 deletions(-) create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v2 4/4] omap3isp: lane shifter support
Hi Laurent, On 03/10/2011 11:21 AM, Laurent Pinchart wrote: > Hi Michael, > [snip] > I've had a closer look at the boards I have here, and it turns out one of > them > connects a 10-bit sensor to DATA[11:2] :-/ data_lane_shift is thus needed for > it. > > I'm fine with leaving data_lane_shift out of this patch, but can you submit a > second patch to add it back ? I'd rather avoid applying a patch that breaks > one of my boards and then have to fix it myself :-) OK, but in that case I'd rather incorporate it into this last patch than introduce a new patch for it. I don't think it will be very complex and they logically belong together. I had just been hoping to avoid implementing it altogether. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v2 4/4] omap3isp: lane shifter support
Hi Laurent,
Thanks for the review. Most of your suggestions didn't warrant
discussion and I will incorporate those changes. The others are below.
On 03/10/2011 01:13 AM, Laurent Pinchart wrote:
> Hi Michael,
>
> Thanks for the patch.
>
> On Wednesday 09 March 2011 17:07:43 Michael Jones wrote:
>> To use the lane shifter, set different pixel formats at each end of
>> the link at the CCDC input.
>>
>> Signed-off-by: Michael Jones
>
> [snip]
>
>> diff --git a/drivers/media/video/omap3-isp/isp.h
>> b/drivers/media/video/omap3-isp/isp.h index 21fa88b..3d13f8b 100644
>> --- a/drivers/media/video/omap3-isp/isp.h
>> +++ b/drivers/media/video/omap3-isp/isp.h
>> @@ -144,8 +144,6 @@ struct isp_reg {
>> * ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
>> */
>> struct isp_parallel_platform_data {
>> -unsigned int width;
>> -unsigned int data_lane_shift:2;
>
> I'm afraid you can't remove the data_lane_shift field completely. Board can
> wire a 8 bits sensor to DATA[9:2] :-/ That needs to be taken into account as
> well when computing the total shift value.
>
> Hardware configuration can be done by adding the requested shift value to
> data_lane_shift for parallel sensors in omap3isp_configure_bridge(), but we
> also need to take it into account when validating the pipeline.
>
> I'm not aware of any board requiring data_lane_shift at the moment though, so
> we could just drop it now and add it back later when needed. This will avoid
> making this patch more complex.
>
I'm in favor of leaving it as is for now and adding it back when needed.
It's a good point, and I do think it should be supported in the long
run, but it'd be nice to not have to worry about it for this patch. Is
it OK with you to leave 'data_lane_shift' out for now?
>> unsigned int clk_pol:1;
>> unsigned int bridge:4;
>> };
>
[snip]
>> /* CCDC_PAD_SINK */
>> diff --git a/drivers/media/video/omap3-isp/ispvideo.c
>> b/drivers/media/video/omap3-isp/ispvideo.c index 3c3b3c4..decc744 100644
>> --- a/drivers/media/video/omap3-isp/ispvideo.c
>> +++ b/drivers/media/video/omap3-isp/ispvideo.c
>> @@ -47,41 +47,59 @@
>>
>> static struct isp_format_info formats[] = {
>
> [snip]
>
>> { V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
>> - V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
>> + V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
>
> Should this be
>
> V4L2_MBUS_FMT_SGRBG10_1X10, 0,
>
> instead ? DPCM formats are not shiftable. It won't make any difference in
> practice, as the format is already 8-bit wide, but you state in the flavor
> field documentation that "=0 if format is not shiftable".
I went back and forth on that since this format is already 8 bits wide
and no non-8-bit format will declare this as its flavor, since it really
is non-shiftable. Although- now that I explicitly say '=0 if format is
not shiftable', I suppose it does make sense to have flavor=0 here. I
will change that.
>
>> + V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
>
> [snip]
>
[snip]
>> +if (link_has_shifter) {
>> +if (!omap3isp_is_shiftable(fmt_source.format.code,
>> +fmt_sink.format.code)) {
>> +pr_debug("%s not shiftable.\n", __func__);
>
> Do we need the pr_debug call ?
No, that was an oversight.
>
>> +return -EPIPE;
>> +}
>> +} else if (fmt_source.format.code != fmt_sink.format.code)
>> +return -EPIPE;
>> }
>>
>> return 0;
>
-Michael
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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/4] omap3isp: lane shifter support
To use the lane shifter, set different pixel formats at each end of
the link at the CCDC input.
Signed-off-by: Michael Jones
---
drivers/media/video/omap3-isp/isp.c |6 +-
drivers/media/video/omap3-isp/isp.h |5 +-
drivers/media/video/omap3-isp/ispccdc.c | 26 +++-
drivers/media/video/omap3-isp/ispvideo.c | 97 +++--
drivers/media/video/omap3-isp/ispvideo.h |3 +
5 files changed, 108 insertions(+), 29 deletions(-)
diff --git a/drivers/media/video/omap3-isp/isp.c
b/drivers/media/video/omap3-isp/isp.c
index 08d90fe..68c6bcd 100644
--- a/drivers/media/video/omap3-isp/isp.c
+++ b/drivers/media/video/omap3-isp/isp.c
@@ -285,7 +285,8 @@ static void isp_power_settings(struct isp_device *isp, int
idle)
*/
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata)
+ const struct isp_parallel_platform_data *pdata,
+ int shift)
{
u32 ispctrl_val;
@@ -298,7 +299,6 @@ void omap3isp_configure_bridge(struct isp_device *isp,
switch (input) {
case CCDC_INPUT_PARALLEL:
ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
- ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
break;
@@ -319,6 +319,8 @@ void omap3isp_configure_bridge(struct isp_device *isp,
return;
}
+ ispctrl_val |= ((shift/2) << ISPCTRL_SHIFT_SHIFT) & ISPCTRL_SHIFT_MASK;
+
ispctrl_val &= ~ISPCTRL_SYNC_DETECT_MASK;
ispctrl_val |= ISPCTRL_SYNC_DETECT_VSRISE;
diff --git a/drivers/media/video/omap3-isp/isp.h
b/drivers/media/video/omap3-isp/isp.h
index 21fa88b..3d13f8b 100644
--- a/drivers/media/video/omap3-isp/isp.h
+++ b/drivers/media/video/omap3-isp/isp.h
@@ -144,8 +144,6 @@ struct isp_reg {
* ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
*/
struct isp_parallel_platform_data {
- unsigned int width;
- unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
unsigned int bridge:4;
};
@@ -322,7 +320,8 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
enum isp_pipeline_stream_state state);
void omap3isp_configure_bridge(struct isp_device *isp,
enum ccdc_input_entity input,
- const struct isp_parallel_platform_data *pdata);
+ const struct isp_parallel_platform_data *pdata,
+ int shift);
#define ISP_XCLK_NONE -1
#define ISP_XCLK_A 0
diff --git a/drivers/media/video/omap3-isp/ispccdc.c
b/drivers/media/video/omap3-isp/ispccdc.c
index 23000b6..923a08f 100644
--- a/drivers/media/video/omap3-isp/ispccdc.c
+++ b/drivers/media/video/omap3-isp/ispccdc.c
@@ -1120,21 +1120,39 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
struct isp_parallel_platform_data *pdata = NULL;
struct v4l2_subdev *sensor;
struct v4l2_mbus_framefmt *format;
+ int depth_in = 0, depth_out = 0;
+ int shift;
+ const struct isp_format_info *fmt_info;
+ struct v4l2_subdev_format fmt_src;
struct media_pad *pad;
unsigned long flags;
u32 syn_mode;
u32 ccdc_pattern;
+ pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
+ sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
- sensor = media_entity_to_v4l2_subdev(pad->entity);
pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
->bus.parallel;
}
- omap3isp_configure_bridge(isp, ccdc->input, pdata);
+ /* set syncif.datsz */
+ fmt_src.pad = pad->index;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
+ fmt_info = omap3isp_video_format_info(fmt_src.format.code);
+ depth_in = fmt_info ? fmt_info->bpp : 0;
+ }
+
+ /* find CCDC input format */
+ fmt_info = omap3isp_video_format_info
+ (isp->isp_ccdc.formats[CCDC_PAD_SINK].code);
+ depth_out = fmt_info ? fmt_info->bpp : 0;
+
+ shift = depth_in - depth_out;
+ omap3isp_configure_bridge(isp, ccdc->input, pdata, shift);
- ccdc->syncif.datsz = pdata ? pdata->width : 10;
+ ccdc->syncif.datsz = depth_out;
ccdc_config_sync_if(ccdc, &ccdc->syncif);
[PATCH v2 3/4] omap3isp: ccdc: support Y10/12, 8-bit bayer fmts
Signed-off-by: Michael Jones
---
drivers/media/video/omap3-isp/ispccdc.c |6 ++
drivers/media/video/omap3-isp/ispvideo.c | 12
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3-isp/ispccdc.c
b/drivers/media/video/omap3-isp/ispccdc.c
index e4d04ce..23000b6 100644
--- a/drivers/media/video/omap3-isp/ispccdc.c
+++ b/drivers/media/video/omap3-isp/ispccdc.c
@@ -43,6 +43,12 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct
v4l2_subdev_fh *fh,
static const unsigned int ccdc_fmts[] = {
V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12,
+ V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10,
V4L2_MBUS_FMT_SRGGB10_1X10,
V4L2_MBUS_FMT_SBGGR10_1X10,
diff --git a/drivers/media/video/omap3-isp/ispvideo.c
b/drivers/media/video/omap3-isp/ispvideo.c
index f16d787..3c3b3c4 100644
--- a/drivers/media/video/omap3-isp/ispvideo.c
+++ b/drivers/media/video/omap3-isp/ispvideo.c
@@ -48,6 +48,18 @@
static struct isp_format_info formats[] = {
{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
+ { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10, 10, },
+ { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_PIX_FMT_Y12, 12, },
+ { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8, },
+ { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8, },
+ { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8, },
+ { V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8, },
{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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/4] media: add 8-bit bayer formats and Y12
Signed-off-by: Michael Jones
Acked-by: Laurent Pinchart
---
include/linux/v4l2-mediabus.h |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 7054a7a..46caecd 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -47,8 +47,9 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
- /* YUV (including grey) - next is 0x2013 */
+ /* YUV (including grey) - next is 0x2014 */
V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
+ V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
V4L2_MBUS_FMT_YUYV8_1_5X8 = 0x2004,
@@ -67,9 +68,11 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
- /* Bayer - next is 0x3013 */
+ /* Bayer - next is 0x3015 */
V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001,
+ V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013,
V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002,
+ V4L2_MBUS_FMT_SRGGB8_1X8 = 0x3014,
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 = 0x300b,
V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c,
V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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/4] v4l: add V4L2_PIX_FMT_Y12 format
Signed-off-by: Michael Jones
Acked-by: Laurent Pinchart
---
include/linux/videodev2.h |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 02da9e7..6fac463 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y12 v4l2_fourcc('Y', '1', '2', ' ') /* 12 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 0/4] OMAP3-ISP lane shifter support
Add support for the ISP's lane shifter. To use the shifter, set different pixel formats at each end of the link at the CCDC input. This has only been tested shifting Y12 and SBGGR12 from a parallel sensor to Y8 and SBGGR8 (respectively) at the CCDC input. Support has also been added for other formats and other shifting values, but is untested. Shifting data coming from one of the serial sensor interfaces (CSI2a, etc) is also untested. As before, ccdc_try_format() does not check that the format at its input is compatible with the format coming from the sensor interface. This consistency check is first done when activating the pipeline. These patches apply to Laurent's media-0005-omap3isp branch, based on 2.6.38-rc5 Changes since v1: -added support for remaining 8-bit Bayer formats (SGBRG8_1X8 & SRGGB8_1X8) -moved omap3isp_is_shiftable() from isp.c to ispvideo.c and return bool -moved 'shift' calculation from omap3isp_configure_bridge() to ccdc_configure() -added 'shift' arg to omap3isp_configure_bridge() -misc minor changes according to feedback (removed unnecessary tests, etc.) Michael Jones (4): v4l: add V4L2_PIX_FMT_Y12 format media: add 8-bit bayer formats and Y12 omap3isp: ccdc: support Y10/12, 8-bit bayer fmts omap3isp: lane shifter support drivers/media/video/omap3-isp/isp.c |6 +- drivers/media/video/omap3-isp/isp.h |5 +- drivers/media/video/omap3-isp/ispccdc.c | 32 +- drivers/media/video/omap3-isp/ispvideo.c | 97 + drivers/media/video/omap3-isp/ispvideo.h |3 + include/linux/v4l2-mediabus.h|7 ++- include/linux/videodev2.h|1 + 7 files changed, 126 insertions(+), 25 deletions(-) -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set
On 03/08/2011 09:31 PM, Guzman Lugo, Fernando wrote:
> On Tue, Mar 8, 2011 at 2:15 PM, David Cohen wrote:
>> From: Michael Jones
>>
>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL
>> address if da_start==0, which would then not get unmapped. Disallow
>> this again if IOVMF_DA_ANON is set. And spell variable 'alignment'
>> correctly.
>>
>> Signed-off-by: Michael Jones
>> ---
>> arch/arm/plat-omap/iovmm.c | 16 ++--
>> 1 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
>> index 6dc1296..e5f8341 100644
>> --- a/arch/arm/plat-omap/iovmm.c
>> +++ b/arch/arm/plat-omap/iovmm.c
>> @@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct
>> iommu *obj, u32 da,
>> size_t bytes, u32 flags)
>> {
>>struct iovm_struct *new, *tmp;
>> - u32 start, prev_end, alignement;
>> + u32 start, prev_end, alignment;
>>
>>if (!obj || !bytes)
>>return ERR_PTR(-EINVAL);
>>
>>start = da;
>> - alignement = PAGE_SIZE;
>> + alignment = PAGE_SIZE;
>>
>>if (flags & IOVMF_DA_ANON) {
>> - start = obj->da_start;
>> + /* Don't map address 0 */
>> + if (obj->da_start)
>> + start = obj->da_start;
>> + else
>> + start = alignment;
>
> looks good to me, just a nitpick comment, that would look better
>
> start = (obj->da_start) ? obj->da_start : alignment;
>
> Regards,
> Fernando.
>
>>
>>if (flags & IOVMF_LINEAR)
>> - alignement = iopgsz_max(bytes);
>> - start = roundup(start, alignement);
>> + alignment = iopgsz_max(bytes);
>> + start = roundup(start, alignment);
>>} else if (start < obj->da_start || start > obj->da_end ||
>>obj->da_end - start < bytes) {
>>return ERR_PTR(-EINVAL);
>> @@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu
>> *obj, u32 da,
>>goto found;
>>
>>if (tmp->da_end >= start && flags & IOVMF_DA_ANON)
>> - start = roundup(tmp->da_end + 1, alignement);
>> + start = roundup(tmp->da_end + 1, alignment);
>>
>>prev_end = tmp->da_end;
>>}
>> --
>> 1.7.0.4
>>
Hi David,
These changes to my patch are fine with me. If you want to incorporate
Fernando's recommendation above, too, go ahead.
-Michael
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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] omap: iommu: disallow mapping NULL address
>From e7dbe4c4b64eb114f9b0804d6af3a3ca0e78acc8 Mon Sep 17 00:00:00 2001 From: Michael Jones Date: Mon, 7 Mar 2011 13:36:15 +0100 Subject: [PATCH] omap: iommu: disallow mapping NULL address commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL address if da_start==0. Force da_start to exclude the first page. Signed-off-by: Michael Jones --- arch/arm/plat-omap/iommu.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c index 5990ea6..dcb5513 100644 --- a/arch/arm/plat-omap/iommu.c +++ b/arch/arm/plat-omap/iommu.c @@ -850,7 +850,7 @@ int iommu_set_da_range(struct iommu *obj, u32 start, u32 end) if (end < start || !PAGE_ALIGN(start | end)) return -EINVAL; - obj->da_start = start; + obj->da_start = max(start, (u32)PAGE_SIZE); obj->da_end = end; return 0; @@ -950,7 +950,9 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev) obj->name = pdata->name; obj->dev = &pdev->dev; obj->ctx = (void *)obj + sizeof(*obj); - obj->da_start = pdata->da_start; + + /* reserve the first page for NULL */ + obj->da_start = max(pdata->da_start, (u32)PAGE_SIZE); obj->da_end = pdata->da_end; mutex_init(&obj->iommu_lock); -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 4/4] omap3isp: lane shifter support
Hi Laurent,
Thanks for the review.
On 03/04/2011 05:33 PM, Laurent Pinchart wrote:
> Hi Michael,
>
> Thanks for the patch.
>
> On Friday 04 March 2011 09:58:04 Michael Jones wrote:
>> Signed-off-by: Michael Jones
>> ---
>> drivers/media/video/omap3-isp/isp.c | 82
>> +- drivers/media/video/omap3-isp/isp.h |
>> 4 +-
>> drivers/media/video/omap3-isp/ispccdc.c |2 +-
>> drivers/media/video/omap3-isp/ispvideo.c | 65 +---
>> drivers/media/video/omap3-isp/ispvideo.h |3 +
>> 5 files changed, 134 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/media/video/omap3-isp/isp.c
>> b/drivers/media/video/omap3-isp/isp.c index 08d90fe..20e6daa 100644
>> --- a/drivers/media/video/omap3-isp/isp.c
>> +++ b/drivers/media/video/omap3-isp/isp.c
>> @@ -273,6 +273,44 @@ static void isp_power_settings(struct isp_device *isp,
>> int idle) }
>>
>> /*
>> + * Decide whether desired output pixel code can be obtained with
>> + * the lane shifter by shifting the input pixel code.
>> + * return 1 if the combination is possible
>> + * return 0 otherwise
>> + */
>> +int omap3isp_is_shiftable(enum v4l2_mbus_pixelcode in,
>> +enum v4l2_mbus_pixelcode out)
>
> As you only use this function in ispvideo.c, I would move it there. You could
> also make the function return a bool.
I thought returning a bool was inconsistent with kernel style (e.g.
isp_pipeline_is_last, ccdc_lsc_is_configured return int).
>
>> +{
>> +const struct isp_format_info *in_info, *out_info;
>> +int shiftable = 0;
>> +if ((in == 0) || (out == 0))
>> +return 0;
>
> Can this happen ?
>
>> +
>> +if (in == out)
>> +return 1;
>> +
>> +in_info = omap3isp_video_format_info(in);
>> +out_info = omap3isp_video_format_info(out);
>> +if ((!in_info) || (!out_info))
>> +return 0;
>
> Can this happen ?
>
These were all relics of previous versions when I was also calling
omap3isp_is_shiftable() from ccdc_try_format(). I will move it to
ispvideo.c and remove the two if statements.
>> +
>> +if (in_info->flavor != out_info->flavor)
>> +return 0;
>> +
>> +switch (in_info->bpp - out_info->bpp) {
>> +case 2:
>> +case 4:
>> +case 6:
>> +shiftable = 1;
>> +break;
>> +default:
>> +shiftable = 0;
>> +}
>
> What about
>
> return in_info->bpp - out_info->bpp <= 6;
As long as there are never formats which are the same flavor but shifted
1, 3, or 5 bits, that's fine. I suppose this is a safe assumption?
>
>> +
>> +return shiftable;
>> +}
>> +
>> +/*
>> * Configure the bridge and lane shifter. Valid inputs are
>> *
>> * CCDC_INPUT_PARALLEL: Parallel interface
>> @@ -288,6 +326,10 @@ void omap3isp_configure_bridge(struct isp_device *isp,
>> const struct isp_parallel_platform_data *pdata)
>> {
>> u32 ispctrl_val;
>> +u32 depth_in = 0, depth_out = 0;
>> +u32 shift;
>> +const struct isp_format_info *fmt_info;
>> +struct media_pad *srcpad;
>>
>> ispctrl_val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL);
>> ispctrl_val &= ~ISPCTRL_SHIFT_MASK;
>> @@ -298,7 +340,6 @@ void omap3isp_configure_bridge(struct isp_device *isp,
>> switch (input) {
>> case CCDC_INPUT_PARALLEL:
>> ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
>> -ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
>> ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
>> ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
>> break;
>> @@ -319,6 +360,45 @@ void omap3isp_configure_bridge(struct isp_device *isp,
>> return;
>> }
>>
>> +/* find output format from the remote end of the link connected to
>> + * CCDC sink pad
>> + */
>> +srcpad = media_entity_remote_source(&isp->isp_ccdc.pads[CCDC_PAD_SINK]);
>> +if (srcpad == NULL)
>> +dev_err(isp->dev, "No active input to CCDC.\n");
>
> There's no need to test for NULL, as this function will only be called on
> streamon, so the pipeline will be valid.
OK.
>
>> +if (media_entity_type(srcpad->entity) == MEDIA_ENT_T_V4L2_SUBDEV)
Re: omap3isp cache error when unloading
Hi David,
On 03/04/2011 02:12 PM, David Cohen wrote:
> Hi,
>
> [snip]
>
>> Sorry, I should've mentioned: I'm using your media-0005-omap3isp branch
>> based on 2.6.38-rc5. I didn't have the problem with 2.6.37, either.
>> It's actually not related to mis-configuring the ISP pipeline like I
>> thought at first- it also happens after I have successfully captured images.
>>
>> I've since tracked down the problem, although I don't understand the
>> cache management well enough to be sure it's a proper fix, so hopefully
>> some new recipients on this can make suggestions/comments.
>>
>> The patch below solves the problem, which modifies a commit by Fernando
>> Guzman Lugo from December.
>>
>> -Michael
>>
>> From db35fb8edca2a4f8fd37197d77fd58676cb1dcac Mon Sep 17 00:00:00 2001
>> From: Michael Jones
>> Date: Thu, 3 Mar 2011 16:50:39 +0100
>> Subject: [PATCH] fix iovmm slab cache error on module unload
>>
>> modify "OMAP: iommu: create new api to set valid da range"
>>
>> This modifies commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb.
>> ---
>> arch/arm/plat-omap/iovmm.c |5 -
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
>> index 6dc1296..2fba6f1 100644
>> --- a/arch/arm/plat-omap/iovmm.c
>> +++ b/arch/arm/plat-omap/iovmm.c
>> @@ -280,7 +280,10 @@ static struct iovm_struct *alloc_iovm_area(struct iommu
>> *obj, u32 da,
>>alignement = PAGE_SIZE;
>>
>>if (flags & IOVMF_DA_ANON) {
>> - start = obj->da_start;
>> + /*
>> +* Reserve the first page for NULL
>> +*/
>> + start = obj->da_start + PAGE_SIZE;
>
> IMO if obj->da_start != 0, no need to add PAGE_SIZE. Otherwise, it
> does make sense to correct wrong obj->da_start == 0. Another thing is
> this piece of code is using alignement (alignment) variable instead of
> PAGE_SIZE (which is the same value).
>
> Br,
>
> David
I believe the following patch addresses your comments. I couldn't
resist also fixing the misspelling of alignment when I was using the
variable in my patch.
-Michael
>From 2712f2fd087ca782e964c912c7f1973e7d84f2b7 Mon Sep 17 00:00:00 2001
From: Michael Jones
Date: Fri, 4 Mar 2011 15:09:48 +0100
Subject: [PATCH] omap: iovmm: disallow mapping NULL address
commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping
the NULL address if da_start==0, which would then not get unmapped.
Disallow this again. And spell variable 'alignment' correctly.
Signed-off-by: Michael Jones
---
arch/arm/plat-omap/iovmm.c | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 6dc1296..11c9b76 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct iommu
*obj, u32 da,
size_t bytes, u32 flags)
{
struct iovm_struct *new, *tmp;
- u32 start, prev_end, alignement;
+ u32 start, prev_end, alignment;
if (!obj || !bytes)
return ERR_PTR(-EINVAL);
start = da;
- alignement = PAGE_SIZE;
+ alignment = PAGE_SIZE;
if (flags & IOVMF_DA_ANON) {
- start = obj->da_start;
+ /* Don't map address 0 */
+ if (obj->da_start)
+ start = obj->da_start;
+ else
+ start = obj->da_start + alignment;
if (flags & IOVMF_LINEAR)
- alignement = iopgsz_max(bytes);
- start = roundup(start, alignement);
+ alignment = iopgsz_max(bytes);
+ start = roundup(start, alignment);
} else if (start < obj->da_start || start > obj->da_end ||
obj->da_end - start < bytes) {
return ERR_PTR(-EINVAL);
@@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu
*obj, u32 da,
goto found;
if (tmp->da_end >= start && flags & IOVMF_DA_ANON)
- start = roundup(tmp->da_end + 1, alignement);
+ start = roundup(tmp->da_end + 1, alignment);
prev_end = tmp->da_end;
}
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 4/4] omap3isp: lane shifter support
Signed-off-by: Michael Jones
---
drivers/media/video/omap3-isp/isp.c | 82 +-
drivers/media/video/omap3-isp/isp.h |4 +-
drivers/media/video/omap3-isp/ispccdc.c |2 +-
drivers/media/video/omap3-isp/ispvideo.c | 65 +---
drivers/media/video/omap3-isp/ispvideo.h |3 +
5 files changed, 134 insertions(+), 22 deletions(-)
diff --git a/drivers/media/video/omap3-isp/isp.c
b/drivers/media/video/omap3-isp/isp.c
index 08d90fe..20e6daa 100644
--- a/drivers/media/video/omap3-isp/isp.c
+++ b/drivers/media/video/omap3-isp/isp.c
@@ -273,6 +273,44 @@ static void isp_power_settings(struct isp_device *isp, int
idle)
}
/*
+ * Decide whether desired output pixel code can be obtained with
+ * the lane shifter by shifting the input pixel code.
+ * return 1 if the combination is possible
+ * return 0 otherwise
+ */
+int omap3isp_is_shiftable(enum v4l2_mbus_pixelcode in,
+ enum v4l2_mbus_pixelcode out)
+{
+ const struct isp_format_info *in_info, *out_info;
+ int shiftable = 0;
+ if ((in == 0) || (out == 0))
+ return 0;
+
+ if (in == out)
+ return 1;
+
+ in_info = omap3isp_video_format_info(in);
+ out_info = omap3isp_video_format_info(out);
+ if ((!in_info) || (!out_info))
+ return 0;
+
+ if (in_info->flavor != out_info->flavor)
+ return 0;
+
+ switch (in_info->bpp - out_info->bpp) {
+ case 2:
+ case 4:
+ case 6:
+ shiftable = 1;
+ break;
+ default:
+ shiftable = 0;
+ }
+
+ return shiftable;
+}
+
+/*
* Configure the bridge and lane shifter. Valid inputs are
*
* CCDC_INPUT_PARALLEL: Parallel interface
@@ -288,6 +326,10 @@ void omap3isp_configure_bridge(struct isp_device *isp,
const struct isp_parallel_platform_data *pdata)
{
u32 ispctrl_val;
+ u32 depth_in = 0, depth_out = 0;
+ u32 shift;
+ const struct isp_format_info *fmt_info;
+ struct media_pad *srcpad;
ispctrl_val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL);
ispctrl_val &= ~ISPCTRL_SHIFT_MASK;
@@ -298,7 +340,6 @@ void omap3isp_configure_bridge(struct isp_device *isp,
switch (input) {
case CCDC_INPUT_PARALLEL:
ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
- ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
break;
@@ -319,6 +360,45 @@ void omap3isp_configure_bridge(struct isp_device *isp,
return;
}
+ /* find output format from the remote end of the link connected to
+* CCDC sink pad
+*/
+ srcpad = media_entity_remote_source(&isp->isp_ccdc.pads[CCDC_PAD_SINK]);
+ if (srcpad == NULL)
+ dev_err(isp->dev, "No active input to CCDC.\n");
+
+ if (media_entity_type(srcpad->entity) == MEDIA_ENT_T_V4L2_SUBDEV) {
+ struct v4l2_subdev *subdev =
+ media_entity_to_v4l2_subdev(srcpad->entity);
+ struct v4l2_subdev_format fmt_src;
+ fmt_src.pad = srcpad->index;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ if (!v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt_src)) {
+ fmt_info =
+ omap3isp_video_format_info(fmt_src.format.code);
+ depth_in = fmt_info ? fmt_info->bpp : 0;
+ }
+ }
+
+ /* find CCDC input format */
+ fmt_info = omap3isp_video_format_info
+ (isp->isp_ccdc.formats[CCDC_PAD_SINK].code);
+ depth_out = fmt_info ? fmt_info->bpp : 0;
+
+ isp->isp_ccdc.syncif.datsz = depth_out;
+
+ /* determine necessary shifting */
+ if (depth_in == depth_out + 6)
+ shift = 3;
+ else if (depth_in == depth_out + 4)
+ shift = 2;
+ else if (depth_in == depth_out + 2)
+ shift = 1;
+ else
+ shift = 0;
+
+ ispctrl_val |= shift << ISPCTRL_SHIFT_SHIFT;
+
ispctrl_val &= ~ISPCTRL_SYNC_DETECT_MASK;
ispctrl_val |= ISPCTRL_SYNC_DETECT_VSRISE;
diff --git a/drivers/media/video/omap3-isp/isp.h
b/drivers/media/video/omap3-isp/isp.h
index 21fa88b..c84d349 100644
--- a/drivers/media/video/omap3-isp/isp.h
+++ b/drivers/media/video/omap3-isp/isp.h
@@ -144,8 +144,6 @@ struct isp_reg {
* ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
*/
struct isp_parallel_platform_data {
- unsigned int width;
- unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
unsigned int bridg
[PATCH 3/4] omap3isp: ccdc: support Y10, Y12, SGRBG8, SBGGR8
Signed-off-by: Michael Jones
---
drivers/media/video/omap3-isp/ispccdc.c |4
drivers/media/video/omap3-isp/ispvideo.c |8
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/omap3-isp/ispccdc.c
b/drivers/media/video/omap3-isp/ispccdc.c
index e4d04ce..166115d 100644
--- a/drivers/media/video/omap3-isp/ispccdc.c
+++ b/drivers/media/video/omap3-isp/ispccdc.c
@@ -43,6 +43,10 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct
v4l2_subdev_fh *fh,
static const unsigned int ccdc_fmts[] = {
V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12,
+ V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10,
V4L2_MBUS_FMT_SRGGB10_1X10,
V4L2_MBUS_FMT_SBGGR10_1X10,
diff --git a/drivers/media/video/omap3-isp/ispvideo.c
b/drivers/media/video/omap3-isp/ispvideo.c
index f16d787..c406043 100644
--- a/drivers/media/video/omap3-isp/ispvideo.c
+++ b/drivers/media/video/omap3-isp/ispvideo.c
@@ -48,6 +48,14 @@
static struct isp_format_info formats[] = {
{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
+ { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10, 10, },
+ { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_PIX_FMT_Y12, 12, },
+ { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8, },
+ { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8, },
{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 2/4] media: add 8-bit bayer formats and Y12
Signed-off-by: Michael Jones
---
include/linux/v4l2-mediabus.h |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 7054a7a..46caecd 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -47,8 +47,9 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
- /* YUV (including grey) - next is 0x2013 */
+ /* YUV (including grey) - next is 0x2014 */
V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
+ V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
V4L2_MBUS_FMT_YUYV8_1_5X8 = 0x2004,
@@ -67,9 +68,11 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
- /* Bayer - next is 0x3013 */
+ /* Bayer - next is 0x3015 */
V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001,
+ V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013,
V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002,
+ V4L2_MBUS_FMT_SRGGB8_1X8 = 0x3014,
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 = 0x300b,
V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c,
V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009,
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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/4] v4l: add V4L2_PIX_FMT_Y12 format
Signed-off-by: Michael Jones
---
include/linux/videodev2.h |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 02da9e7..6fac463 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -288,6 +288,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y4 v4l2_fourcc('Y', '0', '4', ' ') /* 4 Greyscale
*/
#define V4L2_PIX_FMT_Y6 v4l2_fourcc('Y', '0', '6', ' ') /* 6 Greyscale
*/
#define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10 Greyscale
*/
+#define V4L2_PIX_FMT_Y12 v4l2_fourcc('Y', '1', '2', ' ') /* 12 Greyscale
*/
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
/* Palette formats */
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 0/4] OMAP3-ISP lane shifter support
Add support for the ISP's lane shifter. To use the shifter, set different pixel formats at each end of the link at the CCDC input. This has only been tested shifting Y12 and SBGGR12 from a parallel sensor to Y8 and SBGGR12 (respectively) at the CCDC input. Support has also been added for other formats and other shifting values, but is untested. Shifting data coming from one of the serial sensor interfaces (CSI2a, etc) is also untested. As before, ccdc_try_format() does not check that the format at its input is compatible with the format coming from the sensor interface. This consistency check is first done when activating the pipeline. These patches apply to Laurent's media-0005-omap3isp branch, based on 2.6.38-rc5 Michael Jones (4): v4l: add V4L2_PIX_FMT_Y12 format media: add 8-bit bayer formats and Y12 omap3isp: ccdc: support Y10, Y12, SGRBG8, SBGGR8 omap3isp: lane shifter support drivers/media/video/omap3-isp/isp.c | 82 +- drivers/media/video/omap3-isp/isp.h |4 +- drivers/media/video/omap3-isp/ispccdc.c |6 ++- drivers/media/video/omap3-isp/ispvideo.c | 65 ++- drivers/media/video/omap3-isp/ispvideo.h |3 + include/linux/v4l2-mediabus.h|7 ++- include/linux/videodev2.h|1 + 7 files changed, 148 insertions(+), 20 deletions(-) -- 1.7.4.1 MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: omap3isp cache error when unloading
On 03/02/2011 08:18 PM, Laurent Pinchart wrote:
> Hi Michael,
>
> On Tuesday 01 March 2011 17:41:01 Michael Jones wrote:
>> Hi all,
>>
>> I get a warning about a cache error with the following steps:
>>
>> 0. load omap3-isp
>> 1. set up media broken media pipeline. (e.g. set different formats on
>> opposite ends of a link, as will be the case for using the lane shifter)
>> 2. try to capture images. isp_video_streamon() returns -EPIPE from the
>> failed isp_video_validate_pipeline() call.
>> 3. unload omap3-isp module
>>
>> then I get the following from kmem_cache_destroy():
>>
>> slab error in kmem_cache_destroy(): cache `iovm_area_cache': Can't free all
>> objects [] (unwind_backtrace+0x0/0xec) from []
>> (kmem_cache_destroy+0x88/0xf4) [] (kmem_cache_destroy+0x88/0xf4)
>> from [] (sys_delete_module+0x1c4/0x230) []
>> (sys_delete_module+0x1c4/0x230) from []
>> (ret_fast_syscall+0x0/0x30)
>>
>> Then, when reloading the module:
>> SLAB: cache with size 32 has lost its name
>>
>> Can somebody else confirm that they also observe this behavior?
>
> I can't reproduce that (tried both 2.6.32 and 2.6.37). Could you give me some
> more details about your exact test procedure (such as how you configure the
> pipeline) ?
>
Sorry, I should've mentioned: I'm using your media-0005-omap3isp branch
based on 2.6.38-rc5. I didn't have the problem with 2.6.37, either.
It's actually not related to mis-configuring the ISP pipeline like I
thought at first- it also happens after I have successfully captured images.
I've since tracked down the problem, although I don't understand the
cache management well enough to be sure it's a proper fix, so hopefully
some new recipients on this can make suggestions/comments.
The patch below solves the problem, which modifies a commit by Fernando
Guzman Lugo from December.
-Michael
>From db35fb8edca2a4f8fd37197d77fd58676cb1dcac Mon Sep 17 00:00:00 2001
From: Michael Jones
Date: Thu, 3 Mar 2011 16:50:39 +0100
Subject: [PATCH] fix iovmm slab cache error on module unload
modify "OMAP: iommu: create new api to set valid da range"
This modifies commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb.
---
arch/arm/plat-omap/iovmm.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 6dc1296..2fba6f1 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -280,7 +280,10 @@ static struct iovm_struct *alloc_iovm_area(struct iommu
*obj, u32 da,
alignement = PAGE_SIZE;
if (flags & IOVMF_DA_ANON) {
- start = obj->da_start;
+ /*
+* Reserve the first page for NULL
+*/
+ start = obj->da_start + PAGE_SIZE;
if (flags & IOVMF_LINEAR)
alignement = iopgsz_max(bytes);
--
1.7.4.1
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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
omap3isp cache error when unloading
Hi all, I get a warning about a cache error with the following steps: 0. load omap3-isp 1. set up media broken media pipeline. (e.g. set different formats on opposite ends of a link, as will be the case for using the lane shifter) 2. try to capture images. isp_video_streamon() returns -EPIPE from the failed isp_video_validate_pipeline() call. 3. unload omap3-isp module then I get the following from kmem_cache_destroy(): slab error in kmem_cache_destroy(): cache `iovm_area_cache': Can't free all objects [] (unwind_backtrace+0x0/0xec) from [] (kmem_cache_destroy+0x88/0xf4) [] (kmem_cache_destroy+0x88/0xf4) from [] (sys_delete_module+0x1c4/0x230) [] (sys_delete_module+0x1c4/0x230) from [] (ret_fast_syscall+0x0/0x30) Then, when reloading the module: SLAB: cache with size 32 has lost its name Can somebody else confirm that they also observe this behavior? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ISP lane shifter support
Hans, can you weigh in on this? I'm waiting to submit a patch to implement lane shifter support until I get a consensus what the best approach is. Laurent and Sakari favor having a different format on the sensor output than on the CCDC input to indicate a shift. If you agree that this is a sensible approach, I will go ahead and submit my patch soon. thanks, Michael On 02/11/2011 02:06 PM, Laurent Pinchart wrote: > Hi Michael, > > On Friday 11 February 2011 13:07:33 Michael Jones wrote: >> On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote: >>> Looking at the "Data-Lane Shifter" table (12.27 in my datasheet, in the >>> "Bridge-Lane Shifter" chapter), I think, the first two columns are fixed >>> by the board design, right? So, our freedom lies only in one line there >>> and is a single parameter - the shift value. The output shifter (VPIN) is >>> independent from this one, but not unrelated. It seems logical to me to >>> relate the former one to CCDC's input pad, and the latter one to CCDC's >>> output pad. AFAIU, Laurent, your implementation in what concerns pad >>> configuration is: let the user configure all interfaces independently, >>> and first when we have to actually activate the pipeline (start >>> streaming or configure video buffers) we can verify, whether all parts >>> fit together. >> >> I would like to add this lane shifter support. Would you like me to >> implement it as Guennadi suggested- letting the user set all 3 CCDC pad >> formats arbitrarily and postpone the consistency checks to streamon time? > > I've discussed this with Sakari Ailus, and we would implement it with > different formats on the sensor output and the CCDC input. I'd like to get > Hans Verkuil's opinion. > MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: link error w/ media-0006-sensors
Hi Laurent,
sorry to resurrect this from a month ago...
I've continued to export omap_pm_set_min_bus_tput() to enable building
the omap3-isp module, although Paul Wamsley's reply you referred to
clearly indicates that this is the wrong approach.
Aren't you also building omap3-isp as a module? How are you guys
getting around this?
-Michael
On 01/19/2011 12:30 AM, Laurent Pinchart wrote:
> Hi Michael,
>
> On Tuesday 18 January 2011 17:14:37 Michael Jones wrote:
>> Hi Laurent & Sakari,
>>
>> On Laurent's media-0006-sensors branch, when compiling with
>> CONFIG_VIDEO_OMAP3=m, I got the following linking error:
>>
>> ERROR: "omap_pm_set_min_bus_tput" [drivers/media/video/isp/omap3-isp.ko]
>> undefined!
>>
>> I can get rid of the error with the patch below. But as always, I
>> wonder: Why didn't anybody else come across this error? Are you all
>> compiling with VIDEO_OMAP3=y? Is there a config file somewhere I can see
>> where someone is using that?
>>
>> And would anything be wrong with the patch below?
>
> Martin Hostettler sent the same patch to linux-omap today ("[PATCH] OMAP: PM:
> Export omap_pm_set_min_bus_tput to modules"). See Please see Paul Wamsley's
> answer on the list.
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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] ISP lane shifter support
Hi Laurent, On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote: > Looking at the "Data-Lane Shifter" table (12.27 in my datasheet, in the > "Bridge-Lane Shifter" chapter), I think, the first two columns are fixed > by the board design, right? So, our freedom lies only in one line there > and is a single parameter - the shift value. The output shifter (VPIN) is > independent from this one, but not unrelated. It seems logical to me to > relate the former one to CCDC's input pad, and the latter one to CCDC's > output pad. AFAIU, Laurent, your implementation in what concerns pad > configuration is: let the user configure all interfaces independently, and > first when we have to actually activate the pipeline (start streaming or > configure video buffers) we can verify, whether all parts fit together. I would like to add this lane shifter support. Would you like me to implement it as Guennadi suggested- letting the user set all 3 CCDC pad formats arbitrarily and postpone the consistency checks to streamon time? > So, why don't we stay consistent and do the same here? Give the user both > parameters and see how clever they were in the end;) I also think, if we > later decide to add some consistency checks, we can always do it. > > Thanks > Guennadi Thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ISP lane shifter support
Hi Guennadi, Laurent, On 01/27/2011 12:46 AM, Guennadi Liakhovetski wrote: >> >>> I didn't realize the video port can further shift the data. Where can I >>> find this in the TRM? >> >> VPIN field of the CCDC_FMTCFG register. > > This only plays a role, if cam_d is set to 10 bits raw in > CCDC_SYN_MODE.DATSIZ, right? > I didn't understand from the TRM that this is the case. I also didn't see that VPIN is only relevant with parallel data. Is it so? >> It could be, yes. The other option is to modify the format at the CCDC >> input. >> I agree that both options have drawbacks. >> >> Hans, Guennadi, any opinion on this ? > > Looking at the "Data-Lane Shifter" table (12.27 in my datasheet, in the > "Bridge-Lane Shifter" chapter), I think, the first two columns are fixed > by the board design, right? So, our freedom lies only in one line there > and is a single parameter - the shift value. The output shifter (VPIN) is > independent from this one, but not unrelated. It seems logical to me to > relate the former one to CCDC's input pad, and the latter one to CCDC's > output pad. AFAIU, Laurent, your implementation in what concerns pad > configuration is: let the user configure all interfaces independently, and > first when we have to actually activate the pipeline (start streaming or > configure video buffers) we can verify, whether all parts fit together. > So, why don't we stay consistent and do the same here? Give the user both > parameters and see how clever they were in the end;) I also think, if we > later decide to add some consistency checks, we can always do it. > Are you proposing having different formats on each end of the link between the sensor and the CCDC? Or do you agree with my favored approach that the lane shift value be determined by the difference between the CCDC input format and the CCDC output format(s)? I assume the VPIN value will always just select the upper 10 bits of the format which the CCDC is outputting on its other output pad. I understand that Laurent is out until Monday, so I'll wait for him to weigh in on this again before moving forward. > Thanks > Guennadi > thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ISP lane shifter support
Hi Laurent, On 01/24/2011 08:45 PM, Laurent Pinchart wrote: > Hi Michael, > > On Monday 24 January 2011 15:16:28 Michael Jones wrote: >> On 01/24/2011 02:57 PM, Laurent Pinchart wrote: >> >> >>>>> As the lane shifter is located at the CCDC input, it might be easier to >>>>> implement support for this using the CCDC input format. ispvideo.c >>>>> would need to validate the pipeline when the output of the entity >>>>> connected to the CCDC input (parallel sensor, CCP2 or CSI2) is >>>>> configured with a format that can be shifted to the format at the CCDC >>>>> input. >>>> >>>> This crossed my mind, but it seems illogical to have a link with a >>>> different format at each of its ends. >>> >>> I agree in theory, but it might be problematic for the CCDC. Right now >>> the CCDC can write to memory or send the data to the preview engine, but >>> not both at the same time. That's something that I'd like to change in >>> the future. What happens if the user then sets different widths on the >>> output pads ? >> >> Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format >> in the first place? By "prohibit", I mean shouldn't we be sure that the >> pixel format on pad 1 is always the same as on pad 2? > > Yes we should (although we could have a larger width on the memory write > port, > as the video port can further shift the data). Doesn't this conflict with your comment below that we shouldn't silently change pad 1 when setting pad 2? How can we ensure that they're always the same if a change in one doesn't result in a change in the other? See my example below. I didn't realize the video port can further shift the data. Where can I find this in the TRM? > >> Downside: this suggests that set_fmt on pad 2 could change the fmt on pad 1, >> which may be unexpected. But that does at least reflect the reality of the >> hardware, right? > > I don't think it would be a good idea to silently change formats on pad 1 > when > setting the format on pad 2. Applications don't expect that. That's why I've > proposed changing the format on pad 0 instead. I agree that it would be > better > to have the same format on the sensor output and on CCDC pad 0 though. > I don't understand how we can change the pixel format on pad 1 without also changing it on pad 2. Let me take a simple example: 0. Default state: all 3 CCDC pads have SGRBG10. 1. Sensor delivers Y10, so I set CCDC pad 0 to Y10. CCDC then changes format of pad 1&2 to Y10 also. 2. I want 8-bit data written to memory, so I set Y8 on pad 1 to use the shifter. Pad 0 stays Y10, but pad 2 can no longer get Y10, so (?) it must be changed to Y8. And I have to allow the change on pad 1 to be able to use the shifter at all. I agree applications may not expect this behavior. They may _expect_ that they can get Y10 to the video port and Y8 to memory, but they can't. Isn't this just what we pay for the simplicity of building the lane shifter into the CCDC subdev rather than creating its own subdev? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ISP lane shifter support
Hi Laurent, On 01/24/2011 02:57 PM, Laurent Pinchart wrote: >>> >>> As the lane shifter is located at the CCDC input, it might be easier to >>> implement support for this using the CCDC input format. ispvideo.c would >>> need to validate the pipeline when the output of the entity connected to >>> the CCDC input (parallel sensor, CCP2 or CSI2) is configured with a >>> format that can be shifted to the format at the CCDC input. >> >> This crossed my mind, but it seems illogical to have a link with a >> different format at each of its ends. > > I agree in theory, but it might be problematic for the CCDC. Right now the > CCDC can write to memory or send the data to the preview engine, but not both > at the same time. That's something that I'd like to change in the future. > What > happens if the user then sets different widths on the output pads ? > Shouldn't we prohibit the user from doing this in ccdc_[try/set]_format in the first place? By "prohibit", I mean shouldn't we be sure that the pixel format on pad 1 is always the same as on pad 2? Downside: this suggests that set_fmt on pad 2 could change the fmt on pad 1, which may be unexpected. But that does at least reflect the reality of the hardware, right? Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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] ISP lane shifter support
Hi Laurent, Thanks for the feedback. On 01/24/2011 01:10 AM, Laurent Pinchart wrote: > Hi Michael, > > On Friday 21 January 2011 09:40:21 Michael Jones wrote: >> Hi all, >> >> In the OMAP ISP driver, I'm interested in being able to choose between >> 8-bit and 12-bit formats when I have a 12-bit sensor attached. At the >> moment it looks like it's only possible to define this statically with >> data_lane_shift in the board definition. But with the ISP's lane >> shifter, it should be possible for the application to ask for 8-bits >> although it has a 12-bit sensor attached. > > That's right. This would be an interesting feature for the driver. It's also > possible to implement this at the input of the video port (but only when > forwarding data to the preview engine). True, but I do also want the feature available for data written directly to memory. > >> Has anybody already begun implementing this functionality? > > Not that I know of. > >> One approach that comes to mind is to create a subdev for the >> bridge/lane shifter in front of the CCDC, but this also seems a bit >> overkill. Otherwise, perhaps consider the lane shifter a part of the >> CCDC and put the code in there? > > I would keep the code in isp.c, and call it from ccdc_configure(). It should > just be a matter of adding an argument to the function. It seems unnecessary to add an arg to ccdc_configure (that's what I understood you to mean). It gets isp_ccdc_device, which has all the necessary info (pixel format in, which output is active, pixel format out). Seems like I could implement it entirely within isp_configure_bridge(). And of course some changes in ccdc_[try/set]_format(). This is what I will try to do. > >> Then ccdc_try_format() would have to check whether the sink pad has a pixel >> format which is shiftable to the requested pixel format on the source pad. >> A problem with this might be if there are architectures which have a CCDC >> but no shifter. > > The CCDC module already calls isp_configure_bridge(), so I don't think it's > an > issue for now. Let's fix the code when (and if) we start using the driver on > a > platform without a lane shifter. Agreed. > >> Are there other approaches I'm not considering? Or problems I'm >> overlooking? > > As the lane shifter is located at the CCDC input, it might be easier to > implement support for this using the CCDC input format. ispvideo.c would need > to validate the pipeline when the output of the entity connected to the CCDC > input (parallel sensor, CCP2 or CSI2) is configured with a format that can be > shifted to the format at the CCDC input. This crossed my mind, but it seems illogical to have a link with a different format at each of its ends. For instance, I think it is a sensible assumption that media-ctl automatically sets the format at the remote end of a link if you're setting an output pad's format. This is when I thought a subdev of its own would be more logically consistent with the media controller framework (although overkill). > >> Also- it looks like the CCDC now supports writing 12-bit bayer >> data to memory. Is that true? > > That's correct. It will support 8-bit grey data soon (patches have been > submitted internally already). > Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
[RFC] ISP lane shifter support
Hi all, In the OMAP ISP driver, I'm interested in being able to choose between 8-bit and 12-bit formats when I have a 12-bit sensor attached. At the moment it looks like it's only possible to define this statically with data_lane_shift in the board definition. But with the ISP's lane shifter, it should be possible for the application to ask for 8-bits although it has a 12-bit sensor attached. Has anybody already begun implementing this functionality? One approach that comes to mind is to create a subdev for the bridge/lane shifter in front of the CCDC, but this also seems a bit overkill. Otherwise, perhaps consider the lane shifter a part of the CCDC and put the code in there? Then ccdc_try_format() would have to check whether the sink pad has a pixel format which is shiftable to the requested pixel format on the source pad. A problem with this might be if there are architectures which have a CCDC but no shifter. Are there other approaches I'm not considering? Or problems I'm overlooking? Also- it looks like the CCDC now supports writing 12-bit bayer data to memory. Is that true? thanks for your thoughts, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 V2] v4l: OMAP3 ISP CCDC: Add support for 8bit greyscale sensors
Hi Laurent,
On 01/19/2011 05:38 PM, Laurent Pinchart wrote:
> Hi Michael,
>
@@ -1144,10 +1148,15 @@ static void ccdc_configure(struct
isp_ccdc_device *ccdc) else
syn_mode &= ~ISPCCDC_SYN_MODE_SDR2RSZ;
- isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
+ /* Use PACK8 mode for 1byte per pixel formats */
- /* CCDC_PAD_SINK */
- format = &ccdc->formats[CCDC_PAD_SINK];
+ if (isp_video_format_info(format->code)->bpp <= 8)
+ syn_mode |= ISPCCDC_SYN_MODE_PACK8;
+ else
+ syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
+
>>
>> It would make sense to me to move this bit into ispccdc_config_sync_if().
>
> Why do you think so ? This configures how the data is written to memory,
> while
> ispccdc_config_sync_if() configures the CCDC input interface.
I see. I was only thinking of ispccdc_config_sync_if() as configuring
the CCDC_SYN_MODE register. I was in fact wondering why the other
syn_mode assignments weren't made in there. Now that I understand the
division, I agree that setting PACK8 makes sense wherever the other
memory-writing settings are.
>
+
+ isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
/* Mosaic filter */
switch (format->code) {
@@ -2244,7 +2253,12 @@ int isp_ccdc_init(struct isp_device *isp)
ccdc->syncif.vdpol = 0;
ccdc->clamp.oblen = 0;
- ccdc->clamp.dcsubval = 64;
+
+ if (isp->pdata->subdevs->interface == ISP_INTERFACE_PARALLEL
+ && isp->pdata->subdevs->bus.parallel.width <= 8)
+ ccdc->clamp.dcsubval = 0;
+ else
+ ccdc->clamp.dcsubval = 64;
>>>
>>> I don't like this too much. What happens if you have several sensors
>>> connected to the system with different bus width ?
>>
>> I see Laurent's point here. Maybe move the dcsubval assignment into
>> ccdc_configure(). Also, don't we also want to remove dcsubval for an
>> 8-bit serially-attached sensor? In ccdc_configure() you could make it
>> conditional on the mbus format's width on the CCDC sink pad.
>
> This piece of code only sets the default value. If the user sets another
> value, the driver must not override it silently when the video stream is
> started. I'm not really sure how to properly fix this. The best solution is
> of
> course to set the value from userspace.
I see the predicament. 64 is a bad default value for 8-bit data, but we
can't at init time know whether we're going to have 8-bit data or
10(+)-bit data to set a different default. And you can't make a 64-or-0
decision at runtime because the user may have set a custom value after
init (although this isn't currently possible). But I don't think a user
should have to adjust dcsubval just because he changed to an 8-bit image
and wants a decent image. Especially since at the moment the user can't
do that anyway. What I keep coming back to, though it sounds ugly, is 2
different default values for dcsubval.
>
ccdc->vpcfg.pixelclk = 0;
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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 V2] v4l: OMAP3 ISP CCDC: Add support for 8bit greyscale sensors
Hi Martin,
a couple of comments inline below.
On 01/19/2011 12:27 AM, Laurent Pinchart wrote:
> Hi Martin,
>
> Thanks for the patch. One comment below.
>
> On Tuesday 18 January 2011 22:27:42 Martin Hostettler wrote:
>> Adds support for V4L2_MBUS_FMT_Y8_1X8 format and 8bit data width in
>> synchronous interface.
>>
>> When in 8bit mode don't apply DC substraction of 64 per default as this
>> would remove 1/4 of the sensor range.
>>
>> When using V4L2_MBUS_FMT_Y8_1X8 (or possibly another 8bit per pixel) mode
>> set the CDCC to output 8bit per pixel instead of 16bit.
>>
>> Signed-off-by: Martin Hostettler
>> ---
>> drivers/media/video/isp/ispccdc.c | 22 ++
>> drivers/media/video/isp/ispvideo.c |2 ++
>> 2 files changed, 20 insertions(+), 4 deletions(-)
>>
>> Changes since first version:
>> - forward ported to current media.git
>>
>> diff --git a/drivers/media/video/isp/ispccdc.c
>> b/drivers/media/video/isp/ispccdc.c index 578c8bf..c7397c9 100644
>> --- a/drivers/media/video/isp/ispccdc.c
>> +++ b/drivers/media/video/isp/ispccdc.c
>> @@ -43,6 +43,7 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct
>> v4l2_subdev_fh *fh, unsigned int pad, enum v4l2_subdev_format_whence
>> which);
>>
>> static const unsigned int ccdc_fmts[] = {
>> +V4L2_MBUS_FMT_Y8_1X8,
>> V4L2_MBUS_FMT_SGRBG10_1X10,
>> V4L2_MBUS_FMT_SRGGB10_1X10,
>> V4L2_MBUS_FMT_SBGGR10_1X10,
>> @@ -1127,6 +1128,9 @@ static void ccdc_configure(struct isp_ccdc_device
>> *ccdc) ccdc->syncif.datsz = pdata ? pdata->width : 10;
>> ispccdc_config_sync_if(ccdc, &ccdc->syncif);
>>
>> +/* CCDC_PAD_SINK */
>> +format = &ccdc->formats[CCDC_PAD_SINK];
>> +
>> syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
>>
>> /* Use the raw, unprocessed data when writing to memory. The H3A and
>> @@ -1144,10 +1148,15 @@ static void ccdc_configure(struct isp_ccdc_device
>> *ccdc) else
>> syn_mode &= ~ISPCCDC_SYN_MODE_SDR2RSZ;
>>
>> -isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
>> +/* Use PACK8 mode for 1byte per pixel formats */
>>
>> -/* CCDC_PAD_SINK */
>> -format = &ccdc->formats[CCDC_PAD_SINK];
>> +if (isp_video_format_info(format->code)->bpp <= 8)
>> +syn_mode |= ISPCCDC_SYN_MODE_PACK8;
>> +else
>> +syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
>> +
It would make sense to me to move this bit into ispccdc_config_sync_if().
>> +
>> +isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
>>
>> /* Mosaic filter */
>> switch (format->code) {
>> @@ -2244,7 +2253,12 @@ int isp_ccdc_init(struct isp_device *isp)
>> ccdc->syncif.vdpol = 0;
>>
>> ccdc->clamp.oblen = 0;
>> -ccdc->clamp.dcsubval = 64;
>> +
>> +if (isp->pdata->subdevs->interface == ISP_INTERFACE_PARALLEL
>> +&& isp->pdata->subdevs->bus.parallel.width <= 8)
>> +ccdc->clamp.dcsubval = 0;
>> +else
>> +ccdc->clamp.dcsubval = 64;
>
> I don't like this too much. What happens if you have several sensors
> connected
> to the system with different bus width ?
I see Laurent's point here. Maybe move the dcsubval assignment into
ccdc_configure(). Also, don't we also want to remove dcsubval for an
8-bit serially-attached sensor? In ccdc_configure() you could make it
conditional on the mbus format's width on the CCDC sink pad.
>
>> ccdc->vpcfg.pixelclk = 0;
>>
>> diff --git a/drivers/media/video/isp/ispvideo.c
>> b/drivers/media/video/isp/ispvideo.c index 5f984e4..cd3d331 100644
>> --- a/drivers/media/video/isp/ispvideo.c
>> +++ b/drivers/media/video/isp/ispvideo.c
>> @@ -221,6 +221,8 @@ isp_video_check_format(struct isp_video *video, struct
>> isp_video_fh *vfh) }
>>
>> static struct isp_format_info formats[] = {
>> +{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
>> + V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
>> { V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
>>V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
>> { V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
--
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
link error w/ media-0006-sensors
Hi Laurent & Sakari, On Laurent's media-0006-sensors branch, when compiling with CONFIG_VIDEO_OMAP3=m, I got the following linking error: ERROR: "omap_pm_set_min_bus_tput" [drivers/media/video/isp/omap3-isp.ko] undefined! I can get rid of the error with the patch below. But as always, I wonder: Why didn't anybody else come across this error? Are you all compiling with VIDEO_OMAP3=y? Is there a config file somewhere I can see where someone is using that? And would anything be wrong with the patch below? -Michael diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c index e129ce8..9e0bcb6 100644 --- a/arch/arm/plat-omap/omap-pm-noop.c +++ b/arch/arm/plat-omap/omap-pm-noop.c @@ -88,6 +88,7 @@ int omap_pm_set_min_bus_tput(struct device *dev, u8 agent_id, unsigned long r) return 0; } +EXPORT_SYMBOL_GPL(omap_pm_set_min_bus_tput); int omap_pm_set_max_dev_wakeup_lat(struct device *req_dev, struct device *dev, long t) MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: OMAP3530 ISP irqs disabled
Laurent Pinchart wrote: > > Sorry for the late reply, I've been travelling for the past two weeks and had > no hardware to test this on. I will try the latest code on a board with a > parallel sensor and I'll let you know if I can reproduce the problem. > If I'm correct about the problem, it's not about the parallel sensor, it's about writing the data from the CCDC to memory. I expect the problem to occur with a serial sensor too if the CCDC writes to memory. -- Michael Jones MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: OMAP3530 ISP irqs disabled
Sakari Ailus wrote: > (FYI: your lines are quite long, well over 80 characters.) Should be better now, thanks. > >> Here's my suggestion for a fix, hopefully Laurent or Sakari can comment on >> it: >> >> --- a/drivers/media/video/isp/ispccdc.c >> +++ b/drivers/media/video/isp/ispccdc.c >> @@ -1477,7 +1477,7 @@ static void ispccdc_vd1_isr(struct isp_ccdc_device >> *ccdc) >> spin_lock_irqsave(&ccdc->lsc.req_lock, flags); >> >> /* We are about to stop CCDC and/without LSC */ >> - if ((ccdc->output & CCDC_OUTPUT_MEMORY) || >> + if ((ccdc->output & CCDC_OUTPUT_MEMORY) && >> (ccdc->state == ISP_PIPELINE_STREAM_SINGLESHOT)) >> ccdc->stopping = CCDC_STOP_REQUEST; > > Does this fix the problem? ISP_PIPELINE_STREAM_SINGLESHOT is there for > memory sources and I do not think this is a correct fix. I was also able to reproduce Bastian's problem and this fixed it for me. With the condition as it is, it says on VD1, if the output is memory, CCDC will be disabled. This is obviously not what I want just because I'm writing from CCDC to memory. So something needs to be changed in this condition for Bastian. But I'm not clear on what cases the CCDC _does_ need to be disabled here, which is why I'm unsure about the fix. > > Is your VSYNC on falling or rising edge? This is defined for CCP2 and > this is what the driver was originally written for. If it's different > (rising??), you should apply the attached wildly opportunistic patch, > which I do not expect to fix this problem, however. > > But I might be just pointing you to wrong direction, better wait for > Laurent's answer. :-) > > Cheers, > -- Michael Jones MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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: OMAP3530 ISP irqs disabled
Hi Bastian (Laurent, and Sakari), > > I want to clarify this: > > I try to read images with yafta. > I read in 4 images with 5MP size (no skipping). All 4 images contain only > zeros. > I repeat the process some times and keep checking the data. After - > let's say the 6th time - the images contain exactly the data I expect. > WHEN they are read they are good. I just don't want to read 20 black > images before 1 image is transferred right. > > -Bastian > I'm on to your problem, having reproduced it myself. I suspect that you're actually only getting one frame: your very first buffer. You don't touch it, and neither does the CCDC after you requeue it, and after you've cycled through all your other buffers, you get back the non-zero frame. If you clear the "good" frame in your application once, you won't get any more non-zero frames afterwards. Or if you request more buffers, you'll have fewer non-zero frames. That's the behavior I observe. The CCDC is getting disabled by the VD1 interrupt: ispccdc_vd1_isr()->__ispccdc_handle_stopping()->__ispccdc_enable(ccdc, 0) To test this theory I tried disabling the VD1 interrupt, but it didn't solve the problem. In fact, I was still getting VD1 interrupts even though I had disabled them. Has anybody else observed that VD1 cannot be disabled? I also found it strange that the CCDC seemed to continue to generate interrupts when it's disabled. Here's my suggestion for a fix, hopefully Laurent or Sakari can comment on it: --- a/drivers/media/video/isp/ispccdc.c +++ b/drivers/media/video/isp/ispccdc.c @@ -1477,7 +1477,7 @@ static void ispccdc_vd1_isr(struct isp_ccdc_device *ccdc) spin_lock_irqsave(&ccdc->lsc.req_lock, flags); /* We are about to stop CCDC and/without LSC */ - if ((ccdc->output & CCDC_OUTPUT_MEMORY) || + if ((ccdc->output & CCDC_OUTPUT_MEMORY) && (ccdc->state == ISP_PIPELINE_STREAM_SINGLESHOT)) ccdc->stopping = CCDC_STOP_REQUEST; -- Michael Jones MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
