V4L/ARM: videobuf-dma-contig no longer works on my ARM machine (was: [PATCH v3] SoC Camera: add driver for OMAP1 camera interface)
On Wendesday, 23 March 2011, at 11:00:06, Guennadi Liakhovetski wrote: > > You might want to retest ams-delta with the camera on the current > (next or git://linuxtv.org/media_tree.git staging/for_v2.6.39) kernel > ... Hi Guennadi, With the patch I've just submitted to the linux-media list (http://www.spinics.net/lists/linux-media/msg31255.html), the 2.6.39-rc2 OMAP1 camera driver still works for me as before, but only in SG mode. I'm no longer able to use it in CONTIG mode after videobuf-dma-contig.c has been changed with commit 35d9f510b67b10338161aba6229d4f55b4000f5b, "V4L: videobuf, don't use dma addr as physical", supposed to correct potential issues on IOMMU euqipped machines. Since there were no actual problems reported before, I suppose the old code, which was passing to remap_pfn_range() a physical page number calculated from dma_alloc_coherent() privided dma_handle, worked correctly on all platforms actually using videobud-dma-config. Now, on my ARM machine, a completely different, then completely wrong physical address, calculated as virt_to_phys(dma_alloc_coherent()), is used instead of the dma_handle, which causes the machine to hang. AFAICS, incompatibility of the new code with the ARM architecture has been already pointed out as a potential issue by Lauent Pinchart (http://www.spinics.net/lists/linux-media/msg29544.html), but the patch has been accepted regardless. I suspect the problem may affect other ARM subarchitectures, not only OMAP1, but have no way to verify this. Anyone with a suitable hardware, can you please verify and report if your machine is affected or not? I've tried to resolve the problem by conditionally (#ifdef CONFIG_ARM) replacing remap_pfn_range() with ARM specific dma_mmap_coherent(), but with not much success so far. While this still seems a possibly correct solution to me (see sound/core/pcm_native.c for an example, architecture independent driver code which already implements a similiar method), I found that the dma_mmap_coherent() does its job for me only with the first buffer, failing with the second one because of negative buffer size vs. available vma space comparison. It seems to me that there may be something wrong with vma->vm_pgoff handling, but I'm not sure if in the V4L videobuf, or in the ARM DMA, or in a yet another piece of code. Not being able to work out a solution myself, other than reverting back to the old code for ARM, I hope someone can come out with a better idea. Thanks, Janusz -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.39] soc_camera: OMAP1: fix missing bytesperline and sizeimage initialization
Since commit 0e4c180d3e2cc11e248f29d4c604b6194739d05a, bytesperline and sizeimage memebers of v4l2_pix_format structure have no longer been calculated inside soc_camera_g_fmt_vid_cap(), but rather passed via soc_camera_device structure from a host driver callback invoked by soc_camera_set_fmt(). OMAP1 camera host driver has never been providing these parameters, so it no longer works correctly. Fix it by adding suitable assignments to omap1_cam_set_fmt(). Signed-off-by: Janusz Krzysztofik --- drivers/media/video/omap1_camera.c |6 ++ 1 file changed, 6 insertions(+) --- linux-2.6.39-rc2/drivers/media/video/omap1_camera.c.orig2011-04-06 14:30:37.0 +0200 +++ linux-2.6.39-rc2/drivers/media/video/omap1_camera.c 2011-04-09 00:16:36.0 +0200 @@ -1292,6 +1292,12 @@ static int omap1_cam_set_fmt(struct soc_ pix->colorspace = mf.colorspace; icd->current_fmt = xlate; + pix->bytesperline = soc_mbus_bytes_per_line(pix->width, + xlate->host_fmt); + if (pix->bytesperline < 0) + return pix->bytesperline; + pix->sizeimage = pix->height * pix->bytesperline; + return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] v4l: videobuf2: Handle buf_queue errors
Hi Laurent! On 04/08/2011 02:53 PM, Laurent Pinchart wrote: > Hi Sylwester, > > On Thursday 07 April 2011 00:04:45 Sylwester Nawrocki wrote: >> On 04/06/2011 10:05 PM, Sylwester Nawrocki wrote: >>> On 03/07/2011 12:20 AM, Pawel Osciak wrote: On Tue, Mar 1, 2011 at 02:54, Laurent Pinchart wrote: > On Monday 28 February 2011 16:44:38 Pawel Osciak wrote: >> Hi Laurent, >> A few questions from me below. I feel we need to talk about this >> change a bit more, it introduces some recovery and consistency >> problems, unless I'm missing something. >> >> On Sun, Feb 27, 2011 at 10:12, Laurent Pinchart wrote: >>> videobuf2 expects drivers to check buffer in the buf_prepare >>> operation and to return success only if the buffer can queued >>> without any issue. >>> >>> For hot-pluggable devices, disconnection events need to be handled at >>> buf_queue time. Checking the disconnected flag and adding the buffer >>> to the driver queue need to be performed without releasing the >>> driver spinlock, otherwise race conditions can occur in which the >>> driver could still allow a buffer to be queued after the >>> disconnected flag has been set, resulting in a hang during the next >>> DQBUF operation. >>> >>> This problem can be solved either in the videobuf2 core or in the >>> device drivers. To avoid adding a spinlock to videobuf2, make >>> buf_queue return an int and handle buf_queue failures in videobuf2. >>> Drivers must not return an error in buf_queue if the condition >>> leading to the error can be caught in buf_prepare. >> >> ... >> >>> As buf_queue callback is called by vb2 only after start_streaming, >>> for a camera snapshot capture I needed to start a pipeline only from the >>> buf_queue handler level, i.e. subdev's video s_stream op was called from >>> within buf_queue. s_stream couldn't be done in the start_streaming >>> handler as at the time it is invoked there is always no buffer available >>> in the bridge H/W. >>> It's a consequence of how the vb2_streamon() is designed. >>> >>> Before, I used to simply call s_stream in start_streaming, only deferring >>> the actual bridge DMA enable till a buf_queue call, thus letting first >>> frames in the stream to be lost. This of course cannot be done in case >>> of single-frame capture. >>> >>> To make a long story short, it would be useful in my case to have the >>> ability to return error codes as per VIDIOC_STREAMON through buf_queue >>> in the driver (when the first buffer is queued). >>> At the moment mainly EPIPE comes to my mind. This error code has no >>> meaning in the API for QBUF though. Should the pipeline be started from >>> buf_queue >> >> Hmm, the pipeline validation could still be done in start_streaming() >> so we can return any EPIPE error from there directly and effectively >> in VIDIOC_STREAMON. > > That's correct, and that's what the OMAP3 ISP driver does. > >> So the only remaining errors are those related to I2C communication etc. >> when streaming is actually enabled in the subdev. > > buf_queue is called with a spinlock help, so you can't perform I2C > communication there. This sounds like a really simple requirement - configure the capture format, allocate and queue a single buffer and trigger the hardware to fill exactly that buffer. Yet the drivers are forced to perform some acrobatics to achieve that fairly simple task. Luckily videobuf2 does not enforce atomic context in buf_queue as Marek pointed out. And I know it from experience as I've experimented with S5P FIMC and M-5MOLS drivers for still JPEG capture, on top of your patch allowing to return errors from buf_queue btw :) VIDIOC_STREAMON has rather fuzzy meaning for me and I assume there has been an agreement that allowing drivers to return errors from buf_queue and performing blocking I/O from there is the expected way to work around the freedom the API gives to applications in regards to VIDIOC_STREAMON. But someone could prove me wrong here. I guess there are some issues with streaming over USB when buf_queue is called in a non atomic context? I'll have a closer look at your email explaining the USB disconnection handling. -- Regards, Sylwester Nawrocki -- 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] Fix cx88 remote control input
Jarod Wilson wrote: >On Apr 8, 2011, at 2:38 PM, Devin Heitmueller wrote: >... >> I question the notion of introducing the requirement that all keymap >> definitions must have system codes without having really thought >> through the notion that it would result in breaking every existing >> keymap which hadn't been updated. > >Speaks the the "too many of us are only hacking on this in their >limited free time" point I raised. Hack, hack, hack, test with the >hardware available on hand (which is actually quite a bit in my case, >I think I have upwards of 35 receivers and even more remotes now), >see that it works, move on to the next issue. I'm certainly guilty of >not looking at the bigger picture and thinking about possible >ramifications more than once. :) > >... >>> I have quite a few pieces of Hauppauge hardware, several with IR >>> receivers and remotes, but all of which use ir-kbd-i2c (or >>> lirc_zilog), i.e., none of which pass along raw IR. >> >> You don't have an HVR-950 or some other stick which announces RC5 >> codes? If not, let me know and I will send you something. It's kind >> of silly for someone doing that sort of work to not have at least one >> product in each category of receiver. > >I don't think I even fully realized before today that there was >Hauppauge hardware shipping with the grey remotes and a raw IR >receiver. All the Hauppauge stuff I've got is either i2c IR >(PVR-250, PVR-350, HVR-1950, HD-PVR) or came with a bundled mceusb >transceiver (HVR-1500Q, HVR-1800, HVR-950Q -- model 72241, iirc), >and its all working these days (modulo some quirks with the HD-PVR >that still need sorting, but they're not regressions, its actually >better now than it used to be). > >So yeah, I guess I have a gap in my IR hardware collection here, >and would be happy to have something to fill it. > >-- >Jarod Wilson >ja...@wilsonet.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 Jarod, The HVR-1850 uses a raw IR receiver in the CX23888 and older HVR-1250s use the raw IR receiver in the CX23885. They both work for Rx (I need to tweak the Cx23885 rx watermark though), but I never found time to finish Tx (lack of kernel interface when I had time). If you obtain one of these I can answer any driver questions. Regards, Andy -- 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: [linux-dvb] Pinnacle PCTV Dual DVB-T Pro PCI 2000i
On Fri, Apr 8, 2011 at 5:35 AM, pigeonskil...@libero.it wrote: > Pinnacle PCTV Dual DVB-T Pro PCI 2000i (http://linuxtv.org/wiki/index.php/DVB- > T_PCI_Cards#Pinnacle) was introduced in 2006 and after 5 years it is still > unsupported in linux! > Unbelievable! I'm not sure why you find it so unbelievable. This is a project largely composed of volunteers who are working on products in their own time. If it still isn't supported, then it means that no developer owns a board and cares enough to spend a couple dozen hours to make it work. > Yet its chips Zarlink ZL10353 (http://linuxtv.org/wiki/index. > php/Zarlink_ZL10353) and Microtune MT2060 (http://linuxtv.org/wiki/index. > php/Microtune_MT2060) are supported (http://www.linuxtv. > org/downloads/drivers/linux-media-LATEST.tar.bz2)! > So, what is missing? A developer who cares enough to do the work for free, or a corporate entity willing to pay fair market prices to pay to have it supported? > Probably this is the reason why Linux is not so widespread: LACK OF DRIVERS! > And this is the reason why a lot of users cannot migrate to Linux and are > forced to use that stupid O.S. called Win! > If anyone wants to have a look at Windows' drivers and is able to develop > drivers (I'm not), here are the drivers: > ftp://ftp.pctvsystems.com/TV/driver/PCTV%202000i/PCTV%20250i%202000i.zip There are very few developers actively contributing to LinuxTV. With limited developer resources, they have to make decisions about what they are going to work on. if those decisions aren't aligned with what *you* want them working on, then your only option really is to learn to become a developer and add the support yourself. You just have to look at motivation: if a developer doesn't benefit personally from having the card working, doesn't think it's fun to make it work, and isn't being paid, then why invest ten or twenty hours of his/her valuable time? Welcome to a community of volunteers. We'll be happy to refund 100% of the money that you've paid to seeing this device work under Linux. :-) 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: [linux-dvb] Pinnacle PCTV Dual DVB-T Pro PCI 2000i
Op 08-04-11 11:35, pigeonskil...@libero.it schreef: Pinnacle PCTV Dual DVB-T Pro PCI 2000i (http://linuxtv.org/wiki/index.php/DVB- T_PCI_Cards#Pinnacle) was introduced in 2006 and after 5 years it is still unsupported in linux! Unbelievable! Unbelieveable. I bought me a super smaal dvbt stick in a 2 hand shop for 4 euros. I t works. My first was a Terratec T². I also have the pinnacle dual analog/digital... that does not work .. but that's not a big problem.. Too much noise on a dvbt stick... Even a satellite usb box gives too much noise in reception .. Marc -- What's on Shortwave guide: choose an hour, go! http://shortwave.tk 700+ Radio Stations on SW http://swstations.tk 300+ languages on SW http://radiolanguages.tk -- 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] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer
Here is some statistics without likely and with likely functions. It seems that using likely() gives better performance with Phenom I too. % Plain knl % No likely % With likely % dvb_ringbuffer_write5,9 62,88,7 81,35,7 79,2 dvb_dmx_swfilter_packet 1,2 12,80,7 6,5 0,8 11,1 dvb_dmx_swfilter_2042,3 24,51,3 12,10,7 9,7 Here "Plain knl %" is "perf top -d 30" percentage. 24,5 12,1 and 9,7 are percentages without a patch, with basic patch and last is with "likely" functions using patch. Regards, Marko Ristola 08.04.2011 18:40, Marko Ristola kirjoitti: > Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack. > Backtrack one 188 sized packet just after some garbage bytes when possible. > This obsoletes patch https://patchwork.kernel.org/patch/118147/ > > Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi > diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c > b/drivers/media/dvb/dvb-core/dvb_demux.c > index 4a88a3e..faa3671 100644 > --- a/drivers/media/dvb/dvb-core/dvb_demux.c > +++ b/drivers/media/dvb/dvb-core/dvb_demux.c > @@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, > const u8 *buf, > > EXPORT_SYMBOL(dvb_dmx_swfilter_packets); > -- 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
update fi-dna initial tuning file
Moikka Christoph, Merge attached patch. Antti -- http://palosaari.fi/ diff -r 59e55f42ab69 util/scan/dvb-c/fi-dna --- a/util/scan/dvb-c/fi-dnaWed Apr 06 06:50:51 2011 -0300 +++ b/util/scan/dvb-c/fi-dnaFri Apr 08 22:32:15 2011 +0300 @@ -1,15 +1,24 @@ # DNA network reference channels +# updated 2011-04-08 Antti Palosaari +# http://www.dna.fi/YKSITYISILLE/TV/KAAPELITV/Sivut/Taajuudet.aspx +# # freq sr fec mod C 15400 6875000 NONE QAM128 C 16200 6875000 NONE QAM128 +C 16200 6875000 NONE QAM256 C 17000 6875000 NONE QAM128 -C 23200 6875000 NONE QAM128 -C 25000 6875000 NONE QAM128 -C 25800 6875000 NONE QAM128 -C 26600 6875000 NONE QAM128 -C 27400 6875000 NONE QAM128 -C 29000 6875000 NONE QAM128 +C 22600 6875000 NONE QAM128 +C 23400 6875000 NONE QAM128 +C 24200 6875000 NONE QAM128 +C 24200 6875000 NONE QAM256 +C 25000 6875000 NONE QAM256 +C 25800 6875000 NONE QAM256 +C 26600 6875000 NONE QAM256 +C 27400 6875000 NONE QAM256 +C 28200 6875000 NONE QAM256 +C 29000 6875000 NONE QAM256 C 29800 6875000 NONE QAM128 +C 29800 6875000 NONE QAM256 C 30600 6875000 NONE QAM128 C 31400 6875000 NONE QAM128 C 32200 6875000 NONE QAM128 @@ -18,8 +27,8 @@ C 34600 6875000 NONE QAM128 C 35400 6875000 NONE QAM128 C 36200 6875000 NONE QAM128 +C 36200 6875000 NONE QAM256 C 37000 6875000 NONE QAM128 C 37800 6875000 NONE QAM128 +C 38600 6875000 NONE QAM128 C 39400 6875000 NONE QAM128 -C 40200 6875000 NONE QAM128 -C 45000 6875000 NONE QAM128
Re: [PATCH] Fix cx88 remote control input
On Apr 8, 2011, at 2:38 PM, Devin Heitmueller wrote: ... > I question the notion of introducing the requirement that all keymap > definitions must have system codes without having really thought > through the notion that it would result in breaking every existing > keymap which hadn't been updated. Speaks the the "too many of us are only hacking on this in their limited free time" point I raised. Hack, hack, hack, test with the hardware available on hand (which is actually quite a bit in my case, I think I have upwards of 35 receivers and even more remotes now), see that it works, move on to the next issue. I'm certainly guilty of not looking at the bigger picture and thinking about possible ramifications more than once. :) ... >> I have quite a few pieces of Hauppauge hardware, several with IR >> receivers and remotes, but all of which use ir-kbd-i2c (or >> lirc_zilog), i.e., none of which pass along raw IR. > > You don't have an HVR-950 or some other stick which announces RC5 > codes? If not, let me know and I will send you something. It's kind > of silly for someone doing that sort of work to not have at least one > product in each category of receiver. I don't think I even fully realized before today that there was Hauppauge hardware shipping with the grey remotes and a raw IR receiver. All the Hauppauge stuff I've got is either i2c IR (PVR-250, PVR-350, HVR-1950, HD-PVR) or came with a bundled mceusb transceiver (HVR-1500Q, HVR-1800, HVR-950Q -- model 72241, iirc), and its all working these days (modulo some quirks with the HD-PVR that still need sorting, but they're not regressions, its actually better now than it used to be). So yeah, I guess I have a gap in my IR hardware collection here, and would be happy to have something to fill it. -- Jarod Wilson ja...@wilsonet.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix cx88 remote control input
On Fri, Apr 8, 2011 at 2:00 PM, Jarod Wilson wrote: > Have to admit that I don't think it ever registered in my head that > we were going to break that many existing keymaps. But something > to consider: how many of those are *raw* rc-5 scancode keymaps, vs. > cooked scancodes from drivers that only provide command? It may > well be that we should have been more discriminating when building > those keymaps, to distinguish which were truly raw IR scancodes that > the in-kernel decoders ascertained, and which were just scancodes > handed to us directly from the IR hardware. As far as I know, keymaps didn't even support more than eight bit codes until recently. Even if the hardware had supported system codes, there was nothing to compare it against since the keymaps were limited to eight entries. >> This decision was doomed to fail. It basically said, "Yes, I know >> full well that I'm breaking most of the keymaps currently supported, >> but maybe some of those users will eventually report the issue and >> I'll make them provide an updated keymap which will eventually be >> merged upstream for others so that their remotes are no longer >> broken." > > Well, ir-keytable -w is also an option, though that does kill the > "Just Works"-ness when you first have to come up with the new map. Agreed. >> We should have introduced a RC profile property indicating how many >> bits were "significant". Then for those remotes which didn't have the >> system code, we could have continued to match against only the key >> code. > > This was a change in the raw rc-5 IR decoder. There's *always* going > to be a system code (or at least, a resulting byte), isn't there? > Otherwise, its simply not an rc-5 signal. The "no system code" case > should really only apply to hardware decoders that strip it off > internally. I realize that the actual remote controls do have system codes, but our remote control keymaps are missing the info. Wouldn't it have been better if for those cases we only compared against the key code, thereby not resulting in those keymaps being broken? IMHO, it would be better to have had the relatively low risk of the receiver responding to keypresses from an incorrect remote because the keymap wasn't explicit enough than have those remote controls stop working entirely. > Speaking of which, something just occurred to me. Functionality of > Hauppauge receivers and remotes *was* tested. With ir-kbd-i2c. Which > was stripping off the system code at the time. It just wasn't tested > with Hauppauge hardware that actually passed along raw IR. In 2.6.39, > ir-kbd-i2c has been fixed so that it passes along system code and > the Hauppauge keymaps were updated accordingly, which also happens > to fix the cx88 raw IR case. I don't doubt that the Hauppauge remote worked against ir-kbd-i2c because of the deficiency you described in the ir-kbd-i2c driver. I question the notion of introducing the requirement that all keymap definitions must have system codes without having really thought through the notion that it would result in breaking every existing keymap which hadn't been updated. >> Then over time, as keymaps improved, those keymaps could be >> updated and the number of significant bits could be adjusted to >> indicate that the system code was present. >> >> This was a crappy call, and it was completely foreseeable. > > Possibly. I think too many of us are only hacking on this in their > limited free time though, so things like this may get overlooked. > I have quite a few pieces of Hauppauge hardware, several with IR > receivers and remotes, but all of which use ir-kbd-i2c (or > lirc_zilog), i.e., none of which pass along raw IR. You don't have an HVR-950 or some other stick which announces RC5 codes? If not, let me know and I will send you something. It's kind of silly for someone doing that sort of work to not have at least one product in each category of receiver. 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
[cron job] v4l-dvb daily build: ERRORS
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:Fri Apr 8 19:00:39 CEST 2011 git hash:d9954d8547181f9a6a23f835cc1413732700b785 gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: OK linux-git-armv5-davinci: OK linux-git-armv5-ixp: OK linux-git-armv5-omap2: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-x86_64: OK linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: ERRORS linux-2.6.38.2-i686: ERRORS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: ERRORS linux-2.6.38.2-x86_64: ERRORS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.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: [PATCH] Fix cx88 remote control input
On Apr 8, 2011, at 12:50 PM, Lawrence Rust wrote: > On Fri, 2011-04-08 at 12:21 -0400, Jarod Wilson wrote: >> On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: >> >>> On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > This patch restores remote control input for cx2388x based boards on > Linux kernels >= 2.6.38. > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > control input of my Hauppauge Nova-S plus was no longer functioning. > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > some helpful pointers as to the likely cause. > > Turns out that there are 2 problems: ... > 2. The RC5 decoder appends the system code to the scancode and passes > the combination to rc_keydown(). Unfortunately, the combined value is > then forwarded to input_event() which then fails to recognise a valid > scancode and hence no input events are generated. Just to clarify on this one, you're missing a step. We get the scancode, and its passed to rc_keydown. rc_keydown then looks for a match in the loaded keytable, then passes the *keycode* that matches the scancode along to input_event. If you fix the keytable to contain system and command, everything should work just fine again. Throwing away data is a no-no though -- take a look at recent changes to ir-kdb-i2c, which actually just recently made it start *including* system. :) >>> >>> Don't shoot the messenger. >> >> Wasn't my intention. I was simply trying to explain why your "fix" >> isn't correct. >> >> >>> I'm just reporting what I had to do to fix a clumsy hack by someone 6 >>> months ago who didn't test their changes. This patch _restores_ the >>> operation of a subsystem broken by those changes >>> >>> Perhaps those responsible for commit >>> 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: >>> >>> Signed-off-by: David Härdeman >>> Acked-by: Jarod Wilson >>> Signed-off-by: Mauro Carvalho Chehab >>> >>> should fix the keytable. In the meantime (next year) I'll be using this >>> patch. >> >> The entire commit message: >> >>[media] ir-core: convert drivers/media/video/cx88 to ir-core >> >>This patch converts the cx88 driver (for sampling hw) to use the >>decoders provided by ir-core instead of the separate ones provided >>by ir-functions (and gets rid of those). >> >>The value for MO_DDS_IO had a comment saying it corresponded to >>a 4kHz samplerate. That comment was unfortunately misleading. The >>actual samplerate was something like 3250Hz. >> >>The current value has been derived by analyzing the elapsed time >>between interrupts for different values (knowing that each interrupt >>corresponds to 32 samples). >> >>Thanks to Mariusz Bialonczyk for testing my patches >>(about one a day for two weeks!) on actual hardware. >> >> Please note the part about how it *was* tested. And this certainly >> was not a "clumsy hack", I'd actually call it a fairly skilled bit >> of code de-duplication by David. Anyway... >> >> The problem is that there isn't a "the keytable". There are many >> many keytables. And a lot of different hardware. Testing all possible >> combinations of hardware (both receiver side and remote side) is >> next to impossible. > > Hauppauge have the lion's share of the DVB card market so this mod > should have been tested with a Haupauge RC, but clearly wasn't. Its actually not that clear. It clearly wasn't tested with *your* receiver hardware, but a Hauppauge RC *was* tested using other receiver hardware, most notably, ir-kbd-i2c-driven IR hardware in the PVR-250, HVR-1950 and HD-PVR. Umpteen different combinations of bridge drivers, demod drivers, IR chips, etc., make it rather hard to test every possible combination, even when looking solely at products from a single company. :\ > It also > clearly wasn't tested with a cx2388x card because the interrupt handler > has an obvious overflow. I'll give you that one. But look how many different devices are listed in cx88_ir_init(), and you can see how it might be hard to be able to test every single combination. >> We do what we can. Its unfortunate that your >> hardware regressed in functionality. It happens, but it *can* be >> fixed. The fix you provided just wasn't correct. > > It is correct for 2.6.38, which is what was described. I haven't > checked all the keymaps but a sample of 5 show that none include a > system ID. Likely because they're primarily used by non-raw IR drivers and/or ir-kbd-i2c, which until 2.6.39, was stripping out system codes (which made it impossible to use a universal remote programmed for rc-5 signals that aren't using Hauppauge's system codes of choice, despite it being entirely possible for the hardware to handle it). >> The correct fix is >> trivially updating drivers/media/rc/keymaps/. > > So, c
Re: [PATCH] Fix cx88 remote control input
On Apr 8, 2011, at 1:07 PM, Devin Heitmueller wrote: > On Fri, Apr 8, 2011 at 12:21 PM, Jarod Wilson wrote: >> The problem is that there isn't a "the keytable". There are many >> many keytables. And a lot of different hardware. Testing all possible >> combinations of hardware (both receiver side and remote side) is >> next to impossible. We do what we can. Its unfortunate that your >> hardware regressed in functionality. It happens, but it *can* be >> fixed. The fix you provided just wasn't correct. The correct fix is >> trivially updating drivers/media/rc/keymaps/. > > I think the fundamental failure here was avoidable. We introduced a > new requirement that keytables included system codes, knowing full > well that the vast majority of them did not meet the requirement. In > fact, a quick scan through the first 20 or so keymaps show that even > today only *HALF* of them are populated today. That means that half > of the remote keymaps are also completely broken. Have to admit that I don't think it ever registered in my head that we were going to break that many existing keymaps. But something to consider: how many of those are *raw* rc-5 scancode keymaps, vs. cooked scancodes from drivers that only provide command? It may well be that we should have been more discriminating when building those keymaps, to distinguish which were truly raw IR scancodes that the in-kernel decoders ascertained, and which were just scancodes handed to us directly from the IR hardware. > This decision was doomed to fail. It basically said, "Yes, I know > full well that I'm breaking most of the keymaps currently supported, > but maybe some of those users will eventually report the issue and > I'll make them provide an updated keymap which will eventually be > merged upstream for others so that their remotes are no longer > broken." Well, ir-keytable -w is also an option, though that does kill the "Just Works"-ness when you first have to come up with the new map. > We should have introduced a RC profile property indicating how many > bits were "significant". Then for those remotes which didn't have the > system code, we could have continued to match against only the key > code. This was a change in the raw rc-5 IR decoder. There's *always* going to be a system code (or at least, a resulting byte), isn't there? Otherwise, its simply not an rc-5 signal. The "no system code" case should really only apply to hardware decoders that strip it off internally. Speaking of which, something just occurred to me. Functionality of Hauppauge receivers and remotes *was* tested. With ir-kbd-i2c. Which was stripping off the system code at the time. It just wasn't tested with Hauppauge hardware that actually passed along raw IR. In 2.6.39, ir-kbd-i2c has been fixed so that it passes along system code and the Hauppauge keymaps were updated accordingly, which also happens to fix the cx88 raw IR case. > Then over time, as keymaps improved, those keymaps could be > updated and the number of significant bits could be adjusted to > indicate that the system code was present. > > This was a crappy call, and it was completely foreseeable. Possibly. I think too many of us are only hacking on this in their limited free time though, so things like this may get overlooked. I have quite a few pieces of Hauppauge hardware, several with IR receivers and remotes, but all of which use ir-kbd-i2c (or lirc_zilog), i.e., none of which pass along raw IR. -- Jarod Wilson ja...@wilsonet.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: mt9t111 sensor on Beagleboard xM
Hi Javier, On Friday 08 April 2011 17:30:54 javier Martin wrote: > On 8 April 2011 17:07, Laurent Pinchart wrote: > > On Friday 08 April 2011 17:02:48 javier Martin wrote: > >> Hi, > >> I've just received a LI-LBCM3M1 camera module from Leopard Imaging and > >> I want to test it with my Beagleboard xM. This module has a mt9t111 > >> sensor. > >> > >> At first glance, this driver > >> (http://lxr.linux.no/#linux+v2.6.38/drivers/media/video/mt9t112.c) > >> supports mt9t111 sensor and uses both soc-camera and v4l2-subdev > >> frameworks. > >> I am trying to somehow connect this sensor with the omap3isp driver > >> recently merged (I'm working with latest mainline kernel), however, I > >> found an issue when trying to pass "mt9t112_camera_info" data to the > >> sensor driver in my board specific file. > >> > >> It seems that this data is passed through soc-camera but omap3isp > >> doesn't use soc-camera. Do you know what kind of changes are required > >> to adapt this driver so that it can be used with omap3isp? > > > > The OMAP3 ISP driver isn't compatible with the soc-camera framework, as > > you correctly noticed. You will need to port the MT9T111 driver to > > pad-level subdev operations. > > > > You can find a sensor driver (MT9V032) implementing pad-level subdev > > operations at > > http://git.linuxtv.org/pinchartl/media.git?a=commit;h=940b87a5cb7ea3f3cff > > 16454e9085e33ab340064 > > Hi Laurent, > thank you for your quick answer. > > Does the fact of adding pad-level subdev operations for the sensor > break old way of doing things? Adding pad-level operations will not break any existing driver, as long as you keep the existing operations functional. > I mean, if I port MT9T111 driver to pad-level subdev operations would > it be accepted for mainline or would it be rejected since it breaks > something older? The patch will be accepted if you don't break anything :-) We first need to add pad-level operations to subdev drivers. The next step will be to convert bridge drivers to pad-level operations, at which point legacy operations will be removed from subdevs. -- 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] Fix cx88 remote control input
On Fri, Apr 8, 2011 at 12:21 PM, Jarod Wilson wrote: > The problem is that there isn't a "the keytable". There are many > many keytables. And a lot of different hardware. Testing all possible > combinations of hardware (both receiver side and remote side) is > next to impossible. We do what we can. Its unfortunate that your > hardware regressed in functionality. It happens, but it *can* be > fixed. The fix you provided just wasn't correct. The correct fix is > trivially updating drivers/media/rc/keymaps/. I think the fundamental failure here was avoidable. We introduced a new requirement that keytables included system codes, knowing full well that the vast majority of them did not meet the requirement. In fact, a quick scan through the first 20 or so keymaps show that even today only *HALF* of them are populated today. That means that half of the remote keymaps are also completely broken. This decision was doomed to fail. It basically said, "Yes, I know full well that I'm breaking most of the keymaps currently supported, but maybe some of those users will eventually report the issue and I'll make them provide an updated keymap which will eventually be merged upstream for others so that their remotes are no longer broken." We should have introduced a RC profile property indicating how many bits were "significant". Then for those remotes which didn't have the system code, we could have continued to match against only the key code. Then over time, as keymaps improved, those keymaps could be updated and the number of significant bits could be adjusted to indicate that the system code was present. This was a crappy call, and it was completely foreseeable. 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: [PATCH] Fix cx88 remote control input
On Fri, 2011-04-08 at 12:21 -0400, Jarod Wilson wrote: > On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: > > > On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: > >> On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > >> > >>> This patch restores remote control input for cx2388x based boards on > >>> Linux kernels >= 2.6.38. > >>> > >>> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > >>> control input of my Hauppauge Nova-S plus was no longer functioning. > >>> I posted a question on this newsgroup and Mauro Carvalho Chehab gave > >>> some helpful pointers as to the likely cause. > >>> > >>> Turns out that there are 2 problems: > >> ... > >>> 2. The RC5 decoder appends the system code to the scancode and passes > >>> the combination to rc_keydown(). Unfortunately, the combined value is > >>> then forwarded to input_event() which then fails to recognise a valid > >>> scancode and hence no input events are generated. > >> > >> Just to clarify on this one, you're missing a step. We get the scancode, > >> and its passed to rc_keydown. rc_keydown then looks for a match in the > >> loaded keytable, then passes the *keycode* that matches the scancode > >> along to input_event. If you fix the keytable to contain system and > >> command, everything should work just fine again. Throwing away data is > >> a no-no though -- take a look at recent changes to ir-kdb-i2c, which > >> actually just recently made it start *including* system. :) > > > > Don't shoot the messenger. > > Wasn't my intention. I was simply trying to explain why your "fix" > isn't correct. > > > > I'm just reporting what I had to do to fix a clumsy hack by someone 6 > > months ago who didn't test their changes. This patch _restores_ the > > operation of a subsystem broken by those changes > > > > Perhaps those responsible for commit > > 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: > > > >Signed-off-by: David Härdeman > >Acked-by: Jarod Wilson > >Signed-off-by: Mauro Carvalho Chehab > > > > should fix the keytable. In the meantime (next year) I'll be using this > > patch. > > The entire commit message: > > [media] ir-core: convert drivers/media/video/cx88 to ir-core > > This patch converts the cx88 driver (for sampling hw) to use the > decoders provided by ir-core instead of the separate ones provided > by ir-functions (and gets rid of those). > > The value for MO_DDS_IO had a comment saying it corresponded to > a 4kHz samplerate. That comment was unfortunately misleading. The > actual samplerate was something like 3250Hz. > > The current value has been derived by analyzing the elapsed time > between interrupts for different values (knowing that each interrupt > corresponds to 32 samples). > > Thanks to Mariusz Bialonczyk for testing my patches > (about one a day for two weeks!) on actual hardware. > > Please note the part about how it *was* tested. And this certainly > was not a "clumsy hack", I'd actually call it a fairly skilled bit > of code de-duplication by David. Anyway... > > The problem is that there isn't a "the keytable". There are many > many keytables. And a lot of different hardware. Testing all possible > combinations of hardware (both receiver side and remote side) is > next to impossible. Hauppauge have the lion's share of the DVB card market so this mod should have been tested with a Haupauge RC, but clearly wasn't. It also clearly wasn't tested with a cx2388x card because the interrupt handler has an obvious overflow. > We do what we can. Its unfortunate that your > hardware regressed in functionality. It happens, but it *can* be > fixed. The fix you provided just wasn't correct. It is correct for 2.6.38, which is what was described. I haven't checked all the keymaps but a sample of 5 show that none include a system ID. > The correct fix is > trivially updating drivers/media/rc/keymaps/. So, cherrypick the Hauppauge keytable from 2.6.39 and apply it to 2.6.38 together with the 2nd part of this patch. It would be real nice if that was in 2.6.38.3 -- Lawrence -- 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] Fix cx88 remote control input
On Apr 8, 2011, at 11:22 AM, Lawrence Rust wrote: > On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: >> On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: >> >>> This patch restores remote control input for cx2388x based boards on >>> Linux kernels >= 2.6.38. >>> >>> After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote >>> control input of my Hauppauge Nova-S plus was no longer functioning. >>> I posted a question on this newsgroup and Mauro Carvalho Chehab gave >>> some helpful pointers as to the likely cause. >>> >>> Turns out that there are 2 problems: >> ... >>> 2. The RC5 decoder appends the system code to the scancode and passes >>> the combination to rc_keydown(). Unfortunately, the combined value is >>> then forwarded to input_event() which then fails to recognise a valid >>> scancode and hence no input events are generated. >> >> Just to clarify on this one, you're missing a step. We get the scancode, >> and its passed to rc_keydown. rc_keydown then looks for a match in the >> loaded keytable, then passes the *keycode* that matches the scancode >> along to input_event. If you fix the keytable to contain system and >> command, everything should work just fine again. Throwing away data is >> a no-no though -- take a look at recent changes to ir-kdb-i2c, which >> actually just recently made it start *including* system. :) > > Don't shoot the messenger. Wasn't my intention. I was simply trying to explain why your "fix" isn't correct. > I'm just reporting what I had to do to fix a clumsy hack by someone 6 > months ago who didn't test their changes. This patch _restores_ the > operation of a subsystem broken by those changes > > Perhaps those responsible for commit > 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: > >Signed-off-by: David Härdeman >Acked-by: Jarod Wilson >Signed-off-by: Mauro Carvalho Chehab > > should fix the keytable. In the meantime (next year) I'll be using this > patch. The entire commit message: [media] ir-core: convert drivers/media/video/cx88 to ir-core This patch converts the cx88 driver (for sampling hw) to use the decoders provided by ir-core instead of the separate ones provided by ir-functions (and gets rid of those). The value for MO_DDS_IO had a comment saying it corresponded to a 4kHz samplerate. That comment was unfortunately misleading. The actual samplerate was something like 3250Hz. The current value has been derived by analyzing the elapsed time between interrupts for different values (knowing that each interrupt corresponds to 32 samples). Thanks to Mariusz Bialonczyk for testing my patches (about one a day for two weeks!) on actual hardware. Please note the part about how it *was* tested. And this certainly was not a "clumsy hack", I'd actually call it a fairly skilled bit of code de-duplication by David. Anyway... The problem is that there isn't a "the keytable". There are many many keytables. And a lot of different hardware. Testing all possible combinations of hardware (both receiver side and remote side) is next to impossible. We do what we can. Its unfortunate that your hardware regressed in functionality. It happens, but it *can* be fixed. The fix you provided just wasn't correct. The correct fix is trivially updating drivers/media/rc/keymaps/. -- Jarod Wilson ja...@wilsonet.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: [RFCv1 PATCH 6/9] vivi: add support for control events.
On Friday, April 08, 2011 17:19:40 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
> On Monday 04 April 2011 13:51:51 Hans Verkuil wrote:
>
> [snip]
>
> > diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
> > index 21d8f6a..8790e03 100644
> > --- a/drivers/media/video/vivi.c
> > +++ b/drivers/media/video/vivi.c
> > @@ -998,6 +1007,25 @@ static int vivi_s_ctrl(struct v4l2_ctrl *ctrl)
> > File operations for the device
> > --*/
> >
> > +static int vivi_open(struct file *filp)
> > +{
> > + int ret = v4l2_fh_open(filp);
> > + struct v4l2_fh *fh;
> > +
> > + if (ret)
> > + return ret;
> > + fh = filp->private_data;
> > + ret = v4l2_event_init(fh);
> > + if (ret)
> > + goto rel_fh;
> > + ret = v4l2_event_alloc(fh, 10);
> > + if (!ret)
> > + return ret;
> > +rel_fh:
> > + v4l2_fh_release(filp);
> > + return ret;
> > +}
> > +
>
> Should the V4L2 core provide a helper function that does just that ?
Good idea.
>
> > static ssize_t
> > vivi_read(struct file *file, char __user *data, size_t count, loff_t
> > *ppos) {
> > @@ -1012,10 +1040,17 @@ static unsigned int
> > vivi_poll(struct file *file, struct poll_table_struct *wait)
> > {
> > struct vivi_dev *dev = video_drvdata(file);
> > + struct v4l2_fh *fh = file->private_data;
> > struct vb2_queue *q = &dev->vb_vidq;
> > + unsigned int res;
> >
> > dprintk(dev, 1, "%s\n", __func__);
> > - return vb2_poll(q, file, wait);
> > + res = vb2_poll(q, file, wait);
> > + if (v4l2_event_pending(fh))
> > + res |= POLLPRI;
> > + else
> > + poll_wait(file, &fh->events->wait, wait);
>
> Don't you need to call poll_wait unconditionally ?
No, setting POLLPRI will have poll() exit without waiting.
On the other hand, it may be more understandable if I do it unconditionally.
I've been back and forth about this a few times already :-)
I suspect that auditing the way drivers implement poll() would be a great
janitorial project. It's not the greatest API in the world...
Regards,
Hans
>
> > + return res;
> > }
> >
> > static int vivi_close(struct file *file)
>
> [snip]
>
>
--
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] Speed up DVB TS stream delivery from DMA buffer into dvb-core's buffer
Avoid unnecessary DVB TS 188 sized packet copying from DMA buffer into stack.
Backtrack one 188 sized packet just after some garbage bytes when possible.
This obsoletes patch https://patchwork.kernel.org/patch/118147/
Signed-off-by: Marko Ristola marko.rist...@kolumbus.fi
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c
b/drivers/media/dvb/dvb-core/dvb_demux.c
index 4a88a3e..faa3671 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -478,97 +478,94 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux,
const u8 *buf,
EXPORT_SYMBOL(dvb_dmx_swfilter_packets);
-void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count)
+static inline int find_next_packet(const u8 *buf, int pos, size_t count,
+ const int pktsize)
{
- int p = 0, i, j;
+ int start = pos, lost;
- spin_lock(&demux->lock);
-
- if (demux->tsbufp) {
- i = demux->tsbufp;
- j = 188 - i;
- if (count < j) {
- memcpy(&demux->tsbuf[i], buf, count);
- demux->tsbufp += count;
- goto bailout;
- }
- memcpy(&demux->tsbuf[i], buf, j);
- if (demux->tsbuf[0] == 0x47)
- dvb_dmx_swfilter_packet(demux, demux->tsbuf);
- demux->tsbufp = 0;
- p += j;
+ while (pos < count) {
+ if (buf[pos] == 0x47 ||
+ (pktsize == 204 && buf[pos] == 0xB8))
+ break;
+ pos++;
}
- while (p < count) {
- if (buf[p] == 0x47) {
- if (count - p >= 188) {
- dvb_dmx_swfilter_packet(demux, &buf[p]);
- p += 188;
- } else {
- i = count - p;
- memcpy(demux->tsbuf, &buf[p], i);
- demux->tsbufp = i;
- goto bailout;
- }
- } else
- p++;
+ lost = pos - start;
+ if (lost) {
+ /* This garbage is part of a valid packet? */
+ int backtrack = pos - pktsize;
+ if (backtrack >= 0 && (buf[backtrack] == 0x47 ||
+ (pktsize == 204 && buf[backtrack] == 0xB8)))
+ return backtrack;
}
-bailout:
- spin_unlock(&demux->lock);
+ return pos;
}
-EXPORT_SYMBOL(dvb_dmx_swfilter);
-
-void dvb_dmx_swfilter_204(struct dvb_demux *demux, const u8 *buf, size_t count)
+/* Filter all pktsize= 188 or 204 sized packets and skip garbage. */
+static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf,
+ size_t count, const int pktsize)
{
int p = 0, i, j;
- u8 tmppack[188];
+ const u8 *q;
spin_lock(&demux->lock);
- if (demux->tsbufp) {
+ if (demux->tsbufp) { /* tsbuf[0] is now 0x47. */
i = demux->tsbufp;
- j = 204 - i;
+ j = pktsize - i;
if (count < j) {
memcpy(&demux->tsbuf[i], buf, count);
demux->tsbufp += count;
goto bailout;
}
memcpy(&demux->tsbuf[i], buf, j);
- if ((demux->tsbuf[0] == 0x47) || (demux->tsbuf[0] == 0xB8)) {
- memcpy(tmppack, demux->tsbuf, 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- }
+ if (demux->tsbuf[0] == 0x47) /* double check */
+ dvb_dmx_swfilter_packet(demux, demux->tsbuf);
demux->tsbufp = 0;
p += j;
}
- while (p < count) {
- if ((buf[p] == 0x47) || (buf[p] == 0xB8)) {
- if (count - p >= 204) {
- memcpy(tmppack, &buf[p], 188);
- if (tmppack[0] == 0xB8)
- tmppack[0] = 0x47;
- dvb_dmx_swfilter_packet(demux, tmppack);
- p += 204;
- } else {
- i = count - p;
- memcpy(demux->tsbuf, &buf[p], i);
- demux->tsbufp = i;
- goto bailout;
- }
- } else {
- p++;
+ while (1) {
+ p = find_next_packet(buf, p, count, pktsize);
+ if (p >= count)
+ break;
+ if (count - p < pktsize)
+ break;
+
+ q = &buf[p];
+
+
Re: [RFCv1 PATCH 3/9] v4l2-ioctl: add ctrl_handler to v4l2_fh
On Friday, April 08, 2011 17:10:32 Laurent Pinchart wrote: > Hi Hans, > > On Monday 04 April 2011 13:51:48 Hans Verkuil wrote: > > From: Hans Verkuil > > > > This is required to implement control events and is also needed to allow > > for per-filehandle control handlers. > > Thanks for the patch. > > Shouldn't you modify v4l2-subdev.c similarly ? > > Good question. Does it make sense to have per-filehandle controls for a sub-device? On the other hand, does it make sense NOT to have it? I'm inclined to add this functionality if nobody objects. Although a use-case for this would be nice bonus. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mt9t111 sensor on Beagleboard xM
On 8 April 2011 17:07, Laurent Pinchart wrote: > Hi Javier, > > On Friday 08 April 2011 17:02:48 javier Martin wrote: >> Hi, >> I've just received a LI-LBCM3M1 camera module from Leopard Imaging and >> I want to test it with my Beagleboard xM. This module has a mt9t111 >> sensor. >> >> At first glance, this driver >> (http://lxr.linux.no/#linux+v2.6.38/drivers/media/video/mt9t112.c) >> supports mt9t111 sensor and uses both soc-camera and v4l2-subdev >> frameworks. >> I am trying to somehow connect this sensor with the omap3isp driver >> recently merged (I'm working with latest mainline kernel), however, I >> found an issue when trying to pass "mt9t112_camera_info" data to the >> sensor driver in my board specific file. >> >> It seems that this data is passed through soc-camera but omap3isp >> doesn't use soc-camera. Do you know what kind of changes are required >> to adapt this driver so that it can be used with omap3isp? > > The OMAP3 ISP driver isn't compatible with the soc-camera framework, as you > correctly noticed. You will need to port the MT9T111 driver to pad-level > subdev operations. > > You can find a sensor driver (MT9V032) implementing pad-level subdev > operations at > http://git.linuxtv.org/pinchartl/media.git?a=commit;h=940b87a5cb7ea3f3cff16454e9085e33ab340064 > > -- > Regards, > > Laurent Pinchart > Hi Laurent, thank you for your quick answer. Does the fact of adding pad-level subdev operations for the sensor break old way of doing things? I mean, if I port MT9T111 driver to pad-level subdev operations would it be accepted for mainline or would it be rejected since it breaks something older? Thank you, -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix cx88 remote control input
On Fri, 2011-04-08 at 10:41 -0400, Jarod Wilson wrote: > On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > > > This patch restores remote control input for cx2388x based boards on > > Linux kernels >= 2.6.38. > > > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > > control input of my Hauppauge Nova-S plus was no longer functioning. > > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > > some helpful pointers as to the likely cause. > > > > Turns out that there are 2 problems: > ... > > 2. The RC5 decoder appends the system code to the scancode and passes > > the combination to rc_keydown(). Unfortunately, the combined value is > > then forwarded to input_event() which then fails to recognise a valid > > scancode and hence no input events are generated. > > Just to clarify on this one, you're missing a step. We get the scancode, > and its passed to rc_keydown. rc_keydown then looks for a match in the > loaded keytable, then passes the *keycode* that matches the scancode > along to input_event. If you fix the keytable to contain system and > command, everything should work just fine again. Throwing away data is > a no-no though -- take a look at recent changes to ir-kdb-i2c, which > actually just recently made it start *including* system. :) Don't shoot the messenger. I'm just reporting what I had to do to fix a clumsy hack by someone 6 months ago who didn't test their changes. This patch _restores_ the operation of a subsystem broken by those changes Perhaps those responsible for commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d: Signed-off-by: David Härdeman Acked-by: Jarod Wilson Signed-off-by: Mauro Carvalho Chehab should fix the keytable. In the meantime (next year) I'll be using this patch. -- Lawrence -- 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: [RFCv1 PATCH 6/9] vivi: add support for control events.
Hi Hans,
Thanks for the patch.
On Monday 04 April 2011 13:51:51 Hans Verkuil wrote:
[snip]
> diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
> index 21d8f6a..8790e03 100644
> --- a/drivers/media/video/vivi.c
> +++ b/drivers/media/video/vivi.c
> @@ -998,6 +1007,25 @@ static int vivi_s_ctrl(struct v4l2_ctrl *ctrl)
> File operations for the device
> --*/
>
> +static int vivi_open(struct file *filp)
> +{
> + int ret = v4l2_fh_open(filp);
> + struct v4l2_fh *fh;
> +
> + if (ret)
> + return ret;
> + fh = filp->private_data;
> + ret = v4l2_event_init(fh);
> + if (ret)
> + goto rel_fh;
> + ret = v4l2_event_alloc(fh, 10);
> + if (!ret)
> + return ret;
> +rel_fh:
> + v4l2_fh_release(filp);
> + return ret;
> +}
> +
Should the V4L2 core provide a helper function that does just that ?
> static ssize_t
> vivi_read(struct file *file, char __user *data, size_t count, loff_t
> *ppos) {
> @@ -1012,10 +1040,17 @@ static unsigned int
> vivi_poll(struct file *file, struct poll_table_struct *wait)
> {
> struct vivi_dev *dev = video_drvdata(file);
> + struct v4l2_fh *fh = file->private_data;
> struct vb2_queue *q = &dev->vb_vidq;
> + unsigned int res;
>
> dprintk(dev, 1, "%s\n", __func__);
> - return vb2_poll(q, file, wait);
> + res = vb2_poll(q, file, wait);
> + if (v4l2_event_pending(fh))
> + res |= POLLPRI;
> + else
> + poll_wait(file, &fh->events->wait, wait);
Don't you need to call poll_wait unconditionally ?
> + return res;
> }
>
> static int vivi_close(struct file *file)
[snip]
--
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: [RFCv1 PATCH 8/9] vivi: add support for CTRL_CH_STATE events.
Hi Hans,
Thanks for the patch.
On Monday 04 April 2011 13:51:53 Hans Verkuil wrote:
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/v4l2-event.c |6 --
> drivers/media/video/vivi.c |8 ++--
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-event.c
> b/drivers/media/video/v4l2-event.c index c9251a5..9b503aa 100644
> --- a/drivers/media/video/v4l2-event.c
> +++ b/drivers/media/video/v4l2-event.c
> @@ -258,7 +258,8 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
> return -ENOMEM;
> }
>
> - if (sub->type == V4L2_EVENT_CTRL_CH_VALUE) {
> + if (sub->type == V4L2_EVENT_CTRL_CH_VALUE ||
> + sub->type == V4L2_EVENT_CTRL_CH_STATE) {
Indentation looks weird here.
> ctrl = v4l2_ctrl_find(fh->ctrl_handler, sub->id);
> if (ctrl == NULL)
> return -EINVAL;
> @@ -341,7 +342,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
> list_del(&sev->list);
>
> spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> - if (sev->type == V4L2_EVENT_CTRL_CH_VALUE) {
> + if (sev->type == V4L2_EVENT_CTRL_CH_VALUE ||
> + sev->type == V4L2_EVENT_CTRL_CH_STATE) {
And here.
> struct v4l2_ctrl *ctrl = v4l2_ctrl_find(fh->ctrl_handler,
> sev->id);
>
> if (ctrl)
--
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: [RFCv1 PATCH 3/9] v4l2-ioctl: add ctrl_handler to v4l2_fh
Hi Hans, On Monday 04 April 2011 13:51:48 Hans Verkuil wrote: > From: Hans Verkuil > > This is required to implement control events and is also needed to allow > for per-filehandle control handlers. Thanks for the patch. Shouldn't you modify v4l2-subdev.c similarly ? -- 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: [RFCv1 PATCH 1/9] v4l2-ctrls: add new bitmask control type.
Hi Hans,
Thanks for the patch.
On Monday 04 April 2011 13:51:46 Hans Verkuil wrote:
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/v4l2-common.c |3 +++
> drivers/media/video/v4l2-ctrls.c | 17 -
> include/linux/videodev2.h |1 +
> 3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-common.c
> b/drivers/media/video/v4l2-common.c index 06b9f9f..5c6100f 100644
> --- a/drivers/media/video/v4l2-common.c
> +++ b/drivers/media/video/v4l2-common.c
> @@ -105,6 +105,9 @@ int v4l2_ctrl_check(struct v4l2_ext_control *ctrl,
> struct v4l2_queryctrl *qctrl, menu_items[ctrl->value][0] == '\0')
> return -EINVAL;
> }
> + if (qctrl->type == V4L2_CTRL_TYPE_BITMASK &&
> + (ctrl->value & ~qctrl->maximum))
> + return -ERANGE;
> return 0;
> }
> EXPORT_SYMBOL(v4l2_ctrl_check);
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index 2412f08..f75a1d4 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -726,6 +726,10 @@ static int validate_new(struct v4l2_ctrl *ctrl)
> return -EINVAL;
> return 0;
>
> + case V4L2_CTRL_TYPE_BITMASK:
> + ctrl->val &= ctrl->maximum;
> + return 0;
> +
> case V4L2_CTRL_TYPE_BUTTON:
> case V4L2_CTRL_TYPE_CTRL_CLASS:
> ctrl->val64 = 0;
> @@ -962,13 +966,17 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl,
>
> /* Sanity checks */
> if (id == 0 || name == NULL || id >= V4L2_CID_PRIVATE_BASE ||
> - max < min ||
> (type == V4L2_CTRL_TYPE_INTEGER && step == 0) ||
> + (type == V4L2_CTRL_TYPE_BITMASK && max == 0) ||
> (type == V4L2_CTRL_TYPE_MENU && qmenu == NULL) ||
> (type == V4L2_CTRL_TYPE_STRING && max == 0)) {
> handler_set_err(hdl, -ERANGE);
> return NULL;
> }
> + if (type != V4L2_CTRL_TYPE_BITMASK && max < min) {
> + handler_set_err(hdl, -ERANGE);
> + return NULL;
> + }
> if ((type == V4L2_CTRL_TYPE_INTEGER ||
>type == V4L2_CTRL_TYPE_MENU ||
>type == V4L2_CTRL_TYPE_BOOLEAN) &&
> @@ -976,6 +984,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl, handler_set_err(hdl, -ERANGE);
> return NULL;
> }
> + if (type == V4L2_CTRL_TYPE_BITMASK && ((def & ~max) || min || step)) {
> + handler_set_err(hdl, -ERANGE);
> + return NULL;
> + }
>
> if (type == V4L2_CTRL_TYPE_BUTTON)
> flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> @@ -1217,6 +1229,9 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl,
> case V4L2_CTRL_TYPE_MENU:
> printk(KERN_CONT "%s", ctrl->qmenu[ctrl->cur.val]);
> break;
> + case V4L2_CTRL_TYPE_BITMASK:
> + printk(KERN_CONT "0x%x", ctrl->cur.val);
> + break;
What about 0x%08x instead ?
> case V4L2_CTRL_TYPE_INTEGER64:
> printk(KERN_CONT "%lld", ctrl->cur.val64);
> break;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index aa6c393..92d2fdd 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1034,6 +1034,7 @@ enum v4l2_ctrl_type {
> V4L2_CTRL_TYPE_INTEGER64 = 5,
> V4L2_CTRL_TYPE_CTRL_CLASS= 6,
> V4L2_CTRL_TYPE_STRING= 7,
> + V4L2_CTRL_TYPE_BITMASK = 8,
> };
>
> /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
--
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: mt9t111 sensor on Beagleboard xM
Hi Javier, On Friday 08 April 2011 17:02:48 javier Martin wrote: > Hi, > I've just received a LI-LBCM3M1 camera module from Leopard Imaging and > I want to test it with my Beagleboard xM. This module has a mt9t111 > sensor. > > At first glance, this driver > (http://lxr.linux.no/#linux+v2.6.38/drivers/media/video/mt9t112.c) > supports mt9t111 sensor and uses both soc-camera and v4l2-subdev > frameworks. > I am trying to somehow connect this sensor with the omap3isp driver > recently merged (I'm working with latest mainline kernel), however, I > found an issue when trying to pass "mt9t112_camera_info" data to the > sensor driver in my board specific file. > > It seems that this data is passed through soc-camera but omap3isp > doesn't use soc-camera. Do you know what kind of changes are required > to adapt this driver so that it can be used with omap3isp? The OMAP3 ISP driver isn't compatible with the soc-camera framework, as you correctly noticed. You will need to port the MT9T111 driver to pad-level subdev operations. You can find a sensor driver (MT9V032) implementing pad-level subdev operations at http://git.linuxtv.org/pinchartl/media.git?a=commit;h=940b87a5cb7ea3f3cff16454e9085e33ab340064 -- 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
mt9t111 sensor on Beagleboard xM
Hi, I've just received a LI-LBCM3M1 camera module from Leopard Imaging and I want to test it with my Beagleboard xM. This module has a mt9t111 sensor. At first glance, this driver (http://lxr.linux.no/#linux+v2.6.38/drivers/media/video/mt9t112.c) supports mt9t111 sensor and uses both soc-camera and v4l2-subdev frameworks. I am trying to somehow connect this sensor with the omap3isp driver recently merged (I'm working with latest mainline kernel), however, I found an issue when trying to pass "mt9t112_camera_info" data to the sensor driver in my board specific file. It seems that this data is passed through soc-camera but omap3isp doesn't use soc-camera. Do you know what kind of changes are required to adapt this driver so that it can be used with omap3isp? Thank you. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix cx88 remote control input
On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > This patch restores remote control input for cx2388x based boards on > Linux kernels >= 2.6.38. > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > control input of my Hauppauge Nova-S plus was no longer functioning. > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > some helpful pointers as to the likely cause. > > Turns out that there are 2 problems: ... > 2. The RC5 decoder appends the system code to the scancode and passes > the combination to rc_keydown(). Unfortunately, the combined value is > then forwarded to input_event() which then fails to recognise a valid > scancode and hence no input events are generated. Just to clarify on this one, you're missing a step. We get the scancode, and its passed to rc_keydown. rc_keydown then looks for a match in the loaded keytable, then passes the *keycode* that matches the scancode along to input_event. If you fix the keytable to contain system and command, everything should work just fine again. Throwing away data is a no-no though -- take a look at recent changes to ir-kdb-i2c, which actually just recently made it start *including* system. :) -- Jarod Wilson ja...@wilsonet.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: [RFC/PATCH 1/2] v4l: videobuf2: Handle buf_queue errors
Hi Marek, On Friday 08 April 2011 15:09:02 Marek Szyprowski wrote: > On Friday, April 08, 2011 2:53 PM Laurent Pinchart wrote: [snip] > > buf_queue is called with a spinlock help, so you can't perform I2C > > communication there. > > In videobuf2 buf_queue() IS NOT called with any spinlock held. buf_queue > can call functions that require sleeping. This makes a lot of sense > especially for drivers that need to perform a lot of operations for > enabling/disabling hardware. Oops, my bad. > I remember we discussed your solution where you wanted to add a spinlock > for calling buf_queue. This case shows one more reason not go that way. :) Hehe. I totally agree with you that we should avoid locking wherever possible. We still have no solution for the disconnection problem though. > AFAIR buf_queue callback in old videobuf was called with spinlock held. That's correct, yes. > I agree that we definitely need more documentation for vb2 and clarification > what is allowed in each callback... -- 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] Fix cx88 remote control input
On Apr 8, 2011, at 8:50 AM, Lawrence Rust wrote: > This patch restores remote control input for cx2388x based boards on > Linux kernels >= 2.6.38. > > After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote > control input of my Hauppauge Nova-S plus was no longer functioning. > I posted a question on this newsgroup and Mauro Carvalho Chehab gave > some helpful pointers as to the likely cause. > > Turns out that there are 2 problems: > > 1. In the IR interrupt handler of cx88-input.c there's a 32-bit multiply > overflow which causes IR pulse durations to be incorrectly calculated. > > 2. The RC5 decoder appends the system code to the scancode and passes > the combination to rc_keydown(). Unfortunately, the combined value is > then forwarded to input_event() which then fails to recognise a valid > scancode and hence no input events are generated. > > I note that in commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d, which > introduced these changes, David Härdeman changed the IR sample frequency > to a supposed 4kHz. However, the registers dealing with IR input are > undocumented in the cx2388x datasheets and there's no publicly available > information on them. I have to ask the question why this change was > made as it is of no apparent benefit and could have unanticipated > consequences. IMHO that change should also be reverted unless there is > evidence to substantiate it. > > Signed off by: Lawrence Rust Nacked-by: Jarod Wilson > diff --git a/drivers/media/rc/ir-rc5-decoder.c > b/drivers/media/rc/ir-rc5-decoder.c > index ebdba55..c4052da 100644 > --- a/drivers/media/rc/ir-rc5-decoder.c > +++ b/drivers/media/rc/ir-rc5-decoder.c > @@ -144,10 +144,15 @@ again: > system = (data->bits & 0x007C0) >> 6; > toggle = (data->bits & 0x00800) ? 1 : 0; > command += (data->bits & 0x01000) ? 0 : 0x40; > - scancode = system << 8 | command; > - > - IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n", > -scancode, toggle); > +/* Notes > + * 1. Should filter unknown systems e.g Hauppauge use 0x1e or > 0x1f > + * 2. Don't include system in the scancode otherwise > input_event() > + *doesn't recognise the scancode > + */ > + scancode = command; > + > + IR_dprintk(1, "RC5 scancode 0x%02x (system: 0x%02x > toggle: %u)\n", > +scancode, system, toggle); > } > > rc_keydown(dev, scancode, toggle); This part is so very very wrong. We should NOT filter here. Filtering can be achieved on the keymap side, and you *do* include the system here. The fix for your issue is an update to the relevant keymap so that its matching the system byte as well. The divide fix looks sane though. -- Jarod Wilson ja...@wilsonet.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: v4l: Buffer pools
Hi Guennadi, On Thursday 07 April 2011 13:51:12 Guennadi Liakhovetski wrote: > On Fri, 1 Apr 2011, Laurent Pinchart wrote: > > [snip] > > > - Cache management (ISP and DSS) > > > > Cache needs to be synchronized between userspace applications, kernel > > space and hardware. Synchronizing the cache is an expensive operation > > and should be avoided when possible. Userspace applications don't need > > to select memory mapping cache attributes, but should be able to either > > handle cache synchronization explicitly, or override the drivers' > > default behaviour. > > So, what cache attributes are currently used by the driver? Presumably, it > is some cacheable variant? When using MMAP, memory is allocated by the driver from system memory and is mapped as normal cacheable memory. When using USERPTR, mapping cache attributes are not touched by the driver. > And which way should the application be able to override the driver's > behaviour? One of these overrides would probably be "skip cache invalidate > (input) / flush (output)," right? Anything else? Those are the operations I was thinking about. -- 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: Antwort: Re: [PATCH 1/2] mt9v022: fix pixel clock
Hello Guennadi, Am Donnerstag, den 07.04.2011, 14:41 +0200 schrieb Guennadi Liakhovetski: > Hello Teresa > > On Thu, 7 Apr 2011, Teresa Gamez wrote: > > > Hello Guennadi, > > > > the datasheet also says (see table 3): > > > > > > Pixel clock out. DOUT is valid on rising edge of this > > clock. > > > > > > There is a difference between DOUT beeing vaild and DOUT beeing set up. > > So does SOCAM_PCLK_SAMPLE_RISING mean that the data is valid at rising > > edge or > > does it mean the data is set up at rising edge? > > Hm, yeah, looks like a typical example of a copy-paste datasheet to me:-( > And now we don't know which of the two is actually supposed to be true. As > for "set up" vs. "valid" - not sure, whether there is indeed a difference > between them. To me "set up _TO_ the rising edge" is a short way to set > "set up to be valid at the rising edge," however, I might be wrong. Can > you tell me in more detail what and where (at the sensor board or on the > baseboard) you measured and what it looked like? I think, Figure 7 and the > description below it are interesting. From that diagram I would indeed say > indeed the DOUT pins are valid and should be sampled at the rising edge by > default - when bit 4 in 0x74 is not set. SOCAM_PCLK_SAMPLE_RISING means, > that the data should be sampled at the rising of pclkm, i.e., it is valid > there. I meassured the outgoing pins from the baseboard to the camera board and checked the PCLK and D0 to see at which point the data is valid. I have also checked the quality of the image. All tests where made with sensor_type=color My results for pcm038 are with following register settings: mx2_camera 0x0 CSICR1: 0x10020b92 -> rising edge mt9v022 0x74 PIXCLK_FV_LV: 0x0010 -> rising edge (which I think is falling edge) meassured: falling edge (ugly image, wrong colors) Now I set the SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022: mx2_camera 0x0 CSICR10x10020b92 -> rising edge mt9v022 0x74 PIXCLK_FV_LV 0x -> falling edge (which I think is rising edge) meassured: rising edge (image is OK) Now changed the PCLK of the mx2_camera: mx2_camera 0x0 CSICR1 0x10020b90 -> falling edge mt9v022 0x74 PIXCLK_FV_LV0x0010 -> rising edge (which I think is falling edge) meassured: falling edge (image is OK) > > So, yes, if your measurements agree with figure 7 from the datasheet, we > shall assume, that the driver implements the pclk polarity wrongly. But > the fix should be more extensive, than what you've submitted: if we invert > driver's behaviour, we should also invert board configuration of all > driver users: pcm990 and pcm037. Or we have to test them and verify, that > the inverted pclk polarity doesn't megatively affect the image quality, or > maybe even improves it. > > Thanks > Guennadi > > > I have tested this with a pcm038 but I will also make meassurements with > > the pcm037. > > Same results with the pcm037: mx3_camera 0x60 CSI_SENS_CONF: 0x0700 -> rising edge mt9v022 0x74 PIXCLK_FV_LV: 0x0010 -> rising edge (which I think is falling edge) meassured: falling edge (ulgy image, looks like b/w with pixel errors) Set SOCAM_SENSOR_INVERT_PCLK flag in the platformcode for the mt9v022: mx3_camera 0x60 CSI_SENS_CONF: 0x0700 -> rising edge mt9v022 0x74 PIXCLK_FV_LV 0x -> falling edge (which I think is rising edge) meassured: rising edge (image is OK) Additionally set MX3_CAMERA_PCP of the mx3_camera flags mx3_camera 0x60 CSI_SENS_CONF: 0x0708 -> falling edge mt9v022 0x74 PIXCLK_FV_LV: 0x0010 -> rising edge (which I think is falling edge) meassured: falling edge (image is OK) Removed SOCAM_SENSOR_INVERT_PCLK flag for the mt9v022: mx3_camera 0x60 CSI_SENS_CONF: 0x0708 -> falling edge mt9v022 0x74 PIXCLK_FV_LV 0x -> falling edge (which I think is rising edge) meassured: risging edge (ugly image, looks like the first one) I have noticed that on our pcm037 BSP the SOCAM_SENSOR_INVERT_PCLK flag for the camera was set to "fix" this issue. I will continue this test on the pcm990. Teresa -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH 1/2] v4l: videobuf2: Handle buf_queue errors
Hello, On Friday, April 08, 2011 2:53 PM Laurent Pinchart wrote: > > ... > > > > > As buf_queue callback is called by vb2 only after start_streaming, > > > for a camera snapshot capture I needed to start a pipeline only from > the > > > buf_queue handler level, i.e. subdev's video s_stream op was called > from > > > within buf_queue. s_stream couldn't be done in the start_streaming > > > handler as at the time it is invoked there is always no buffer > available > > > in the bridge H/W. > > > It's a consequence of how the vb2_streamon() is designed. > > > > > > Before, I used to simply call s_stream in start_streaming, only > deferring > > > the actual bridge DMA enable till a buf_queue call, thus letting first > > > frames in the stream to be lost. This of course cannot be done in case > > > of single-frame capture. > > > > > > To make a long story short, it would be useful in my case to have the > > > ability to return error codes as per VIDIOC_STREAMON through buf_queue > > > in the driver (when the first buffer is queued). > > > At the moment mainly EPIPE comes to my mind. This error code has no > > > meaning in the API for QBUF though. Should the pipeline be started from > > > buf_queue > > > > Hmm, the pipeline validation could still be done in start_streaming() > > so we can return any EPIPE error from there directly and effectively > > in VIDIOC_STREAMON. > > That's correct, and that's what the OMAP3 ISP driver does. > > > So the only remaining errors are those related to I2C communication etc. > > when streaming is actually enabled in the subdev. > > buf_queue is called with a spinlock help, so you can't perform I2C > communication there. In videobuf2 buf_queue() IS NOT called with any spinlock held. buf_queue can call functions that require sleeping. This makes a lot of sense especially for drivers that need to perform a lot of operations for enabling/disabling hardware. I remember we discussed your solution where you wanted to add a spinlock for calling buf_queue. This case shows one more reason not go that way. :) AFAIR buf_queue callback in old videobuf was called with spinlock held. I agree that we definitely need more documentation for vb2 and clarification what is allowed in each callback... Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] v4l: videobuf2: Handle buf_queue errors
Hi Sylwester, On Thursday 07 April 2011 00:04:45 Sylwester Nawrocki wrote: > On 04/06/2011 10:05 PM, Sylwester Nawrocki wrote: > > On 03/07/2011 12:20 AM, Pawel Osciak wrote: > >> On Tue, Mar 1, 2011 at 02:54, Laurent Pinchart wrote: > >>> On Monday 28 February 2011 16:44:38 Pawel Osciak wrote: > Hi Laurent, > A few questions from me below. I feel we need to talk about this > change a bit more, it introduces some recovery and consistency > problems, unless I'm missing something. > > On Sun, Feb 27, 2011 at 10:12, Laurent Pinchart wrote: > > videobuf2 expects drivers to check buffer in the buf_prepare > > operation and to return success only if the buffer can queued > > without any issue. > > > > For hot-pluggable devices, disconnection events need to be handled at > > buf_queue time. Checking the disconnected flag and adding the buffer > > to the driver queue need to be performed without releasing the > > driver spinlock, otherwise race conditions can occur in which the > > driver could still allow a buffer to be queued after the > > disconnected flag has been set, resulting in a hang during the next > > DQBUF operation. > > > > This problem can be solved either in the videobuf2 core or in the > > device drivers. To avoid adding a spinlock to videobuf2, make > > buf_queue return an int and handle buf_queue failures in videobuf2. > > Drivers must not return an error in buf_queue if the condition > > leading to the error can be caught in buf_prepare. > > ... > > > As buf_queue callback is called by vb2 only after start_streaming, > > for a camera snapshot capture I needed to start a pipeline only from the > > buf_queue handler level, i.e. subdev's video s_stream op was called from > > within buf_queue. s_stream couldn't be done in the start_streaming > > handler as at the time it is invoked there is always no buffer available > > in the bridge H/W. > > It's a consequence of how the vb2_streamon() is designed. > > > > Before, I used to simply call s_stream in start_streaming, only deferring > > the actual bridge DMA enable till a buf_queue call, thus letting first > > frames in the stream to be lost. This of course cannot be done in case > > of single-frame capture. > > > > To make a long story short, it would be useful in my case to have the > > ability to return error codes as per VIDIOC_STREAMON through buf_queue > > in the driver (when the first buffer is queued). > > At the moment mainly EPIPE comes to my mind. This error code has no > > meaning in the API for QBUF though. Should the pipeline be started from > > buf_queue > > Hmm, the pipeline validation could still be done in start_streaming() > so we can return any EPIPE error from there directly and effectively > in VIDIOC_STREAMON. That's correct, and that's what the OMAP3 ISP driver does. > So the only remaining errors are those related to I2C communication etc. > when streaming is actually enabled in the subdev. buf_queue is called with a spinlock help, so you can't perform I2C communication there. > > the errors from buf_queue would be seen in userspace via VIDIOC_STREAMON > > and/or VIDIOC_QBUF. > > > > It should be also possible to signal any errors originating from the > > subdev when s_stream is called on it, perhaps by EIO ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] V4L: Extended crop/compose API, ver2
Hi Tomasz!
Some comments below...
On Wednesday, April 06, 2011 10:44:17 Tomasz Stanislawski wrote:
> Hello everyone,
>
> This patch-set introduces new ioctls to V4L2 API. The new method for
> configuration of cropping and composition is presented.
>
> This is the second version of extcrop RFC. It was enriched with new features
> like additional hint flags, and a support for auxiliary crop/compose
> rectangles.
>
> There is some confusion in understanding of a cropping in current version of
> V4L2. For CAPTURE devices cropping refers to choosing only a part of input
> data stream and processing it and storing it in a memory buffer. The buffer is
> fully filled by data. It is not possible to choose only a part of a buffer for
> being updated by hardware.
>
> In case of OUTPUT devices, the whole content of a buffer is passed by hardware
> to output display. Cropping means selecting only a part of an output
> display/signal. It is not possible to choose only a part for a memory buffer
> to be processed.
>
> The overmentioned flaws in cropping API were discussed in post:
> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/28945
>
> A solution was proposed during brainstorming session in Warsaw.
>
>
> 1. Data structures.
>
> The structure v4l2_crop used by current API lacks any place for further
> extensions. Therefore new structure is proposed.
>
> struct v4l2_selection {
> u32 type;
> u32 target;
> struct v4l2_rect r;
> u32 flags;
> u32 reserved[9];
> };
>
> Where,
> type - type of buffer queue: V4L2_BUF_TYPE_VIDEO_CAPTURE,
>V4L2_BUF_TYPE_VIDEO_OUTPUT, etc.
> target - choose one of cropping/composing rectangles
> r - selection rectangle
> flags - control over coordinates adjustments
> reserved - place for further extensions, adjust struct size to 64 bytes
>
> At first, the distinction between cropping and composing was stated. The
> cropping operation means choosing only part of input data bounding it by a
> cropping rectangle. All other data must be discarded. On the other hand,
> composing means pasting processed data into rectangular part of data sink. The
> sink may be output device, user buffer, etc.
>
>
> 2. Crop/Compose ioctl.
> Four new ioctls would be added to V4L2.
>
> Name
> VIDIOC_S_EXTCROP - set cropping rectangle on an input of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_S_EXTCROP, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to configure:
> - for input devices, a part of input data that is processed in hardware
> - for output devices, a part of a data buffer to be passed to hardware
> Drivers may adjust a cropping area. The adjustment can be controlled
> by v4l2_selection::flags. Please refer to Hints section.
> - an adjusted crop rectangle is returned in v4l2_selection::r
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> ERANGE - failed to find a rectangle that satisfy all constraints
> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>
> -
>
> Name
> VIDIOC_G_EXTCROP - get cropping rectangle on an input of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_G_EXTCROP, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to query:
> - for input devices, a part of input data that is processed in hardware
> Other crop rectangles might be examined using this ioctl.
> Please refer to Targets section.
> - for output devices, a part of data buffer to be passed to hardware
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> EINVAL - incorrect buffer type, incorrect target, cropping not supported
>
> -
>
> Name
> VIDIOC_S_COMPOSE - set destination rectangle on an output of a device
>
> Synopsis
> int ioctl(fd, VIDIOC_S_COMPOSE, struct v4l2_selection *s)
>
> Description:
> The ioctl is used to configure:
> - for input devices, a part of a data buffer that is filled by hardware
> - for output devices, a area on output device where image is inserted
> Drivers may adjust a composing area. The adjustment can be controlled
> by v4l2_selection::flags. Please refer to Hints section.
> - an adjusted composing rectangle is returned in v4l2_selection::r
>
> Return value
> On success 0 is returned, on error -1 and the errno variable is set
> appropriately:
> ERANGE - failed to find a rectangle that satisfy all constraints
> EINVAL - incorrect buffer type, incorrect target, composing not
> supported
>
> -
>
> Name
> VIDIOC_G_COMPOSE - get destin
[PATCH] Fix cx88 remote control input
This patch restores remote control input for cx2388x based boards on
Linux kernels >= 2.6.38.
After upgrading from Linux 2.6.37 to 2.6.38 I found that the remote
control input of my Hauppauge Nova-S plus was no longer functioning.
I posted a question on this newsgroup and Mauro Carvalho Chehab gave
some helpful pointers as to the likely cause.
Turns out that there are 2 problems:
1. In the IR interrupt handler of cx88-input.c there's a 32-bit multiply
overflow which causes IR pulse durations to be incorrectly calculated.
2. The RC5 decoder appends the system code to the scancode and passes
the combination to rc_keydown(). Unfortunately, the combined value is
then forwarded to input_event() which then fails to recognise a valid
scancode and hence no input events are generated.
I note that in commit 2997137be8eba5bf9c07a24d5fda1f4225f9ca7d, which
introduced these changes, David Härdeman changed the IR sample frequency
to a supposed 4kHz. However, the registers dealing with IR input are
undocumented in the cx2388x datasheets and there's no publicly available
information on them. I have to ask the question why this change was
made as it is of no apparent benefit and could have unanticipated
consequences. IMHO that change should also be reverted unless there is
evidence to substantiate it.
Signed off by: Lawrence Rust
diff --git a/drivers/media/rc/ir-rc5-decoder.c
b/drivers/media/rc/ir-rc5-decoder.c
index ebdba55..c4052da 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -144,10 +144,15 @@ again:
system = (data->bits & 0x007C0) >> 6;
toggle = (data->bits & 0x00800) ? 1 : 0;
command += (data->bits & 0x01000) ? 0 : 0x40;
- scancode = system << 8 | command;
-
- IR_dprintk(1, "RC5 scancode 0x%04x (toggle: %u)\n",
- scancode, toggle);
+/* Notes
+ * 1. Should filter unknown systems e.g Hauppauge use 0x1e or 0x1f
+ * 2. Don't include system in the scancode otherwise input_event()
+ *doesn't recognise the scancode
+ */
+ scancode = command;
+
+ IR_dprintk(1, "RC5 scancode 0x%02x (system: 0x%02x
toggle: %u)\n",
+ scancode, system, toggle);
}
rc_keydown(dev, scancode, toggle);
diff --git a/drivers/media/video/cx88/cx88-input.c
b/drivers/media/video/cx88/cx88-input.c
index c820e2f..7281db4 100644
--- a/drivers/media/video/cx88/cx88-input.c
+++ b/drivers/media/video/cx88/cx88-input.c
@@ -524,7 +524,7 @@ void cx88_ir_irq(struct cx88_core *core)
for (todo = 32; todo > 0; todo -= bits) {
ev.pulse = samples & 0x8000 ? false : true;
bits = min(todo, 32U - fls(ev.pulse ? samples : ~samples));
- ev.duration = (bits * NSEC_PER_SEC) / (1000 * ir_samplerate);
+ ev.duration = bits * (NSEC_PER_SEC / (1000 * ir_samplerate));
/* NB avoid 32-bit overflow */
ir_raw_event_store_with_filter(ir->dev, &ev);
samples <<= bits;
}
--
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: vb2: stop_streaming() callback redesign
Hi Pawel, On Tuesday 05 April 2011 17:12:29 Pawel Osciak wrote: > On Mon, Apr 4, 2011 at 03:27, Laurent Pinchart wrote: > > On Monday 04 April 2011 01:51:05 Pawel Osciak wrote: > >> Hi, > >> > >> This series implements a slight redesign of the stop_streaming() > >> callback in vb2. The callback has been made obligatory. The drivers are > >> expected to finish all hardware operations and cede ownership of all > >> buffers before returning, but are not required to call > >> vb2_buffer_done() for any of them. The return value from this callback > >> has also been removed. > > > > What's the rationale behind this patch set ? I've always been against vb2 > > controlling the stream state (vb2 should handle buffer management only in > > my opinion) and I'd like to understand why you want to make it required. > > I might have overstated the intention saying it was a 'redesign'. It > actually doesn't change the overall stop_streaming callback idea, I am > just simplifying it with this patch, while also emphasizing its role > by making it obligatory. Drivers were always required to finish > everything they were doing with the buffers before returning from > stop_streaming. But until now, stop_streaming was expecting the driver > to call vb2_buffer_done for all buffers it received via buf_queue. > We've decided it's superfluous, so I am removing this requirement. > Also, I didn't see any use for the return value from stop_streaming so > I removed it as well. Apart from the above, nothing has really > changed. Does that mean that drivers don't have to implement the stop_streaming callback if they finish all buffer-related operations before calling vb2 functions that stop streaming ? > > I plan to use vb2 in the uvcvideo driver (when vb2 will provide a way to > > handle device disconnection), and uvcvideo will stop the stream before > > calling vb2_queue_release() and vb2_streamoff(). Would will I need a > > stop_stream operation ? > > I actually just yesterday noticed your response from a couple of weeks ago > to my comments to your original buf_queue proposal in my ever growing pile > of mail, sorry about that, I will reply to that as soon as I have time to > properly read it and think about it. Nevertheless, I have the same question > as Marek here, would there be anything preventing you from doing that in > stop_streaming? See my reply to Marek :-) -- 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: vb2: stop_streaming() callback redesign
Hi Marek,
On Tuesday 05 April 2011 16:27:54 Marek Szyprowski wrote:
> On Monday, April 04, 2011 12:27 PM Laurent Pinchart wrote:
> > On Monday 04 April 2011 01:51:05 Pawel Osciak wrote:
> > >
> > > This series implements a slight redesign of the stop_streaming()
> > > callback in vb2. The callback has been made obligatory. The drivers
> > > are expected to finish all hardware operations and cede ownership of
> > > all buffers before returning, but are not required to call
> > > vb2_buffer_done() for any of them. The return value from this callback
> > > has also been removed.
> >
> > What's the rationale behind this patch set ? I've always been against vb2
> > controlling the stream state (vb2 should handle buffer management only in
> > my opinion) and I'd like to understand why you want to make it required.
>
> Let me remind the rationale behind {start,stop}_streaming. Basically there
> are more than one place where you should change the DMA streaming state,
> some of which are quite obvious (like stream_{on,off}), the others are a
> bit more surprising (like the recently discussed first call to poll()).
stream_on and stream_off are not difficult to handle on the driver side, but I
agree that it becomes more complex when poll() and read() get involved. I
still believe that stream on/off is out of scope of the video buffer
management implemementation.
> Also some of the vb2 operations behaves differently if streaming is
> enabled or not (like dqbuf), so vb2 needs to be aware of streaming state
> change.
I have no issue with vb2 being aware of stream state changes, my issues comes
from vb2 managing the stream state itself.
> The idea is also to simplify the drivers and provide a one-to-one functions
> for all typical v4l2 operations: req_bufs, query_bufs, q_buf, dq_buf,
> stream_on, stream_off, mmap, read/write, poll, so implementation of all
> from this list can be a simple 4 lines of code, like the following:
>
> static int vidioc_streamon(struct file *file, void *priv, enum
> v4l2_buf_type i) {
> struct vivi_dev *dev = video_drvdata(file);
> return vb2_streamon(&dev->vb_vidq, i);
> }
All those functions deal with buffer management, except for streamon and
streamoff.
> > I plan to use vb2 in the uvcvideo driver (when vb2 will provide a way to
> > handle device disconnection), and uvcvideo will stop the stream before
> > calling vb2_queue_release() and vb2_streamoff(). Would will I need a
> > stop_stream operation ?
>
> What's prevents you from moving the dma streaming stop call from
> stop_streaming ioctl and release file operation?
Probably not much. What bothers me is that the vb2 stream on/off callbacks to
drivers are not properly documented, so driver authors might implement them
without thinking about all possible call paths, and crash in corner cases. I
don't like implementing a callback in a driver when I don't know exactly when
and how it can be called.
--
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
Pinnacle PCTV Dual DVB-T Pro PCI 2000i
Pinnacle PCTV Dual DVB-T Pro PCI 2000i (http://linuxtv.org/wiki/index.php/DVB- T_PCI_Cards#Pinnacle) was introduced in 2006 and after 5 years it is still unsupported in linux! Unbelievable! Yet its chips Zarlink ZL10353 (http://linuxtv.org/wiki/index. php/Zarlink_ZL10353) and Microtune MT2060 (http://linuxtv.org/wiki/index. php/Microtune_MT2060) are supported (http://www.linuxtv. org/downloads/drivers/linux-media-LATEST.tar.bz2)! So, what is missing? Probably this is the reason why Linux is not so widespread: LACK OF DRIVERS! And this is the reason why a lot of users cannot migrate to Linux and are forced to use that stupid O.S. called Win! If anyone wants to have a look at Windows' drivers and is able to develop drivers (I'm not), here are the drivers: ftp://ftp.pctvsystems.com/TV/driver/PCTV%202000i/PCTV%20250i%202000i.zip Sorry for the outburst. A user wishing to migrate to Linux. -- 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: [RFCv1 PATCH 5/9] vb2_poll: don't start DMA, leave that to the first read().
Hello, On Friday, April 08, 2011 10:08 AM Hans Verkuil wrote: > On Friday, April 08, 2011 09:00:55 Marek Szyprowski wrote: > > Hello, > > > > On Monday, April 04, 2011 1:52 PM Hans Verkuil wrote: > > > > > The vb2_poll function would start read DMA if called without any > streaming > > > in progress. This unfortunately does not work if the application just > wants > > > to poll for exceptions. This information of what the application is > polling > > > for is sadly unavailable in the driver. > > > > > > Andy Walls suggested to just return POLLIN | POLLRDNORM and let the > first > > > call to read start the DMA. This initial read() call will return EAGAIN > > > since no actual data is available yet, but it does start the DMA. > > > > The current implementation of vb2_read() will just start streaming on > first > > call without returning EAGAIN. Do you think this should be changed? > > In the non-blocking case vb2_read will also return EAGAIN. Which is what > I meant. So nothing needs to be changed. Right, I got confused again. vb2_read internally calls vb2_dqbuf, which in case of nonblocking io returns EAGAIN. Acked-by: Marek Szyprowski Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 5/9] vb2_poll: don't start DMA, leave that to the first read().
On Friday, April 08, 2011 09:00:55 Marek Szyprowski wrote:
> Hello,
>
> On Monday, April 04, 2011 1:52 PM Hans Verkuil wrote:
>
> > The vb2_poll function would start read DMA if called without any streaming
> > in progress. This unfortunately does not work if the application just wants
> > to poll for exceptions. This information of what the application is polling
> > for is sadly unavailable in the driver.
> >
> > Andy Walls suggested to just return POLLIN | POLLRDNORM and let the first
> > call to read start the DMA. This initial read() call will return EAGAIN
> > since no actual data is available yet, but it does start the DMA.
>
> The current implementation of vb2_read() will just start streaming on first
> call without returning EAGAIN. Do you think this should be changed?
In the non-blocking case vb2_read will also return EAGAIN. Which is what
I meant. So nothing needs to be changed.
Regards,
Hans
>
> >
> > Application are supposed to handle EAGAIN. MythTV does handle this
> > correctly.
> >
> > Signed-off-by: Hans Verkuil
> > ---
> > drivers/media/video/videobuf2-core.c | 16 +++-
> > 1 files changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/video/videobuf2-core.c
> > b/drivers/media/video/videobuf2-core.c
> > index 6698c77..2dea57a 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c
> > @@ -1372,20 +1372,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> > file *file, poll_table *wait)
> > * Start file I/O emulator only if streaming API has not been used
> > yet.
> > */
> > if (q->num_buffers == 0 && q->fileio == NULL) {
> > - if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
> > {
> > - ret = __vb2_init_fileio(q, 1);
> > - if (ret)
> > - return POLLERR;
> > - }
> > - if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
> > {
> > - ret = __vb2_init_fileio(q, 0);
> > - if (ret)
> > - return POLLERR;
> > - /*
> > -* Write to OUTPUT queue can be done immediately.
> > -*/
> > + if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
> > + return POLLIN | POLLRDNORM;
> > + if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
> > return POLLOUT | POLLWRNORM;
> > - }
> > }
> >
> > /*
> > --
>
> Best regards
>
--
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: [RFCv1 PATCH 5/9] vb2_poll: don't start DMA, leave that to the first read().
Hello,
On Monday, April 04, 2011 1:52 PM Hans Verkuil wrote:
> The vb2_poll function would start read DMA if called without any streaming
> in progress. This unfortunately does not work if the application just wants
> to poll for exceptions. This information of what the application is polling
> for is sadly unavailable in the driver.
>
> Andy Walls suggested to just return POLLIN | POLLRDNORM and let the first
> call to read start the DMA. This initial read() call will return EAGAIN
> since no actual data is available yet, but it does start the DMA.
The current implementation of vb2_read() will just start streaming on first
call without returning EAGAIN. Do you think this should be changed?
>
> Application are supposed to handle EAGAIN. MythTV does handle this
> correctly.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/videobuf2-core.c | 16 +++-
> 1 files changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 6698c77..2dea57a 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1372,20 +1372,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> file *file, poll_table *wait)
>* Start file I/O emulator only if streaming API has not been used
> yet.
>*/
> if (q->num_buffers == 0 && q->fileio == NULL) {
> - if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
> {
> - ret = __vb2_init_fileio(q, 1);
> - if (ret)
> - return POLLERR;
> - }
> - if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
> {
> - ret = __vb2_init_fileio(q, 0);
> - if (ret)
> - return POLLERR;
> - /*
> - * Write to OUTPUT queue can be done immediately.
> - */
> + if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
> + return POLLIN | POLLRDNORM;
> + if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
> return POLLOUT | POLLWRNORM;
> - }
> }
>
> /*
> --
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
