RE: [PATCH 7/9] ARM: DMA: steal memory for DMA coherent mappings
Hello, On Tuesday, August 16, 2011 4:26 PM Arnd Bergmann wrote: On Tuesday 16 August 2011, Russell King - ARM Linux wrote: On Tue, Aug 16, 2011 at 03:28:48PM +0200, Arnd Bergmann wrote: Hmm, I don't remember the point about dynamically sizing the pool for ARMv6K, but that can well be an oversight on my part. I do remember the part about taking that memory pool from the CMA region as you say. If you're setting aside a pool of pages, then you have to dynamically size it. I did mention during our discussion about this. The problem is that a pool of fixed size is two fold: you need it to be sufficiently large that it can satisfy all allocations which come along in atomic context. Yet, we don't want the pool to be too large because then it prevents the memory being used for other purposes. Basically, the total number of pages in the pool can be a fixed size, but as they are depleted through allocation, they need to be re-populated from CMA to re-build the reserve for future atomic allocations. If the pool becomes larger via frees, then obviously we need to give pages back. Ok, thanks for the reminder. I must have completely missed this part of the discussion. When I briefly considered this problem, my own conclusion was that the number of atomic DMA allocations would always be very low because they tend to be short-lived (e.g. incoming network packets), so we could ignore this problem and just use a smaller reservation size. While this seems to be true in general (see git grep -w -A3 dma_alloc_coherent | grep ATOMIC), there is one very significant case that we cannot ignore, which is pci_alloc_consistent. This function is still called by hundreds of PCI drivers and always does dma_alloc_coherent(..., GFP_ATOMIC), even for long-lived allocations and those that are too large to be ignored. So at least for the case where we have PCI devices, I agree that we need to have the dynamic pool. Do we really need the dynamic pool for the first version? I would like to know how much memory can be allocated in GFP_ATOMIC context. What are the typical sizes of such allocations? Maybe for the first version a static pool with reasonably small size (like 128KiB) will be more than enough? This size can be even board depended or changed with kernel command line for systems that really needs more memory. I noticed one more problem. The size of the CMA managed area must be the multiple of 16MiBs (MAX_ORDER+1). This means that the smallest CMA area is 16MiB. These values comes from the internals of the kernel memory management design and page blocks are the only entities that can be managed with page migration code. I'm not sure if all ARMv6+ boards have at least 32MiB of memory be able to create a CMA area. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
On Sat, 6 Aug 2011, Sakari Ailus wrote: Guennadi Liakhovetski wrote: A possibility to preallocate and initialise buffers of different sizes in V4L2 is required for an efficient implementation of asnapshot mode. This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF and defines respective data structures. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- Hi Guennadi, [snip] +paraWhen the I/O method is not supported the ioctl +returns an EINVAL;./para + +table pgwide=1 frame=none id=v4l2-create-buffers + titlestruct structnamev4l2_create_buffers/structname/title + tgroup cols=3 + cs-str; + tbody valign=top + row + entry__u32/entry + entrystructfieldindex/structfield/entry + entryThe starting buffer index, returned by the driver./entry + /row + row + entry__u32/entry + entrystructfieldcount/structfield/entry + entryThe number of buffers requested or granted./entry + /row + row + entry__u32/entry + entrystructfieldtype/structfield/entry + entryV4L2 buffer type: one of constantV4L2_BUF_TYPE_*/constant +values./entry v4l2-buf-type; here? No idea and I don't care all that much, tbh. I certainly copy-pasted those constructs from other documents, and yes, they are inconsistent across v4l: some use my version, some yours, others xref linkend= I'm happy with either or none of those. Just tell me _something_, that's approapriate. + /row + row + entry__u32/entry + entrystructfieldmemory/structfield/entry + entryApplications set this field to +constantV4L2_MEMORY_MMAP/constant or +constantV4L2_MEMORY_USERPTR/constant./entry v4l2-memory; [snip] +paraApplications can optionally call the +constantVIDIOC_PREPARE_BUF/constant ioctl to pass ownership of the buffer +to the driver before actually enqueuing it, using the +constantVIDIOC_QBUF/constant ioctl, and to prepare it for future I/O. s/constantVIDIOC_QBUF/constant/VIDIOC_QBUF;/ *shrug* +Such preparations may include cache invalidation or cleaning. Performing them +in advance saves time during the actual I/O. In case such cache operations are +not required, the application can use one of +constantV4L2_BUF_FLAG_NO_CACHE_INVALIDATE/constant and +constantV4L2_BUF_FLAG_NO_CACHE_CLEAN/constant flags to skip the respective +step./para + +paraThe structnamev4l2_buffer/structname structure is s/structnamev4l2_buffer/structname/v4l2-buffer;/ structname seems to be more precise, but well... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
On Tue, 16 Aug 2011, Pawel Osciak wrote: Hi Guennadi, On Tue, Aug 16, 2011 at 06:13, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Mon, 15 Aug 2011, Guennadi Liakhovetski wrote: On Mon, 15 Aug 2011, Hans Verkuil wrote: On Monday, August 15, 2011 13:28:23 Guennadi Liakhovetski wrote: Hi Hans On Mon, 8 Aug 2011, Hans Verkuil wrote: [snip] but I've changed my mind: I think this should use a struct v4l2_format after all. While switching back, I have to change the struct vb2_ops::queue_setup() operation to take a struct v4l2_create_buffers pointer. An earlier version of this patch just added one more parameter to .queue_setup(), which is easier - changes to videobuf2-core.c are smaller, but it is then redundant. We could use the create pointer for both input and output. The video plane configuration in frame format is the same as what is calculated in .queue_setup(), IIUC. So, we could just let the driver fill that one in. This would require then the videobuf2-core.c to parse struct v4l2_format to decide which union member we need, depending on the buffer type. Do we want this or shall drivers duplicate plane sizes in separate .queue_setup() parameters? Let me explain my question a bit. The current .queue_setup() method is int (*queue_setup)(struct vb2_queue *q, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); To support multiple-size buffers we also have to pass a pointer to struct v4l2_create_buffers to this function now. We can either do it like this: int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); and let all drivers fill in respective fields in *create, e.g., either do create-format.fmt.pix_mp.plane_fmt[i].sizeimage = ...; create-format.fmt.pix_mp.num_planes = ...; and also duplicate it in method parameters *num_planes = create-format.fmt.pix_mp.num_planes; sizes[i] = create-format.fmt.pix_mp.plane_fmt[i].sizeimage; or with create-format.fmt.pix.sizeimage = ...; for single-plane. Alternatively we make the prototype int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, void *alloc_ctxs[]); then drivers only fill in *create, and the videobuf2-core will have to check create-format.type to decide, which of create-format.fmt.* is relevant and extract plane sizes from there. Could we try exploring an alternative idea? The queue_setup callback was added to decouple formats from vb2 (and add some asynchronousness). But now we are doing the opposite, adding format awareness to vb2. Does vb2 really need to know about formats? I really believe it doesn't. It only needs sizes and counts. This kind of objection was expected:-) However, I think, you're a bit exaggerating. VB2 does not have to _fill_ the format. All frame-format fields like fourcc code, width, height, colorspace are only input from the user. If the user didn't fill them in, they should not be used. The only thing, that vb2 will have to learn about formats is to find the location of (plane-)buffer sizes in struct v4l2_format, for which it will have to interpret the .type value. I.e., we just have to add this: static int vb2_parse_planes(const struct v4l2_create_buffers *create, unsigned int *num_planes, unsigned int *plane_sizes) { int i; switch (create-format.type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: *num_planes = 1; plane_sizes[0] = create-format.fmt.pix.sizeimage; return 0; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: *num_planes = create-format.fmt.pix_mp.num_planes; for (i = 0; i *num_planes; i++) plane_sizes[i] = create-format.fmt.pix_mp.plane_fmt[i].sizeimage; return 0; default: return -EINVAL; } } Can you live with this or you still think it's too much format-knowledge for vb2? Or am I missing something, why we need more knowledge? Also, we are actually complicating things I think. The proposal, IIUC, would look like this: driver_queue_setup(..., create, num_buffers, [num_planes], ...) { if (create != NULL create-format != NULL) { /* use create-fmt to fill sizes */ } else if (create != NULL) { /* this assumes we have both format or sizes */
[PATCH] Media controller: Define media_entity_init() and media_entity_cleanup() conditionally
From: Vaibhav Hiremath hvaib...@ti.com Defines the two functions only when CONFIG_MEDIA_CONTROLLER is enabled. Signed-off-by: Vaibhav Hiremath hvaib...@ti.com Signed-off-by: Deepthy Ravi deepthy.r...@ti.com --- include/media/media-entity.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/include/media/media-entity.h b/include/media/media-entity.h index cd8bca6..c90916e 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -121,9 +121,18 @@ struct media_entity_graph { int top; }; +#ifdef CONFIG_MEDIA_CONTROLLER int media_entity_init(struct media_entity *entity, u16 num_pads, struct media_pad *pads, u16 extra_links); void media_entity_cleanup(struct media_entity *entity); +#else +static inline int media_entity_init(struct media_entity *entity, u16 num_pads, + struct media_pad *pads, u16 extra_links) +{ + return 0; +} +static inline void media_entity_cleanup(struct media_entity *entity) {} +#endif int media_entity_create_link(struct media_entity *source, u16 source_pad, struct media_entity *sink, u16 sink_pad, u32 flags); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] V4L/DVB: Add chip ID of TVP5146 video decoder
From: Vaibhav Hiremath hvaib...@ti.com This patch is to add chip identifier entry for TVP5146 video decoder. Signed-off-by: Vaibhav Hiremath hvaib...@ti.com Signed-off-by: Deepthy Ravi deepthy.r...@ti.com --- include/media/v4l2-chip-ident.h |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h index 63fd9d3..cd9b66f 100644 --- a/include/media/v4l2-chip-ident.h +++ b/include/media/v4l2-chip-ident.h @@ -122,6 +122,9 @@ enum { /* Other via devs could use 3314, 3324, 3327, 3336, 3364, 3353 */ V4L2_IDENT_VIA_VX855 = 3409, + /* module tvp514x */ + V4L2_IDENT_TVP5146 = 5146, + /* module tvp5150 */ V4L2_IDENT_TVP5150 = 5150, -- 1.7.0.4 -- 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 controller: Define media_entity_init() and media_entity_cleanup() conditionally
On Wed, 2011-08-17 at 16:04 +0530, Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com Defines the two functions only when CONFIG_MEDIA_CONTROLLER is enabled. Is it not a driver's option to be dependent on MEDIA_CONTROLLER? -- Andy Shevchenko andriy.shevche...@linux.intel.com Intel Finland Oy -- 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] adp1653: set media entity type
On Thu, Aug 11, 2011 at 02:35:04PM +0300, Andy Shevchenko wrote: The type of a media entity is default for this driver. This patch makes it explicitly defined as MEDIA_ENT_T_V4L2_SUBDEV_FLASH. Thanks again for the patch, Andy! Applied to my tree in linuxtv.org; the branch is called media-for-3.2-adp1653-1. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/media/video/adp1653.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c index 7f2e710..0fd9579 100644 --- a/drivers/media/video/adp1653.c +++ b/drivers/media/video/adp1653.c @@ -438,6 +438,8 @@ static int adp1653_probe(struct i2c_client *client, if (ret 0) goto free_and_quit; + flash-subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH; + return 0; free_and_quit: -- 1.7.5.4 -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
On Wed, Aug 17, 2011 at 10:41:51AM +0200, Guennadi Liakhovetski wrote: On Sat, 6 Aug 2011, Sakari Ailus wrote: Guennadi Liakhovetski wrote: A possibility to preallocate and initialise buffers of different sizes in V4L2 is required for an efficient implementation of asnapshot mode. This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF and defines respective data structures. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- Hi Guennadi, [snip] +paraWhen the I/O method is not supported the ioctl +returns an EINVAL;./para + +table pgwide=1 frame=none id=v4l2-create-buffers + titlestruct structnamev4l2_create_buffers/structname/title + tgroup cols=3 + cs-str; + tbody valign=top + row + entry__u32/entry + entrystructfieldindex/structfield/entry + entryThe starting buffer index, returned by the driver./entry + /row + row + entry__u32/entry + entrystructfieldcount/structfield/entry + entryThe number of buffers requested or granted./entry + /row + row + entry__u32/entry + entrystructfieldtype/structfield/entry + entryV4L2 buffer type: one of constantV4L2_BUF_TYPE_*/constant +values./entry v4l2-buf-type; here? No idea and I don't care all that much, tbh. I certainly copy-pasted those constructs from other documents, and yes, they are inconsistent across v4l: some use my version, some yours, others xref linkend= I'm happy with either or none of those. Just tell me _something_, that's approapriate. Links are being used in other parts of V4L2 documentation. I think PREPARE_BUF documentation should use them, too, rather than duplicating parts of definitions. + /row + row + entry__u32/entry + entrystructfieldmemory/structfield/entry + entryApplications set this field to +constantV4L2_MEMORY_MMAP/constant or +constantV4L2_MEMORY_USERPTR/constant./entry v4l2-memory; [snip] +paraApplications can optionally call the +constantVIDIOC_PREPARE_BUF/constant ioctl to pass ownership of the buffer +to the driver before actually enqueuing it, using the +constantVIDIOC_QBUF/constant ioctl, and to prepare it for future I/O. s/constantVIDIOC_QBUF/constant/VIDIOC_QBUF;/ *shrug* +Such preparations may include cache invalidation or cleaning. Performing them +in advance saves time during the actual I/O. In case such cache operations are +not required, the application can use one of +constantV4L2_BUF_FLAG_NO_CACHE_INVALIDATE/constant and +constantV4L2_BUF_FLAG_NO_CACHE_CLEAN/constant flags to skip the respective +step./para + +paraThe structnamev4l2_buffer/structname structure is s/structnamev4l2_buffer/structname/v4l2-buffer;/ structname seems to be more precise, but well... It's precise but precision isn't everything: giving the user a link to the definition of e.g. v4l2_buffer is more useful than forcing him or her to look for it elsewhere in the documentation. These are small issues but the usability of the documentation counts a lot. Regards, -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Em 17-08-2011 00:57, Laurent Pinchart escreveu: Hi Mauro, On Wednesday 17 August 2011 00:36:44 Mauro Carvalho Chehab wrote: Em 16-08-2011 08:44, Laurent Pinchart escreveu: On Tuesday 16 August 2011 17:30:47 Mauro Carvalho Chehab wrote: Em 16-08-2011 01:57, Laurent Pinchart escreveu: My point is that the ISP driver developer can't know in advance which sensor will be used systems that don't exist yet. As far as such hardware is projected, someone will know. It is a simple trivial patch to associate a new hardware with a hardware profile at the platform data. Platform data must contain hardware descriptions only, not policies. This is even clearer now that ARM is moving away from board code to the Device Tree. Again, a cell phone with one frontal camera and one hear camera has two sensor inputs only. This is not a policy. It is a hardware constraint. The driver should allow setting the pipeline for both sensors via S_INPUT, otherwise a V4L2 only userspace application won't work. It is as simple as that. When capturing from the main sensor on the OMAP3 ISP you need to capture raw data to memory on a video node, feed it back to the hardware through another video node, and finally capture it on a third video node. A V4L2-only userspace application won't work. That's how the hardware is, we can't do much about that. The raw data conversion is one of the functions that libv4l should do. So, if you've already submitted the patches for libv4l to do the hardware loopback trick, or add a function there to convert the raw data into a common format, that should be ok. Otherwise, we have a problem, that needs to be fixed. That's not what this is about. Bayer to YUV conversion needs to happen in hardware in this case for performance reasons. The hardware will also perform scaling on YUV, as well as many image processing tasks (white balance, defect pixel correction, gamma correction, noise filtering, ...). Also, on most cases, probing a sensor is as trivial as reading a sensor ID during device probe. This applies, for example, for all Omnivision sensors. We do things like that all the times for PC world, as nobody knows what webcam someone would plug on his PC. Sorry, but that's not related. You simply can't decide in an embedded ISP driver how to deal with sensor controls, as the system will be used in a too wide variety of applications and hardware configurations. All controls need to be exposed, period. We're not talking about controls. We're talking about providing the needed V4L2 support to allow an userspace application to access the hardware sensor. OK, so we're discussing S_INPUT. Let's discuss controls later :-) I never saw an embedded hardware that allows physically changing the sensor. Beagleboard + pluggable sensor board. Development systems like beagleboard, pandaboard, Exynos SMDK, etc, aren't embeeded hardware. They're development kits. People create end-user products based on those kits. That make them first- class embedded hardware like any other. No doubt they should be supported, but it doesn't make sense to create tons of input pipelines to be used for S_INPUT for each different type of possible sensor. Somehow, userspace needs to tell what's the sensor that he attached to the hardware, or the driver should suport auto-detecting it. We're not creating tons of input pipelines. Look at http://www.ideasonboard.org/media/omap3isp.ps , every video node (in yellow) has its purpose. Not sure if I it understood well. The subdevs 8-11 are the sensors, right? et8ek8 and vs6555 are the sensors. ad5820 is the lens controller and adp1653 the flash controller. All other subdevs (green blocks) are part of the ISP. In other words, I see 2 options for that: 1) add hardware auto-detection at the sensor logic. At driver probe, try to probe all sensors, if it is a hardware development kit; We've worked quite hard to remove I2C device probing from the kernel, let's not add it back. We do I2C probing on several drivers. It is there for devices where the cards entry is not enough to identify the hardware. For example, two different devices with the same USB ID generally uses that. If the hardware information is not enough, there's nothing wrong on doing that. Except that probing might destroy the hardware in the general case. We can only probe I2C devices on a bus that we know will not contain any other sensitive devices. 2) add one new parameter at the driver: sensors. If the hardware is one of those kits, this parameter will allow the developer to specify the used sensors. It is the same logic as we do with userspace TV and grabber cards without eeprom or any other way to auto-detect the hardware. This will likely be done through the Device Tree. I don't mind much about the way it is implemented, but it should be there on a place where people can find it and use it. Devices that requires
Re: [PATCH 7/9] ARM: DMA: steal memory for DMA coherent mappings
On Wednesday 17 August 2011, Marek Szyprowski wrote: Do we really need the dynamic pool for the first version? I would like to know how much memory can be allocated in GFP_ATOMIC context. What are the typical sizes of such allocations? I think this highly depends on the board and on the use case. We know that 2 MB is usually enough, because that is the current CONSISTENT_DMA_SIZE on most platforms. Most likely something a lot smaller will be ok in practice. CONSISTENT_DMA_SIZE is currently used for both atomic and non-atomic allocations. Maybe for the first version a static pool with reasonably small size (like 128KiB) will be more than enough? This size can be even board depended or changed with kernel command line for systems that really needs more memory. For a first version that sounds good enough. Maybe we could use a fraction of the CONSISTENT_DMA_SIZE as an estimate? For the long-term solution, I see two options: 1. make the preallocated pool rather small so we normally don't need it. 2. make it large enough so we can also fulfill most nonatomic allocations from that pool to avoid the TLB flushes and going through the CMA code. Only use the real CMA region when the pool allocation fails. In either case, there should be some method for balancing the pool size. I noticed one more problem. The size of the CMA managed area must be the multiple of 16MiBs (MAX_ORDER+1). This means that the smallest CMA area is 16MiB. These values comes from the internals of the kernel memory management design and page blocks are the only entities that can be managed with page migration code. I'm not sure if all ARMv6+ boards have at least 32MiB of memory be able to create a CMA area. My guess is that you can assume to have 64 MB or more on ARMv6 running Linux, but other people may have more accurate data. Also, there is the option of setting a lower value for FORCE_MAX_ZONEORDER for some platforms if it becomes a problem. Arnd -- 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: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Hi Mauro, On Tue, Aug 16, 2011 at 03:36:44PM -0700, Mauro Carvalho Chehab wrote: [clip] For instance, when switching between a bw and a color sensor you will need to reconfigure the whole pipeline to select the right gamma table, white balance parameters, color conversion matrix, ... That's not something we want to hardcode in the kernel. This needs to be done from userspace. This is something that, if it is not written somehwere, no userspace applications not developed by the hardware vendor will ever work. I don't see any code for that any at the kernel or at libv4l. Am I missing something? There actually is. The plugin interface patches went in to libv4l 0.9.0. That's just an interface and it doesn't yet have support for any embedded devices. -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Hi everybody, On Wed, 2011-08-17 at 05:25 -0700, Mauro Carvalho Chehab wrote: Em 17-08-2011 00:57, Laurent Pinchart escreveu: Hi Mauro, On Wednesday 17 August 2011 00:36:44 Mauro Carvalho Chehab wrote: Em 16-08-2011 08:44, Laurent Pinchart escreveu: On Tuesday 16 August 2011 17:30:47 Mauro Carvalho Chehab wrote: Em 16-08-2011 01:57, Laurent Pinchart escreveu: My point is that the ISP driver developer can't know in advance which sensor will be used systems that don't exist yet. As far as such hardware is projected, someone will know. It is a simple trivial patch to associate a new hardware with a hardware profile at the platform data. Platform data must contain hardware descriptions only, not policies. This is even clearer now that ARM is moving away from board code to the Device Tree. Again, a cell phone with one frontal camera and one hear camera has two sensor inputs only. This is not a policy. It is a hardware constraint. The driver should allow setting the pipeline for both sensors via S_INPUT, otherwise a V4L2 only userspace application won't work. It is as simple as that. When capturing from the main sensor on the OMAP3 ISP you need to capture raw data to memory on a video node, feed it back to the hardware through another video node, and finally capture it on a third video node. A V4L2-only userspace application won't work. That's how the hardware is, we can't do much about that. The raw data conversion is one of the functions that libv4l should do. So, if you've already submitted the patches for libv4l to do the hardware loopback trick, or add a function there to convert the raw data into a common format, that should be ok. Otherwise, we have a problem, that needs to be fixed. That's not what this is about. Bayer to YUV conversion needs to happen in hardware in this case for performance reasons. The hardware will also perform scaling on YUV, as well as many image processing tasks (white balance, defect pixel correction, gamma correction, noise filtering, ...). Also, on most cases, probing a sensor is as trivial as reading a sensor ID during device probe. This applies, for example, for all Omnivision sensors. We do things like that all the times for PC world, as nobody knows what webcam someone would plug on his PC. Sorry, but that's not related. You simply can't decide in an embedded ISP driver how to deal with sensor controls, as the system will be used in a too wide variety of applications and hardware configurations. All controls need to be exposed, period. We're not talking about controls. We're talking about providing the needed V4L2 support to allow an userspace application to access the hardware sensor. OK, so we're discussing S_INPUT. Let's discuss controls later :-) I never saw an embedded hardware that allows physically changing the sensor. Beagleboard + pluggable sensor board. Development systems like beagleboard, pandaboard, Exynos SMDK, etc, aren't embeeded hardware. They're development kits. People create end-user products based on those kits. That make them first- class embedded hardware like any other. No doubt they should be supported, but it doesn't make sense to create tons of input pipelines to be used for S_INPUT for each different type of possible sensor. Somehow, userspace needs to tell what's the sensor that he attached to the hardware, or the driver should suport auto-detecting it. We're not creating tons of input pipelines. Look at http://www.ideasonboard.org/media/omap3isp.ps , every video node (in yellow) has its purpose. Not sure if I it understood well. The subdevs 8-11 are the sensors, right? et8ek8 and vs6555 are the sensors. ad5820 is the lens controller and adp1653 the flash controller. All other subdevs (green blocks) are part of the ISP. In other words, I see 2 options for that: 1) add hardware auto-detection at the sensor logic. At driver probe, try to probe all sensors, if it is a hardware development kit; We've worked quite hard to remove I2C device probing from the kernel, let's not add it back. We do I2C probing on several drivers. It is there for devices where the cards entry is not enough to identify the hardware. For example, two different devices with the same USB ID generally uses that. If the hardware information is not enough, there's nothing wrong on doing that. Except that probing might destroy the hardware in the general case. We can only probe I2C devices on a bus that we know will not contain any other sensitive devices. 2) add one new parameter at the driver: sensors. If the hardware is one of those kits, this parameter will allow the developer to specify the used sensors. It is the same logic as we do with userspace TV and grabber cards without eeprom or any other way to auto-detect the hardware.
RE: [PATCH 7/9] ARM: DMA: steal memory for DMA coherent mappings
Hello, On Wednesday, August 17, 2011 10:01 AM Marek Szyprowski wrote: On Tuesday, August 16, 2011 4:26 PM Arnd Bergmann wrote: On Tuesday 16 August 2011, Russell King - ARM Linux wrote: On Tue, Aug 16, 2011 at 03:28:48PM +0200, Arnd Bergmann wrote: Hmm, I don't remember the point about dynamically sizing the pool for ARMv6K, but that can well be an oversight on my part. I do remember the part about taking that memory pool from the CMA region as you say. If you're setting aside a pool of pages, then you have to dynamically size it. I did mention during our discussion about this. The problem is that a pool of fixed size is two fold: you need it to be sufficiently large that it can satisfy all allocations which come along in atomic context. Yet, we don't want the pool to be too large because then it prevents the memory being used for other purposes. Basically, the total number of pages in the pool can be a fixed size, but as they are depleted through allocation, they need to be re-populated from CMA to re-build the reserve for future atomic allocations. If the pool becomes larger via frees, then obviously we need to give pages back. Ok, thanks for the reminder. I must have completely missed this part of the discussion. When I briefly considered this problem, my own conclusion was that the number of atomic DMA allocations would always be very low because they tend to be short-lived (e.g. incoming network packets), so we could ignore this problem and just use a smaller reservation size. While this seems to be true in general (see git grep -w -A3 dma_alloc_coherent | grep ATOMIC), there is one very significant case that we cannot ignore, which is pci_alloc_consistent. This function is still called by hundreds of PCI drivers and always does dma_alloc_coherent(..., GFP_ATOMIC), even for long-lived allocations and those that are too large to be ignored. So at least for the case where we have PCI devices, I agree that we need to have the dynamic pool. Do we really need the dynamic pool for the first version? I would like to know how much memory can be allocated in GFP_ATOMIC context. What are the typical sizes of such allocations? Maybe for the first version a static pool with reasonably small size (like 128KiB) will be more than enough? This size can be even board depended or changed with kernel command line for systems that really needs more memory. I noticed one more problem. The size of the CMA managed area must be the multiple of 16MiBs (MAX_ORDER+1). This means that the smallest CMA area is 16MiB. These values comes from the internals of the kernel memory management design and page blocks are the only entities that can be managed with page migration code. I'm really sorry for the confusion. This 16MiB value worried me too much and I've checked the code once again and found that this MAX_ORDER+1 value was a miscalculation, which appeared in v11 of the patches. The true minimal CMA area size is 8MiB for ARM architecture. I believe this shouldn't be an issue for the current ARMv6+ based machines. I've checked it with mem=16M cma=8M kernel arguments. System booted fine and CMA area has been successfully created. Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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 7/9] ARM: DMA: steal memory for DMA coherent mappings
Hello, On Wednesday, August 17, 2011 2:29 PM Arnd Bergmann wrote: On Wednesday 17 August 2011, Marek Szyprowski wrote: Do we really need the dynamic pool for the first version? I would like to know how much memory can be allocated in GFP_ATOMIC context. What are the typical sizes of such allocations? I think this highly depends on the board and on the use case. We know that 2 MB is usually enough, because that is the current CONSISTENT_DMA_SIZE on most platforms. Most likely something a lot smaller will be ok in practice. CONSISTENT_DMA_SIZE is currently used for both atomic and non-atomic allocations. Ok. The platforms that increased CONSISTENT_DMA_SIZE usually did that to enable support for framebuffer or other multimedia devices, which won't be allocated in ATOMIC context anyway. Maybe for the first version a static pool with reasonably small size (like 128KiB) will be more than enough? This size can be even board depended or changed with kernel command line for systems that really needs more memory. For a first version that sounds good enough. Maybe we could use a fraction of the CONSISTENT_DMA_SIZE as an estimate? Ok, good. For the initial values I will probably use 1/8 of CONSISTENT_DMA_SIZE for coherent allocations. Writecombine atomic allocations are extremely rare and rather ARM specific. 1/32 of CONSISTENT_DMA_SIZE should be more than enough for them. For the long-term solution, I see two options: 1. make the preallocated pool rather small so we normally don't need it. 2. make it large enough so we can also fulfill most nonatomic allocations from that pool to avoid the TLB flushes and going through the CMA code. Only use the real CMA region when the pool allocation fails. In either case, there should be some method for balancing the pool size. Right. The most obvious method is to use additional kernel thread which will periodically call the balance function. In the implementation both usage scenarios are very similar, so this can even be a kernel parameter or Kconfig option, but lets leave this for the future vesions. I noticed one more problem. The size of the CMA managed area must be the multiple of 16MiBs (MAX_ORDER+1). This means that the smallest CMA area is 16MiB. These values comes from the internals of the kernel memory management design and page blocks are the only entities that can be managed with page migration code. I'm not sure if all ARMv6+ boards have at least 32MiB of memory be able to create a CMA area. My guess is that you can assume to have 64 MB or more on ARMv6 running Linux, but other people may have more accurate data. Also, there is the option of setting a lower value for FORCE_MAX_ZONEORDER for some platforms if it becomes a problem. Ok. I figured out an error in the above calculation, so 8MiB is the smallest CMA area size. Assuming that there are at least 32MiB of memory available this is not an issue anymore. Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
Hello, On Monday, August 15, 2011 3:46 PM Guennadi Liakhovetski wrote: On Mon, 15 Aug 2011, Hans Verkuil wrote: On Monday, August 15, 2011 13:28:23 Guennadi Liakhovetski wrote: On Mon, 8 Aug 2011, Hans Verkuil wrote: On Friday, August 05, 2011 09:47:13 Guennadi Liakhovetski wrote: A possibility to preallocate and initialise buffers of different sizes in V4L2 is required for an efficient implementation of asnapshot mode. This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF and defines respective data structures. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v4: 1. CREATE_BUFS now takes an array of plane sizes and a fourcc code in its argument, instead of a frame format specification, including documentation update 2. documentation improvements, as suggested by Hans 3. increased reserved fields to 18, as suggested by Sakari Documentation/DocBook/media/v4l/io.xml | 17 ++ Documentation/DocBook/media/v4l/v4l2.xml |2 + .../DocBook/media/v4l/vidioc-create-bufs.xml | 161 .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 drivers/media/video/v4l2-compat-ioctl32.c |6 + drivers/media/video/v4l2-ioctl.c | 26 +++ include/linux/videodev2.h | 18 +++ include/media/v4l2-ioctl.h |2 + 8 files changed, 328 insertions(+), 0 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml snip diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index fca24cc..3cd0cb3 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -653,6 +653,9 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_ERROR 0x0040 #define V4L2_BUF_FLAG_TIMECODE 0x0100 /* timecode field is valid */ #define V4L2_BUF_FLAG_INPUT 0x0200 /* input field is valid */ +/* Cache handling flags */ +#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE0x0400 +#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x0800 /* * O V E R L A Y P R E V I E W @@ -2092,6 +2095,18 @@ struct v4l2_dbg_chip_ident { __u32 revision;/* chip revision, chip specific */ } __attribute__ ((packed)); +/* VIDIOC_CREATE_BUFS */ +struct v4l2_create_buffers { + __u32 index; /* output: buffers index...index + count - 1 have been created */ + __u32 count; + __u32 type; + __u32 memory; + __u32 fourcc; + __u32 num_planes; + __u32 sizes[VIDEO_MAX_PLANES]; + __u32 reserved[18]; +}; I know you are going to hate me for this, hm, I'll consider this possibility;-) but I've changed my mind: I think this should use a struct v4l2_format after all. While switching back, I have to change the struct vb2_ops::queue_setup() operation to take a struct v4l2_create_buffers pointer. An earlier version of this patch just added one more parameter to .queue_setup(), which is easier - changes to videobuf2-core.c are smaller, but it is then redundant. We could use the create pointer for both input and output. The video plane configuration in frame format is the same as what is calculated in .queue_setup(), IIUC. So, we could just let the driver fill that one in. This would require then the videobuf2-core.c to parse struct v4l2_format to decide which union member we need, depending on the buffer type. Do we want this or shall drivers duplicate plane sizes in separate .queue_setup() parameters? IMHO if possible we should have only one callback for the driver. Please notice that the driver should be also allowed to increase (or decrease) the number of buffers for particular format/fourcc. Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Em 17-08-2011 05:37, Ivan T. Ivanov escreveu: Hi everybody, On Wed, 2011-08-17 at 05:25 -0700, Mauro Carvalho Chehab wrote: Em 17-08-2011 00:57, Laurent Pinchart escreveu: Hi Mauro, On Wednesday 17 August 2011 00:36:44 Mauro Carvalho Chehab wrote: Em 16-08-2011 08:44, Laurent Pinchart escreveu: On Tuesday 16 August 2011 17:30:47 Mauro Carvalho Chehab wrote: Em 16-08-2011 01:57, Laurent Pinchart escreveu: No. S_INPUT shouldn't be use to select between sensors. The hardware pipeline is more complex than just that. We can't make it all fit in the S_INPUT API. For instance, when switching between a bw and a color sensor you will need to reconfigure the whole pipeline to select the right gamma table, white balance parameters, color conversion matrix, ... That's not something we want to hardcode in the kernel. This needs to be done from userspace. This is something that, if it is not written somehwere, no userspace applications not developed by the hardware vendor will ever work. I don't see any code for that any at the kernel or at libv4l. Am I missing something? Code for that needs to be written in libv4l. It's not there yet as I don't think we have any hardware for this particular example at the moment :-) As no pure V4L2 application would set the pipelines as you've said, and no libv4l code exists yet, Actually there is such code for OMAP3 ISP driver. Plug-in support in libv4l have been extended a little bit [1] and plugin-in which handle request for regular video device nodes /dev/video0 and /dev/video1 and translate them to MC and sub-device API have been posted here [2], but is still not merged. Regards, iivanov [1] http://www.spinics.net/lists/linux-media/msg35570.html [2] http://www.spinics.net/lists/linux-media/msg32539.html Ah, ok. So, it is just the usual delay of having some features merged. Hans G., FYI. Please review the OMAP3 MC-aware patches for libv4l when you have some time for that. Thanks! Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
On Wed, Aug 17, 2011 at 06:22, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On Monday, August 15, 2011 3:46 PM Guennadi Liakhovetski wrote: While switching back, I have to change the struct vb2_ops::queue_setup() operation to take a struct v4l2_create_buffers pointer. An earlier version of this patch just added one more parameter to .queue_setup(), which is easier - changes to videobuf2-core.c are smaller, but it is then redundant. We could use the create pointer for both input and output. The video plane configuration in frame format is the same as what is calculated in .queue_setup(), IIUC. So, we could just let the driver fill that one in. This would require then the videobuf2-core.c to parse struct v4l2_format to decide which union member we need, depending on the buffer type. Do we want this or shall drivers duplicate plane sizes in separate .queue_setup() parameters? IMHO if possible we should have only one callback for the driver. Please notice that the driver should be also allowed to increase (or decrease) the number of buffers for particular format/fourcc. Or remove queue_setup altogether (please see my example above). What do you think Marek? -- Best regards, Pawel Osciak -- 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: How to git and build HVR-2200 drivers from Kernel labs ?
On Monday 15 August 2011 23:04:03 Steven Toth wrote: So how do I get a 8940 edition of a HVR-2200 working with Ubuntu ? Hello Declan, You'll need to install the entire new kernel and all of its modules, you should avoid cherry picking small pieces. Incidentally, I've had confirmation from another user that the tree works and automatically detects type 9 cards. - Steve Hi Steve I'm wanting to get the source for a version of the saa7164 driver that supports my 8940 revision of the HVR-2200 card. I've had a look at the version in git clone git://kernellabs.com/stoth/saa7164-stable.git, but it doesn't seem to support the 8940 revision. Could you please recommend where/how I should get a version of the source that does support this card revision. Many thanks. Eg should I use the source from git clone git://kernellabs.com/stoth/saa7164- dev.git or do you recommend something else that might be more stable ? Many thanks, Declan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
Hi On Wed, Aug 17, 2011 at 02:11, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Tue, 16 Aug 2011, Pawel Osciak wrote: Hi Guennadi, On Tue, Aug 16, 2011 at 06:13, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Mon, 15 Aug 2011, Guennadi Liakhovetski wrote: On Mon, 15 Aug 2011, Hans Verkuil wrote: On Monday, August 15, 2011 13:28:23 Guennadi Liakhovetski wrote: Hi Hans On Mon, 8 Aug 2011, Hans Verkuil wrote: [snip] but I've changed my mind: I think this should use a struct v4l2_format after all. While switching back, I have to change the struct vb2_ops::queue_setup() operation to take a struct v4l2_create_buffers pointer. An earlier version of this patch just added one more parameter to .queue_setup(), which is easier - changes to videobuf2-core.c are smaller, but it is then redundant. We could use the create pointer for both input and output. The video plane configuration in frame format is the same as what is calculated in .queue_setup(), IIUC. So, we could just let the driver fill that one in. This would require then the videobuf2-core.c to parse struct v4l2_format to decide which union member we need, depending on the buffer type. Do we want this or shall drivers duplicate plane sizes in separate .queue_setup() parameters? Let me explain my question a bit. The current .queue_setup() method is int (*queue_setup)(struct vb2_queue *q, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); To support multiple-size buffers we also have to pass a pointer to struct v4l2_create_buffers to this function now. We can either do it like this: int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); and let all drivers fill in respective fields in *create, e.g., either do create-format.fmt.pix_mp.plane_fmt[i].sizeimage = ...; create-format.fmt.pix_mp.num_planes = ...; and also duplicate it in method parameters *num_planes = create-format.fmt.pix_mp.num_planes; sizes[i] = create-format.fmt.pix_mp.plane_fmt[i].sizeimage; or with create-format.fmt.pix.sizeimage = ...; for single-plane. Alternatively we make the prototype int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, void *alloc_ctxs[]); then drivers only fill in *create, and the videobuf2-core will have to check create-format.type to decide, which of create-format.fmt.* is relevant and extract plane sizes from there. Could we try exploring an alternative idea? The queue_setup callback was added to decouple formats from vb2 (and add some asynchronousness). But now we are doing the opposite, adding format awareness to vb2. Does vb2 really need to know about formats? I really believe it doesn't. It only needs sizes and counts. This kind of objection was expected:-) However, I think, you're a bit exaggerating. VB2 does not have to _fill_ the format. All frame-format fields like fourcc code, width, height, colorspace are only input from the user. If the user didn't fill them in, they should not be used. The only thing, that vb2 will have to learn about formats is to find the location of (plane-)buffer sizes in struct v4l2_format, for which it will have to interpret the .type value. I.e., we just have to add this: static int vb2_parse_planes(const struct v4l2_create_buffers *create, unsigned int *num_planes, unsigned int *plane_sizes) { int i; switch (create-format.type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: *num_planes = 1; plane_sizes[0] = create-format.fmt.pix.sizeimage; return 0; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: *num_planes = create-format.fmt.pix_mp.num_planes; for (i = 0; i *num_planes; i++) plane_sizes[i] = create-format.fmt.pix_mp.plane_fmt[i].sizeimage; return 0; default: return -EINVAL; } } Can you live with this or you still think it's too much format-knowledge for vb2? Or am I missing something, why we need more knowledge? True, I exaggerated a bit there. You are right with the above (I was also thinking of a case when the driver would want to adjust the format passed to CREATE_BUFS, but that seems to work as well). But I think my point still stands though: the drivers will
RE: [PATCH] Media controller: Define media_entity_init() and media_entity_cleanup() conditionally
-Original Message- From: Ravi, Deepthy Sent: Wednesday, August 17, 2011 4:05 PM To: mche...@infradead.org; linux-media@vger.kernel.org; linux- ker...@vger.kernel.org Cc: linux-o...@vger.kernel.org; Hiremath, Vaibhav; Ravi, Deepthy Subject: [PATCH] Media controller: Define media_entity_init() and media_entity_cleanup() conditionally From: Vaibhav Hiremath hvaib...@ti.com Defines the two functions only when CONFIG_MEDIA_CONTROLLER is enabled. [Hiremath, Vaibhav] Deepthy, You may want to mention about build failure without MEDIA_CONTROLLER option being enabled, especially if any sensor driver is being used between MC and non-MC framework compatible devices. For example, OMAP3 and AM3517, where TVP5146 is being used but OMAP3 is based on MC framework and AM3517 is based on simple sub-dev based interface. Thanks, Vaibhav Signed-off-by: Vaibhav Hiremath hvaib...@ti.com Signed-off-by: Deepthy Ravi deepthy.r...@ti.com --- include/media/media-entity.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/include/media/media-entity.h b/include/media/media-entity.h index cd8bca6..c90916e 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -121,9 +121,18 @@ struct media_entity_graph { int top; }; +#ifdef CONFIG_MEDIA_CONTROLLER int media_entity_init(struct media_entity *entity, u16 num_pads, struct media_pad *pads, u16 extra_links); void media_entity_cleanup(struct media_entity *entity); +#else +static inline int media_entity_init(struct media_entity *entity, u16 num_pads, + struct media_pad *pads, u16 extra_links) +{ + return 0; +} +static inline void media_entity_cleanup(struct media_entity *entity) {} +#endif int media_entity_create_link(struct media_entity *source, u16 source_pad, struct media_entity *sink, u16 sink_pad, u32 flags); -- 1.7.0.4 -- 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: Add support for arbitrary resolution for the ov5642 camera driver
This patch adds the ability to get arbitrary resolutions with a width up to 2592 and a height up to 720 pixels instead of the standard 1280x720 only. Signed-off-by: Bastian Hecht hec...@gmail.com --- diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c index 6410bda..1b40d90 100644 --- a/drivers/media/video/ov5642.c +++ b/drivers/media/video/ov5642.c @@ -14,8 +14,10 @@ * published by the Free Software Foundation. */ +#include linux/bitops.h #include linux/delay.h #include linux/i2c.h +#include linux/kernel.h #include linux/slab.h #include linux/videodev2.h @@ -28,13 +30,25 @@ #define REG_CHIP_ID_HIGH 0x300a #define REG_CHIP_ID_LOW0x300b +#define REG_RED_GAIN_HIGH 0x3400 +#define REG_RED_GAIN_LOW 0x3401 +#define REG_BLUE_GAIN_HIGH 0x3404 +#define REG_BLUE_GAIN_LOW 0x3405 +#define REG_AWB_MANUAL 0x3406 +#define REG_EXP_HIGH 0x3500 +#define REG_EXP_MIDDLE 0x3501 +#define REG_EXP_LOW0x3502 +#define REG_EXP_GAIN_CTRL 0x3503 +#define REG_GAIN 0x350b +#define REG_EXTEND_FRAME_TIME_HIGH 0x350c +#define REG_EXTEND_FRAME_TIME_LOW 0x350d #define REG_WINDOW_START_X_HIGH0x3800 #define REG_WINDOW_START_X_LOW 0x3801 #define REG_WINDOW_START_Y_HIGH0x3802 #define REG_WINDOW_START_Y_LOW 0x3803 #define REG_WINDOW_WIDTH_HIGH 0x3804 #define REG_WINDOW_WIDTH_LOW 0x3805 -#define REG_WINDOW_HEIGHT_HIGH 0x3806 +#define REG_WINDOW_HEIGHT_HIGH 0x3806 #define REG_WINDOW_HEIGHT_LOW 0x3807 #define REG_OUT_WIDTH_HIGH 0x3808 #define REG_OUT_WIDTH_LOW 0x3809 @@ -44,19 +58,56 @@ #define REG_OUT_TOTAL_WIDTH_LOW0x380d #define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e #define REG_OUT_TOTAL_HEIGHT_LOW 0x380f +#define REG_FLIP_SUBSAMPLE 0x3818 +#define REG_OUTPUT_FORMAT 0x4300 +#define REG_ISP_CTRL_010x5001 +#define REG_DIGITAL_EFFECTS0x5580 +#define REG_HUE_COS0x5581 +#define REG_HUE_SIN0x5582 +#define REG_BLUE_SATURATION0x5583 +#define REG_RED_SATURATION 0x5584 +#define REG_CONTRAST 0x5588 +#define REG_BRIGHTNESS 0x5589 +#define REG_D_E_AUXILLARY 0x558a +#define REG_AVG_WINDOW_END_X_HIGH 0x5682 +#define REG_AVG_WINDOW_END_X_LOW 0x5683 +#define REG_AVG_WINDOW_END_Y_HIGH 0x5686 +#define REG_AVG_WINDOW_END_Y_LOW 0x5687 + +/* active pixel array size */ +#define OV5642_SENSOR_SIZE_X 2592 +#define OV5642_SENSOR_SIZE_Y 1944 + +/* current maximum working size */ +#define OV5642_MAX_WIDTH OV5642_SENSOR_SIZE_X +#define OV5642_MAX_HEIGHT 720 + +/* default sizes */ +#define OV5642_DEFAULT_WIDTH 1280 +#define OV5642_DEFAULT_HEIGHT OV5642_MAX_HEIGHT + +/* minimum extra blanking */ +#define BLANKING_EXTRA_WIDTH 500 +#define BLANKING_EXTRA_HEIGHT 20 /* - * define standard resolution. - * Works currently only for up to 720 lines - * eg. 320x240, 640x480, 800x600, 1280x720, 2048x720 + * the sensor's autoexposure is buggy when setting total_height low. + * It tries to expose longer than 1 frame period without taking care of it + * and this leads to weird output. So we set 1000 lines as minimum. */ -#define OV5642_WIDTH 1280 -#define OV5642_HEIGHT 720 -#define OV5642_TOTAL_WIDTH 3200 -#define OV5642_TOTAL_HEIGHT2000 -#define OV5642_SENSOR_SIZE_X 2592 -#define OV5642_SENSOR_SIZE_Y 1944 +#define BLANKING_MIN_HEIGHT1000 + +/* + * About OV5642 resolution, cropping and binning: + * This sensor supports it all, at least in the feature description. + * Unfortunately, no combination of appropriate registers settings could make + * the chip work the intended way. As it works with predefined register lists, + * some undocumented registers are presumably changed there to achieve their + * goals. + * This driver currently only works for resolutions up to 720 lines with a + * 1:1 scale. Hopefully these restrictions will be removed in the future. + */ struct regval_list { u16 reg_num; @@ -105,10 +156,8 @@ static struct regval_list ov5642_default_regs_init[] = { { 0x471d, 0x5 }, { 0x4708, 0x6 }, { 0x370c, 0xa0 }, - { 0x5687, 0x94 }, { 0x501f, 0x0 }, { 0x5000, 0x4f }, - { 0x5001, 0xcf }, { 0x4300, 0x30 }, { 0x4300, 0x30 }, { 0x460b, 0x35 }, @@ -121,11 +170,8 @@ static struct regval_list ov5642_default_regs_init[] = { { 0x4402, 0x90 }, { 0x460c, 0x22 }, { 0x3815, 0x44 }, - { 0x3503, 0x7 }, { 0x3501, 0x73 }, { 0x3502, 0x80 }, - { 0x350b, 0x0 }, - { 0x3818, 0xc8 },
[PATCH] media: Add camera controls for the ov5642 driver
The driver now supports automatic/manual gain, automatic/manual white balance, automatic/manual exposure control, vertical flip, brightness control, contrast control and saturation control. Additionally the following effects are available now: rotating the hue in the colorspace, gray scale image and solarize effect. Signed-off-by: Bastian Hecht hec...@gmail.com --- diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c index 1b40d90..069a720 100644 --- a/drivers/media/video/ov5642.c +++ b/drivers/media/video/ov5642.c @@ -74,6 +74,34 @@ #define REG_AVG_WINDOW_END_Y_HIGH 0x5686 #define REG_AVG_WINDOW_END_Y_LOW 0x5687 +/* default values in native space */ +#define DEFAULT_RBBALANCE 0x400 +#define DEFAULT_CONTRAST 0x20 +#define DEFAULT_SATURATION 0x40 + +#define MAX_EXP_NATIVE 0x01 +#define MAX_GAIN_NATIVE0x1f +#define MAX_RBBALANCE_NATIVE 0x0fff +#define MAX_EXP0x +#define MAX_GAIN 0xff +#define MAX_RBBALANCE 0xff +#define MAX_HUE_TRIG_NATIVE0x80 + +#define OV5642_CONTROL_BLUE_SATURATION (V4L2_CID_PRIVATE_BASE + 0) +#define OV5642_CONTROL_RED_SATURATION (V4L2_CID_PRIVATE_BASE + 1) +#define OV5642_CONTROL_GRAY_SCALE (V4L2_CID_PRIVATE_BASE + 2) +#define OV5642_CONTROL_SOLARIZE(V4L2_CID_PRIVATE_BASE + 3) + +#define EXP_V4L2_TO_NATIVE(x) ((x) 4) +#define EXP_NATIVE_TO_V4L2(x) ((x) 4) +#define GAIN_V4L2_TO_NATIVE(x) ((x) * MAX_GAIN_NATIVE / MAX_GAIN) +#define GAIN_NATIVE_TO_V4L2(x) ((x) * MAX_GAIN / MAX_GAIN_NATIVE) +#define RBBALANCE_V4L2_TO_NATIVE(x) ((x) * MAX_RBBALANCE_NATIVE / MAX_RBBALANCE) +#define RBBALANCE_NATIVE_TO_V4L2(x) ((x) * MAX_RBBALANCE / MAX_RBBALANCE_NATIVE) + +/* flaw in the datasheet. we need some extra lines */ +#define MANUAL_LONG_EXP_SAFETY_DISTANCE20 + /* active pixel array size */ #define OV5642_SENSOR_SIZE_X 2592 #define OV5642_SENSOR_SIZE_Y 1944 @@ -109,6 +137,9 @@ * 1:1 scale. Hopefully these restrictions will be removed in the future. */ +static int ov5642_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl); +static int ov5642_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl); + struct regval_list { u16 reg_num; u8 value; @@ -608,6 +639,145 @@ static struct regval_list ov5642_default_regs_finalise[] = { { 0x, 0xff }, }; +static const struct v4l2_queryctrl ov5642_controls[] = { + { + .id = V4L2_CID_AUTOGAIN, + .type = V4L2_CTRL_TYPE_BOOLEAN, + .name = Automatic Gain Control, + .minimum= 0, + .maximum= 1, + .step = 1, + .default_value = 1, + }, + { + .id = V4L2_CID_GAIN, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Gain, + .minimum= 0, + .maximum= MAX_GAIN, + .step = 1, + .default_value = 0, + }, + { + .id = V4L2_CID_AUTO_WHITE_BALANCE, + .type = V4L2_CTRL_TYPE_BOOLEAN, + .name = Automatic White Balance, + .minimum= 0, + .maximum= 1, + .step = 1, + .default_value = 1, + }, + { + .id = V4L2_CID_BLUE_BALANCE, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Blue Balance, + .minimum= 0, + .maximum= MAX_RBBALANCE, + .step = 1, + .default_value = DEFAULT_RBBALANCE, + }, + { + .id = V4L2_CID_RED_BALANCE, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Red Balance, + .minimum= 0, + .maximum= MAX_RBBALANCE, + .step = 1, + .default_value = DEFAULT_RBBALANCE, + }, + { + .id = V4L2_CID_EXPOSURE_AUTO, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Automatic Exposure Control, + .minimum= 0, + .maximum= 1, + .step = 1, + .default_value = V4L2_EXPOSURE_AUTO, + }, + { + .id = V4L2_CID_EXPOSURE, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Exposure, + .minimum= 0, + .maximum= MAX_EXP, + .step = 1, + .default_value = 0, + }, + /* vflip works out of
Re: [PATCH] media: Added extensive feature set to the OV5642 camera driver
2011/8/16 Laurent Pinchart laurent.pinch...@ideasonboard.com: Hi Bastian, On Tuesday 16 August 2011 14:58:58 Bastian Hecht wrote: The driver now supports arbitray resolutions (width up to 2592, height up to 720), automatic/manual gain, automatic/manual white balance, automatic/manual exposure control, vertical flip, brightness control, contrast control and saturation control. Additionally the following effects are available now: rotating the hue in the colorspace, gray scale image and solarize effect. That's a big patch, thus quite hard to review. What about splitting it in one patch per feature (or group of features, at least separating format configuration and controls) ? :-) Hello Laurent, I have reposted the my code split into 2 patches. The first with changes related to the image sizes and the other with all the controls. I hope that this separation makes it easy enough to review it. If not, tell me and I can split it up further. Thanks for reviewing, Bastian Hecht -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Afatech AF9013
On Wed, 2011-08-17 at 01:23 +0200, Jose Alberto Reguero wrote: On Martes, 16 de Agosto de 2011 22:57:24 Antti Palosaari escribió: On 08/16/2011 11:27 PM, Jose Alberto Reguero wrote: options dvb-usb force_pid_filter_usage=1 I change the signal timeout and tuning timeout and now it works perfect! I can watch two HD channels, thanks for your help. I really don't understand what force_pid_filter_usage do on the module, is there any documentation? Thanks and best regards. For usb devices with usb 2.0 when tunned to a channel there is enought usb bandwith to deliver the whole transponder. With pid filters they only deliver the pids needed for the channel. The only limit is that the pid filters is limited normaly to 32 pids. May I ask how wide DVB-T streams you have? Here in Finland it is about 22 Mbit/sec and I think two such streams should be too much for one USB bus. I suspect there is some other bug in back of this. regards Antti Here the transport stream is like yours. About 4 Mbit/sec by channel, and about 5 channels by transport stream. The problem I have is that when I have the two tuners working I have a few packets lost, and I have some TS discontinuitys. With pid filters the stream is perfect. Perhaps Josu have another problem. I am certain it is the configuration of the second frontend that ripples through Afatech devices. I have only got a single AF9015 device so can't test the dual configuration. Does the same problems exist when running the second frontend solo or dual with the Windows driver? With the IT1937(aka AF9035) the second frontend appeared not to work at all in Windows in dual mode. tvboxspy -- 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: How to git and build HVR-2200 drivers from Kernel labs ?
Eg should I use the source from git clone git://kernellabs.com/stoth/saa7164- dev.git or do you recommend something else that might be more stable ? pull a snapshot: http://www.kernellabs.com/gitweb/?p=stoth/saa7164-stable.git;a=snapshot;h=87e0c0378bf2068df5d0c43acd66aea9ba71bd89;sf=tgz -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Afatech AF9013
2011/8/17 Malcolm Priestley tvbox...@gmail.com: On Wed, 2011-08-17 at 01:23 +0200, Jose Alberto Reguero wrote: On Martes, 16 de Agosto de 2011 22:57:24 Antti Palosaari escribió: On 08/16/2011 11:27 PM, Jose Alberto Reguero wrote: options dvb-usb force_pid_filter_usage=1 I change the signal timeout and tuning timeout and now it works perfect! I can watch two HD channels, thanks for your help. I really don't understand what force_pid_filter_usage do on the module, is there any documentation? Thanks and best regards. For usb devices with usb 2.0 when tunned to a channel there is enought usb bandwith to deliver the whole transponder. With pid filters they only deliver the pids needed for the channel. The only limit is that the pid filters is limited normaly to 32 pids. May I ask how wide DVB-T streams you have? Here in Finland it is about 22 Mbit/sec and I think two such streams should be too much for one USB bus. I suspect there is some other bug in back of this. regards Antti Here the transport stream is like yours. About 4 Mbit/sec by channel, and about 5 channels by transport stream. The problem I have is that when I have the two tuners working I have a few packets lost, and I have some TS discontinuitys. With pid filters the stream is perfect. Perhaps Josu have another problem. I am certain it is the configuration of the second frontend that ripples through Afatech devices. I have only got a single AF9015 device so can't test the dual configuration. Does the same problems exist when running the second frontend solo or dual with the Windows driver? With the IT1937(aka AF9035) the second frontend appeared not to work at all in Windows in dual mode. tvboxspy Thanks Malcolm, sorry but I don't understand very good your post (my poor english). On Microsoft Windows XP the dual device works great, I can watch two different channels (different transponder). I want to know if there is an other command-line tool to test them, because it will be MythTV playback problem. The most problem is that sometimes one adapter work, then no adapter work and then both adapter work but there is no clear image (pixeled). How could I test if there is some packet drop? Thanks for your great help. Best regards. -- Josu Lazkano -- 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
[cron job] v4l-dvb daily build: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Wed Aug 17 19:00:30 CEST 2011 git hash:9bed77ee2fb46b74782d0d9d14b92e9d07f3df6e gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Negotiating frame buffer size between sensor subdevs and bridge devices
On 08/17/2011 12:25 AM, Sakari Ailus wrote: On Thu, Jul 28, 2011 at 07:04:11PM +0200, Sylwester Nawrocki wrote: Hello, Hi Sylwester, Hi Sakari, kiitos commentti ;) Trying to capture images in JPEG format with regular image sensor - mipi-csi receiver - host interface H/W configuration I've found there is no standard way to communicate between the sensor subdev and the host driver what is exactly a required maximum buffer size to capture a frame. For the raw formats there is no issue as the buffer size can be easily determined from the pixel format and resolution (or sizeimage set on a video node). However compressed data formats are a bit more complicated, the required memory buffer size depends on multiple factors, like compression ratio, exact file header structure etc. Often it is at the sensor driver where all information required to determine size of the allocated memory is present. Bridge/host devices just do plain DMA without caring much what is transferred. I know of hardware which, for some pixel formats, once data capture is started, writes to memory whatever amount of data the sensor is transmitting, without any means to interrupt on the host side. So it is critical to assure the buffer allocation is done right, according to the sensor requirements, to avoid buffer overrun. Here is a link to somehow related discussion I could find: [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg27138.html In order to let the host drivers query or configure subdevs with required frame buffer size one of the following changes could be done at V4L2 API: 1. Add a 'sizeimage' field in struct v4l2_mbus_framefmt and make subdev drivers optionally set/adjust it when setting or getting the format with set_fmt/get_fmt pad level ops (and s/g_mbus_fmt ?) There could be two situations: - effective required frame buffer size is specified by the sensor and the host driver relies on that value when allocating a buffer; - the host driver forces some arbitrary buffer size and the sensor performs any required action to limit transmitted amount of data to that amount of data; Both cases could be covered similarly as it's done with VIDIOC_S_FMT. Introducing 'sizeimage' field is making the media bus format struct looking more similar to struct v4l2_pix_format and not quite in line with media bus format meaning, i.e. describing data on a physical bus, not in the memory. The other option I can think of is to create separate subdev video ops. 2. Add new s/g_sizeimage subdev video operations The best would be to make this an optional callback, not sure if it makes sense though. It has an advantage of not polluting the user space API. Although 'sizeimage' in user space might be useful for some purposes I rather tried to focus on in-kernel calls. I prefer this second approach over the first once since the maxiumu size of the image in bytes really isn't a property of the bus. After thinking some more about it I came to similar conclusion. I intended to find some better name for s/g_sizeimage callbacks and post relevant patch for consideration. Although I haven't yet found some time to carry on with this. How about a regular V4L2 control? You would also have minimum and maximum values, I'm not quite sure whather this is a plus, though. :) My intention was to have these calls used only internally in the kernel and do not allow the userspace to mess with it. All in all, if anything had interfered and the host driver would have allocated too small buffer the system would crash miserably due to buffer overrun. The final buffer size for a JFIF/EXIF file will depend on other factors, like main image resolution, JPEG compression ratio, the thumbnail inclusion and its format/resolution, etc. I imagine we should be rather creating controls for those parameters. Also the driver would most likely have to validate the buffer size during STREAMON call. Btw. how does v4l2_mbus_framefmt suit for compressed formats in general? Well, there is really nothing particularly addressing the compressed formats in that struct. But we need to use it as the compressed data flows through the media bus in same way as the raw data. It's rather hard to define the pixel codes using existing convention as there is no simple relationship between the pixel data and what is transferred on the bus. Yet I haven't run into issues other than no means to specify the whole image size. -- Regards, Sylwester -- 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: Afatech AF9013
On 08/17/2011 10:36 AM, Josu Lazkano wrote: I don't know how wide is the stream, but it could be a USB wide limitation. My board is a little ION based and I have some USB devices: $ lsusb Bus 001 Device 002: ID 1b80:e399 Afatech Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub I don't think so. Total under 50Mbit/sec stream should not be too much for one USB2.0 root hub. Which is chipset used ION (it is southbridge which contains usually USB ports)? The problematic twin device is the Afatech one, there is an DVB-S2 USB tuner, a bluetooth dongle, a IR receiver and a wireless mouse/keybord receiver. Now I am at work, I will try to disconnect all devices and try with just the DVB-T device. I use to try with MythTV if it works or not. Is there any other tool to test and debug more deep about USB or DVB wide? You can look stream sizes using dvbtraffic tool. It is last line of output which shows total stream size. tzap can be used to tune channel. But it you can use some other app like MythTV and then run dvbtraffic same time. I apreciate your help. Thanks and best regards. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Smart card reader support for Anysee DVB devices
On 08/15/2011 02:14 PM, Antti Palosaari wrote: On 08/15/2011 02:51 AM, Antti Palosaari wrote: Biggest problem I see whole thing is poor application support. OpenCT is rather legacy but there is no good alternative. All this kind of serial drivers seems to be OpenCT currently. I wonder if it is possible to make virtual CCID device since CCID seems to be unfortunately the only interface SmartCard guys currently care. I studied scenario and looks like it is possible to implement way like, register virtual USB HCI (virtual motherboard USB controller) then register virtual PC/SC device to that which hooks all calls to HW via Anysee driver. Some glue surely needed for emulate PC/SC. I think there is not any such driver yet. Anyhow, there is virtual USB HCI driver currently in staging which can be used as example, or even use it to register virtual device. That kind of functionality surely needs more talking... regards, Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Afatech AF9013
On Wed, 2011-08-17 at 19:27 +0200, Josu Lazkano wrote: 2011/8/17 Malcolm Priestley tvbox...@gmail.com: On Wed, 2011-08-17 at 01:23 +0200, Jose Alberto Reguero wrote: On Martes, 16 de Agosto de 2011 22:57:24 Antti Palosaari escribió: On 08/16/2011 11:27 PM, Jose Alberto Reguero wrote: options dvb-usb force_pid_filter_usage=1 I change the signal timeout and tuning timeout and now it works perfect! I can watch two HD channels, thanks for your help. I really don't understand what force_pid_filter_usage do on the module, is there any documentation? Thanks and best regards. For usb devices with usb 2.0 when tunned to a channel there is enought usb bandwith to deliver the whole transponder. With pid filters they only deliver the pids needed for the channel. The only limit is that the pid filters is limited normaly to 32 pids. May I ask how wide DVB-T streams you have? Here in Finland it is about 22 Mbit/sec and I think two such streams should be too much for one USB bus. I suspect there is some other bug in back of this. regards Antti Here the transport stream is like yours. About 4 Mbit/sec by channel, and about 5 channels by transport stream. The problem I have is that when I have the two tuners working I have a few packets lost, and I have some TS discontinuitys. With pid filters the stream is perfect. Perhaps Josu have another problem. I am certain it is the configuration of the second frontend that ripples through Afatech devices. I have only got a single AF9015 device so can't test the dual configuration. Does the same problems exist when running the second frontend solo or dual with the Windows driver? With the IT1937(aka AF9035) the second frontend appeared not to work at all in Windows in dual mode. tvboxspy Thanks Malcolm, sorry but I don't understand very good your post (my poor english). On Microsoft Windows XP the dual device works great, I can watch two different channels (different transponder). I want to know if there is an other command-line tool to test them, because it will be MythTV playback problem. The most problem is that sometimes one adapter work, then no adapter work and then both adapter work but there is no clear image (pixeled). How could I test if there is some packet drop? Looks like you need pid filtering on, but if you force the pid filter on, it won't work because there is no functions setup for it on the driver for the second frontend. btw, you need to force the pid filter on at boot time, add a line in file. /etc/modules (or /etc/modules.preload) dvb_usb force_pid_filter_usage=1 Otherwise, it keeps going off. But, it will only work on the first frontend(adapter). It works fine on my single one. Regards Malcolm -- 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] Fix locking and resource problems for PCTV 290e (em28xx / em28xx-dvb)
Hi, This is my third draft at fixing the em28xx modules for the PCTV 290e DVB adapter. The highlights are: - resolve the locking issue when replugging the USB adapter, - remove race condition when initialising an adapter and simultaneously loading the em28xx-dvb module, - more reliable releasing of resources on error paths, - use atomic bit operations for em28xx_devused mask, and enforce hard limit of EM28XX_MAXBOARDS adapters. The patch is against 3.0, but these particular files don't seem to have changed much by 3.1. Any review comments welcome. Cheers, Chris --- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-17 08:52:25.0 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-17 08:52:37.0 +0100 @@ -1160,10 +1160,9 @@ static DEFINE_MUTEX(em28xx_devlist_mutex); /* - * em28xx_realease_resources() - * unregisters the v4l2,i2c and usb devices - * called when the device gets disconected or at module unload -*/ + * em28xx_remove_from_devlist() + * Removes the device from the list of active devices. + */ void em28xx_remove_from_devlist(struct em28xx *dev) { mutex_lock(em28xx_devlist_mutex); @@ -1171,13 +1170,6 @@ mutex_unlock(em28xx_devlist_mutex); }; -void em28xx_add_into_devlist(struct em28xx *dev) -{ - mutex_lock(em28xx_devlist_mutex); - list_add_tail(dev-devlist, em28xx_devlist); - mutex_unlock(em28xx_devlist_mutex); -}; - /* * Extension interface */ @@ -1193,8 +1185,8 @@ list_for_each_entry(dev, em28xx_devlist, devlist) { ops-init(dev); } - printk(KERN_INFO Em28xx: Initialized (%s) extension\n, ops-name); mutex_unlock(em28xx_devlist_mutex); + printk(KERN_INFO Em28xx: Initialized (%s) extension\n, ops-name); return 0; } EXPORT_SYMBOL(em28xx_register_extension); @@ -1207,9 +1199,9 @@ list_for_each_entry(dev, em28xx_devlist, devlist) { ops-fini(dev); } - printk(KERN_INFO Em28xx: Removed (%s) extension\n, ops-name); list_del(ops-next); mutex_unlock(em28xx_devlist_mutex); + printk(KERN_INFO Em28xx: Removed (%s) extension\n, ops-name); } EXPORT_SYMBOL(em28xx_unregister_extension); @@ -1218,11 +1210,10 @@ struct em28xx_ops *ops = NULL; mutex_lock(em28xx_devlist_mutex); - if (!list_empty(em28xx_extension_devlist)) { - list_for_each_entry(ops, em28xx_extension_devlist, next) { - if (ops-init) -ops-init(dev); - } + list_add_tail(dev-devlist, em28xx_devlist); + list_for_each_entry(ops, em28xx_extension_devlist, next) { + if (ops-init) + ops-init(dev); } mutex_unlock(em28xx_devlist_mutex); } --- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-17 19:22:29.0 +0100 @@ -60,7 +60,7 @@ module_param_array(card, int, NULL, 0444); MODULE_PARM_DESC(card, card type); -/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS */ +/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */ static unsigned long em28xx_devused; struct em28xx_hash_table { @@ -2738,9 +2738,9 @@ #endif /* CONFIG_MODULES */ /* - * em28xx_realease_resources() + * em28xx_release_resources() * unregisters the v4l2,i2c and usb devices - * called when the device gets disconected or at module unload + * called when the device gets disconnected or at module unload */ void em28xx_release_resources(struct em28xx *dev) { @@ -2763,7 +2763,7 @@ usb_put_dev(dev-udev); /* Mark device as unused */ - em28xx_devused = ~(1 dev-devno); + clear_bit(dev-devno, em28xx_devused); }; /* @@ -2776,7 +2776,6 @@ { struct em28xx *dev = *devhandle; int retval; - int errCode; dev-udev = udev; mutex_init(dev-ctrl_urb_lock); @@ -2872,12 +2871,11 @@ } /* register i2c bus */ - errCode = em28xx_i2c_register(dev); - if (errCode 0) { - v4l2_device_unregister(dev-v4l2_dev); + retval = em28xx_i2c_register(dev); + if (retval 0) { em28xx_errdev(%s: em28xx_i2c_register - errCode [%d]!\n, - __func__, errCode); - return errCode; + __func__, retval); + goto fail_reg_i2c; } /* @@ -2891,11 +2889,11 @@ em28xx_card_setup(dev); /* Configure audio */ - errCode = em28xx_audio_setup(dev); - if (errCode 0) { - v4l2_device_unregister(dev-v4l2_dev); + retval = em28xx_audio_setup(dev); + if (retval 0) { em28xx_errdev(%s: Error while setting audio - errCode [%d]!\n, - __func__, errCode); + __func__, retval); + goto fail_setup_audio; } /* wake i2c devices */ @@ -2909,31 +2907,28 @@ if (dev-board.has_msp34xx) { /* Send a reset to other chips via gpio */ - errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7); - if (errCode 0) { - em28xx_errdev(%s: em28xx_write_regs_req - + retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7); + if (retval 0) { + em28xx_errdev(%s: em28xx_write_reg - msp34xx(1) failed! errCode [%d]\n, - __func__, errCode); - return errCode; + __func__, retval); + goto fail_write_reg; }
RE: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
Hello, On Wednesday, August 17, 2011 4:58 PM Pawel Osciak wrote: On Wed, Aug 17, 2011 at 06:22, Marek Szyprowski m.szyprow...@samsung.com wrote: On Monday, August 15, 2011 3:46 PM Guennadi Liakhovetski wrote: While switching back, I have to change the struct vb2_ops::queue_setup() operation to take a struct v4l2_create_buffers pointer. An earlier version of this patch just added one more parameter to .queue_setup(), which is easier - changes to videobuf2-core.c are smaller, but it is then redundant. We could use the create pointer for both input and output. The video plane configuration in frame format is the same as what is calculated in .queue_setup(), IIUC. So, we could just let the driver fill that one in. This would require then the videobuf2-core.c to parse struct v4l2_format to decide which union member we need, depending on the buffer type. Do we want this or shall drivers duplicate plane sizes in separate .queue_setup() parameters? IMHO if possible we should have only one callback for the driver. Please notice that the driver should be also allowed to increase (or decrease) the number of buffers for particular format/fourcc. Or remove queue_setup altogether (please see my example above). What do you think Marek? I'm perfectly fine with replacing queue_setup callback with something else. Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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