[GIT PATCHES FOR 2.6.39] gspca_sn9c20x fixes
Hi Mauro, Please pull from my gspca tree, for some gspca_sn9c20x fixes I've been doing. The following changes since commit 5ed4bbdae09d207d141759e013a0f3c24ae76ecc: [media] tuner-core: Don't touch at standby during tuner_lookup (2011-02-15 10:31:01 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git gspca-for_v2.6.39 Hans de Goede (5): gspca_sn9c20x: Fix colored borders with ov7660 sensor gspca_sn9c20x: Add hflip and vflip controls for the ov7660 sensor gspca_sn9c20x: Add LED_REVERSE flag for 0c45:62bb gspca_sn9c20x: Make buffers slightly larger for JPEG frames gspca_sn9c20x: Add another MSI laptop to the sn9c20x upside down list drivers/media/video/gspca/sn9c20x.c | 40 ++ 1 files changed, 30 insertions(+), 10 deletions(-) Thanks, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR for remote control not working for Hauppauge WinTV-HVR-1150 (SAA7134)
Ok, Devin. I'm not sure about how it would went but if you think it would be worth trying to get info about the components of the 1150 directly from Hauppauge, I can give them a call and see what happens. I've just ordered 20 of those boards for a project that initially wouldn't relly on RC but since the IR seems to be there. I'm not saying they would help, it would be just a try if you think it could help... Regards, Fernando On Wed, Feb 16, 2011 at 3:03 PM, Devin Heitmueller dheitmuel...@kernellabs.com wrote: On Wed, Feb 16, 2011 at 11:54 AM, Fernando Laudares Camargos fernando.laudares.camar...@gmail.com wrote: Thanks, Jarod, for re-directing the message and explaining why the fix from Mauro's wouldn't suffice here. Devin: I was hoping they (Hauppauge) had used the same componentry as of the predecessor board (HVR-1120) for the IR, looks that's not the case. I would like to help to get the card supported but unless there's a procedure documented somewhere showing what to do I guess I don't qualify for the job (don't even understand what you meant by TLC). But if I can help somehow, I would be glad to do it. It is the same layout as the 1120 (I cheated and just looked at the schematic), but despite some user having submitted the patch, it wasn't working at least for me. Hence, the fact that you see code in the driver referencing the 1120 doesn't mean it actually works. There is no real documented procedure for adding this sort of support. It needs to be investigated by an actual driver developer familiar with the underlying hardware design. TLC is just an English expression for Tender Love and Care, an expression that is synonymous with giving something the requisite attention. I've got the board, but just need to find a few hours to debug the IRQ handler. Devin -- Devin J. Heitmueller - 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: MCEUSB: falsly claims mass storage device
On 09.02.2011 06:19, Jarod Wilson wrote: [...] Looks like bInterfaceNumber == 2 on this device. The patch to handle this similar to the conexant polaris devices should be pretty trivial. I'll try to get something together tomorrow. Hi, any news on this one? Regards, Lucian -- 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: soc-camera: Benefits of soc-camera interface over specific char drivers that use Gstreamer lib
Hi Bhupesh, On Wednesday 16 February 2011 14:57:12 Bhupesh SHARMA wrote: On Wednesday, February 16, 2011 7:20 PM Guennadi Liakhovetski wrote: On Wed, 16 Feb 2011, Laurent Pinchart wrote: On Wednesday 16 February 2011 06:57:11 Bhupesh SHARMA wrote: Hi Guennadi, As I mentioned in one of my previous mails , we are developing a Camera Host and Sensor driver for our ST specific SoC and considering using the soc-camera framework for the same. One of our open-source customers has raised a interesting case though: It seems they have an existing solution (for another SoC) in which they do not use V4L2 framework and instead use the Gstreamer with framebuffer. They specifically wish us to implement a solution which is compatible with ANDROID applications. Could you please help us in deciding which approach is preferable in terms of performance, maintenance and ease-of-design. That's a difficult question that can't be answered without more details about your SoC. Could you share some documentation, such as a high-level block diagram of the video-related blocks in the SoC ? Laurent, IIUC, the choice above referred not to soc-camera vs. plain v4l2, but to v4l2 vs. original android-style video character device, which doesn't seem so difficult to me;) That's correct Guennadi :) The choice I have to make is to between v4ls (soc-camera) vs. a specific video char driver written to support android-style applications. Also I am not sure about how gstreamer over framebuffer can interface with such a design and the respective merits/demerits. GStreamer is often used on top of V4L2 devices. I'm not sure to understand what GStreamer over framebuffer is supposed to mean here, as you're looking for a solution for your camera driver, and the framebuffer API is used for video output only, not video capture. -- 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: soc-camera: Benefits of soc-camera interface over specific char drivers that use Gstreamer lib
Hi Bhupesh, On Wednesday 16 February 2011 14:54:02 Bhupesh SHARMA wrote: On Wednesday, February 16, 2011 7:14 PM Laurent Pinchart wrote: On Wednesday 16 February 2011 06:57:11 Bhupesh SHARMA wrote: Hi Guennadi, As I mentioned in one of my previous mails , we are developing a Camera Host and Sensor driver for our ST specific SoC and considering using the soc-camera framework for the same. One of our open-source customers has raised a interesting case though: It seems they have an existing solution (for another SoC) in which they do not use V4L2 framework and instead use the Gstreamer with framebuffer. They specifically wish us to implement a solution which is compatible with ANDROID applications. Could you please help us in deciding which approach is preferable in terms of performance, maintenance and ease-of-design. That's a difficult question that can't be answered without more details about your SoC. Could you share some documentation, such as a high-level block diagram of the video-related blocks in the SoC ? At the top-level the interface is very simple: - 8-bit data interface --- | Camera sensor | --- | SoC (ARM Based) | | | | | | | I2C control Interface | | | | --- | | | | | | | | SYNC signals (HS/VS) | | | | --- | | | | | | | | Pixel CLK | | | | --- | | | | | | | | Sensor CLK| | | | --- | | - --- The SoC itself has a simple interface to transfer the data received via system DMA and has buffer to store an incoming line data. Also one can program the SoC logic to apply transformations on the input data. If the SoC just writes incoming data to memory using DMA, a V4L2 driver should be pretty easy to develop. If your SoC can capture images in different formats, possible applying complex processing (such as scaling the image) and supports memory-to-memory processing modes, you might need the media controller API in addition to the V4L2 API. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] uvcvideo: Add support for MPEG-2 TS payload
Hi Stephan, On Friday 28 January 2011 20:35:05 Stephan Lachowsky wrote: Parse the UVC 1.0 and UVC 1.1 VS_FORMAT_MPEG2TS descriptors. This a stream based format, so we generate a dummy frame descriptor with a dummy frame interval range. Thanks for the patch, and sorry for the late reply. Don't you also need to implement support for the V4L2 MPEG CIDs ? I would expect the driver to support at least the controls used to select the MPEG format (MPEG2, TS), even if they're hardcoded to MPEG2-TS. --- drivers/media/video/uvc/uvc_driver.c | 41 ++ drivers/media/video/uvc/uvcvideo.h | 3 ++ 2 files changed, 44 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c index a1e9dfb..6bcb9e1 100644 --- a/drivers/media/video/uvc/uvc_driver.c +++ b/drivers/media/video/uvc/uvc_driver.c @@ -103,6 +103,11 @@ static struct uvc_format_desc uvc_fmts[] = { .guid = UVC_GUID_FORMAT_BY8, .fcc= V4L2_PIX_FMT_SBGGR8, }, + { + .name = MPEG2 TS, + .guid = UVC_GUID_FORMAT_MPEG, + .fcc= V4L2_PIX_FMT_MPEG, + }, }; /* @@ -398,6 +403,33 @@ static int uvc_parse_format(struct uvc_device *dev, break; case UVC_VS_FORMAT_MPEG2TS: + n = dev-uvc_version = 0x0110 ? 23 : 7; + if (buflen n) { + uvc_trace(UVC_TRACE_DESCR, device %d videostreaming +interface %d FORMAT error\n, +dev-udev-devnum, +alts-desc.bInterfaceNumber); + return -EINVAL; + } + + strlcpy(format-name, MPEG2 TS, sizeof format-name); + format-fcc = V4L2_PIX_FMT_MPEG; + format-flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM; + format-bpp = 0; + ftype = 0; + + /* Create a dummy frame descriptor. */ + frame = format-frame[0]; + memset(format-frame[0], 0, sizeof format-frame[0]); + frame-bFrameIntervalType = 0; + frame-dwDefaultFrameInterval = 1; + frame-dwFrameInterval = *intervals; + *(*intervals)++ = 1; + *(*intervals)++ = 1000; + *(*intervals)++ = 1; + format-nframes = 1; + break; + case UVC_VS_FORMAT_STREAM_BASED: /* Not supported yet. */ default: @@ -673,6 +705,14 @@ static int uvc_parse_streaming(struct uvc_device *dev, break; case UVC_VS_FORMAT_MPEG2TS: + /* MPEG2TS format has no frame descriptor. We will create a + * dummy frame descriptor with a dummy frame interval range. + */ + nformats++; + nframes++; + nintervals += 3; + break; + case UVC_VS_FORMAT_STREAM_BASED: uvc_trace(UVC_TRACE_DESCR, device %d videostreaming interface %d FORMAT %u is not supported.\n, @@ -724,6 +764,7 @@ static int uvc_parse_streaming(struct uvc_device *dev, switch (buffer[2]) { case UVC_VS_FORMAT_UNCOMPRESSED: case UVC_VS_FORMAT_MJPEG: + case UVC_VS_FORMAT_MPEG2TS: case UVC_VS_FORMAT_DV: case UVC_VS_FORMAT_FRAME_BASED: format-frame = frame; diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h index 45f01e7..e522f99 100644 --- a/drivers/media/video/uvc/uvcvideo.h +++ b/drivers/media/video/uvc/uvcvideo.h @@ -152,6 +152,9 @@ struct uvc_xu_control { #define UVC_GUID_FORMAT_BY8 \ { 'B', 'Y', '8', ' ', 0x00, 0x00, 0x10, 0x00, \ 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71} +#define UVC_GUID_FORMAT_MPEG \ + { 'M', 'P', 'E', 'G', 0x00, 0x00, 0x10, 0x00, \ + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71} /* * Driver specific constants. -- 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: [RFD] frame-size switching: preview / single-shot use-case
Hi all Let me try to further elaborate on this topic. So far, I think, the following provides the cleanest solution to our quick format-switching / multiple video-queue problem: On Wed, 16 Feb 2011, Guennadi Liakhovetski wrote: (2) cleanly separate setting video data format (S_FMT) from specifying the allocated buffer size. The control flow then would look like: struct v4l2_requestbuffers req; fd = open(); /* initialise and configure the first still-shot buffer-queue */ bufq.id = -EINVAL; ret = ioctl(fd, VIDIOC_BUFQ_SELECT, bufq); /* * bufq.id now contains a handle to a newly allocated videobuf * queue, it also becomes the current queue. */ still_qid = buf.id; /* * specify explicitly required buffer size - use one of reserved * fields */ req.size = still_x * still_y * bpp; ret = ioctl(fd, VIDIOC_REQBUFS, req); for (i = 0; i req.count; i++) { buf.index = i; ret = ioctl(fd, VIDIOC_QUERYBUF, buf); start = mmap(); ret = ioctl(fd, VIDIOC_QBUF, buf); } /* Allocate a second preview queue */ bufq.id = -EINVAL; ret = ioctl(fd, VIDIOC_BUFQ_SELECT, bufq); preview_qid = bufq.id; req.size = preview_x * preview_y * bpp; ret = ioctl(fd, VIDIOC_REQBUFS, req); for (i = 0; i req.count; i++) { buf.index = i; ret = ioctl(fd, VIDIOC_QUERYBUF, buf); start = mmap(); ret = ioctl(fd, VIDIOC_QBUF, buf); } /* Stay on the preview queue */ ret = ioctl(fd, VIDIOC_STREAMON, type); for (i = 0; i n; i++) { ret = ioctl(fd, VIDIOC_DQBUF, buf); ret = ioctl(fd, VIDIOC_QBUF, buf); } ret = ioctl(fd, VIDIOC_STREAMOFF, type); /* Switch to the still-shot queue */ bufq.id = still_qid; ret = ioctl(fd, VIDIOC_BUFQ_SELECT, bufq); ret = ioctl(fd, VIDIOC_STREAMON, type); for (i = 0; i n; i++) { ret = ioctl(fd, VIDIOC_DQBUF, buf); ret = ioctl(fd, VIDIOC_QBUF, buf); } ret = ioctl(fd, VIDIOC_STREAMOFF, type); And so on. If the above is _conceptually_ acceptable, we need 1 to 2 new ioctl()s and an extension to the VIDIOC_REQBUFS ioctl() to specify the buffer size. I wrote 1 or 2 new ioctl()s, because above the new VIDIOC_BUFQ_SELECT ioctl() performs two functions: if the handle is negative, it allocates a new queue, if it containes a valid (per-device or per file-handle?) queue ID, it switches to it. We might also decide to use two ioctl()s for these two functions. Comments? 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 RFC] uvcvideo: Add support for MPEG-2 TS payload
Hi Laurent, On Thu, 2011-02-17 at 08:03 -0800, Laurent Pinchart wrote: Hi Stephan, On Friday 28 January 2011 20:35:05 Stephan Lachowsky wrote: Parse the UVC 1.0 and UVC 1.1 VS_FORMAT_MPEG2TS descriptors. This a stream based format, so we generate a dummy frame descriptor with a dummy frame interval range. Thanks for the patch, and sorry for the late reply. No worries, just glad to have the moss knocked off the stone. Don't you also need to implement support for the V4L2 MPEG CIDs ? I would expect the driver to support at least the controls used to select the MPEG format (MPEG2, TS), even if they're hardcoded to MPEG2-TS. That would be possible, for the stream type there is your choice of MPEG2-TS so that is trivial. There are a very limited set of standardized controls that can be mapped: wKeyFrameRate, wPFrameRate, wCompQuality from the VS probe/commit (GOP size, B frames, bitrate). Since these controls are optional in the spec, and an overly simplistic projection of the encoder's actual configuration space, device manufactures (typically) choose instead to use custom XUs that expose richer more representative ones. Given this state of affairs, I think it would be prudent to blindly forward the data stream (Which is all, in essence, this patch enables) leaving the configuration to userspace. I'm not suggesting we preclude adding XU - MPEG2 CID mappings into uvcvideo later, just that as is this is a valuable step forward. Stephan --- drivers/media/video/uvc/uvc_driver.c | 41 ++ drivers/media/video/uvc/uvcvideo.h | 3 ++ 2 files changed, 44 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c index a1e9dfb..6bcb9e1 100644 --- a/drivers/media/video/uvc/uvc_driver.c +++ b/drivers/media/video/uvc/uvc_driver.c @@ -103,6 +103,11 @@ static struct uvc_format_desc uvc_fmts[] = { .guid = UVC_GUID_FORMAT_BY8, .fcc= V4L2_PIX_FMT_SBGGR8, }, + { + .name = MPEG2 TS, + .guid = UVC_GUID_FORMAT_MPEG, + .fcc= V4L2_PIX_FMT_MPEG, + }, }; /* @@ -398,6 +403,33 @@ static int uvc_parse_format(struct uvc_device *dev, break; case UVC_VS_FORMAT_MPEG2TS: + n = dev-uvc_version = 0x0110 ? 23 : 7; + if (buflen n) { + uvc_trace(UVC_TRACE_DESCR, device %d videostreaming + interface %d FORMAT error\n, + dev-udev-devnum, + alts-desc.bInterfaceNumber); + return -EINVAL; + } + + strlcpy(format-name, MPEG2 TS, sizeof format-name); + format-fcc = V4L2_PIX_FMT_MPEG; + format-flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM; + format-bpp = 0; + ftype = 0; + + /* Create a dummy frame descriptor. */ + frame = format-frame[0]; + memset(format-frame[0], 0, sizeof format-frame[0]); + frame-bFrameIntervalType = 0; + frame-dwDefaultFrameInterval = 1; + frame-dwFrameInterval = *intervals; + *(*intervals)++ = 1; + *(*intervals)++ = 1000; + *(*intervals)++ = 1; + format-nframes = 1; + break; + case UVC_VS_FORMAT_STREAM_BASED: /* Not supported yet. */ default: @@ -673,6 +705,14 @@ static int uvc_parse_streaming(struct uvc_device *dev, break; case UVC_VS_FORMAT_MPEG2TS: + /* MPEG2TS format has no frame descriptor. We will create a +* dummy frame descriptor with a dummy frame interval range. +*/ + nformats++; + nframes++; + nintervals += 3; + break; + case UVC_VS_FORMAT_STREAM_BASED: uvc_trace(UVC_TRACE_DESCR, device %d videostreaming interface %d FORMAT %u is not supported.\n, @@ -724,6 +764,7 @@ static int uvc_parse_streaming(struct uvc_device *dev, switch (buffer[2]) { case UVC_VS_FORMAT_UNCOMPRESSED: case UVC_VS_FORMAT_MJPEG: + case UVC_VS_FORMAT_MPEG2TS: case UVC_VS_FORMAT_DV: case UVC_VS_FORMAT_FRAME_BASED: format-frame = frame; diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h index 45f01e7..e522f99 100644 --- a/drivers/media/video/uvc/uvcvideo.h +++ b/drivers/media/video/uvc/uvcvideo.h @@ -152,6 +152,9 @@ struct uvc_xu_control { #define UVC_GUID_FORMAT_BY8 \ { 'B', 'Y',
Re: [RFD] frame-size switching: preview / single-shot use-case
Em 15-02-2011 15:33, Guennadi Liakhovetski escreveu: Hi This topic has been slightly discussed several times [1] before, but there has been no conclusion, nor I'm aware of any implementation, suitably resolving this problem. I've added to CC all involved in earlier discussions, that I managed to find. What seems a typical use-case to me is a system with a vewfinder or a display, providing a live preview of the video data from a source, like a camera, with a relatively low resolution, and a possibility to take high-resolution still photos with a very short delay. Currently this is pretty difficult to realise, e.g., with soc-camera drivers you have to free the videobuf(2) queue, by either closing and re-opening the interface, or by issuing an ioctl(VIDIOC_REQBUFS, count=0) if your driver is already using videobuf2 and if this is really working;), configure with a different resolution and re-allocate videobuffers (or use different buffers, allocated per USERPTR). Another possibility would be to allocate and use buffers large enough for still photos, also for the preview, which would be wasteful, because one can well need many more preview than still-shot buffers. So, it seems to me, we could live with a better solution. 1. We could use separate inputs for different capture modes and support per-input videobuf queues. Advantage: no API changes required. Disadvantages: confusing, especially, if a driver already exports multiple inputs. The driver does not know, whether this mode is required or not, always exporting 2 inputs for this purpose doesn't seem like a good idea. Eventually, the user might want not 2, but 3 or more of such videobuf queues. Very bad. The high res is not a new input. 2. Use different IO methods, e.g., mmap() for preview and read() for still shots. I'm just mentioning this possibility here, because it occurred in one of previous threads, but I don't really like it either. What if you want to use the same IO method for all? Etc. Can be done. 3. Not liking either of the above, it seems we need yet a new API for this... How about extending VIDIOC_REQBUFS with a videobuf queue index, thus using up one of the remaining two 32-bit reserved fields? Then we need one more ioctl() like VIDIOC_BUFQ_SELECT to switch from one queue to another, after which setting frame format and queuing and dequeuing buffers will affect this currently selected queue. We could also keep REQBUFS as is and require BUFQ_SELECT to be called before it for any queue except the default 0. Yes, I know, that some video sensors have a double register set for this dual-format operation, so, for them it is natural to support two queues, and drivers are certainly most welcome to use this feature for, say, the first two queues. On other sensors and for any further queues switching will have to be done in software. Seems too hacky to me. There's a 4th alternative: open the device twice, and use different settings on each open. Userspace can stop streaming on one file descriptor and start the other one when he wants to take a high-res picture. Cheers, 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: [RFD] frame-size switching: preview / single-shot use-case
Em 16-02-2011 08:35, Guennadi Liakhovetski escreveu: But from the point of view of the application it makes more sense to actually have two video nodes. The only difference is that when one starts streaming it pre-empts the other. Well, I don't think I like it all that much... One reason - yes, two independent video device-nodes, which actually only can stream in turn seems somewhat counterintuitive to me, specifically, I don't think there is a way to tell the application about this. What if we have more than two video devices on the system? Or what if you need more than two video queues / formats? Or actually only one? The kernel doesn't know initially how many, this is a dynamic resource, not a fixed static interface, IMHO. I agree with this, which is why I don't think two (or more) video nodes would be a good solution. I agree. Video nodes shouldn't be bind to an specific format. A device with 2 video nodes should be able to retrieve images from two independent video sources. Unfortunately, ivtv driver was merged with the bad concept of one video node per different type of formats (partially my fault: I remember I commented about it at the time it was submitted, but, as its merge took a long time, and there were several other issues that were needed to be solved there, I ended by giving up and letting it to come with this API non-compliance, hoping that a fix would happen at the next kernel release. Unfortunately, it was never fixed). We've hit the exact same issue with the OMAP3 ISP driver. Our current solution is to allocate video buffer queues at the file handle level instead of the video node level. Applications can open the same video device twice and allocate buffers for the viewfinder on one of the instances and for still image capture on the other. When switching from viewfinder to still image capture, all it needs to do (beside obviously reconfiguring the media controller pipeline if required) is to issue VIDIOC_STREAMOFF on the viewfinder file handle and VIDIOC_STREAMON on the still capture file handle. This seems to be the proper way. solution (2) of using read()/mmap() is just an special case of per-file handle stream control, as applications that used this approach in the past were, in fact, using two opens, one for read, and another for mmap (to be clear, I'm not in favor of 2, I'm just saying that a per-file handle solution will allow (2) also). One issue with this approach is that allocating buffers requires knowledge of the format and resolution. The driver traditionally computes the buffer size from the parameters set by VIDIOC_S_FMT. This would prevent an application opening the video node a second time and setting up buffers for still image capture a second time while the viewfinder is running, as the VIDIOC_S_FMT on the second file handle won't be allowed then. Changes to the V4L2 spec would be needed to allow this to work properly. The spec is actually saying about the S_FMT ioctl(): On success the driver may program the hardware, allocate resources and generally prepare for data exchange. - _may_ program the hardware. So, if we don't do that and instead only verify the format and store it for future activation upon a call to STREAMON we are not violating the spec, thus, no change is required. OTOH, another spec sections V4L2 close() says: data format parameters, current input or output, control values or other properties remain unchanged. which is usually interpreted as a sequence open(); ioctl(S_FMT); close(); open(); ioctl(STREAMON); _must_ use the format, set by the call to S_FMT, which is not necessarily logical, if we adopt the per-file-descriptor format / stream approach. Every time a may appears on a spec, we'll have troubles, as some drivers will follow the may and others won't follow. Changing the behaviour will likely cause regressions, whatever direction is taken. One alternative would be to have a better way to negotiate features than what's provided by QUERYCAP. If we look for some protocols with a long life, like telnet, they don't have a one-way to check/set capabilities. Instead, both parties should present their capabilities and the client need to negotiate what he wants. We could do something like: ret = ioctl(fd, VIDIOC_QUERYCAP, cap); if (cap.capabilities CAN_PER_FD_FMT) { setcap.capabilities |= SHOULD_PER_FD_FMT; ret = ioctl(fd, VIDIOC_SETCAP, setcap); } To be sure that the kernel driver will behave fine. Yet, in this particular case, this would mean that drivers or core will need to handle both per-fd and per-node S_FMT friends. I think, we have two options: (1) adopt Laurent's proposal of per-fd contexts, but that would require a pretty heave modification of the spec - S_FMT is not kept across close() / open() pair. Whatever done, we'll need to change the specs in a lot of places.
Re: [RFD] frame-size switching: preview / single-shot use-case
Hi Mauro Thanks for your comments. On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: Em 16-02-2011 08:35, Guennadi Liakhovetski escreveu: [snip] (2) cleanly separate setting video data format (S_FMT) from specifying the allocated buffer size. This would break existing applications. Too late for that, except if negotiated with a SETCAP like approach. Sorry, don't see how my proposal from my last mail would change existing applications. As long as no explicit buffer-queue management is performed, no new queues are allocated, the driver will just implicitly allocate one queue and use it. I.e., no change in behaviour. There's an additional problem with that: assume that streaming is happening, and a S_FMT changing the resolution was sent. There's no way to warrant that the very next frame will have the new resolution. So, a meta-data with the frame resolution (and format) would be needed. Sorry, we are not going to allow format changes during a running capture. You have to stop streaming, set new formats (possibly switch to another queue) and restart streaming. What am I missing? 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
[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:Thu Feb 17 19:00:23 CET 2011 git master: 1b59be2a6cdcb5a12e18d8315c07c94a624de48f git media-master: gcc version: i686-linux-gcc (GCC) 4.5.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: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-x86_64: OK 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.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: OK linux-2.6.33-x86_64: OK linux-2.6.34-x86_64: OK linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.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: soc-camera and videobuf2
On Wed, 16 Feb 2011, Andrew Chew wrote: Reposting. Sorry for the rich text in my previous email. I'm looking at the videobuf2 stuff, and would like to use it because it solves a bunch of problems for me that were in videobuf (for example, I'm writing a variant of videobuf-dma-contig, and there's some private memory allocator state I needed to track, but the videobuf stuff didn't seem to let you do that). But I'm also using soc_camera. I was wondering if there's a time estimate for soc_camera to be converted over to the videobuf2 framework. Also, are SoC camera host drivers expected to call the methods in the videobuf2 ops table directly (as in, vb2_ops-alloc())? Or will there be wrappers around these in the future (the wrappers can take the videobuf2's alloc_ctx as a parameter and thereby figure out which method to call). Please, have a look at this thread: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/28782 I plan to push soc-camera videobuf2 support for 2.6.39. 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] Zarlink zl10036 DVB-S: Fix mem leak in zl10036_attach
On Sunday 06 February 2011, Jesper Juhl wrote: If the memory allocation to 'state' succeeds but we jump to the 'error' label before 'state' is assigned to fe-tuner_priv, then the call to 'zl10036_release(fe)' at the 'error:' label will not free 'state', but only what was previously assigned to 'tuner_priv', thus leaking the memory allocated to 'state'. There are may ways to fix this, including assigning the allocated memory directly to 'fe-tuner_priv', but I did not go for that since the additional pointer derefs are more expensive than the local variable, so I just added a 'kfree(state)' call. I guess the call to 'zl10036_release' might not even be needed in this case, but I wasn't sure, so I left it in. Yeah, that call to zl10036_release can be completely eleminated. Another thing is: jumping to the error label only makes sense when memory was already allocated. So the jump in line 471 can be replaced by return NULL, as the other error handling before allocation: if (NULL == config) { printk(KERN_ERR %s: no config specified, __func__); goto error; } I suggest to improve the patch to clean the code up when changing that. But I am fine with commiting this patch also if you do not want to change it. Regards Matthias Signed-off-by: Matthias Schwarzott z...@gentoo.org Signed-off-by: Jesper Juhl j...@chaosbits.net --- zl10036.c |1 + 1 file changed, 1 insertion(+) compile tested only. diff --git a/drivers/media/dvb/frontends/zl10036.c b/drivers/media/dvb/frontends/zl10036.c index 4627f49..b4fb8e8 100644 --- a/drivers/media/dvb/frontends/zl10036.c +++ b/drivers/media/dvb/frontends/zl10036.c @@ -508,6 +508,7 @@ struct dvb_frontend *zl10036_attach(struct dvb_frontend *fe, error: zl10036_release(fe); + kfree(state); return NULL; } EXPORT_SYMBOL(zl10036_attach); -- 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] Zarlink zl10036 DVB-S: Fix mem leak in zl10036_attach
On Thu, 17 Feb 2011, Matthias Schwarzott wrote: On Sunday 06 February 2011, Jesper Juhl wrote: If the memory allocation to 'state' succeeds but we jump to the 'error' label before 'state' is assigned to fe-tuner_priv, then the call to 'zl10036_release(fe)' at the 'error:' label will not free 'state', but only what was previously assigned to 'tuner_priv', thus leaking the memory allocated to 'state'. There are may ways to fix this, including assigning the allocated memory directly to 'fe-tuner_priv', but I did not go for that since the additional pointer derefs are more expensive than the local variable, so I just added a 'kfree(state)' call. I guess the call to 'zl10036_release' might not even be needed in this case, but I wasn't sure, so I left it in. Yeah, that call to zl10036_release can be completely eleminated. Another thing is: jumping to the error label only makes sense when memory was already allocated. So the jump in line 471 can be replaced by return NULL, as the other error handling before allocation: if (NULL == config) { printk(KERN_ERR %s: no config specified, __func__); goto error; } I suggest to improve the patch to clean the code up when changing that. But I am fine with commiting this patch also if you do not want to change it. Thank you for your feedback. It makes a lot of sense. Changing it is not a problem :) How about the updated patch below? If the memory allocation to 'state' succeeds but we jump to the 'error' label before 'state' is assigned to fe-tuner_priv, then the call to 'zl10036_release(fe)' at the 'error:' label will not free 'state', but only what was previously assigned to 'tuner_priv', thus leaking the memory allocated to 'state'. This patch fixes the leak and also does not jump to 'error:' before mem has been allocated but instead just returns. Also some small style cleanups. Signed-off-by: Jesper Juhl j...@chaosbits.net --- zl10036.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/media/dvb/frontends/zl10036.c b/drivers/media/dvb/frontends/zl10036.c index 4627f49..81aa984 100644 --- a/drivers/media/dvb/frontends/zl10036.c +++ b/drivers/media/dvb/frontends/zl10036.c @@ -463,16 +463,16 @@ struct dvb_frontend *zl10036_attach(struct dvb_frontend *fe, const struct zl10036_config *config, struct i2c_adapter *i2c) { - struct zl10036_state *state = NULL; + struct zl10036_state *state; int ret; - if (NULL == config) { + if (!config) { printk(KERN_ERR %s: no config specified, __func__); - goto error; + return NULL; } state = kzalloc(sizeof(struct zl10036_state), GFP_KERNEL); - if (NULL == state) + if (!state) return NULL; state-config = config; @@ -507,7 +507,7 @@ struct dvb_frontend *zl10036_attach(struct dvb_frontend *fe, return fe; error: - zl10036_release(fe); + kfree(state); return NULL; } EXPORT_SYMBOL(zl10036_attach); -- Jesper Juhl j...@chaosbits.nethttp://www.chaosbits.net/ Plain text mails only, please. Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.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: [Patch] Zarlink zl10036 DVB-S: Fix mem leak in zl10036_attach
On Thursday 17 February 2011, Jesper Juhl wrote: On Thu, 17 Feb 2011, Matthias Schwarzott wrote: On Sunday 06 February 2011, Jesper Juhl wrote: If the memory allocation to 'state' succeeds but we jump to the 'error' label before 'state' is assigned to fe-tuner_priv, then the call to 'zl10036_release(fe)' at the 'error:' label will not free 'state', but only what was previously assigned to 'tuner_priv', thus leaking the memory allocated to 'state'. There are may ways to fix this, including assigning the allocated memory directly to 'fe-tuner_priv', but I did not go for that since the additional pointer derefs are more expensive than the local variable, so I just added a 'kfree(state)' call. I guess the call to 'zl10036_release' might not even be needed in this case, but I wasn't sure, so I left it in. Yeah, that call to zl10036_release can be completely eleminated. Another thing is: jumping to the error label only makes sense when memory was already allocated. So the jump in line 471 can be replaced by return NULL, as the other error handling before allocation: if (NULL == config) { printk(KERN_ERR %s: no config specified, __func__); goto error; } I suggest to improve the patch to clean the code up when changing that. But I am fine with commiting this patch also if you do not want to change it. Thank you for your feedback. It makes a lot of sense. Changing it is not a problem :) How about the updated patch below? Looks good. @Mauro: Please apply. If the memory allocation to 'state' succeeds but we jump to the 'error' label before 'state' is assigned to fe-tuner_priv, then the call to 'zl10036_release(fe)' at the 'error:' label will not free 'state', but only what was previously assigned to 'tuner_priv', thus leaking the memory allocated to 'state'. This patch fixes the leak and also does not jump to 'error:' before mem has been allocated but instead just returns. Also some small style cleanups. Signed-off-by: Jesper Juhl j...@chaosbits.net Signed-off-by: Matthias Schwarzott z...@gentoo.org --- zl10036.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/media/dvb/frontends/zl10036.c b/drivers/media/dvb/frontends/zl10036.c index 4627f49..81aa984 100644 --- a/drivers/media/dvb/frontends/zl10036.c +++ b/drivers/media/dvb/frontends/zl10036.c @@ -463,16 +463,16 @@ struct dvb_frontend *zl10036_attach(struct dvb_frontend *fe, const struct zl10036_config *config, struct i2c_adapter *i2c) { - struct zl10036_state *state = NULL; + struct zl10036_state *state; int ret; - if (NULL == config) { + if (!config) { printk(KERN_ERR %s: no config specified, __func__); - goto error; + return NULL; } state = kzalloc(sizeof(struct zl10036_state), GFP_KERNEL); - if (NULL == state) + if (!state) return NULL; state-config = config; @@ -507,7 +507,7 @@ struct dvb_frontend *zl10036_attach(struct dvb_frontend *fe, return fe; error: - zl10036_release(fe); + kfree(state); return NULL; } EXPORT_SYMBOL(zl10036_attach); -- 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: [RFD] frame-size switching: preview / single-shot use-case
Em 17-02-2011 17:26, Guennadi Liakhovetski escreveu: Hi Mauro Thanks for your comments. On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: Em 16-02-2011 08:35, Guennadi Liakhovetski escreveu: [snip] (2) cleanly separate setting video data format (S_FMT) from specifying the allocated buffer size. This would break existing applications. Too late for that, except if negotiated with a SETCAP like approach. Sorry, don't see how my proposal from my last mail would change existing applications. As long as no explicit buffer-queue management is performed, no new queues are allocated, the driver will just implicitly allocate one queue and use it. I.e., no change in behaviour. Using the same ioctl to explicitly or to implicitly allocating memory depending on the context would make the API more complicated than it should be. There's an additional problem with that: assume that streaming is happening, and a S_FMT changing the resolution was sent. There's no way to warrant that the very next frame will have the new resolution. So, a meta-data with the frame resolution (and format) would be needed. Sorry, we are not going to allow format changes during a running capture. You have to stop streaming, set new formats (possibly switch to another queue) and restart streaming. What am I missing? If you're stopping the stream, the current API will work as-is. If all of your concerns is about reserving a bigger buffer queue, I think that one of the reasons for the CMA allocator it for such usage. Am I missing something? Cheers, 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: tm6000 and radio
Em 17-02-2011 03:12, Dmitri Belimov escreveu: Hi All Now I have working radio for our TV cards based on tm6000 and xc5000. TV works too. Great news! I think that the only remaining issue is audio that still doesn't work with some audio standards. The last time I tested here, PAL/M didn't work. After some time I'll send my changes for discuss here. With my best regards, Dmitry. -- 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 -- 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: [RFD] frame-size switching: preview / single-shot use-case
On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: Em 17-02-2011 17:26, Guennadi Liakhovetski escreveu: Hi Mauro Thanks for your comments. On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: Em 16-02-2011 08:35, Guennadi Liakhovetski escreveu: [snip] (2) cleanly separate setting video data format (S_FMT) from specifying the allocated buffer size. This would break existing applications. Too late for that, except if negotiated with a SETCAP like approach. Sorry, don't see how my proposal from my last mail would change existing applications. As long as no explicit buffer-queue management is performed, no new queues are allocated, the driver will just implicitly allocate one queue and use it. I.e., no change in behaviour. Using the same ioctl to explicitly or to implicitly allocating memory depending on the context would make the API more complicated than it should be. Sorry again, of course, we're adding new functionality, so, it may well happen, that the API will become more complicated too. But that was not the question. The question was - would it break any existing users? And it looks like it wouldn't, at the same time giving new users the required additional flexibility and functionality. There's an additional problem with that: assume that streaming is happening, and a S_FMT changing the resolution was sent. There's no way to warrant that the very next frame will have the new resolution. So, a meta-data with the frame resolution (and format) would be needed. Sorry, we are not going to allow format changes during a running capture. You have to stop streaming, set new formats (possibly switch to another queue) and restart streaming. What am I missing? If you're stopping the stream, the current API will work as-is. If all of your concerns is about reserving a bigger buffer queue, I think that one of the reasons for the CMA allocator it for such usage. Not just bigger, say, with our preview / still-shot example, we would have one queue with a larger number of small buffers for drop-free preview, and a small number of larger buffers for still images. Currently you would have to allocate a large number of large buffers, which would waste memory. Or you would have to reallocate the queue, losing time. AFAICS, CMA doesn't manage our memory for us. It only provides an API to reserve memory for various uses with various restrictions (alignment, etc.) and use different allocators to obtain that memory. So, are you suggesting, that with that in place, we would first allocate the preview queue from this memory, then free it, when switching to snapshooting, allocate our large-buffer queue from the _same_ memory, capture images, free and allocate preview queue again? Would that be fast enough? In fact, it would be kind of smart to reuse the same memory for both queues, but if we could do it without re-allocations?... 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: Kernel configuration for ov9655 on the PXA27x Quick Capture Interface
Hi Guennadi, thank you for the useful information. Does exist an older kernel version that I can patch ( https://patchwork.kernel.org/patch/16548/) in order to use the ov9655 sensor ? Paolo 2011/2/17 Guennadi Liakhovetski g.liakhovet...@gmx.de: (replaced the old mailing list address) On Thu, 17 Feb 2011, Paolo Santinelli wrote: Hi Guennadi, thank you for the information. Can I use or adapt this patch: https://patchwork.kernel.org/patch/16548/ ? You'd have to port it to the current kernel, the patch is almost 2 years old... I Could use the code from the patch to direct control the sensor register configuration and use the PXA27x Quick Capture Interface to capture data by mean soc_camera and pxa_camera driver modules. But now when I try to load the soc_camera module i get this error: insmod soc_camera.ko insmod: cannot insert 'soc_camera.ko': No such device Please, could you give mi some tips and indication No, all the drivers: soc-camera core, camera host driver (pxa_camera) and a camera sensor driver (ov9655) have to work together. And their mutual work is configured at the platform level. Sorry, I don't think, I can guide you in detail through a complete v4l2-subdev / soc-camera driver architecture. You can try to have a look at one of the multiple examples, e.g., arch/arm/mach-pxa/ezx.c (see a780_camera) drivers/media/video/mt9m111.c drivers/media/video/pxa_camera.c Good luck Guennadi Thanks Paolo 2011/2/17 Guennadi Liakhovetski g.liakhovet...@gmx.de: On Wed, 16 Feb 2011, Paolo Santinelli wrote: Hi all, I have an embedded smart camera equipped with an XScal-PXA270 processor running Linux 2.6.37 and the OV9655 Image sensor connected on the PXA27x Quick Capture Interface. Please, what kernel module I have to select in order to use the Image sensor ? You need to write a new or adapt an existing driver for your ov9655 sensor, currently, there's no driver available to work with your pxa270. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- -- Paolo Santinelli ImageLab Computer Vision and Pattern Recognition Lab Dipartimento di Ingegneria dell'Informazione Universita' di Modena e Reggio Emilia via Vignolese 905/B, 41125, Modena, Italy Cell. +39 3472953357, Office +39 059 2056270, Fax +39 059 2056129 email: mailto:paolo.santine...@unimore.it paolo.santine...@unimore.it URL: http://imagelab.ing.unimo.it/ http://imagelab.ing.unimo.it -- --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- -- Paolo Santinelli ImageLab Computer Vision and Pattern Recognition Lab Dipartimento di Ingegneria dell'Informazione Universita' di Modena e Reggio Emilia via Vignolese 905/B, 41125, Modena, Italy Cell. +39 3472953357, Office +39 059 2056270, Fax +39 059 2056129 email: mailto:paolo.santine...@unimore.it paolo.santine...@unimore.it URL: http://imagelab.ing.unimo.it/ http://imagelab.ing.unimo.it -- -- 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 AF9015 dual tuner - dual_mode B.R.O.K.E.N.
poma wrote: poma wrote: To num_adapters = 2, or num_adapters = 1: that is the question! In dual tuner mode, after a while device become unrensponsive, eventually after S5 aka 'Soft Off' system doesn't even boot! Didn't even mention all sorts of 'mumbo-jumbo' with S3 aka 'Suspend to RAM'. Antti, please consider adding 'dual_mode' parameter back. dvb_usb_af9015 dual_mode=0 Devices to consider: Not Only TV/LifeView DUAL DVB-T USB LV52T (equivalent to TerraTec Cinergy T Stick Dual RC) Afatech AF9013/AF9015 2x MaxLinear MxL5007T http://www.notonlytv.net/p_lv52t.html KWorld USB Dual DVB-T TV Stick (DVB-T 399U) Afatech AF9013/AF9015 2x MaxLinear MxL5003S http://www.kworld-global.com/main/prod_in.aspx?mnuid=1248modid=6prodid=73 DigitalNow TinyTwin DVB-T Receiver Afatech AF9013/AF9015 2x MaxLinear MxL5005S http://www.digitalnow.com.au/product_pages/TinyTwin.html http://www.spinics.net/lists/linux-dvb/msg31616.html http://www.spinics.net/lists/linux-dvb/msg31621.html This patch restore dvb_usb_af9015 'dual mode' parameter - disable dual mode by default because it is buggy. Enabled mode: options dvb_usb_af9015 dual_mode=1 in modprobe referent file. .. --- a/linux/drivers/media/dvb/dvb-usb/af9015.c 2011-01-10 16:24:45.0 +0100 +++ b/linux/drivers/media/dvb/dvb-usb/af9015.c 2011-02-17 21:58:42.099040739 +0100 @@ -40,6 +40,9 @@ static int dvb_usb_af9015_remote; module_param_named(remote, dvb_usb_af9015_remote, int, 0644); MODULE_PARM_DESC(remote, select remote); +static int dvb_usb_af9015_dual_mode; +module_param_named(dual_mode, dvb_usb_af9015_dual_mode, int, 0644); +MODULE_PARM_DESC(dual_mode, enable dual mode); DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static DEFINE_MUTEX(af9015_usb_mutex); @@ -841,6 +844,9 @@ goto error; af9015_config.dual_mode = val; deb_info(%s: TS mode:%d\n, __func__, af9015_config.dual_mode); + /* disable dual mode by default because it is buggy */ + if (!dvb_usb_af9015_dual_mode) + af9015_config.dual_mode = 0; /* Set adapter0 buffer size according to USB port speed, adapter1 buffer size can be static because it is enabled only USB2.0 */ .. rgds, poma --- a/linux/drivers/media/dvb/dvb-usb/af9015.c 2011-01-10 16:24:45.0 +0100 +++ b/linux/drivers/media/dvb/dvb-usb/af9015.c 2011-02-17 21:58:42.099040739 +0100 @@ -40,6 +40,9 @@ static int dvb_usb_af9015_remote; module_param_named(remote, dvb_usb_af9015_remote, int, 0644); MODULE_PARM_DESC(remote, select remote); +static int dvb_usb_af9015_dual_mode; +module_param_named(dual_mode, dvb_usb_af9015_dual_mode, int, 0644); +MODULE_PARM_DESC(dual_mode, enable dual mode); DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static DEFINE_MUTEX(af9015_usb_mutex); @@ -841,6 +844,9 @@ goto error; af9015_config.dual_mode = val; deb_info(%s: TS mode:%d\n, __func__, af9015_config.dual_mode); + /* disable dual mode by default because it is buggy */ + if (!dvb_usb_af9015_dual_mode) + af9015_config.dual_mode = 0; /* Set adapter0 buffer size according to USB port speed, adapter1 buffer size can be static because it is enabled only USB2.0 */
[PATCH v4 1/1] [media] ov9740: Initial submit of OV9740 driver.
From: Andrew Chew ac...@nvidia.com This soc_camera driver is for Omnivision's OV9740 sensor. This initial submission provides support for YUV422 output at 1280x720 (720p), which is the sensor's native resolution. 640x480 (VGA) is also supported, with cropping and scaling performed by the sensor's ISP. This driver is heavily based off of the existing OV9640 driver. Change-Id: Ie974475ce987588d87ff2e4ca720ac054f8adc0b Signed-off-by: Andrew Chew ac...@nvidia.com --- Applied more suggestions from Guennadi Liakhovetski: Changed some dev_info prints to dev_dbg to reduce driver chattiness. Small modification in s_stream method for improved readability. Removed code parameter from ov9740_set_res(). Fixed try_fmt method that I butchered due to my misunderstanding. drivers/media/video/Kconfig |6 + drivers/media/video/Makefile|1 + drivers/media/video/ov9740.c| 1009 +++ include/media/v4l2-chip-ident.h |1 + 4 files changed, 1017 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9740.c diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index d40a8fc..52b6271 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -837,6 +837,12 @@ config SOC_CAMERA_OV9640 help This is a ov9640 camera driver +config SOC_CAMERA_OV9740 + tristate ov9740 camera support + depends on SOC_CAMERA I2C + help + This is a ov9740 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 251b7ca..ac54652 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -79,6 +79,7 @@ obj-$(CONFIG_SOC_CAMERA_OV2640) += ov2640.o obj-$(CONFIG_SOC_CAMERA_OV6650)+= ov6650.o obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o +obj-$(CONFIG_SOC_CAMERA_OV9740)+= ov9740.o obj-$(CONFIG_SOC_CAMERA_RJ54N1)+= rj54n1cb0c.o obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c new file mode 100644 index 000..4d4ee4f --- /dev/null +++ b/drivers/media/video/ov9740.c @@ -0,0 +1,1009 @@ +/* + * OmniVision OV9740 Camera Driver + * + * Copyright (C) 2011 NVIDIA Corporation + * + * Based on ov9640 camera driver. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/init.h +#include linux/module.h +#include linux/i2c.h +#include linux/slab.h +#include media/v4l2-chip-ident.h +#include media/soc_camera.h + +#define to_ov9740(sd) container_of(sd, struct ov9740_priv, subdev) + +/* General Status Registers */ +#define OV9740_MODEL_ID_HI 0x +#define OV9740_MODEL_ID_LO 0x0001 +#define OV9740_REVISION_NUMBER 0x0002 +#define OV9740_MANUFACTURER_ID 0x0003 +#define OV9740_SMIA_VERSION0x0004 + +/* General Setup Registers */ +#define OV9740_MODE_SELECT 0x0100 +#define OV9740_IMAGE_ORT 0x0101 +#define OV9740_SOFTWARE_RESET 0x0103 +#define OV9740_GRP_PARAM_HOLD 0x0104 +#define OV9740_MSK_CORRUP_FM 0x0105 + +/* Timing Setting */ +#define OV9740_FRM_LENGTH_LN_HI0x0340 /* VTS */ +#define OV9740_FRM_LENGTH_LN_LO0x0341 /* VTS */ +#define OV9740_LN_LENGTH_PCK_HI0x0342 /* HTS */ +#define OV9740_LN_LENGTH_PCK_LO0x0343 /* HTS */ +#define OV9740_X_ADDR_START_HI 0x0344 +#define OV9740_X_ADDR_START_LO 0x0345 +#define OV9740_Y_ADDR_START_HI 0x0346 +#define OV9740_Y_ADDR_START_LO 0x0347 +#define OV9740_X_ADDR_END_HI 0x0348 +#define OV9740_X_ADDR_END_LO 0x0349 +#define OV9740_Y_ADDR_END_HI 0x034A +#define OV9740_Y_ADDR_END_LO 0x034B +#define OV9740_X_OUTPUT_SIZE_HI0x034C +#define OV9740_X_OUTPUT_SIZE_LO0x034D +#define OV9740_Y_OUTPUT_SIZE_HI0x034E +#define OV9740_Y_OUTPUT_SIZE_LO0x034F + +/* IO Control Registers */ +#define OV9740_IO_CREL00 0x3002 +#define OV9740_IO_CREL01 0x3004 +#define OV9740_IO_CREL02 0x3005 +#define OV9740_IO_OUTPUT_SEL01 0x3026 +#define OV9740_IO_OUTPUT_SEL02 0x3027 + +/* AWB Registers */ +#define OV9740_AWB_MANUAL_CTRL 0x3406 + +/* Analog Control Registers */ +#define OV9740_ANALOG_CTRL01 0x3601 +#define OV9740_ANALOG_CTRL02 0x3602 +#define OV9740_ANALOG_CTRL03 0x3603 +#define OV9740_ANALOG_CTRL04 0x3604 +#define OV9740_ANALOG_CTRL10 0x3610 +#define OV9740_ANALOG_CTRL12 0x3612 +#define
Re: [RFD] frame-size switching: preview / single-shot use-case
On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: There's an additional problem with that: assume that streaming is happening, and a S_FMT changing the resolution was sent. There's no way to warrant that the very next frame will have the new resolution. So, a meta-data with the frame resolution (and format) would be needed. Em 17-02-2011 17:26, Guennadi Liakhovetski escreveu: Sorry, we are not going to allow format changes during a running capture. You have to stop streaming, set new formats (possibly switch to another queue) and restart streaming. What am I missing? On Thu, 17 Feb 2011, Mauro Carvalho Chehab wrote: If you're stopping the stream, the current API will work as-is. If all of your concerns is about reserving a bigger buffer queue, I think that one of the reasons for the CMA allocator it for such usage. From: Guennadi Liakhovetski g.liakhovet...@gmx.de Not just bigger, say, with our preview / still-shot example, we would have one queue with a larger number of small buffers for drop-free preview, and a small number of larger buffers for still images. Ie. waste memory? As in you have both those queues allocated but only one is used at given time? Currently you would have to allocate a large number of large buffers, which would waste memory. Or you would have to reallocate the queue, losing time. AFAICS, CMA doesn't manage our memory for us. It only provides an API to reserve memory for various uses with various restrictions (alignment, etc.) and use different allocators to obtain that memory. I'm not sure if I understand you here. CMA has some API for reserving memory at boot time but it sure does manage this reserved memory, ie. when system is running you can allocate chunks of memory from this reserved block. Also note, that latest CMA uses only one allocator. So, are you suggesting, that with that in place, we would first allocate the preview queue from this memory, then free it, when switching to snapshooting, allocate our large-buffer queue from the _same_ memory, capture images, free and allocate preview queue again? Would that be fast enough? If CMA is considered, the most important thing to note is that CMA may share memory with page allocator (so that other parts of the system can use it if CMA-compatible devices are not using it). When CMA allocates memory chunk it may potentially need to migrate memory pages which may take so time (there is room for improvement, but still). Sharing can be disabled in which case allocation should be quite fast (the last CMA patchset uses a first-fit bitmap-based gen_allocator API but O(log n) best-fit algorithm can easily used instead). To sum things up, if sharing is disabled, CMA should be able to fulfil your requirements, however it may be undesirable as it wastes space. If sharing is enabled, on the other hand, the delay may potentially be noticeable. In fact, it would be kind of smart to reuse the same memory for both queues, but if we could do it without re-allocations?... What I would do is allocate a few big buffers and when needed divide them into smaller chunks (or even allocate one big block and later divide it in whatever way needed). I'm not sure if such usage would map well to V4L2 API. This usage is, as a matter of fact, supported by CMA. You can allocate a big block and then run cma_create() on it to create a new CMA context. Using this context you can allocate a lot of small blocks, then free them all, to finally allocate few big blocks. Again, I'm not sure how it maps to V4L2 API. If you can change formats while retaining V4L device instance's state, this should be doable. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michal mina86 Nazarewicz(o o) ooo +-email/xmpp: mnazarew...@google.com-ooO--(_)--Ooo-- -- 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: soc-camera: vb2 continued: mx3_camera
On Mon, 7 Feb 2011, Guennadi Liakhovetski wrote: Hi One more soc-camera host driver has been converted to videobuf2: mx3_camera for i.MX3* SoCs. I also added Anatolij's patches to the branch. Please, review / test. The branch is available at git://linuxtv.org/gliakhovetski/v4l-dvb.git soc_camera-vb2 Two persons reported problems, when trying to directly clone the above tree. The following worked for me: git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git cd linux-2.6/ git remote add soc-camera git://linuxtv.org/gliakhovetski/v4l-dvb.git git remote update git checkout -b v4l-devel soc-camera/soc_camera-vb2 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 v4 1/1] [media] ov9740: Initial submit of OV9740 driver.
This looks good now, thanks, I'll queue it for 2.6.39. Just one question: On Thu, 17 Feb 2011, ac...@nvidia.com wrote: From: Andrew Chew ac...@nvidia.com This soc_camera driver is for Omnivision's OV9740 sensor. This initial submission provides support for YUV422 output at 1280x720 (720p), which is the sensor's native resolution. 640x480 (VGA) is also supported, with cropping and scaling performed by the sensor's ISP. Didn't you mean to say just scaling? You're not implementing cropping. You could, certainly, fake scaling with cropping, but are you really using both to switch between 720p and VGA? No need for a v5, just tell me how to change the comment. Thanks Guennadi This driver is heavily based off of the existing OV9640 driver. Change-Id: Ie974475ce987588d87ff2e4ca720ac054f8adc0b Signed-off-by: Andrew Chew ac...@nvidia.com --- Applied more suggestions from Guennadi Liakhovetski: Changed some dev_info prints to dev_dbg to reduce driver chattiness. Small modification in s_stream method for improved readability. Removed code parameter from ov9740_set_res(). Fixed try_fmt method that I butchered due to my misunderstanding. drivers/media/video/Kconfig |6 + drivers/media/video/Makefile|1 + drivers/media/video/ov9740.c| 1009 +++ include/media/v4l2-chip-ident.h |1 + 4 files changed, 1017 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9740.c diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index d40a8fc..52b6271 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -837,6 +837,12 @@ config SOC_CAMERA_OV9640 help This is a ov9640 camera driver +config SOC_CAMERA_OV9740 + tristate ov9740 camera support + depends on SOC_CAMERA I2C + help + This is a ov9740 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 251b7ca..ac54652 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -79,6 +79,7 @@ obj-$(CONFIG_SOC_CAMERA_OV2640) += ov2640.o obj-$(CONFIG_SOC_CAMERA_OV6650) += ov6650.o obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o +obj-$(CONFIG_SOC_CAMERA_OV9740) += ov9740.o obj-$(CONFIG_SOC_CAMERA_RJ54N1) += rj54n1cb0c.o obj-$(CONFIG_SOC_CAMERA_TW9910) += tw9910.o diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c new file mode 100644 index 000..4d4ee4f --- /dev/null +++ b/drivers/media/video/ov9740.c @@ -0,0 +1,1009 @@ +/* + * OmniVision OV9740 Camera Driver + * + * Copyright (C) 2011 NVIDIA Corporation + * + * Based on ov9640 camera driver. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/init.h +#include linux/module.h +#include linux/i2c.h +#include linux/slab.h +#include media/v4l2-chip-ident.h +#include media/soc_camera.h + +#define to_ov9740(sd)container_of(sd, struct ov9740_priv, subdev) + +/* General Status Registers */ +#define OV9740_MODEL_ID_HI 0x +#define OV9740_MODEL_ID_LO 0x0001 +#define OV9740_REVISION_NUMBER 0x0002 +#define OV9740_MANUFACTURER_ID 0x0003 +#define OV9740_SMIA_VERSION 0x0004 + +/* General Setup Registers */ +#define OV9740_MODE_SELECT 0x0100 +#define OV9740_IMAGE_ORT 0x0101 +#define OV9740_SOFTWARE_RESET0x0103 +#define OV9740_GRP_PARAM_HOLD0x0104 +#define OV9740_MSK_CORRUP_FM 0x0105 + +/* Timing Setting */ +#define OV9740_FRM_LENGTH_LN_HI 0x0340 /* VTS */ +#define OV9740_FRM_LENGTH_LN_LO 0x0341 /* VTS */ +#define OV9740_LN_LENGTH_PCK_HI 0x0342 /* HTS */ +#define OV9740_LN_LENGTH_PCK_LO 0x0343 /* HTS */ +#define OV9740_X_ADDR_START_HI 0x0344 +#define OV9740_X_ADDR_START_LO 0x0345 +#define OV9740_Y_ADDR_START_HI 0x0346 +#define OV9740_Y_ADDR_START_LO 0x0347 +#define OV9740_X_ADDR_END_HI 0x0348 +#define OV9740_X_ADDR_END_LO 0x0349 +#define OV9740_Y_ADDR_END_HI 0x034A +#define OV9740_Y_ADDR_END_LO 0x034B +#define OV9740_X_OUTPUT_SIZE_HI 0x034C +#define OV9740_X_OUTPUT_SIZE_LO 0x034D +#define OV9740_Y_OUTPUT_SIZE_HI 0x034E +#define OV9740_Y_OUTPUT_SIZE_LO 0x034F + +/* IO Control Registers */ +#define OV9740_IO_CREL00 0x3002 +#define OV9740_IO_CREL01 0x3004 +#define OV9740_IO_CREL02 0x3005 +#define
RE: [PATCH v4 1/1] [media] ov9740: Initial submit of OV9740 driver.
This looks good now, thanks, I'll queue it for 2.6.39. Just one question: On Thu, 17 Feb 2011, ac...@nvidia.com wrote: From: Andrew Chew ac...@nvidia.com This soc_camera driver is for Omnivision's OV9740 sensor. This initial submission provides support for YUV422 output at 1280x720 (720p), which is the sensor's native resolution. 640x480 (VGA) is also supported, with cropping and scaling performed by the sensor's ISP. Didn't you mean to say just scaling? You're not implementing cropping. You could, certainly, fake scaling with cropping, but are you really using both to switch between 720p and VGA? No need for a v5, just tell me how to change the comment. There really is cropping. The native resolution of the OV9740 sensor is a different aspect ratio than VGA. To preserve the square pixels, first we crop both the left and right sides, and then scale down.-- 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 RFC] uvcvideo: Add a mapping for H.264 payloads
Hi, On Thu, 2011-02-17 at 08:01 -0800, Laurent Pinchart wrote: HI Stephan, Thanks for the patch, and sorry for the late reply. On Friday 28 January 2011 20:38:58 Stephan Lachowsky wrote: Associate the H.264 GUID with an H.264 pixel format so that frame and stream based format descriptors with this GUID are recognized by the UVC video driver. --- drivers/media/video/uvc/uvc_driver.c |5 + drivers/media/video/uvc/uvcvideo.h |3 +++ include/linux/videodev2.h|1 + 3 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c index 6bcb9e1..a5a86ce 100644 --- a/drivers/media/video/uvc/uvc_driver.c +++ b/drivers/media/video/uvc/uvc_driver.c @@ -108,6 +108,11 @@ static struct uvc_format_desc uvc_fmts[] = { .guid = UVC_GUID_FORMAT_MPEG, .fcc= V4L2_PIX_FMT_MPEG, }, + { + .name = H.264, + .guid = UVC_GUID_FORMAT_H264, + .fcc= V4L2_PIX_FMT_H264, + }, }; /* diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h index e522f99..4f65ac6 100644 --- a/drivers/media/video/uvc/uvcvideo.h +++ b/drivers/media/video/uvc/uvcvideo.h @@ -155,6 +155,9 @@ struct uvc_xu_control { #define UVC_GUID_FORMAT_MPEG \ { 'M', 'P', 'E', 'G', 0x00, 0x00, 0x10, 0x00, \ 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71} +#define UVC_GUID_FORMAT_H264 \ + { 'H', '2', '6', '4', 0x00, 0x00, 0x10, 0x00, \ +0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71} /* * Driver specific constants. diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 5f6f470..d3b5877 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -341,6 +341,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_JPEG v4l2_fourcc('J', 'P', 'E', 'G') /* JFIF JPEG */ #define V4L2_PIX_FMT_DV v4l2_fourcc('d', 'v', 's', 'd') /* 1394 */ #define V4L2_PIX_FMT_MPEG v4l2_fourcc('M', 'P', 'E', 'G') /* MPEG-1/2/4*/ +#define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H.264 Annex-B NAL Units */ I've discussed H.264 support with Hans Verkuil (CC'ed) some time ago, and his opinion was that we shouldn't use a new V4L2 format for it. H.264 is essentially an MPEG version, so drivers should use V4L2_PIX_FMT_MPEG and select the details using the MPEG CIDs. Of course feel free to disagree with Hans and discuss the matter with him :-) Well MPEG is a loaded acronym to throw around, it means many, many things in different contexts. Saying H.264 is essentially an MPEG version is true, but putting something in a labelled drawer doesn't necessarily make it square (unless the drawer is square, structurally solid, smaller than the something, and the insertion is done with extreme prejudice). Let me throw a few points out for critique: * There is a 1-1 mapping between between the contents of this UVC stream, and what would be found inside an AVI container with the same fourcc code... Why fight an existing defacto labelling? * There is currently a straightforward correspondence between UVC payload formats and v4l2 fourcc types... Why would you want to add indirection through an overloaded fourcc type, increasing the complexity for all parties involved? * If you don't use the fourcc code to denote the payload format, you lose the ability to enumerate supported formats generically. You require interface users to understand MPEG CID, and use it to sub-enumerate any formats considered MPEG -- which is an arbitrary label in this context -- you wouldn't try to shim Theora or WebM in this way, so why H.264? * MPEG CID (http://v4l2spec.bytesex.org/spec/x802.htm#MPEG-CONTROLS) does not currently have defined controls beyond MPEG2 (No MPEG4 ASP, no MPEG4 AVC)... configuring the minutiae of these codecs is complex enough that they probably deserve their own extended control classes. I'm quite persuadable by reason, and quite partial to wit; so do cc anyone else that will respond with insight (and/or humour). Stephan /* Vendor-specific formats */ #define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */ -- 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: Terratec Cinergy Hybrid stick
One more strange thing: Kernel 2.6.31 reports USB ID 0ccd:00a5 and with Kernel 2.6.38-rc the device has a different USB ID: 6000:0002 What does that mean? Thanx Malte -- 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/13] lirc_zilog: Ref-counting and locking cleanup
The following 13 patches are a substantial rework of lirc_zilog reference counting, object allocation and deallocation, and object locking. With these changes, devices can now disappear out from under lircd + lirc_dev + lirc_zilog with no adverse effects. I tested this with irw + lircd + lirc_dev + lirc_zilog + cx18 + HVR-1600. I could unload the cx18 driver without any oops or application crashes. When I reloaded the cx18 driver, irw started receiving RX button presses again, and irsend worked without a problem (and I didn't even need to restart lircd!). The ref counting fixes aren't finished as lirc_zilog itself can still be unloaded by the user when it shouldn't be, but a hot unplug of an HD-PVR, PVR-USB2, or HVR-1950 isn't going to trigger that. These changes are base off of Jarod Wilson's git repo http://git.linuxtv.org/jarod/linux-2.6-ir.git for-2.6.38 (IIRC) Regards, Andy The following changes since commit c369acfb63914f9f502baef032bacfd5a53a871f: mceusb: really fix remaining keybounce issues (2011-01-26 10:56:29 -0500) are available in the git repository at: ssh://linuxtv.org/git/awalls/media_tree.git z8-wilson-38 Andy Walls (13): lirc_zilog: Restore checks for existence of the IR_tx object lirc_zilog: Remove broken, ineffective reference counting lirc_zilog: Convert ir_device instance array to a linked list lirc_zilog: Convert the instance open count to an atomic_t lirc_zilog: Use kernel standard methods for marking device non-seekable lirc_zilog: Don't acquire the rx-buf_lock in the poll() function lirc_zilog: Remove unneeded rx-buf_lock lirc_zilog: Always allocate a Rx lirc_buffer object lirc_zilog: Move constants from ir_probe() into the lirc_driver template lirc_zilog: Add ref counting of struct IR, IR_tx, and IR_rx lirc_zilog: Add locking of the i2c_clients when in use lirc_zilog: Fix somewhat confusing information messages in ir_probe() lirc_zilog: Update TODO list based on work completed and revised plans drivers/staging/lirc/TODO.lirc_zilog | 51 +-- drivers/staging/lirc/lirc_zilog.c| 802 +- 2 files changed, 523 insertions(+), 330 deletions(-) -- 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 01/13] lirc_zilog: Restore checks for existence of the IR_tx object
This reverts commit 8090232a237ab62e22307fc060097da1a283dd66 and adds an additional check for ir-tx == NULL. The user may need us to handle an RX only unit. Apparently there are TV capture units in existence with Rx only wiring and/or RX only firmware for the on-board Zilog Z8 IR unit. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 27 --- 1 files changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 0aad0d7..7389b77 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -209,7 +209,8 @@ static int add_to_buf(struct IR *ir) return -ENODATA; } schedule_timeout((100 * HZ + 999) / 1000); - ir-tx-need_boot = 1; + if (ir-tx != NULL) + ir-tx-need_boot = 1; ++failures; mutex_unlock(ir-ir_lock); @@ -1032,9 +1033,10 @@ static long ioctl(struct file *filep, unsigned int cmd, unsigned long arg) int result; unsigned long mode, features = 0; - features |= LIRC_CAN_SEND_PULSE; if (ir-rx != NULL) features |= LIRC_CAN_REC_LIRCCODE; + if (ir-tx != NULL) + features |= LIRC_CAN_SEND_PULSE; switch (cmd) { case LIRC_GET_LENGTH: @@ -1061,9 +1063,15 @@ static long ioctl(struct file *filep, unsigned int cmd, unsigned long arg) result = -EINVAL; break; case LIRC_GET_SEND_MODE: + if (!(featuresLIRC_CAN_SEND_MASK)) + return -ENOSYS; + result = put_user(LIRC_MODE_PULSE, (unsigned long *) arg); break; case LIRC_SET_SEND_MODE: + if (!(featuresLIRC_CAN_SEND_MASK)) + return -ENOSYS; + result = get_user(mode, (unsigned long *) arg); if (!result mode != LIRC_MODE_PULSE) return -EINVAL; @@ -1242,8 +1250,10 @@ static int ir_remove(struct i2c_client *client) } /* Good-bye Tx */ - i2c_set_clientdata(ir-tx-c, NULL); - kfree(ir-tx); + if (ir-tx != NULL) { + i2c_set_clientdata(ir-tx-c, NULL); + kfree(ir-tx); + } /* Good-bye IR */ del_ir_device(ir); @@ -1393,9 +1403,12 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) * after registering with lirc as otherwise hotplug seems to take * 10s to create the lirc device. */ - ret = tx_init(ir-tx); - if (ret != 0) - goto out_unregister; + if (ir-tx != NULL) { + /* Special TX init */ + ret = tx_init(ir-tx); + if (ret != 0) + goto out_unregister; + } zilog_info(probe of IR %s on %s (i2c-%d) done. IR unit ready.\n, tx_probe ? Tx : Rx, adap-name, adap-nr); -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/13] lirc_zilog: Remove broken, ineffective reference counting
The set_use_inc() and set_use_dec() functions tried to lock the underlying bridge driver device instance in memory by changing the use count on the device's i2c_clients. This worked for PCI devices (ivtv, cx18, bttv). It doesn't work for hot-pluggable usb devices (pvrusb2 and hdpvr). With usb device instances, the driver may get locked into memory, but the unplugged hardware is gone. The set_use_inc() set_use_dec() functions also tried to have lirc_zilog change its own module refernce count, which is racy and not guaranteed to work. The lirc_dev module does actually perform proper module ref count manipulation on the lirc_zilog module, so there is need for lirc_zilog to attempt a buggy module get on itself anyway. lirc_zilog also errantly called these functions on itself in open() and close(), but lirc_dev did that already too. So let's just gut the bodies of the set_use_*() functions, and remove the extra calls to them from within lirc_zilog. Proper reference counting of the struct IR, IR_rx, and IR_tx objects -- to handle the case when the underlying bttv, ivtv, cx18, hdpvr, or pvrusb2 bridge driver module or device instance goes away -- will be added in subsequent patches. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 32 +--- 1 files changed, 1 insertions(+), 31 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 7389b77..3a91257 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -305,34 +305,12 @@ static int lirc_thread(void *arg) static int set_use_inc(void *data) { - struct IR *ir = data; - - if (ir-l.owner == NULL || try_module_get(ir-l.owner) == 0) - return -ENODEV; - - /* lock bttv in memory while /dev/lirc is in use */ - /* -* this is completely broken code. lirc_unregister_driver() -* must be possible even when the device is open -*/ - if (ir-rx != NULL) - i2c_use_client(ir-rx-c); - if (ir-tx != NULL) - i2c_use_client(ir-tx-c); - return 0; } static void set_use_dec(void *data) { - struct IR *ir = data; - - if (ir-rx) - i2c_release_client(ir-rx-c); - if (ir-tx) - i2c_release_client(ir-tx-c); - if (ir-l.owner != NULL) - module_put(ir-l.owner); + return; } /* safe read of a uint32 (always network byte order) */ @@ -1098,7 +1076,6 @@ static struct IR *find_ir_device_by_minor(unsigned int minor) static int open(struct inode *node, struct file *filep) { struct IR *ir; - int ret; unsigned int minor = MINOR(node-i_rdev); /* find our IR struct */ @@ -1112,12 +1089,6 @@ static int open(struct inode *node, struct file *filep) /* increment in use count */ mutex_lock(ir-ir_lock); ++ir-open; - ret = set_use_inc(ir); - if (ret != 0) { - --ir-open; - mutex_unlock(ir-ir_lock); - return ret; - } mutex_unlock(ir-ir_lock); /* stash our IR struct */ @@ -1139,7 +1110,6 @@ static int close(struct inode *node, struct file *filep) /* decrement in use count */ mutex_lock(ir-ir_lock); --ir-open; - set_use_dec(ir); mutex_unlock(ir-ir_lock); return 0; -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/13] lirc_zilog: Convert ir_device instance array to a linked list
Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 59 +++- 1 files changed, 31 insertions(+), 28 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 3a91257..39f7b53 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -89,6 +89,8 @@ struct IR_tx { }; struct IR { + struct list_head list; + struct lirc_driver l; struct mutex ir_lock; @@ -99,9 +101,9 @@ struct IR { struct IR_tx *tx; }; -/* Minor - data mapping */ -static struct mutex ir_devices_lock; -static struct IR *ir_devices[MAX_IRCTL_DEVICES]; +/* IR transceiver instance object list */ +static DEFINE_MUTEX(ir_devices_lock); +static LIST_HEAD(ir_devices_list); /* Block size for IR transmitter */ #define TX_BLOCK_SIZE 99 @@ -1063,10 +1065,16 @@ static long ioctl(struct file *filep, unsigned int cmd, unsigned long arg) /* ir_devices_lock must be held */ static struct IR *find_ir_device_by_minor(unsigned int minor) { - if (minor = MAX_IRCTL_DEVICES) + struct IR *ir; + + if (list_empty(ir_devices_list)) return NULL; - return ir_devices[minor]; + list_for_each_entry(ir, ir_devices_list, list) + if (ir-l.minor == minor) + return ir; + + return NULL; } /* @@ -1172,25 +1180,21 @@ static void destroy_rx_kthread(struct IR_rx *rx) /* ir_devices_lock must be held */ static int add_ir_device(struct IR *ir) { - int i; - - for (i = 0; i MAX_IRCTL_DEVICES; i++) - if (ir_devices[i] == NULL) { - ir_devices[i] = ir; - break; - } - - return i == MAX_IRCTL_DEVICES ? -ENOMEM : i; + list_add_tail(ir-list, ir_devices_list); + return 0; } /* ir_devices_lock must be held */ static void del_ir_device(struct IR *ir) { - int i; + struct IR *p; + + if (list_empty(ir_devices_list)) + return; - for (i = 0; i MAX_IRCTL_DEVICES; i++) - if (ir_devices[i] == ir) { - ir_devices[i] = NULL; + list_for_each_entry(p, ir_devices_list, list) + if (p == ir) { + list_del(p-list); break; } } @@ -1237,17 +1241,16 @@ static int ir_remove(struct i2c_client *client) /* ir_devices_lock must be held */ static struct IR *find_ir_device_by_adapter(struct i2c_adapter *adapter) { - int i; - struct IR *ir = NULL; + struct IR *ir; - for (i = 0; i MAX_IRCTL_DEVICES; i++) - if (ir_devices[i] != NULL - ir_devices[i]-adapter == adapter) { - ir = ir_devices[i]; - break; - } + if (list_empty(ir_devices_list)) + return NULL; + + list_for_each_entry(ir, ir_devices_list, list) + if (ir-adapter == adapter) + return ir; - return ir; + return NULL; } static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) @@ -1284,6 +1287,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) goto out_no_ir; } /* store for use in ir_probe() again, and open() later on */ + INIT_LIST_HEAD(ir-list); ret = add_ir_device(ir); if (ret) goto out_free_ir; @@ -1421,7 +1425,6 @@ static int __init zilog_init(void) zilog_notify(Zilog/Hauppauge IR driver initializing\n); mutex_init(tx_data_lock); - mutex_init(ir_devices_lock); request_module(firmware_class); -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/13] lirc_zilog: Convert the instance open count to an atomic_t
The open count is simply used for deciding if the Rx polling thread needs to poll the IR chip for userspace. Simplify the manipulation of the open count by using an atomic_t and not requiring a lock The polling thread errantly didn't try to take the lock anyway. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 15 +-- 1 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 39f7b53..c857b99 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -94,7 +94,7 @@ struct IR { struct lirc_driver l; struct mutex ir_lock; - int open; + atomic_t open_count; struct i2c_adapter *adapter; struct IR_rx *rx; @@ -279,7 +279,7 @@ static int lirc_thread(void *arg) set_current_state(TASK_INTERRUPTIBLE); /* if device not opened, we can sleep half a second */ - if (!ir-open) { + if (atomic_read(ir-open_count) == 0) { schedule_timeout(HZ/2); continue; } @@ -1094,10 +1094,7 @@ static int open(struct inode *node, struct file *filep) if (ir == NULL) return -ENODEV; - /* increment in use count */ - mutex_lock(ir-ir_lock); - ++ir-open; - mutex_unlock(ir-ir_lock); + atomic_inc(ir-open_count); /* stash our IR struct */ filep-private_data = ir; @@ -1115,10 +1112,7 @@ static int close(struct inode *node, struct file *filep) return -ENODEV; } - /* decrement in use count */ - mutex_lock(ir-ir_lock); - --ir-open; - mutex_unlock(ir-ir_lock); + atomic_dec(ir-open_count); return 0; } @@ -1294,6 +1288,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) ir-adapter = adap; mutex_init(ir-ir_lock); + atomic_set(ir-open_count, 0); /* set lirc_dev stuff */ memcpy(ir-l, lirc_template, sizeof(struct lirc_driver)); -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/13] lirc_zilog: Use kernel standard methods for marking device non-seekable
lirc_zilog had its own llseek stub that returned -ESPIPE. Get rid of it and use the kernel's no_llseek() and nonseekable_open() functions instead. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c |9 ++--- 1 files changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index c857b99..720ef67 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -712,12 +712,6 @@ static int tx_init(struct IR_tx *tx) return 0; } -/* do nothing stub to make LIRC happy */ -static loff_t lseek(struct file *filep, loff_t offset, int orig) -{ - return -ESPIPE; -} - /* copied from lirc_dev */ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) { @@ -1099,6 +1093,7 @@ static int open(struct inode *node, struct file *filep) /* stash our IR struct */ filep-private_data = ir; + nonseekable_open(node, filep); return 0; } @@ -1150,7 +1145,7 @@ static struct i2c_driver driver = { static const struct file_operations lirc_fops = { .owner = THIS_MODULE, - .llseek = lseek, + .llseek = no_llseek, .read = read, .write = write, .poll = poll, -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/13] lirc_zilog: Don't acquire the rx-buf_lock in the poll() function
There is no need to take the rx-buf_lock in the the poll() function as all the underling calls made on objects in the rx-buf lirc_buffer object are protected by spinlocks. Corrected a bad error return value in poll(): return POLLERR instead of -ENODEV. Added some comments to poll() for when, in the future, I forget what poll() and poll_wait() are supposed to do. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 21 ++--- 1 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 720ef67..dfa6a42 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -985,19 +985,26 @@ static unsigned int poll(struct file *filep, poll_table *wait) unsigned int ret; dprintk(poll called\n); - if (rx == NULL) - return -ENODEV; - mutex_lock(rx-buf_lock); + if (rx == NULL) { + /* +* Revisit this, if our poll function ever reports writeable +* status for Tx +*/ + dprintk(poll result = POLLERR\n); + return POLLERR; + } + /* +* Add our lirc_buffer's wait_queue to the poll_table. A wake up on +* that buffer's wait queue indicates we may have a new poll status. +*/ poll_wait(filep, rx-buf.wait_poll, wait); - dprintk(poll result = %s\n, - lirc_buffer_empty(rx-buf) ? 0 : POLLIN|POLLRDNORM); - + /* Indicate what ops could happen immediately without blocking */ ret = lirc_buffer_empty(rx-buf) ? 0 : (POLLIN|POLLRDNORM); - mutex_unlock(rx-buf_lock); + dprintk(poll result = %s\n, ret ? POLLIN|POLLRDNORM : 0); return ret; } -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/13] lirc_zilog: Remove unneeded rx-buf_lock
Remove the rx-buf_lock that protected the rx-buf lirc_buffer. The underlying operations on the objects within the lirc_buffer are already protected by spinlocks, or the objects are constant (e.g. chunk_size). Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 23 +-- 1 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index dfa6a42..0f2fa58 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -67,9 +67,8 @@ struct IR_rx { /* RX device */ struct i2c_client *c; - /* RX device buffer lock */ + /* RX device buffer */ struct lirc_buffer buf; - struct mutex buf_lock; /* RX polling thread data */ struct task_struct *task; @@ -718,18 +717,15 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) struct IR *ir = filep-private_data; struct IR_rx *rx = ir-rx; int ret = 0, written = 0; + unsigned int m; DECLARE_WAITQUEUE(wait, current); dprintk(read called\n); if (rx == NULL) return -ENODEV; - if (mutex_lock_interruptible(rx-buf_lock)) - return -ERESTARTSYS; - if (n % rx-buf.chunk_size) { dprintk(read result = -EINVAL\n); - mutex_unlock(rx-buf_lock); return -EINVAL; } @@ -767,19 +763,19 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) set_current_state(TASK_INTERRUPTIBLE); } else { unsigned char buf[rx-buf.chunk_size]; - lirc_buffer_read(rx-buf, buf); - ret = copy_to_user((void *)outbuf+written, buf, - rx-buf.chunk_size); - written += rx-buf.chunk_size; + m = lirc_buffer_read(rx-buf, buf); + if (m == rx-buf.chunk_size) { + ret = copy_to_user((void *)outbuf+written, buf, + rx-buf.chunk_size); + written += rx-buf.chunk_size; + } } } remove_wait_queue(rx-buf.wait_poll, wait); set_current_state(TASK_RUNNING); - mutex_unlock(rx-buf_lock); - dprintk(read result = %s (%d)\n, - ret ? -EFAULT : OK, ret); + dprintk(read result = %d (%s)\n, ret, ret ? Error : OK); return ret ? ret : written; } @@ -1327,7 +1323,6 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) if (ret) goto out_free_xx; - mutex_init(ir-rx-buf_lock); ir-rx-c = client; ir-rx-hdpvr_data_fmt = (id-driver_data ID_FLAG_HDPVR) ? true : false; -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/13] lirc_zilog: Always allocate a Rx lirc_buffer object
Always allocate a lirc_buffer object, instead of just upon setup of the Rx i2c_client. If we do not allocate a lirc_buffer object, because we are not handling the Rx i2c_client, lirc_dev will allocate its own lirc_buffer anyway and not tell us about its location. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 62 ++-- 1 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 0f2fa58..a94b10a 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -67,9 +67,6 @@ struct IR_rx { /* RX device */ struct i2c_client *c; - /* RX device buffer */ - struct lirc_buffer buf; - /* RX polling thread data */ struct task_struct *task; @@ -91,6 +88,7 @@ struct IR { struct list_head list; struct lirc_driver l; + struct lirc_buffer rbuf; struct mutex ir_lock; atomic_t open_count; @@ -157,12 +155,13 @@ static int add_to_buf(struct IR *ir) int ret; int failures = 0; unsigned char sendbuf[1] = { 0 }; + struct lirc_buffer *rbuf = ir-l.rbuf; struct IR_rx *rx = ir-rx; if (rx == NULL) return -ENXIO; - if (lirc_buffer_full(rx-buf)) { + if (lirc_buffer_full(rbuf)) { dprintk(buffer overflow\n); return -EOVERFLOW; } @@ -250,9 +249,9 @@ static int add_to_buf(struct IR *ir) codes[1] = code 0xff; /* return it */ - lirc_buffer_write(rx-buf, codes); + lirc_buffer_write(rbuf, codes); ++got_data; - } while (!lirc_buffer_full(rx-buf)); + } while (!lirc_buffer_full(rbuf)); return 0; } @@ -270,7 +269,7 @@ static int add_to_buf(struct IR *ir) static int lirc_thread(void *arg) { struct IR *ir = arg; - struct IR_rx *rx = ir-rx; + struct lirc_buffer *rbuf = ir-l.rbuf; dprintk(poll thread started\n); @@ -297,7 +296,7 @@ static int lirc_thread(void *arg) if (kthread_should_stop()) break; if (!add_to_buf(ir)) - wake_up_interruptible(rx-buf.wait_poll); + wake_up_interruptible(rbuf-wait_poll); } dprintk(poll thread ended\n); @@ -716,6 +715,7 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) { struct IR *ir = filep-private_data; struct IR_rx *rx = ir-rx; + struct lirc_buffer *rbuf = ir-l.rbuf; int ret = 0, written = 0; unsigned int m; DECLARE_WAITQUEUE(wait, current); @@ -724,7 +724,7 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) if (rx == NULL) return -ENODEV; - if (n % rx-buf.chunk_size) { + if (n % rbuf-chunk_size) { dprintk(read result = -EINVAL\n); return -EINVAL; } @@ -734,7 +734,7 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) * to avoid losing scan code (in case when queue is awaken somewhere * between while condition checking and scheduling) */ - add_wait_queue(rx-buf.wait_poll, wait); + add_wait_queue(rbuf-wait_poll, wait); set_current_state(TASK_INTERRUPTIBLE); /* @@ -742,7 +742,7 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) * mode and 'copy_to_user' is happy, wait for data. */ while (written n ret == 0) { - if (lirc_buffer_empty(rx-buf)) { + if (lirc_buffer_empty(rbuf)) { /* * According to the read(2) man page, 'written' can be * returned as less than 'n', instead of blocking @@ -762,17 +762,17 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) schedule(); set_current_state(TASK_INTERRUPTIBLE); } else { - unsigned char buf[rx-buf.chunk_size]; - m = lirc_buffer_read(rx-buf, buf); - if (m == rx-buf.chunk_size) { + unsigned char buf[rbuf-chunk_size]; + m = lirc_buffer_read(rbuf, buf); + if (m == rbuf-chunk_size) { ret = copy_to_user((void *)outbuf+written, buf, - rx-buf.chunk_size); - written += rx-buf.chunk_size; + rbuf-chunk_size); + written += rbuf-chunk_size; } } } -
[PATCH 09/13] lirc_zilog: Move constants from ir_probe() into the lirc_driver template
ir_probe() makes a number of constant assignments into the lirc_driver object after copying in a template. Make better use of the template. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 27 +++ 1 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index a94b10a..8ab60e9 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -1116,13 +1116,6 @@ static int close(struct inode *node, struct file *filep) return 0; } -static struct lirc_driver lirc_template = { - .name = lirc_zilog, - .set_use_inc= set_use_inc, - .set_use_dec= set_use_dec, - .owner = THIS_MODULE -}; - static int ir_remove(struct i2c_client *client); static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id); @@ -1161,6 +1154,19 @@ static const struct file_operations lirc_fops = { .release= close }; +static struct lirc_driver lirc_template = { + .name = lirc_zilog, + .minor = -1, + .code_length= 13, + .buffer_size= BUFLEN / 2, + .sample_rate= 0, /* tell lirc_dev to not start its own kthread */ + .chunk_size = 2, + .set_use_inc= set_use_inc, + .set_use_dec= set_use_dec, + .fops = lirc_fops, + .owner = THIS_MODULE, +}; + static void destroy_rx_kthread(struct IR_rx *rx) { /* end up polling thread */ @@ -1292,14 +1298,9 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) /* set lirc_dev stuff */ memcpy(ir-l, lirc_template, sizeof(struct lirc_driver)); ir-l.minor = minor; /* module option */ - ir-l.code_length = 13; - ir-l.chunk_size = 2; - ir-l.buffer_size = BUFLEN / 2; ir-l.rbuf= ir-rbuf; - ir-l.fops= lirc_fops; ir-l.data= ir; ir-l.dev = adap-dev; - ir-l.sample_rate = 0; ret = lirc_buffer_init(ir-l.rbuf, ir-l.chunk_size, ir-l.buffer_size); if (ret) @@ -1314,6 +1315,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) goto out_free_xx; } + ir-l.features |= LIRC_CAN_SEND_PULSE; ir-tx-c = client; ir-tx-need_boot = 1; ir-tx-post_tx_ready_poll = @@ -1326,6 +1328,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) goto out_free_xx; } + ir-l.features |= LIRC_CAN_REC_LIRCCODE; ir-rx-c = client; ir-rx-hdpvr_data_fmt = (id-driver_data ID_FLAG_HDPVR) ? true : false; -- 1.7.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] lirc_zilog: Add ref counting of struct IR, IR_tx, and IR_rx
This is a major change to add pointer reference counting for struct IR, struct IR_tx, and struct IR_rx object instances. This ref counting gets lirc_zilog closer to gracefully handling bridge drivers and hot-unplugged USB devices disappearing out from under lirc_zilog when the /dev/lircN node is still open. (mutexes to protect the i2c_client pointers in struct IR_tx and struct IR_rx still need to be added.) This reference counting also helps lirc_zilog clean up properly when the i2c_clients disappear. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 582 - 1 files changed, 380 insertions(+), 202 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 8ab60e9..755cb39 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -63,8 +63,14 @@ #include media/lirc_dev.h #include media/lirc.h +struct IR; + struct IR_rx { + struct kref ref; + struct IR *ir; + /* RX device */ + /* FIXME mutex lock access to this pointer */ struct i2c_client *c; /* RX polling thread data */ @@ -76,7 +82,11 @@ struct IR_rx { }; struct IR_tx { + struct kref ref; + struct IR *ir; + /* TX device */ + /* FIXME mutex lock access to this pointer */ struct i2c_client *c; /* TX additional actions needed */ @@ -85,8 +95,10 @@ struct IR_tx { }; struct IR { + struct kref ref; struct list_head list; + /* FIXME spinlock access to l.features */ struct lirc_driver l; struct lirc_buffer rbuf; @@ -94,11 +106,21 @@ struct IR { atomic_t open_count; struct i2c_adapter *adapter; + + spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */ struct IR_rx *rx; + + spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */ struct IR_tx *tx; }; /* IR transceiver instance object list */ +/* + * This lock is used for the following: + * a. ir_devices_list access, insertions, deletions + * b. struct IR kref get()s and put()s + * c. serialization of ir_probe() for the two i2c_clients for a Z8 + */ static DEFINE_MUTEX(ir_devices_lock); static LIST_HEAD(ir_devices_list); @@ -146,6 +168,157 @@ static int minor = -1;/* minor number */ ## args); \ } while (0) + +/* struct IR reference counting */ +static struct IR *get_ir_device(struct IR *ir, bool ir_devices_lock_held) +{ + if (ir_devices_lock_held) { + kref_get(ir-ref); + } else { + mutex_lock(ir_devices_lock); + kref_get(ir-ref); + mutex_unlock(ir_devices_lock); + } + return ir; +} + +static void release_ir_device(struct kref *ref) +{ + struct IR *ir = container_of(ref, struct IR, ref); + + /* +* Things should be in this state by now: +* ir-rx set to NULL and deallocated - happens before ir-rx-ir put() +* ir-rx-task kthread stopped - happens before ir-rx-ir put() +* ir-tx set to NULL and deallocated - happens before ir-tx-ir put() +* ir-open_count == 0 - happens on final close() +* ir_lock, tx_ref_lock, rx_ref_lock, all released +*/ + if (ir-l.minor = 0 ir-l.minor MAX_IRCTL_DEVICES) { + lirc_unregister_driver(ir-l.minor); + ir-l.minor = MAX_IRCTL_DEVICES; + } + if (ir-rbuf.fifo_initialized) + lirc_buffer_free(ir-rbuf); + list_del(ir-list); + kfree(ir); +} + +static int put_ir_device(struct IR *ir, bool ir_devices_lock_held) +{ + int released; + + if (ir_devices_lock_held) + return kref_put(ir-ref, release_ir_device); + + mutex_lock(ir_devices_lock); + released = kref_put(ir-ref, release_ir_device); + mutex_unlock(ir_devices_lock); + + return released; +} + +/* struct IR_rx reference counting */ +static struct IR_rx *get_ir_rx(struct IR *ir) +{ + struct IR_rx *rx; + + spin_lock(ir-rx_ref_lock); + rx = ir-rx; + if (rx != NULL) + kref_get(rx-ref); + spin_unlock(ir-rx_ref_lock); + return rx; +} + +static void destroy_rx_kthread(struct IR_rx *rx, bool ir_devices_lock_held) +{ + /* end up polling thread */ + if (!IS_ERR_OR_NULL(rx-task)) { + kthread_stop(rx-task); + rx-task = NULL; + /* Put the ir ptr that ir_probe() gave to the rx poll thread */ + put_ir_device(rx-ir, ir_devices_lock_held); + } +} + +static void release_ir_rx(struct kref *ref) +{ + struct IR_rx *rx = container_of(ref, struct IR_rx, ref); + struct IR *ir = rx-ir; + + /* +* This release function can't do all the work, as we want +* to keep the rx_ref_lock a spinlock, and killing the poll thread +
[PATCH 11/13] lirc_zilog: Add locking of the i2c_clients when in use
Lock the i2c_client pointers and prevent i2c_client removal when lirc_zilog is perfoming a series of operations that require valid i2c_client pointers. Signed-off-by: Andy Walls awa...@md.metrocast.net --- drivers/staging/lirc/lirc_zilog.c | 41 +--- 1 files changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 755cb39..a59d32d 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -70,7 +70,7 @@ struct IR_rx { struct IR *ir; /* RX device */ - /* FIXME mutex lock access to this pointer */ + struct mutex client_lock; struct i2c_client *c; /* RX polling thread data */ @@ -86,7 +86,7 @@ struct IR_tx { struct IR *ir; /* TX device */ - /* FIXME mutex lock access to this pointer */ + struct mutex client_lock; struct i2c_client *c; /* TX additional actions needed */ @@ -341,6 +341,14 @@ static int add_to_buf(struct IR *ir) if (rx == NULL) return -ENXIO; + /* Ensure our rx-c i2c_client remains valid for the duration */ + mutex_lock(rx-client_lock); + if (rx-c == NULL) { + mutex_unlock(rx-client_lock); + put_ir_rx(rx, false); + return -ENXIO; + } + tx = get_ir_tx(ir); /* @@ -442,6 +450,7 @@ static int add_to_buf(struct IR *ir) ret = 0; } while (!lirc_buffer_full(rbuf)); + mutex_unlock(rx-client_lock); if (tx != NULL) put_ir_tx(tx, false); put_ir_rx(rx, false); @@ -1089,6 +1098,14 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, if (tx == NULL) return -ENXIO; + /* Ensure our tx-c i2c_client remains valid for the duration */ + mutex_lock(tx-client_lock); + if (tx-c == NULL) { + mutex_unlock(tx-client_lock); + put_ir_tx(tx, false); + return -ENXIO; + } + /* Lock i2c bus for the duration */ mutex_lock(ir-ir_lock); @@ -1099,6 +1116,7 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, if (copy_from_user(command, buf + i, sizeof(command))) { mutex_unlock(ir-ir_lock); + mutex_unlock(tx-client_lock); put_ir_tx(tx, false); return -EFAULT; } @@ -1109,6 +1127,7 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, ret = fw_load(tx); if (ret != 0) { mutex_unlock(ir-ir_lock); + mutex_unlock(tx-client_lock); put_ir_tx(tx, false); if (ret != -ENOMEM) ret = -EIO; @@ -1126,6 +1145,7 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, (unsigned)command 0x); if (ret == -EPROTO) { mutex_unlock(ir-ir_lock); + mutex_unlock(tx-client_lock); put_ir_tx(tx, false); return ret; } @@ -1144,6 +1164,7 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, zilog_error(unable to send to the IR chip after 3 resets, giving up\n); mutex_unlock(ir-ir_lock); + mutex_unlock(tx-client_lock); put_ir_tx(tx, false); return ret; } @@ -1158,6 +1179,8 @@ static ssize_t write(struct file *filep, const char *buf, size_t n, /* Release i2c bus */ mutex_unlock(ir-ir_lock); + mutex_unlock(tx-client_lock); + /* Give back our struct IR_tx reference */ put_ir_tx(tx, false); @@ -1367,12 +1390,20 @@ static int ir_remove(struct i2c_client *client) { if (strncmp(ir_tx_z8, client-name, 8) == 0) { struct IR_tx *tx = i2c_get_clientdata(client); - if (tx != NULL) + if (tx != NULL) { + mutex_lock(tx-client_lock); + tx-c = NULL; + mutex_unlock(tx-client_lock); put_ir_tx(tx, false); + } } else if (strncmp(ir_rx_z8, client-name, 8) == 0) { struct IR_rx *rx = i2c_get_clientdata(client); - if (rx != NULL) + if (rx != NULL) { + mutex_lock(rx-client_lock); + rx-c = NULL; +
[PATCH 10/29] wl1273: mfd_cell is now implicitly available to drivers
The cell's platform_data is now accessed with a helper function; change clients to use that, and remove the now-unused data_size. Signed-off-by: Andres Salomon dilin...@queued.net --- drivers/media/radio/radio-wl1273.c |2 +- drivers/mfd/wl1273-core.c |2 -- sound/soc/codecs/wl1273.c |3 ++- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/radio/radio-wl1273.c b/drivers/media/radio/radio-wl1273.c index 7ecc8e6..4698eb0 100644 --- a/drivers/media/radio/radio-wl1273.c +++ b/drivers/media/radio/radio-wl1273.c @@ -2138,7 +2138,7 @@ static int wl1273_fm_radio_remove(struct platform_device *pdev) static int __devinit wl1273_fm_radio_probe(struct platform_device *pdev) { - struct wl1273_core **core = pdev-dev.platform_data; + struct wl1273_core **core = mfd_get_data(pdev); struct wl1273_device *radio; struct v4l2_ctrl *ctrl; int r = 0; diff --git a/drivers/mfd/wl1273-core.c b/drivers/mfd/wl1273-core.c index d2ecc24..703085e 100644 --- a/drivers/mfd/wl1273-core.c +++ b/drivers/mfd/wl1273-core.c @@ -80,7 +80,6 @@ static int __devinit wl1273_core_probe(struct i2c_client *client, cell = core-cells[children]; cell-name = wl1273_fm_radio; cell-platform_data = core; - cell-data_size = sizeof(core); children++; if (pdata-children WL1273_CODEC_CHILD) { @@ -89,7 +88,6 @@ static int __devinit wl1273_core_probe(struct i2c_client *client, dev_dbg(client-dev, %s: Have codec.\n, __func__); cell-name = wl1273-codec; cell-platform_data = core; - cell-data_size = sizeof(core); children++; } diff --git a/sound/soc/codecs/wl1273.c b/sound/soc/codecs/wl1273.c index 861b28f..1ad0d5a 100644 --- a/sound/soc/codecs/wl1273.c +++ b/sound/soc/codecs/wl1273.c @@ -436,7 +436,8 @@ EXPORT_SYMBOL_GPL(wl1273_get_format); static int wl1273_probe(struct snd_soc_codec *codec) { - struct wl1273_core **core = codec-dev-platform_data; + struct wl1273_core **core = + mfd_get_data(to_platform_device(codec-dev)); struct wl1273_priv *wl1273; int r; -- 1.7.2.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver
Kamil, I have a question about MFC state FINISHING FINISHED as below. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Kamil Debski Sent: Saturday, January 08, 2011 1:26 AM To: linux-media@vger.kernel.org; linux-samsung-...@vger.kernel.org Cc: m.szyprow...@samsung.com; pa...@osciak.com; kyungmin.p...@samsung.com; k.deb...@samsung.com; jaeryul...@samsung.com; kgene@samsung.com Subject: [RFC/PATCH v6 3/4] MFC: Add MFC 5.1 V4L2 driver Multi Format Codec 5.1 is capable of handling a range of video codecs and this driver provides V4L2 interface for video decoding. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/video/Kconfig |8 + drivers/media/video/Makefile |1 + drivers/media/video/s5p-mfc/Makefile |3 + drivers/media/video/s5p-mfc/regs-mfc5.h | 335 + drivers/media/video/s5p-mfc/s5p_mfc.c| 2072 ++ drivers/media/video/s5p-mfc/s5p_mfc_common.h | 224 +++ drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h | 173 +++ drivers/media/video/s5p-mfc/s5p_mfc_debug.h | 47 + drivers/media/video/s5p-mfc/s5p_mfc_intr.c | 92 ++ drivers/media/video/s5p-mfc/s5p_mfc_intr.h | 26 + drivers/media/video/s5p-mfc/s5p_mfc_memory.h | 43 + drivers/media/video/s5p-mfc/s5p_mfc_opr.c| 885 +++ drivers/media/video/s5p-mfc/s5p_mfc_opr.h| 160 ++ 13 files changed, 4069 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/s5p-mfc/Makefile create mode 100644 drivers/media/video/s5p-mfc/regs-mfc5.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc.c create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_common.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_ctrls.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_debug.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.c create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_intr.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_memory.h create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.c create mode 100644 drivers/media/video/s5p-mfc/s5p_mfc_opr.h Snipped... +/* Videobuf opts */ +static struct vb2_ops s5p_mfc_qops = { + .buf_queue = s5p_mfc_buf_queue, + .queue_setup = s5p_mfc_queue_setup, + .start_streaming = s5p_mfc_start_streaming, + .buf_init = s5p_mfc_buf_init, + .stop_streaming = s5p_mfc_stop_streaming, + .wait_prepare = s5p_mfc_unlock, + .wait_finish = s5p_mfc_lock, +}; + +static void s5p_mfc_handle_frame_all_extracted(struct s5p_mfc_ctx *ctx) +{ + struct s5p_mfc_dev *dev = ctx-dev; + struct s5p_mfc_buf *dst_buf; + + ctx-state = MFCINST_DEC_FINISHED; + mfc_debug(Decided to finish\n); + ctx-sequence++; + while (!list_empty(ctx-dst_queue)) { + dst_buf = list_entry(ctx-dst_queue.next, + struct s5p_mfc_buf, list); + mfc_debug(Cleaning up buffer: %d\n, + dst_buf-b-v4l2_buf.index); + vb2_set_plane_payload(dst_buf-b, 0, 0); + vb2_set_plane_payload(dst_buf-b, 1, 0); + list_del(dst_buf-list); + ctx-dst_queue_cnt--; + dst_buf-b-v4l2_buf.sequence = (ctx-sequence++); + if (s5p_mfc_get_pic_time_top(ctx) == + s5p_mfc_get_pic_time_bottom(ctx)) + dst_buf-b-v4l2_buf.field = V4L2_FIELD_NONE; + else + dst_buf-b-v4l2_buf.field = + V4L2_FIELD_INTERLACED; + ctx-dec_dst_flag = ~(1 dst_buf-b-v4l2_buf.index); + vb2_buffer_done(dst_buf-b, VB2_BUF_STATE_DONE); + mfc_debug(Cleaned up buffer: %d\n, + dst_buf-b-v4l2_buf.index); + } + mfc_debug(After cleanup\n); +} + +static void s5p_mfc_handle_frame_new(struct s5p_mfc_ctx *ctx, unsigned int err) +{ + struct s5p_mfc_dev *dev = ctx-dev; + struct s5p_mfc_buf *dst_buf; + size_t dspl_y_addr = s5p_mfc_get_dspl_y_adr(); + + ctx-sequence++; + /* If frame is same as previous then skip and do not dequeue */ + if (s5p_mfc_get_frame_type() == S5P_FIMV_DECODE_FRAME_SKIPPED) + return; + /* The MFC returns address of the buffer, now we have to + * check which videobuf does it correspond to */ + list_for_each_entry(dst_buf, ctx-dst_queue, list) { + mfc_debug(Listing: %d\n, dst_buf-b-v4l2_buf.index); + /* Check if this is the buffer we're looking for */ + if (vb2_cma_plane_paddr(dst_buf-b, 0) == dspl_y_addr) { + list_del(dst_buf-list); + ctx-dst_queue_cnt--; + dst_buf-b-v4l2_buf.sequence =