Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Wed, Apr 22, 2015 at 01:39:07AM +0200, Luis R. Rodriguez wrote: > > Mike, do you think the time is right to just remove the iPath driver? > > With PAT now being default the driver effectively won't work > with write-combining on modern kernels. Even if systems are old > they likely had PAT support, when upgrading kernels PAT will work > but write-combing won't on ipath. Sorry, do you mean the driver already doesn't get WC? Or do you mean after some more pending patches are applied? Jason -- 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: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Apr 22 04:00:22 CEST 2015 git branch: test git hash: e183201b9e917daf2530b637b2f34f1d5afb934d gcc version:i686-linux-gcc (GCC) 4.9.1 sparse version: v0.5.0-44-g40791b9 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:3.19.0-1.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: WARNINGS linux-3.18.7-i686: WARNINGS linux-3.19-i686: WARNINGS linux-4.0-rc1-i686: WARNINGS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Tue, Apr 21, 2015 at 04:57:32PM -0600, Jason Gunthorpe wrote: > On Wed, Apr 22, 2015 at 12:46:01AM +0200, Luis R. Rodriguez wrote: > > > are talking about annotating the qib driver as "known to be broken without > > PAT" > > and since the ipath driver needs considerable work to be ported to > > use PAT (the > > This only seems to be true for one of the chips that driver supports, > not all possibilities. > > > userspace register is just one area) I wanted to review if we can just > > remove > > MTRR use on the ipath driver and annotate write-combining with PAT as a TODO > > item. > > AFAIK, dropping MTRR support will completely break the performance to > the point the driver is unusable. If we drop MTRR we may as well > remove the driver. To be clear, the arch_phys_wc_add() API will be a no-op when PAT is enabled on a system. Although some folks think PAT is new, its not, its just that our implementation on Linux lacked a bit of push, recent changes however make PAT support complete and that means now we'll have PAT enabled on systems that likely didn't before on recent kernels. There are other important motivations to use PAT: * Xen won't work with MTRR, having a unified PAT enabled kernel will ensure that when Xen is used write-combinging will take effect * Long term we want to make strong UC the default to ioremap() on x86, right now its not, its UC-, we need to convert all drivers that want write-combining over to use ioremap_wc() for their specific area, and it must not overlap. There are issues with using mtrr_add() on regions marked as UC-, since its the default this means that mtrr_add() use on ioramp'd areas on PAT systems will actually make write-combining *void*. Refer to this table for combinatorial effects for non-PAT / PAT of wc MTRR: https://marc.info/?l=linux-kernel&m=142964809710517&w=1 So as the train of PAT enablement moves forward it means systems that have PAT enabled now might not have write-combining effective. In order to get the best of both worlds, non-PAT and PAT systems we must design drivers cleanly for the non-writecombining and write-combining areas. This mean using ioremap_nocache() for MMIO and ioremap_wc() *only* for the desired, write-combining area, followed by arch_phys_wc_add(). > Mike, do you think the time is right to just remove the iPath driver? With PAT now being default the driver effectively won't work with write-combining on modern kernels. Even if systems are old they likely had PAT support, when upgrading kernels PAT will work but write-combing won't on ipath. Luis -- 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Tue, Apr 21, 2015 at 06:51:26PM -0400, Andy Walls wrote: > Sorry for the top post; mobile work email account. > > Luis, > > You do the changes to remove MTTR and point me to your dev repo and branch. > Also point me to the new functions/primitives I'll need. There is nothing new actually needed for ivtv, unless of course the ivtv driver is bounded to use a large MTRR that includes the non-framebuffer region, if so then the ioremap_uc() would be needed, and you can just cherry pick that patch: https://marc.info/?l=linux-kernel&m=142964809110516&w=1 I'll bounce that patch to you as well. Might help reading this patch too: https://marc.info/?l=linux-kernel&m=142964809710517&w=1 If your write-combining area is not restricted by size constraints so that it also include the non-framebuffer areas then you can just do a simple conversion of the driver to use ioremap_wc() on the framebuffer followed by arch_phys_wc_add(). An example driver that required changes to split this with size contraints is atyfb, here are the changes for it: https://marc.info/?l=linux-kernel&m=142964818810539&w=1 https://marc.info/?l=linux-kernel&m=142964813610531&w=1 https://marc.info/?l=linux-kernel&m=142964811010524&w=1 https://marc.info/?l=linux-kernel&m=142964814810532&w=1 If you are not constrained by MTRR's limitation on size then a simple trivial driver conversion is sufficient. For example: https://marc.info/?l=linux-kernel&m=142964744610286&w=1 I should also note that we are strivoing to also not use overlapping ioremap() calls as we want to avoid that mess. Overlapping iroemap() calls with different types could in theory work but its best we just design clean drivers and avoid this. As per Andy Lutomirski, what we'd need done on ivtv likely is for it to ioremap() for an initial bring up of the device, then infer the framebuffer offset, and only when that is being used then iounmap and then ioremap() again split areas on the driver, one with ioremap. > I'll do the changes to add write-combining back into ivtv and ivtvfb, test > them with my hardware and push them to my linuxtv.org git repo. Great! The above sounded like a complexity you did not wish to take on, but if you're up for the change, that'd be awesome! > I know there is at least one English speaking user in India using ivtv with > old PVR hardware, and probably folks in less developed places also using it. If the above is too much work for that few amount of users I'd hope we can just have them use older kernels, for the sake of sane APIs and clean driver architecture. Luis -- 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Wed, Apr 22, 2015 at 12:46:01AM +0200, Luis R. Rodriguez wrote: > are talking about annotating the qib driver as "known to be broken without > PAT" > and since the ipath driver needs considerable work to be ported to > use PAT (the This only seems to be true for one of the chips that driver supports, not all possibilities. > userspace register is just one area) I wanted to review if we can just remove > MTRR use on the ipath driver and annotate write-combining with PAT as a TODO > item. AFAIK, dropping MTRR support will completely break the performance to the point the driver is unusable. If we drop MTRR we may as well remove the driver. Mike, do you think the time is right to just remove the iPath driver? Jason -- 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Wed, Apr 15, 2015 at 01:42:47PM -0700, Andy Lutomirski wrote: > On Mon, Apr 13, 2015 at 10:49 AM, Luis R. Rodriguez wrote: > > > c) ivtv: the driver does not have the PCI space mapped out separately, and > > in fact it actually does not do the math for the framebuffer, instead it > > lets > > the device's own CPU do that and assume where its at, see > > ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get > > but not a setter. Its not clear if the firmware would make a split easy. > > We'd need ioremap_ucminus() here too and __arch_phys_wc_add(). > > > > IMO this should be conceptually easy to split. Once we get the > framebuffer address, just unmap it (or don't prematurely map it) and > then ioremap the thing. Side note to ipath driver folks: as reviewed with Andy Walls, the ivtv driver cannot easily be ported to use PAT so we are evaluating simply removing write-combing from that driver on future kernels. > > > From the beginning it seems only framebuffer devices used MTRR/WC, lately it > > seems infiniband drivers also find good use for for it for PIO TX buffers to > > blast some sort of data, in the future I would not be surprised if other > > devices found use for it. > > IMO the Infiniband maintainers should fix their code. Especially in > the server space, there aren't that many MTRRs to go around. I wrote > arch_phys_wc_add in the first place because my server *ran out of > MTRRs*. > > Hey, IB people: can you fix your drivers to use arch_phys_wc_add > (which is permitted to be a no-op) along with ioremap_wc? Your users ipath driver maintainers: The ipath driver is one of two drivers left to convert over to arch_phys_wc_add(). MTRR use is being deprecated, and its use is actually highly discouraged now that we have proper PAT implemenation on Linux. Since we are talking about annotating the qib driver as "known to be broken without PAT" and since the ipath driver needs considerable work to be ported to use PAT (the userspace register is just one area) I wanted to review if we can just remove MTRR use on the ipath driver and annotate write-combining with PAT as a TODO item. This would help a lot in our journey to bury MTRR use. Luis -- 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Tue, Apr 21, 2015 at 3:08 PM, Luis R. Rodriguez wrote: > On Tue, Apr 21, 2015 at 3:02 PM, Luis R. Rodriguez wrote: >> Andy, can we live without MTRR support on this driver for future kernels? >> This >> would only leave ipath as the only offending driver. > > Sorry to be clear, can we live with removal of write-combining on this driver? > I personally think so, but a driver maintainer's ack would be nice. --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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Tue, Apr 21, 2015 at 3:02 PM, Luis R. Rodriguez wrote: > Andy, can we live without MTRR support on this driver for future kernels? This > would only leave ipath as the only offending driver. Sorry to be clear, can we live with removal of write-combining on this driver? Luis -- 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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Wed, Apr 15, 2015 at 09:07:37PM -0400, Andy Walls wrote: > On Thu, 2015-04-16 at 01:58 +0200, Luis R. Rodriguez wrote: > > Hey Andy, thanks for your review, adding Hyong-Youb Kim for review of the > > full range ioremap_wc() idea below. > > > > On Wed, Apr 15, 2015 at 06:38:51PM -0400, Andy Walls wrote: > > > Hi All, > > > > > > On Mon, 2015-04-13 at 19:49 +0200, Luis R. Rodriguez wrote: > > > > From the beginning it seems only framebuffer devices used MTRR/WC, > > > [snip] > > > > The ivtv device is a good example of the worst type of > > > > situations and these days. So perhap __arch_phys_wc_add() and a > > > > ioremap_ucminus() might be something more than transient unless > > > > hardware folks > > > > get a good memo or already know how to just Do The Right Thing (TM). > > > > > > Just to reiterate a subtle point, use of the ivtvfb is *optional*. A > > > user may or may not load it. When the user does load the ivtvfb driver, > > > the ivtv driver has already been initialized and may have functions of > > > the card already in use by userspace. > > > > I suspected this and its why I note that a rewrite to address a clean > > split with separate ioremap seems rather difficult in this case. > > > > > Hopefully no one is trying to use the OSD as framebuffer and the video > > > decoder/output engine for video display at the same time. > > > > Worst case concern I have also is the implications of having overlapping > > ioremap() calls (as proposed in my last reply) for different memory types > > and having the different virtual memory addresse used by different parts > > of the driver. Its not clear to me what the hardware implications of this > > are. > > > > > But the video > > > decoder/output device nodes may already be open for performing ioctl() > > > functions so unmapping the decoder IO space out from under them, when > > > loading the ivtvfb driver module, might not be a good thing. > > > > Using overlapping ioremap() calls with different memory types would address > > this concern provided hardware won't barf both on the device and CPU. > > Hardware > > folks could provide feedback or an ivtvfb user could test the patch supplied > > on both non-PAT and PAT systems. Even so, who knows, this might work on > > some > > systems while not on others, only hardware folks would know. > > The CX2341[56] firmware+hardware has a track record for being really > picky about sytem hardware. It's primary symptoms are for the DMA > engine or Mailbox protocol to get hung up. So yeah, it could barf > easily on some users. > > > An alternative... is to just ioremap_wc() the entire region, including > > MMIO registers for these old devices. > > That's my thought; as long as implementing PCI write then read can force > writes to be posted and that setting that many pages as WC doesn't cause > some sort of PAT resource exhaustion. (I know very little about PAT). So upon review that strategy won't work well unless we implemnt some sort of of hack on the driver. That's also quite a bit of work. Andy, can we live without MTRR support on this driver for future kernels? This would only leave ipath as the only offending driver. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE
Hello Laurent On Tue, Apr 21, 2015 at 7:44 PM, Laurent Pinchart wrote: > Hi Ricardo, > > Thank you for the patch, and sorry for the late review (so late that the patch > has already been merged). No worries. > > On Friday 20 March 2015 14:30:46 Ricardo Ribalda Delgado wrote: >> Volatile controls should not generate CH_VALUE events. > > What's the rationale for that ? I would actually expect the value change > events to be more useful for volatile controls than non-volatile controls. > Volatile controls can have their value changed by the hardware without > software intervention, and it makes sense to me to report that to userspace. Imagine a temperature register on the sensor. It is changing constantly, resolution 10 milidegrees: Do you want to get an event for every change? Who will poll the temperature? The driver? The hardware will irq the driver? So I guess the less wrong solution is not throwing the ch_value event. This is just my two cents, probably Hans has a much better global view :) 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: [PATCH 2/3] omap3isp: Disable CCDC's VD0 and VD1 interrupts when stream is not enabled
Laurent - On 04/21/15 12:58, Laurent Pinchart wrote: Hi Tim, On Wednesday 18 March 2015 10:25:34 Tim Nordell wrote: I'll give that a shot and try add code into the adv7180 driver to turn on and off its output signals. However, it seems like if the driver can avoid a problem presented by external hardware (or other drivers), that it should. Something like either turning off the VD0 and VD1 interrupts when not in use, or by simply moving the trigger points for those interrupts (as I did here) to avoid problems by presented by signals to the system is probably a good thing for robustness. I don't disagree with that. I'll have to review the patch in details, as the CCDC code is quite sensitive. In order to do so, I'd like to know whether the problem in your case was caused by the adv7180 always being enabled. Any luck with adding a s_stream implementation in the adv7180 driver ? :-) I did add the stream on/off code, but it still seemed to have some difficulties. The codebase has effectively been handed off to our client, however, at this point. I still happen to have hardware (we're wrapping things up with the client), but likely I won't have the hardware in a week or so. I still think that the driver should avoid having the interrupts enabled if it knows it shouldn't be receiving any at a given point. I personally like the approach of modifying the VD0/VD1 trigger points as it effectively silences those interrupts without touching the central interrupt register (less potential locking issues between the various components in the OMAP3 ISP), but it could be reworked of course to touch the central interrupt register too. - Tim -- 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 2/3] omap3isp: Disable CCDC's VD0 and VD1 interrupts when stream is not enabled
Hi Tim, On Wednesday 18 March 2015 10:25:34 Tim Nordell wrote: > On 03/18/15 10:19, Laurent Pinchart wrote: > > On Tuesday 10 March 2015 14:24:53 Tim Nordell wrote: > >> During testing there appeared to be a race condition where the IRQs > >> for VD0 and VD1 could be triggered while enabling the CCDC module > >> before the pipeline status was updated. Simply modify the trigger > >> conditions for VD0 and VD1 so they won't occur when the CCDC module > >> is not enabled. > >> > >> (When this occurred during testing, the VD0 interrupt was occurring > >> over and over again starving the rest of the system.) > > > > I'm curious, might this be caused by the input (adv7180 in your case) > > being enabled before the ISP ? The CCDC is very sensitive to any glitch in > > its input signals, you need to make sure that the source is disabled > > before its subdev s_stream operation is called. Given that the adv7180 > > driver doesn't implement s_stream, I expect it to be free-running, which > > is definitely a problem. > > I'll give that a shot and try add code into the adv7180 driver to turn on > and off its output signals. However, it seems like if the driver can avoid > a problem presented by external hardware (or other drivers), that it should. > Something like either turning off the VD0 and VD1 interrupts when not in > use, or by simply moving the trigger points for those interrupts (as I did > here) to avoid problems by presented by signals to the system is probably a > good thing for robustness. I don't disagree with that. I'll have to review the patch in details, as the CCDC code is quite sensitive. In order to do so, I'd like to know whether the problem in your case was caused by the adv7180 always being enabled. Any luck with adding a s_stream implementation in the adv7180 driver ? :-) -- 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 v2 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE
Hi Ricardo,
Thank you for the patch, and sorry for the late review (so late that the patch
has already been merged).
On Friday 20 March 2015 14:30:46 Ricardo Ribalda Delgado wrote:
> Volatile controls should not generate CH_VALUE events.
What's the rationale for that ? I would actually expect the value change
events to be more useful for volatile controls than non-volatile controls.
Volatile controls can have their value changed by the hardware without
software intervention, and it makes sense to me to report that to userspace.
> Set has_changed to false to prevent this happening.
>
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> v2: By Sakari Ailus
>
> Fix CodeStyle (sorry :S)
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c index 45c5b47..2ebc33e 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1609,6 +1609,15 @@ static int cluster_changed(struct v4l2_ctrl *master)
>
> if (ctrl == NULL)
> continue;
> + /*
> + * Set has_changed to false to avoid generating
> + * the event V4L2_EVENT_CTRL_CH_VALUE
> + */
> + if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
> + ctrl->has_changed = false;
> + continue;
> + }
> +
> for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++)
> ctrl_changed = !ctrl->type_ops->equal(ctrl, idx,
> ctrl->p_cur, ctrl->p_new);
--
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: ioremap_uc() followed by set_memory_wc() - burrying MTRR
On Wed, Apr 15, 2015 at 05:58:14PM -0700, Andy Lutomirski wrote: > On Wed, Apr 15, 2015 at 4:59 PM, Andy Walls wrote: > > On Wed, 2015-04-15 at 16:42 -0700, Andy Lutomirski wrote: > >> On Wed, Apr 15, 2015 at 3:38 PM, Andy Walls > >> wrote: > > > >> > > >> > >> IMO the right solution would be to avoid ioremapping the whole bar at > >> startup. Instead ioremap pieces once the driver learns what they are. > >> This wouldn't have any of these problems -- you'd ioremap() register > >> regions and you'd ioremap_wc() the framebuffer once you find it. If > >> there are regions of unknown purpose, just don't map them all. > >> > >> Would this be feasible? > > > > Feasible? Maybe. > > > > Worth the time and effort for end-of-life, convential PCI hardware so I > > can have an optimally performing X display on a Standard Def Analog TV > > screen? Nope. I don't have that level of nostalgia. > > > > The point is actually to let us unexport or delete mtrr_add. We can > either severely regress performance on ivtv on PAT-capable hardware if > we naively switch it to arch_phys_wc_add or we can do something else. > The something else remains to be determined. Back to square one: I can't come up with anything not too instrusive or that dotes not requires substantial amount of work as an alternative to removing MTRR completely right now (with the long term goal of also making strong UC default) and its because of 2 device drivers: * ivtv: firmware API is poo and device is legacy, no one cares * ipath: driver needs a clean split and work is considerable, maintainers have not been responsive, do they care? What do we want to do with these drivers? Let us be straight shooters, if we are serious about having a performance regression on the drivers for the sake of removing MTRR why not just seriously discuss removal of these drivers. This way we can remain sane, upkeep a policy to never even consider overlapping ioremap*() calls, and have a clean expected strategy we can expect for new drivers. I'm going to split up my patches now into 4 series: 1) things which are straight forward in converting drivers over to arch_phys_wc_add() and ioremap_wc(). These are subsystem wide though, so just a heads up, my hope is that each subsystem maintainer can take their own series unless someone else is comfortable in taking this in for x86 2) a few helpers in the like of ioremap_wc() needed for other drivers. These are straight forward but since they depend on x86 / core helpers it would be nice for them to go I guess through x86 folks. What maintainer is up to take these? 3) MTRR run time changes 4) corner cases - TBD - lets discuss here what we want to do with ivtv and ipath. I will however remove fusion's mtrr code use as its all commented out. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP3 ISP previewer Y10 to UYVY conversion
Hi Chris,
On Tuesday 21 April 2015 11:04:14 Chris Whittenburg wrote:
> On Fri, Apr 17, 2015 at 4:39 AM, Laurent Pinchart wrote:
> > On Thursday 16 April 2015 13:05:30 Chris Whittenburg wrote:
> >> On Tue, Apr 7, 2015 at 10:51 AM, Laurent Pinchart wrote:
> >>> Black level compensation is applied by the CCDC before writing raw
> >>> frames to memory. If your raw frames are correct BLC is probably not to
> >>> blame.
> >>>
> >>> The default contrast is x1.0 and the default brightness is +0.0, so I
> >>> don't think those should be blame either.
> >>>
> >>> I suspect the RGB2RGB conversion matrix to be wrong. The default
> >>> setting is supposed to handle fluorescent lighting. You could try
> >>> setting the RGB2RGB matrix to the identity matrix and see if this
> >>> helps. See
> >>> http://git.ideasonboard.org/omap3-isp-live.git/blob/HEAD:/isp/controls.
> >>> c#l184 for sample code.
> >>>
> >>> Another matrix that could be worth being reprogrammed is the RGB2YUV
> >>> matrix, which also defaults to fluorescent lighting. Sample code to
> >>> reprogram it is available in the same location.
> >>
> >> I tried changing the rgb2rgb matrx to the identity matrix:
> >>
> >> {0x0100, 0x, 0x},
> >> {0x, 0x0100, 0x},
> >> {0x, 0x, 0x0100}
> >>
> >> And the csc (rgb2yuv) to this:
> >> {256, 0, 0},
> >> {0, 0, 0},
> >> {0, 0, 0}
> >>
> >> But I couldn't see much, if any, difference.
> >>
> >> However, when I forced the gamma correction to be bypassed, it seemed to
> >> fix it.
> >>
> >> Does that make sense? I guess I don't understand it enough to understand
> >> if gamma correction would have compressed all my luma values.
> >
> > Yes, it makes sense. Gamma correction applies a non-linear transformation
> > to the pixel values and can explain the problems you were seeing.
> >
> > I've checked the default rgb2rgb matrix, and it should work fine for your
> > case as all lines add up to 1.0. The default rgb2yuv matrix, however,
> > limits Y values to 220, so you should modify it.
>
> I believe the formula used is:
> Y = CSCRY*Rin + CSCGY*Gin + CSCBY*Bin +Yoffset
>
> If CSCRY=1.0, and Rin is in the range 0 to 255, then the resulting Y
> would be in the range [0, 255] as well, so why would the matrix limit
> Y values to 220? Is it because Rin, Gin, and Bin have already been
> limited to the YCbCr range of [16, 240]?
I meant that the default matrix programmed by the omap3isp driver will limit
the range to 220.
{Y} { 66, 129, 25} {R}
{U} = { -38, -75, 112} {G}
{V} { 112, -94, -18} {B}
Y = 66*R + 129*G + 25*B
As R = G = B = y in your case, that's
Y = 66*y + 129*y + 25*y = 220*y
Coefficients are expressed in S10Q8 format (signed, 10 bits in total, 8
decimal bits), so we end up with Y = 220/256 * y, limiting the Y range to 220
(assuming the input value is limited to 8 bits).
--
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: OMAP3 ISP previewer Y10 to UYVY conversion
On Fri, Apr 17, 2015 at 4:39 AM, Laurent Pinchart
wrote:
> Hi Chris,
>
> On Thursday 16 April 2015 13:05:30 Chris Whittenburg wrote:
>> On Tue, Apr 7, 2015 at 10:51 AM, Laurent Pinchart wrote:
>> > Black level compensation is applied by the CCDC before writing raw frames
>> > to memory. If your raw frames are correct BLC is probably not to blame.
>> >
>> > The default contrast is x1.0 and the default brightness is +0.0, so I
>> > don't think those should be blame either.
>> >
>> > I suspect the RGB2RGB conversion matrix to be wrong. The default setting
>> > is supposed to handle fluorescent lighting. You could try setting the
>> > RGB2RGB matrix to the identity matrix and see if this helps. See
>> > http://git.ideasonboard.org/omap3-isp-live.git/blob/HEAD:/isp/controls.c#l
>> > 184 for sample code.
>> >
>> > Another matrix that could be worth being reprogrammed is the RGB2YUV
>> > matrix, which also defaults to fluorescent lighting. Sample code to
>> > reprogram it is available in the same location.
>>
>> I tried changing the rgb2rgb matrx to the identity matrix:
>>
>> {0x0100, 0x, 0x},
>> {0x, 0x0100, 0x},
>> {0x, 0x, 0x0100}
>>
>> And the csc (rgb2yuv) to this:
>> {256, 0, 0},
>> {0, 0, 0},
>> {0, 0, 0}
>>
>> But I couldn't see much, if any, difference.
>>
>> However, when I forced the gamma correction to be bypassed, it seemed to fix
>> it.
>>
>> Does that make sense? I guess I don't understand it enough to understand if
>> gamma correction would have compressed all my luma values.
>
> Yes, it makes sense. Gamma correction applies a non-linear transformation to
> the pixel values and can explain the problems you were seeing.
>
> I've checked the default rgb2rgb matrix, and it should work fine for your case
> as all lines add up to 1.0. The default rgb2yuv matrix, however, limits Y
> values to 220, so you should modify it.
I believe the formula used is:
Y = CSCRY*Rin + CSCGY*Gin + CSCBY*Bin +Yoffset
If CSCRY=1.0, and Rin is in the range 0 to 255, then the resulting Y
would be in the range [0, 255] as well, so why would the matrix limit
Y values to 220? Is it because Rin, Gin, and Bin have already been
limited to the YCbCr range of [16, 240]?
--
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
[GIT PULL for v4.1-rc1] media updates
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v4.1-2 For: - A new frontend driver for new ATSC devices: lgdt3306a - A new sensor driver: ov2659 - a new platform driver: xilinx - The m88ts2022 tuner driver was merged at ts2020 driver - The media controller gained experimental support for DVB and hybrid devices; - Lots of random cleanups, fixes and improvements on media drivers Thanks, Mauro - The following changes since commit 2c33ce009ca2389dbf0535d0672214d09738e35e: Merge Linus master into drm-next (2015-04-20 13:05:20 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v4.1-2 for you to fetch changes up to 64131a87f2aae2ed9e05d8227c5b009ca6c50d98: Merge branch 'drm-next-merged' of git://people.freedesktop.org/~airlied/linux into v4l_for_linus (2015-04-21 09:44:55 -0300) media updates for v4.1-rc1 Alexey Khoroshilov (2): [media] sh_vou: fix memory leak on error paths in sh_vou_open() [media] usbvision: fix leak of usb_dev on failure paths in usbvision_probe() Andrzej Pietrasiewicz (1): [media] s5p-jpeg: add 5420 family support Antti Palosaari (11): [media] mn88473: define symbol rate limits [media] mn88472: define symbol rate limits [media] ts2020: add support for TS2022 [media] ts2020: implement I2C client bindings [media] em28xx: switch PCTV 461e to ts2020 driver [media] cx23885: switch ts2022 to ts2020 driver [media] smipcie: switch ts2022 to ts2020 driver [media] dvbsky: switch ts2022 to ts2020 driver [media] dw2102: switch ts2022 to ts2020 driver [media] m88ts2022: remove obsolete driver [media] ts2020: do not use i2c_transfer() on sleep() Arnd Bergmann (2): [media] wl128x-radio really depends on TI_ST [media] Add and use IS_REACHABLE macro Benjamin Larsson (14): [media] mn88473: calculate the IF register values [media] mn88473: simplify bandwidth registers setting code [media] r820t: add DVBC profile in sysfreq_sel [media] r820t: change read_gain() code to match register layout [media] rtl28xxu: lower the rc poll time to mitigate i2c transfer errors [media] mn88472: implement lock for all delivery systems [media] mn88472: implement firmware parity check [media] mn88473: implement firmware parity check [media] mn88473: check if firmware is already running before loading it [media] mn88472: check if firmware is already running before loading it [media] mn88473: implement lock for all delivery systems [media] mn88472: add ts mode and ts clock to driver [media] r820t: add settings for SYS_DVBC_ANNEX_C standard [media] r820t: enable flt_ext_wide for SYS_DVBC_ANNEX_A standard Benoit Parrot (1): [media] media: i2c: add support for omnivision's ov2659 sensor Christian Dale (1): [media] WinFast DTV2000 DS Plus Christian Engelmayer (2): [media] cx88: Fix possible leak in cx8802_probe() [media] si2165: Fix possible leak in si2165_upload_firmware() David Howells (3): [media] m88ts2022: Nested loops shouldn't use the same index variable [media] cx23885: Always initialise dev->slock spinlock [media] cxusb: Use enum to represent table offsets rather than hard-coding numbers Devin Heitmueller (1): [media] xc5000: fix memory corruption when unplugging device Dimitris Lampridis (1): [media] rtl28xxu: add support for Turbo-X DTT2000 Enrico Scholz (1): [media] mt9p031: fixed calculation of clk_div Ezequiel Garcia (2): [media] stk1160: Make sure current buffer is released [media] MAINTAINERS: Update the maintainer mail address for stk1160 Fabian Frederick (1): [media] saa7146: replace current->state by set_current_state() Florian Echtler (2): [media] add raw video stream support for Samsung SUR40 [media] sur40: fix occasional hard freeze due to buffer queue underrun Fred Richter (1): [media] DVB: add support for LG Electronics LGDT3306A ATSC/QAM-B Demodulator Geert Uytterhoeven (2): [media] am437x: VIDEO_AM437X_VPFE should depend on HAS_DMA [media] timberdale: VIDEO_TIMBERDALE should depend on HAS_DMA Gilles Risch (1): [media] Basic support for the Elgato EyeTV Hybrid INT 2008 USB Stick Guennadi Liakhovetski (2): [media] V4L: remove clock name from v4l2_clk API [media] V4L: add CCF support to the v4l2_clk API Hans Verkuil (118): [media] media.h: mark alsa struct in media_entity_desc as TODO [media] DocBook media: fix validation error [media] pvrusb2: replace .ioctl by .unlocked_ioctl [media] radio-bcm2048: use unlocked_ioctl instead of ioctl [media] uvc gadget: switch to v4l
Re: [PATCH 2/2] gspca: sn9c2028: Add gain and autogain controls Genius Videocam Live v2
Hi Hans,
On Tue, Apr 21, 2015 at 5:32 PM, Hans de Goede wrote:
>> diff --git a/drivers/media/usb/gspca/sn9c2028.h
>> b/drivers/media/usb/gspca/sn9c2028.h
>> index 8fd1d3e..6f20c0f 100644
>> --- a/drivers/media/usb/gspca/sn9c2028.h
>> +++ b/drivers/media/usb/gspca/sn9c2028.h
>> @@ -21,8 +21,17 @@
>>*
>>*/
>>
>> -static const unsigned char sn9c2028_sof_marker[5] =
>> - { 0xff, 0xff, 0x00, 0xc4, 0xc4 };
>> +static const unsigned char sn9c2028_sof_marker[] = {
>> + 0xff, 0xff, 0x00, 0xc4, 0xc4, 0x96,
>> + 0x00,
>> + 0x00, /* seq */
>> + 0x00,
>> + 0x00,
>> + 0x00, /* avg luminance lower 8 bit */
>> + 0x00, /* avg luminance higher 8 bit */
>> + 0x00,
>> + 0x00,
>> +};
>>
>
> This seems wrong, the header is only 12 bytes the extra 2 0x00 bytes you add
> are
> actually part of the compressed data and are parsed by the userspace code,
> please drop them.
OK, I've found average lumimance value in header heuristically,
based on info that it's present in header of sn9c1xx and sn9c201 cams,
and I didn't check actual header length - my fault.
Regards,
Vasily
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] gspca: sn9c2028: Add support for Genius Videocam Live v2
Hi Hans,
On Tue, Apr 21, 2015 at 5:21 PM, Hans de Goede wrote:
>> @@ -128,7 +129,7 @@ static int sn9c2028_long_command(struct gspca_dev
>> *gspca_dev, u8 *command)
>> status = -1;
>> for (i = 0; i < 256 && status < 2; i++)
>> status = sn9c2028_read1(gspca_dev);
>> - if (status != 2) {
>> + if (status < 0) {
>> pr_err("long command status read error %d\n", status);
>> return (status < 0) ? status : -EIO;
>> }
>
>
> Do you really need this change ? sn9c2028_read1 returns either a negative
> error code, or the byte read from the sn9c2028 chip. This functions wait for
> the sn9c2028 to return a read value of 2. I admit that the check in the for
> vs the check in the error reporting is not chosen well, both should probably
> be != 2. But checking for status < 0 is not good as this does not catch
> a successful read from the chip not returning 2.
For this cam it returns 1 on some commands. Anyway, this value is not
used anywhere later, so I just extended condition.
Regards,
Vasily
--
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 2/2] gspca: sn9c2028: Add gain and autogain controls Genius Videocam Live v2
Hi,
On 19-04-15 20:52, Vasily Khoruzhick wrote:
Autogain algorithm is very simple, if average luminance is low - increase gain,
if it's high - decrease gain. Gain granularity is low enough for this algo to
stabilize quickly.
Signed-off-by: Vasily Khoruzhick
---
drivers/media/usb/gspca/sn9c2028.c | 121 +
drivers/media/usb/gspca/sn9c2028.h | 20 +-
2 files changed, 138 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/gspca/sn9c2028.c
b/drivers/media/usb/gspca/sn9c2028.c
index 317b02c..0ff390f 100644
--- a/drivers/media/usb/gspca/sn9c2028.c
+++ b/drivers/media/usb/gspca/sn9c2028.c
@@ -34,6 +34,16 @@ struct sd {
struct gspca_dev gspca_dev; /* !! must be the first item */
u8 sof_read;
u16 model;
+
+#define MIN_AVG_LUM 8500
+#define MAX_AVG_LUM 1
+ int avg_lum;
+ u8 avg_lum_l;
+
+ struct { /* autogain and gain control cluster */
+ struct v4l2_ctrl *autogain;
+ struct v4l2_ctrl *gain;
+ };
};
struct init_command {
@@ -252,6 +262,77 @@ static int run_start_commands(struct gspca_dev *gspca_dev,
return 0;
}
+static void set_gain(struct gspca_dev *gspca_dev, s32 g)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ struct init_command genius_vcam_live_gain_cmds[] = {
+ {{0x1d, 0x25, 0x10, 0x20, 0xab, 0x00}, 0},
+ };
+ if (!gspca_dev->streaming)
+ return;
+
+ switch (sd->model) {
+ case 0x7003:
+ genius_vcam_live_gain_cmds[0].instruction[2] = g;
+ run_start_commands(gspca_dev, genius_vcam_live_gain_cmds,
+ ARRAY_SIZE(genius_vcam_live_gain_cmds));
+ break;
+ default:
+ break;
+ }
+}
+
+static int sd_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct gspca_dev *gspca_dev =
+ container_of(ctrl->handler, struct gspca_dev, ctrl_handler);
+ struct sd *sd = (struct sd *)gspca_dev;
+
+ gspca_dev->usb_err = 0;
+
+ if (!gspca_dev->streaming)
+ return 0;
+
+ switch (ctrl->id) {
+ /* standalone gain control */
+ case V4L2_CID_GAIN:
+ set_gain(gspca_dev, ctrl->val);
+ break;
+ /* autogain */
+ case V4L2_CID_AUTOGAIN:
+ set_gain(gspca_dev, sd->gain->val);
+ break;
+ }
+ return gspca_dev->usb_err;
+}
+
+static const struct v4l2_ctrl_ops sd_ctrl_ops = {
+ .s_ctrl = sd_s_ctrl,
+};
+
+
+static int sd_init_controls(struct gspca_dev *gspca_dev)
+{
+ struct v4l2_ctrl_handler *hdl = &gspca_dev->ctrl_handler;
+ struct sd *sd = (struct sd *)gspca_dev;
+
+ gspca_dev->vdev.ctrl_handler = hdl;
+ v4l2_ctrl_handler_init(hdl, 2);
+
+ switch (sd->model) {
+ case 0x7003:
+ sd->gain = v4l2_ctrl_new_std(hdl, &sd_ctrl_ops,
+ V4L2_CID_GAIN, 0, 20, 1, 0);
+ sd->autogain = v4l2_ctrl_new_std(hdl, &sd_ctrl_ops,
+ V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
static int start_spy_cam(struct gspca_dev *gspca_dev)
{
struct init_command spy_start_commands[] = {
@@ -641,6 +722,9 @@ static int start_genius_videocam_live(struct gspca_dev
*gspca_dev)
if (r < 0)
return r;
+ if (sd->gain)
+ set_gain(gspca_dev, v4l2_ctrl_g_ctrl(sd->gain));
+
return r;
}
@@ -757,6 +841,8 @@ static int sd_start(struct gspca_dev *gspca_dev)
return -ENXIO;
}
+ sd->avg_lum = -1;
+
return err_code;
}
@@ -776,6 +862,39 @@ static void sd_stopN(struct gspca_dev *gspca_dev)
PERR("Camera Stop command failed");
}
+static void do_autogain(struct gspca_dev *gspca_dev, int avg_lum)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+ s32 cur_gain = v4l2_ctrl_g_ctrl(sd->gain);
+
+ if (avg_lum == -1)
+ return;
+
+ if (avg_lum < MIN_AVG_LUM) {
+ if (cur_gain == sd->gain->maximum)
+ return;
+ cur_gain++;
+ v4l2_ctrl_s_ctrl(sd->gain, cur_gain);
+ }
+ if (avg_lum > MAX_AVG_LUM) {
+ if (cur_gain == sd->gain->minimum)
+ return;
+ cur_gain--;
+ v4l2_ctrl_s_ctrl(sd->gain, cur_gain);
+ }
+
+}
+
+static void sd_dqcallback(struct gspca_dev *gspca_dev)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ if (sd->autogain == NULL || !v4l2_ctrl_g_ctrl(sd->autogain))
+ return;
+
+ do_autogain(gspca_dev, sd->avg_lum);
+}
+
/* Include sn9c2028 sof detection functions */
#include "sn9c2028.h"
@@ -810,8 +929,10 @@ static const struct sd_desc sd_desc = {
.name = MODULE_NAME,
.config = sd_config,
.init = sd_ini
Re: [PATCH 1/2] gspca: sn9c2028: Add support for Genius Videocam Live v2
Hi Vasily,
Thanks for the patches.
On 19-04-15 20:52, Vasily Khoruzhick wrote:
Signed-off-by: Vasily Khoruzhick
---
drivers/media/usb/gspca/sn9c2028.c | 120 -
1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/sn9c2028.c
b/drivers/media/usb/gspca/sn9c2028.c
index 39b6b2e..317b02c 100644
--- a/drivers/media/usb/gspca/sn9c2028.c
+++ b/drivers/media/usb/gspca/sn9c2028.c
@@ -2,6 +2,7 @@
* SN9C2028 library
*
* Copyright (C) 2009 Theodore Kilgore
+ * Copyright (C) 2015 Vasily Khoruzhick
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -128,7 +129,7 @@ static int sn9c2028_long_command(struct gspca_dev
*gspca_dev, u8 *command)
status = -1;
for (i = 0; i < 256 && status < 2; i++)
status = sn9c2028_read1(gspca_dev);
- if (status != 2) {
+ if (status < 0) {
pr_err("long command status read error %d\n", status);
return (status < 0) ? status : -EIO;
}
Do you really need this change ? sn9c2028_read1 returns either a negative
error code, or the byte read from the sn9c2028 chip. This functions wait for
the sn9c2028 to return a read value of 2. I admit that the check in the for
vs the check in the error reporting is not chosen well, both should probably
be != 2. But checking for status < 0 is not good as this does not catch
a successful read from the chip not returning 2.
@@ -178,6 +179,9 @@ static int sd_config(struct gspca_dev *gspca_dev,
case 0x7005:
PDEBUG(D_PROBE, "Genius Smart 300 camera");
break;
+ case 0x7003:
+ PDEBUG(D_PROBE, "Genius Videocam Live v2");
+ break;
case 0x8000:
PDEBUG(D_PROBE, "DC31VC");
break;
@@ -530,6 +534,116 @@ static int start_genius_cam(struct gspca_dev *gspca_dev)
ARRAY_SIZE(genius_start_commands));
}
+static int start_genius_videocam_live(struct gspca_dev *gspca_dev)
+{
+ int r;
+ struct sd *sd = (struct sd *) gspca_dev;
+ struct init_command genius_vcam_live_start_commands[] = {
+ {{0x0c, 0x01, 0x00, 0x00, 0x00, 0x00}, 0},
+ {{0x16, 0x01, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x10, 0x00, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x25, 0x01, 0x16, 0x00, 0x00}, 4},
+ {{0x13, 0x26, 0x01, 0x12, 0x00, 0x00}, 4},
+
+ {{0x13, 0x28, 0x01, 0x0e, 0x00, 0x00}, 4},
+ {{0x13, 0x27, 0x01, 0x20, 0x00, 0x00}, 4},
+ {{0x13, 0x29, 0x01, 0x22, 0x00, 0x00}, 4},
+ {{0x13, 0x2c, 0x01, 0x02, 0x00, 0x00}, 4},
+ {{0x13, 0x2d, 0x01, 0x02, 0x00, 0x00}, 4},
+ {{0x13, 0x2e, 0x01, 0x09, 0x00, 0x00}, 4},
+ {{0x13, 0x2f, 0x01, 0x07, 0x00, 0x00}, 4},
+ {{0x11, 0x20, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x21, 0x2d, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x22, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x23, 0x03, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x10, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x11, 0x64, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x12, 0x00, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x13, 0x91, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x14, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x15, 0x20, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x16, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x17, 0x60, 0x00, 0x00, 0x00}, 4},
+ {{0x1c, 0x20, 0x00, 0x2d, 0x00, 0x00}, 4},
+ {{0x13, 0x20, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x21, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x22, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x23, 0x01, 0x01, 0x00, 0x00}, 4},
+ {{0x13, 0x24, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x25, 0x01, 0x16, 0x00, 0x00}, 4},
+ {{0x13, 0x26, 0x01, 0x12, 0x00, 0x00}, 4},
+ {{0x13, 0x27, 0x01, 0x20, 0x00, 0x00}, 4},
+ {{0x13, 0x28, 0x01, 0x0e, 0x00, 0x00}, 4},
+ {{0x13, 0x29, 0x01, 0x22, 0x00, 0x00}, 4},
+ {{0x13, 0x2a, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x2b, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x2c, 0x01, 0x02, 0x00, 0x00}, 4},
+ {{0x13, 0x2d, 0x01, 0x02, 0x00, 0x00}, 4},
+ {{0x13, 0x2e, 0x01, 0x09, 0x00, 0x00}, 4},
+ {{0x13, 0x2f, 0x01, 0x07, 0x00, 0x00}, 4},
+ {{0x12, 0x34, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x13, 0x34, 0x01, 0xa1, 0x00, 0x00}, 4},
+ {{0x13, 0x35, 0x01, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x01, 0x04, 0x00, 0x00, 0x00}, 4},
+ {{0x11, 0x02, 0x92, 0x00, 0x00, 0x00}, 4}
[GIT PULL FOR v4.2] Fill in the description for VIDIOC_ENUM_FMT
Ensure standard format descriptions by filling it in in the v4l2 core. Currently these descriptions are all over the place and every driver dreams up its own description. That's not good. Regards, Hans The following changes since commit e183201b9e917daf2530b637b2f34f1d5afb934d: [media] uvcvideo: add support for VIDIOC_QUERY_EXT_CTRL (2015-04-10 10:29:27 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.2c for you to fetch changes up to b1a4374e55d847ae0b17b33c52a974de833ce228: vivid: drop format description (2015-04-21 15:39:49 +0200) Hans Verkuil (4): v4l2-ioctl: fill in the description for VIDIOC_ENUM_FMT v4l2-pci-skeleton: drop format description vim2m: drop format description vivid: drop format description Documentation/video4linux/v4l2-pci-skeleton.c | 2 - drivers/media/platform/vim2m.c | 4 -- drivers/media/platform/vivid/vivid-core.h | 1 - drivers/media/platform/vivid/vivid-vid-cap.c| 4 -- drivers/media/platform/vivid/vivid-vid-common.c | 50 --- drivers/media/v4l2-core/v4l2-ioctl.c| 199 +--- 6 files changed, 192 insertions(+), 68 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 2/4] v4l2-pci-skeleton: drop format description
From: Hans Verkuil The format description is now filled in by the core, so we can drop this in this skeleton driver. Signed-off-by: Hans Verkuil --- Documentation/video4linux/v4l2-pci-skeleton.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c b/Documentation/video4linux/v4l2-pci-skeleton.c index 7bd1b97..9c80c09 100644 --- a/Documentation/video4linux/v4l2-pci-skeleton.c +++ b/Documentation/video4linux/v4l2-pci-skeleton.c @@ -406,9 +406,7 @@ static int skeleton_enum_fmt_vid_cap(struct file *file, void *priv, if (f->index != 0) return -EINVAL; - strlcpy(f->description, "4:2:2, packed, YUYV", sizeof(f->description)); f->pixelformat = V4L2_PIX_FMT_YUYV; - f->flags = 0; return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 3/4] vim2m: drop format description
From: Hans Verkuil
The format description is now filled in by the core, so we can
drop this in this virtual m2m driver.
Signed-off-by: Hans Verkuil
---
drivers/media/platform/vim2m.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 4d6b4cc..cecfd75 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -80,7 +80,6 @@ static struct platform_device vim2m_pdev = {
};
struct vim2m_fmt {
- char*name;
u32 fourcc;
int depth;
/* Types the format can be used for */
@@ -89,14 +88,12 @@ struct vim2m_fmt {
static struct vim2m_fmt formats[] = {
{
- .name = "RGB565 (BE)",
.fourcc = V4L2_PIX_FMT_RGB565X, /* rggg gggb */
.depth = 16,
/* Both capture and output format */
.types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
},
{
- .name = "4:2:2, packed, YUYV",
.fourcc = V4L2_PIX_FMT_YUYV,
.depth = 16,
/* Output-only format */
@@ -458,7 +455,6 @@ static int enum_fmt(struct v4l2_fmtdesc *f, u32 type)
if (i < NUM_FORMATS) {
/* Format found */
fmt = &formats[i];
- strncpy(f->description, fmt->name, sizeof(f->description) - 1);
f->pixelformat = fmt->fourcc;
return 0;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 1/4] v4l2-ioctl: fill in the description for VIDIOC_ENUM_FMT
From: Hans Verkuil
The descriptions used in drivers for the formats returned with ENUM_FMT
are all over the place.
So instead allow the core to fill them in if the driver didn't. This
allows drivers to drop the description and flags.
Based on an earlier patch from Philipp Zabel:
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/81411
Signed-off-by: Hans Verkuil
Acked-by: Philipp Zabel
Acked-by: Laurent Pinchart
Acked-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-ioctl.c | 199 +--
1 file changed, 192 insertions(+), 7 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index aa407cb..24dc45f 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1103,6 +1103,182 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops
*ops,
return ops->vidioc_enum_output(file, fh, p);
}
+static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
+{
+ const unsigned sz = sizeof(fmt->description);
+ const char *descr = NULL;
+ u32 flags = 0;
+
+ /*
+* We depart from the normal coding style here since the descriptions
+* should be aligned so it is easy to see which descriptions will be
+* longer than 31 characters (the max length for a description).
+* And frankly, this is easier to read anyway.
+*
+* Note that gcc will use O(log N) comparisons to find the right case.
+*/
+ switch (fmt->pixelformat) {
+ /* Max description length mask: descr =
"0123456789012345678901234567890" */
+ case V4L2_PIX_FMT_RGB332: descr = "8-bit RGB 3-3-2"; break;
+ case V4L2_PIX_FMT_RGB444: descr = "16-bit A/XRGB 4-4-4-4"; break;
+ case V4L2_PIX_FMT_ARGB444: descr = "16-bit ARGB 4-4-4-4"; break;
+ case V4L2_PIX_FMT_XRGB444: descr = "16-bit XRGB 4-4-4-4"; break;
+ case V4L2_PIX_FMT_RGB555: descr = "16-bit A/XRGB 1-5-5-5"; break;
+ case V4L2_PIX_FMT_ARGB555: descr = "16-bit ARGB 1-5-5-5"; break;
+ case V4L2_PIX_FMT_XRGB555: descr = "16-bit XRGB 1-5-5-5"; break;
+ case V4L2_PIX_FMT_RGB565: descr = "16-bit RGB 5-6-5"; break;
+ case V4L2_PIX_FMT_RGB555X: descr = "16-bit A/XRGB 1-5-5-5 BE";
break;
+ case V4L2_PIX_FMT_ARGB555X: descr = "16-bit ARGB 1-5-5-5 BE"; break;
+ case V4L2_PIX_FMT_XRGB555X: descr = "16-bit XRGB 1-5-5-5 BE"; break;
+ case V4L2_PIX_FMT_RGB565X: descr = "16-bit RGB 5-6-5 BE"; break;
+ case V4L2_PIX_FMT_BGR666: descr = "18-bit BGRX 6-6-6-14"; break;
+ case V4L2_PIX_FMT_BGR24:descr = "24-bit BGR 8-8-8"; break;
+ case V4L2_PIX_FMT_RGB24:descr = "24-bit RGB 8-8-8"; break;
+ case V4L2_PIX_FMT_BGR32:descr = "32-bit BGRA/X 8-8-8-8"; break;
+ case V4L2_PIX_FMT_ABGR32: descr = "32-bit BGRA 8-8-8-8"; break;
+ case V4L2_PIX_FMT_XBGR32: descr = "32-bit BGRX 8-8-8-8"; break;
+ case V4L2_PIX_FMT_RGB32:descr = "32-bit A/XRGB 8-8-8-8"; break;
+ case V4L2_PIX_FMT_ARGB32: descr = "32-bit ARGB 8-8-8-8"; break;
+ case V4L2_PIX_FMT_XRGB32: descr = "32-bit XRGB 8-8-8-8"; break;
+ case V4L2_PIX_FMT_GREY: descr = "8-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y4: descr = "4-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y6: descr = "6-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y10: descr = "10-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y12: descr = "12-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y16: descr = "16-bit Greyscale"; break;
+ case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)";
break;
+ case V4L2_PIX_FMT_PAL8: descr = "8-bit Palette"; break;
+ case V4L2_PIX_FMT_UV8: descr = "8-bit Chrominance UV 4-4";
break;
+ case V4L2_PIX_FMT_YVU410: descr = "Planar YVU 4:1:0"; break;
+ case V4L2_PIX_FMT_YVU420: descr = "Planar YVU 4:2:0"; break;
+ case V4L2_PIX_FMT_YUYV: descr = "YUYV 4:2:2"; break;
+ case V4L2_PIX_FMT_YYUV: descr = "YYUV 4:2:2"; break;
+ case V4L2_PIX_FMT_YVYU: descr = "YVYU 4:2:2"; break;
+ case V4L2_PIX_FMT_UYVY: descr = "UYVY 4:2:2"; break;
+ case V4L2_PIX_FMT_VYUY: descr = "VYUY 4:2:2"; break;
+ case V4L2_PIX_FMT_YUV422P: descr = "Planar YVU 4:2:2"; break;
+ case V4L2_PIX_FMT_YUV411P: descr = "Planar YUV 4:1:1"; break;
+ case V4L2_PIX_FMT_Y41P: descr = "YUV 4:1:1 (Packed)"; break;
+ case V4L2_PIX_FMT_YUV444: descr = "16-bit A/XYUV 4-4-4-4"; break;
+ case V4L2_PIX_FMT_YUV555: descr = "16-bit A/XYUV 1-5-5-5"; break;
+ case V4L2_PIX_FMT_YUV565: descr = "16-bit YUV 5-6-5"; break;
+ case V4L2_PIX_FMT_YUV32:descr = "32-bit A/XYUV
[PATCHv3 4/4] vivid: drop format description
From: Hans Verkuil
The format description is now filled in by the core, so we can
drop this in this virtual driver.
Signed-off-by: Hans Verkuil
---
drivers/media/platform/vivid/vivid-core.h | 1 -
drivers/media/platform/vivid/vivid-vid-cap.c| 4 --
drivers/media/platform/vivid/vivid-vid-common.c | 50 -
3 files changed, 55 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-core.h
b/drivers/media/platform/vivid/vivid-core.h
index 9e15aee..bf26173 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -77,7 +77,6 @@ extern const struct v4l2_rect vivid_max_rect;
extern unsigned vivid_debug;
struct vivid_fmt {
- const char *name;
u32 fourcc; /* v4l2 format id */
boolis_yuv;
boolcan_do_overlay;
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
b/drivers/media/platform/vivid/vivid-vid-cap.c
index dab5990..8a20137 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -40,7 +40,6 @@ static const struct v4l2_fract
static const struct vivid_fmt formats_ovl[] = {
{
- .name = "RGB565 (LE)",
.fourcc = V4L2_PIX_FMT_RGB565, /* gggb rggg */
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -48,7 +47,6 @@ static const struct vivid_fmt formats_ovl[] = {
.buffers = 1,
},
{
- .name = "XRGB555 (LE)",
.fourcc = V4L2_PIX_FMT_XRGB555, /* gggb argg */
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -56,7 +54,6 @@ static const struct vivid_fmt formats_ovl[] = {
.buffers = 1,
},
{
- .name = "ARGB555 (LE)",
.fourcc = V4L2_PIX_FMT_ARGB555, /* gggb argg */
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -1030,7 +1027,6 @@ int vidioc_enum_fmt_vid_overlay(struct file *file, void
*priv,
fmt = &formats_ovl[f->index];
- strlcpy(f->description, fmt->name, sizeof(f->description));
f->pixelformat = fmt->fourcc;
return 0;
}
diff --git a/drivers/media/platform/vivid/vivid-vid-common.c
b/drivers/media/platform/vivid/vivid-vid-common.c
index aa44627..6ba8744 100644
--- a/drivers/media/platform/vivid/vivid-vid-common.c
+++ b/drivers/media/platform/vivid/vivid-vid-common.c
@@ -45,7 +45,6 @@ const struct v4l2_dv_timings_cap vivid_dv_timings_cap = {
struct vivid_fmt vivid_formats[] = {
{
- .name = "4:2:2, packed, YUYV",
.fourcc = V4L2_PIX_FMT_YUYV,
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -55,7 +54,6 @@ struct vivid_fmt vivid_formats[] = {
.data_offset = { PLANE0_DATA_OFFSET },
},
{
- .name = "4:2:2, packed, UYVY",
.fourcc = V4L2_PIX_FMT_UYVY,
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -64,7 +62,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "4:2:2, packed, YVYU",
.fourcc = V4L2_PIX_FMT_YVYU,
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -73,7 +70,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "4:2:2, packed, VYUY",
.fourcc = V4L2_PIX_FMT_VYUY,
.vdownsampling = { 1 },
.bit_depth = { 16 },
@@ -82,7 +78,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "YUV 4:2:2 triplanar",
.fourcc = V4L2_PIX_FMT_YUV422P,
.vdownsampling = { 1, 1, 1 },
.bit_depth = { 8, 4, 4 },
@@ -91,7 +86,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "YUV 4:2:0 triplanar",
.fourcc = V4L2_PIX_FMT_YUV420,
.vdownsampling = { 1, 2, 2 },
.bit_depth = { 8, 4, 4 },
@@ -100,7 +94,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "YVU 4:2:0 triplanar",
.fourcc = V4L2_PIX_FMT_YVU420,
.vdownsampling = { 1, 2, 2 },
.bit_depth = { 8, 4, 4 },
@@ -109,7 +102,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name = "YUV 4:2:0 biplanar",
.fourcc = V4L2_PIX_FMT_NV12,
.vdownsampling = { 1, 2 },
.bit_depth = { 8, 8 },
@@ -118,7 +110,6 @@ struct vivid_fmt vivid_formats[] = {
.buffers = 1,
},
{
- .name
[PATCHv3 0/4] Fill in the description for VIDIOC_ENUM_FMT
From: Hans Verkuil This patch series is identical to https://patchwork.linuxtv.org/patch/29080/ but it removes the description from the skeleton driver and the virtual drivers. Since those are basically reference drivers it makes sense to update those first. I'll make a pull request of this as well, since this is ready for 4.2. Regards, Hans Hans Verkuil (4): v4l2-ioctl: fill in the description for VIDIOC_ENUM_FMT v4l2-pci-skeleton: drop format description vim2m: drop format description vivid: drop format description Documentation/video4linux/v4l2-pci-skeleton.c | 2 - drivers/media/platform/vim2m.c | 4 - drivers/media/platform/vivid/vivid-core.h | 1 - drivers/media/platform/vivid/vivid-vid-cap.c| 4 - drivers/media/platform/vivid/vivid-vid-common.c | 50 -- drivers/media/v4l2-core/v4l2-ioctl.c| 199 +++- 6 files changed, 192 insertions(+), 68 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v4.2] Replace most duplicate video ops by pad ops
These patches replace most duplicate video ops by their pad ops counterparts. The only remaining duplicate ops deal with cropping and selection, and I want to be able to test that first. I have difficulty doing that, but I am expecting hardware in the near future that should enable me to do this work. These six patches replace the *_mbus_fmt video ops by their pad ops versions. While not particularly complex, this does cause a lot of unavoidable churn, so I would like to get this merged early in the 4.2 cycle to get the maximum testing time. This work just has to be done, since duplication of ops defeats the whole purpose of reusability. The patches in this pull request are identical to patches 1-6 of this post, except that patch 1 has a trivial bug fix that Guennadi found: http://www.spinics.net/lists/linux-media/msg88549.html Regards, Hans The following changes since commit e183201b9e917daf2530b637b2f34f1d5afb934d: [media] uvcvideo: add support for VIDIOC_QUERY_EXT_CTRL (2015-04-10 10:29:27 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.2b for you to fetch changes up to a3912adac27072237c62f479c581d00f41ef2a46: v4l2: replace s_mbus_fmt by set_fmt in bridge drivers (2015-04-21 15:23:33 +0200) Hans Verkuil (6): v4l2: replace enum_mbus_fmt by enum_mbus_code v4l2: replace video op g_mbus_fmt by pad op get_fmt v4l2: replace try_mbus_fmt by set_fmt v4l2: replace s_mbus_fmt by set_fmt v4l2: replace try_mbus_fmt by set_fmt in bridge drivers v4l2: replace s_mbus_fmt by set_fmt in bridge drivers drivers/media/i2c/adv7170.c | 42 + drivers/media/i2c/adv7175.c | 42 + drivers/media/i2c/adv7183.c | 61 +--- drivers/media/i2c/adv7842.c | 25 +-- drivers/media/i2c/ak881x.c | 39 +++ drivers/media/i2c/cx25840/cx25840-core.c | 15 +++-- drivers/media/i2c/ml86v7667.c| 29 +++-- drivers/media/i2c/mt9v011.c | 53 --- drivers/media/i2c/ov7670.c | 38 -- drivers/media/i2c/saa6752hs.c| 42 - drivers/media/i2c/saa7115.c | 16 -- drivers/media/i2c/saa717x.c | 16 -- drivers/media/i2c/soc_camera/imx074.c| 66 -- drivers/media/i2c/soc_camera/mt9m001.c | 43 + drivers/media/i2c/soc_camera/mt9m111.c | 57 ++--- drivers/media/i2c/soc_camera/mt9t031.c | 74 +-- drivers/media/i2c/soc_camera/mt9t112.c | 41 +--- drivers/media/i2c/soc_camera/mt9v022.c | 43 + drivers/media/i2c/soc_camera/ov2640.c| 62 +--- drivers/media/i2c/soc_camera/ov5642.c| 60 ++- drivers/media/i2c/soc_camera/ov6650.c| 43 + drivers/media/i2c/soc_camera/ov772x.c| 41 +--- drivers/media/i2c/soc_camera/ov9640.c| 32 +-- drivers/media/i2c/soc_camera/ov9740.c| 35 ++--- drivers/media/i2c/soc_camera/rj54n1cb0c.c| 66 +++--- drivers/media/i2c/soc_camera/tw9910.c| 41 +--- drivers/media/i2c/sr030pc30.c| 62 drivers/media/i2c/tvp514x.c | 55 ++-- drivers/media/i2c/tvp5150.c | 30 +++--- drivers/media/i2c/tvp7002.c | 48 drivers/media/i2c/vs6624.c | 55 ++-- drivers/media/pci/cx18/cx18-av-core.c| 16 -- drivers/media/pci/cx18/cx18-controls.c | 13 +--- drivers/media/pci/cx18/cx18-ioctl.c | 12 --- drivers/media/pci/cx23885/cx23885-video.c| 12 --- drivers/media/pci/ivtv/ivtv-controls.c | 12 --- drivers/media/pci/ivtv/ivtv-ioctl.c | 12 --- drivers/media/pci/saa
Re: [patch] [media] v4l: xilinx: harmless buffer overflow
Hi Dan,
Thank you for the patch.
On Tuesday 21 April 2015 12:31:10 Dan Carpenter wrote:
> My static checker warns that the name of the port can be 15 characters
> when you consider the NUL terminator and that's one more than the 14
> characters in name[]. Maybe it's an off-by-one?
>
> It's unlikely that we hit the limit and even if we do the overflow will
> only affect one of the two bytes of padding so it's harmless. Still
> let's fix it and also change the sprintf() to snprintf().
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
> b/drivers/media/platform/xilinx/xilinx-dma.c index efde88a..98e50e4 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -653,7 +653,7 @@ static const struct v4l2_file_operations xvip_dma_fops =
> { int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma
> *dma, enum v4l2_buf_type type, unsigned int port)
> {
> - char name[14];
> + char name[16];
Being pedantic we could use name[15], but it wouldn't make any difference.
> int ret;
>
> dma->xdev = xdev;
> @@ -725,7 +725,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev,
> struct xvip_dma *dma, }
>
> /* ... and the DMA channel. */
> - sprintf(name, "port%u", port);
> + snprintf(name, sizeof(name), "port%u", port);
Nitpicking again, I'd say that sprintf is enough as we know it won't overflow.
However, as the sprintf implementation is a wrapper around vsnprintf with size
set to INT_MAX, using snprintf won't incur any runtime performance penalty.
Acked-by: Laurent Pinchart
and applied to my tree.
> dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
> if (dma->dma == NULL) {
> dev_err(dma->xdev->dev, "no VDMA channel found\n");
--
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
[RFCv2 PATCH 13/15] vivid: add request support for video capture.
From: Hans Verkuil
In order to test the request API in applications we add request support to
vivid. The brightness, contrast, saturation and hue controls now can be used
in requests. Those were chosen because the test pattern generator supports
those controls and will adjust the TPG colors accordingly, so this gives a
good visual feedback.
Just before a buffer with a specific request is ready to be filled, any
controls set for that request are applied and the TPG will use the new
values for filling in the buffer, so this matches what a well-written driver
will do in actual hardware.
Finally, support for req_queue is added using the new v4l2_device_req_queue
helper function.
Signed-off-by: Hans Verkuil
---
drivers/media/platform/vivid/vivid-core.c| 2 ++
drivers/media/platform/vivid/vivid-ctrls.c | 4
drivers/media/platform/vivid/vivid-kthread-cap.c | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index d33f164..23c5bc0 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -669,6 +669,7 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
return ret;
}
dev->v4l2_dev.release = vivid_dev_release;
+ dev->v4l2_dev.req_queue = v4l2_device_req_queue;
/* start detecting feature set */
@@ -1044,6 +1045,7 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
q->lock = &dev->mutex;
+ q->allow_requests = 1;
ret = vb2_queue_init(q);
if (ret)
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c
b/drivers/media/platform/vivid/vivid-ctrls.c
index 2b90700..bb66608 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -1301,12 +1301,16 @@ int vivid_create_controls(struct vivid_dev *dev, bool
show_ccs_cap,
V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
for (i = 0; i < MAX_INPUTS; i++)
dev->input_brightness[i] = 128;
+ v4l2_ctrl_s_max_reqs(dev->brightness, VIDEO_MAX_FRAME);
dev->contrast = v4l2_ctrl_new_std(hdl_user_vid,
&vivid_user_vid_ctrl_ops,
V4L2_CID_CONTRAST, 0, 255, 1, 128);
+ v4l2_ctrl_s_max_reqs(dev->contrast, VIDEO_MAX_FRAME);
dev->saturation = v4l2_ctrl_new_std(hdl_user_vid,
&vivid_user_vid_ctrl_ops,
V4L2_CID_SATURATION, 0, 255, 1, 128);
+ v4l2_ctrl_s_max_reqs(dev->saturation, VIDEO_MAX_FRAME);
dev->hue = v4l2_ctrl_new_std(hdl_user_vid,
&vivid_user_vid_ctrl_ops,
V4L2_CID_HUE, -128, 128, 1, 0);
+ v4l2_ctrl_s_max_reqs(dev->hue, VIDEO_MAX_FRAME);
v4l2_ctrl_new_std(hdl_user_vid, &vivid_user_vid_ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(hdl_user_vid, &vivid_user_vid_ctrl_ops,
diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c
b/drivers/media/platform/vivid/vivid-kthread-cap.c
index 1727f54..6b4d3b7 100644
--- a/drivers/media/platform/vivid/vivid-kthread-cap.c
+++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
@@ -681,6 +681,8 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev
*dev, int dropped_bufs)
if (!list_empty(&dev->vid_cap_active)) {
vid_cap_buf = list_entry(dev->vid_cap_active.next, struct
vivid_buffer, list);
list_del(&vid_cap_buf->list);
+ v4l2_ctrl_apply_request(dev->vid_cap_dev.ctrl_handler,
+ vid_cap_buf->vb.v4l2_buf.request);
}
if (!list_empty(&dev->vbi_cap_active)) {
if (dev->field_cap != V4L2_FIELD_ALTERNATE ||
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 08/15] v4l2-ctrls: add VIDIOC_REQUEST_CMD
From: Hans Verkuil
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls.c | 60 --
drivers/media/v4l2-core/v4l2-dev.c| 1 +
drivers/media/v4l2-core/v4l2-ioctl.c | 81 ++-
drivers/media/v4l2-core/v4l2-subdev.c | 63 ---
include/media/v4l2-ctrls.h| 8 ++--
include/media/v4l2-fh.h | 1 +
include/uapi/linux/videodev2.h| 19 +++-
7 files changed, 189 insertions(+), 44 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 43fb3c2..d262e2e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2715,9 +2715,13 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
EXPORT_SYMBOL(v4l2_query_ext_ctrl);
/* Implement VIDIOC_QUERYCTRL */
-int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl *qc)
+int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl,
+ unsigned request, struct v4l2_queryctrl *qc)
{
- struct v4l2_query_ext_ctrl qec = { qc->id };
+ struct v4l2_query_ext_ctrl qec = {
+ .id = qc->id,
+ .request = request,
+ };
int rc;
rc = v4l2_query_ext_ctrl(hdl, &qec);
@@ -2755,7 +2759,7 @@ int v4l2_subdev_queryctrl(struct v4l2_subdev *sd, struct
v4l2_queryctrl *qc)
{
if (qc->id & (V4L2_CTRL_FLAG_NEXT_CTRL | V4L2_CTRL_FLAG_NEXT_COMPOUND))
return -EINVAL;
- return v4l2_queryctrl(sd->ctrl_handler, qc);
+ return v4l2_queryctrl(sd->ctrl_handler, 0, qc);
}
EXPORT_SYMBOL(v4l2_subdev_queryctrl);
@@ -3058,7 +3062,8 @@ int v4l2_subdev_g_ext_ctrls(struct v4l2_subdev *sd,
struct v4l2_ext_controls *cs
EXPORT_SYMBOL(v4l2_subdev_g_ext_ctrls);
/* Helper function to get a single control */
-static int get_ctrl(struct v4l2_ctrl *ctrl, struct v4l2_ext_control *c)
+static int get_ctrl(struct v4l2_ctrl *ctrl,
+ unsigned request, struct v4l2_ext_control *c)
{
struct v4l2_ctrl *master = ctrl->cluster[0];
int ret = 0;
@@ -3076,11 +3081,17 @@ static int get_ctrl(struct v4l2_ctrl *ctrl, struct
v4l2_ext_control *c)
v4l2_ctrl_lock(master);
/* g_volatile_ctrl will update the current control values */
- if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
+ if (request == 0 && (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE)) {
for (i = 0; i < master->ncontrols; i++)
cur_to_new(master->cluster[i]);
ret = call_op(master, g_volatile_ctrl);
new_to_user(c, ctrl);
+ } else if (request) {
+ ctrl->request = get_request(ctrl, request);
+ if (ctrl->request)
+ ptr_to_user(c, ctrl, ctrl->request->ptr);
+ else
+ ret = -EINVAL;
} else {
cur_to_user(c, ctrl);
}
@@ -3088,7 +3099,8 @@ static int get_ctrl(struct v4l2_ctrl *ctrl, struct
v4l2_ext_control *c)
return ret;
}
-int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *control)
+int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl,
+ unsigned request, struct v4l2_control *control)
{
struct v4l2_ctrl *ctrl = v4l2_ctrl_find(hdl, control->id);
struct v4l2_ext_control c;
@@ -3096,7 +3108,7 @@ int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl, struct
v4l2_control *control)
if (ctrl == NULL || !ctrl->is_int)
return -EINVAL;
- ret = get_ctrl(ctrl, &c);
+ ret = get_ctrl(ctrl, request, &c);
control->value = c.value;
return ret;
}
@@ -3104,7 +3116,7 @@ EXPORT_SYMBOL(v4l2_g_ctrl);
int v4l2_subdev_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *control)
{
- return v4l2_g_ctrl(sd->ctrl_handler, control);
+ return v4l2_g_ctrl(sd->ctrl_handler, 0, control);
}
EXPORT_SYMBOL(v4l2_subdev_g_ctrl);
@@ -3115,7 +3127,7 @@ s32 v4l2_ctrl_g_ctrl(struct v4l2_ctrl *ctrl)
/* It's a driver bug if this happens. */
WARN_ON(!ctrl->is_int);
c.value = 0;
- get_ctrl(ctrl, &c);
+ get_ctrl(ctrl, 0, &c);
return c.value;
}
EXPORT_SYMBOL(v4l2_ctrl_g_ctrl);
@@ -3127,7 +3139,7 @@ s64 v4l2_ctrl_g_ctrl_int64(struct v4l2_ctrl *ctrl)
/* It's a driver bug if this happens. */
WARN_ON(ctrl->is_ptr || ctrl->type != V4L2_CTRL_TYPE_INTEGER64);
c.value = 0;
- get_ctrl(ctrl, &c);
+ get_ctrl(ctrl, 0, &c);
return c.value;
}
EXPORT_SYMBOL(v4l2_ctrl_g_ctrl_int64);
@@ -3384,7 +3396,8 @@ int v4l2_subdev_s_ext_ctrls(struct v4l2_subdev *sd,
struct v4l2_ext_controls *cs
EXPORT_SYMBOL(v4l2_subdev_s_ext_ctrls);
/* Helper function for VIDIOC_S_CTRL compatibility */
-static int set_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 ch_flags)
+static int set_ctrl(struct v4l2_fh *fh, unsigned request,
+
[RFCv2 PATCH 15/15] Documentation: add v4l2-requests.txt
From: Hans Verkuil Add documentation about requests. Signed-off-by: Hans Verkuil --- Documentation/video4linux/v4l2-requests.txt | 233 1 file changed, 233 insertions(+) create mode 100644 Documentation/video4linux/v4l2-requests.txt diff --git a/Documentation/video4linux/v4l2-requests.txt b/Documentation/video4linux/v4l2-requests.txt new file mode 100644 index 000..324a840 --- /dev/null +++ b/Documentation/video4linux/v4l2-requests.txt @@ -0,0 +1,233 @@ +Introduction + + +It is often useful to apply certain settings when a buffer is about to be +filled by the DMA capture of a video capture device, ensuring that those +settings are applied in time for them to be used with that buffer. + +One of the prime use-cases of this is Android's CameraHAL v3 which requires +per-frame configuration support. + +But other use-cases are possible as well: changing codec settings (bit rate, +etc.) starting with a specific buffer, preparing a configuration to be applied +at a certain time, etc. + +The request API is the way V4L2 solved this problem. + + +Basic Mechanism +=== + +Requests are implemented by building on top of the control framework, adding +new 'request' fields to various v4l2 structs and by adding a new ioctl: +VIDIOC_REQUEST_CMD. + +The core are the additions to the control framework: normally when a control +is set the control's value is applied immediately in the hardware. But the +request API allows you to specify the control value for a specific request +ID (from 1-0x). The control value is validated and stored for that request, +but it won't be applied until later. + +Each control can support up to N requests where N is expected to be a multiple +of VIDEO_MAX_FRAME. If N is 0, then it cannot be used with requests. Many +controls (e.g. V4L2_CID_POWER_LINE_FREQUENCY) just make no sense in a request +context. + +How many requests each control supports can be queried with VIDIOC_QUERY_EXT_CTRL +and by checking the max_reqs field. + +Getting/setting/trying controls for a request can be done in two ways: one +is to use the VIDIOC_REQUEST_CMD ioctl (more about that later), the other is +to use VIDIOC_G/S/TRY_EXT/CTRLS and fill in a non-zero request ID. If the +request ID is 0, then the normal behavior applies: so controls are set immediately. + +Request IDs are in the range of 1-65535. Applications can freely choose them, +but controls won't support more than a fixed number of different requests. +Requests can be deleted, see the section on VIDIOC_REQUEST_CMD. + +The expected usage is that request IDs are some base value + the buffer index. +Request lookup is optimized for that scheme inside the control framework: there +is a hash table of size VIDEO_MAX_FRAME, each element being a linked list. +The hash function used is 'request % VIDEO_MAX_FRAME', and then the linked list +is walked to find whether the request is in there. The expectation is that +having more than VIDEO_MAX_FRAME requests will be very unusual, and certainly +that the maximum number of possible requests in flight at the same time won't +be more than a small multiple of VIDEO_MAX_FRAME. + +Associating a buffer with a specific request can be done by setting the new +'request' field in struct v4l2_buffer to the request ID. + +Once a buffer is queued with a non-zero request ID, then it is up to the +driver to apply the control values for that request at the right time ensuring +that when the buffer is DMAed those values are in effect. + +This is entirely hardware specific, so it is the driver's responsibility. It +will know the request ID of a buffer as soon as the vb2 buf_queue op is called. + +For simple cases a helper function v4l2_ctrl_apply_request() is available that +will just apply any controls for the given request immediately. The driver can +call that when the buffer is about to be filled. This is likely to be too crude +for the more complex devices, in particular when dealing with settings that can +take some time before that will take effect (e.g. focussing). + + +Complex Devices +=== + +For complex devices with lots of video and v4l-subdev device nodes you want to +be able to synchronize buffers and controls for multiple nodes and apply them +all as a single request. + +The way to do that is to: + +1) use the same request ID for all device nodes when setting controls for that + request. +2) instead of using VIDIOC_QBUF use VIDIOC_PREPARE_BUF with the request field + set everywhere you need to have a buffer associated with the request. +3) when all is done, call VIDIOC_REQUEST_CMD(V4L2_REQ_CMD_QUEUE) with the + request ID on any video or v4l-subdev node. + +The V4L2_REQ_CMD_QUEUE command will call a top-level 'req_queue' callback in +the driver, at which point it is the responsibility of the driver to pull +everything together and ensure that all prepared buffers are queued up at the +right time to the DMA queues and all controls for that request
[RFCv2 PATCH 10/15] vb2: add helper function to queue request-specific buffer.
From: Hans Verkuil
The vb2_qbuf_request() function will queue any buffers for the given request
that are in state PREPARED.
Useful when drivers have to implement the req_queue callback.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 18 ++
include/media/videobuf2-core.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index da513b1..4ab65b6 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1923,6 +1923,24 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
}
EXPORT_SYMBOL_GPL(vb2_qbuf);
+int vb2_qbuf_request(struct vb2_queue *q, u16 request, struct vb2_buffer
**p_buf)
+{
+ int buffer;
+
+ for (buffer = 0; buffer < q->num_buffers; buffer++) {
+ struct vb2_buffer *vb = q->bufs[buffer];
+
+ if (vb->v4l2_buf.request == request &&
+ vb->state == VB2_BUF_STATE_PREPARED) {
+ if (p_buf)
+ *p_buf = vb;
+ return vb2_qbuf(q, &vb->v4l2_buf);
+ }
+ }
+ return -ENOENT;
+}
+EXPORT_SYMBOL_GPL(vb2_qbuf_request);
+
/**
* __vb2_wait_for_done_vb() - wait for a buffer to become available
* for dequeuing
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index a3e3596..82fa2a6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -461,6 +461,7 @@ void vb2_queue_release(struct vb2_queue *q);
void vb2_queue_error(struct vb2_queue *q);
int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b);
+int vb2_qbuf_request(struct vb2_queue *q, u16 request, struct vb2_buffer
**p_buf);
int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb);
int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 09/15] v4l2: add initial V4L2_REQ_CMD_QUEUE support
From: Hans Verkuil
Add the V4L2_REQ_CMD_QUEUE command and the req_queue callback to struct
v4l2_device. Call it if set from v4l2-ioctl.c and v4l2-subdev.c. Make sure
in v4l2-ioctl.c to unlock any current lock first (and relock afterwards).
That way req_queue is called with the assurance that there are no video_device
locks taken. Since req_queue operates device-wide that would make that code
much more complex.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ioctl.c | 11 +++
drivers/media/v4l2-core/v4l2-subdev.c | 6 ++
include/media/v4l2-device.h | 2 ++
include/uapi/linux/videodev2.h| 1 +
4 files changed, 20 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 51830c2..44c33f3 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1996,6 +1996,17 @@ static int v4l_request_cmd(const struct v4l2_ioctl_ops
*ops,
return v4l2_ctrl_delete_request(vfh->ctrl_handler, p->request);
case V4L2_REQ_CMD_APPLY:
return v4l2_ctrl_apply_request(vfh->ctrl_handler, p->request);
+ case V4L2_REQ_CMD_QUEUE:
+ if (vfd->v4l2_dev->req_queue == NULL)
+ return -ENOSYS;
+ if (p->request == 0)
+ return -EINVAL;
+ if (vfd->lock)
+ mutex_unlock(vfd->lock);
+ ret = vfd->v4l2_dev->req_queue(vfd->v4l2_dev, p->request);
+ if (vfd->lock)
+ mutex_lock(vfd->lock);
+ return ret;
default:
return -EINVAL;
}
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index cabbddc..7113b95 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -266,6 +266,12 @@ static long subdev_do_ioctl(struct file *file, unsigned
int cmd, void *arg)
case V4L2_REQ_CMD_APPLY:
return v4l2_ctrl_apply_request(vfh->ctrl_handler,
p->request);
+ case V4L2_REQ_CMD_QUEUE:
+ if (sd->v4l2_dev->req_queue == NULL)
+ return -ENOSYS;
+ if (p->request == 0)
+ return -EINVAL;
+ return sd->v4l2_dev->req_queue(sd->v4l2_dev,
p->request);
default:
return -EINVAL;
}
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 9c58157..603e7f3 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -62,6 +62,8 @@ struct v4l2_device {
struct kref ref;
/* Release function that is called when the ref count goes to 0. */
void (*release)(struct v4l2_device *v4l2_dev);
+ /* Queue a request */
+ int (*req_queue)(struct v4l2_device *v4l2_dev, u16 request);
};
static inline void v4l2_device_get(struct v4l2_device *v4l2_dev)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 98a7717..f3164f6 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2089,6 +2089,7 @@ struct v4l2_create_buffers {
#define V4L2_REQ_CMD_END (1)
#define V4L2_REQ_CMD_DELETE(2)
#define V4L2_REQ_CMD_APPLY (3)
+#define V4L2_REQ_CMD_QUEUE (4)
struct v4l2_request_cmd {
__u32 cmd;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 14/15] v4l2-ctrls: add REQ_KEEP flag
From: Hans Verkuil
Experimental: I am still not certain whether this is desired or not.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls.c | 28
drivers/media/v4l2-core/v4l2-ioctl.c | 9 -
drivers/media/v4l2-core/v4l2-subdev.c | 11 ++-
include/media/v4l2-ctrls.h| 3 +++
include/media/v4l2-fh.h | 3 +++
include/uapi/linux/videodev2.h| 4
6 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d262e2e..480bdb6 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2693,6 +2693,8 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
if (req) {
if (ctrl_req->applied)
qc->flags |= V4L2_CTRL_FLAG_REQ_APPLIED;
+ if (ctrl_req->keep)
+ qc->flags |= V4L2_CTRL_FLAG_REQ_KEEP;
}
qc->max_reqs = ctrl->max_reqs;
qc->type = ctrl->type;
@@ -3148,7 +3150,7 @@ EXPORT_SYMBOL(v4l2_ctrl_g_ctrl_int64);
copied to the current value on a set.
Must be called with ctrl->handler->lock held. */
static int try_or_set_cluster(struct v4l2_fh *fh, struct v4l2_ctrl *master,
- u16 request, bool set, u32 ch_flags)
+ u16 request, bool keep, bool set, u32 ch_flags)
{
bool update_flag;
int ret;
@@ -3172,6 +3174,8 @@ static int try_or_set_cluster(struct v4l2_fh *fh, struct
v4l2_ctrl *master,
if (ret)
return ret;
}
+ if (set)
+ req->keep = keep;
}
ctrl->request = req;
if (!ctrl->is_new) {
@@ -3272,14 +3276,17 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct
v4l2_ctrl_handler *hdl,
struct v4l2_ctrl_helper helper[4];
struct v4l2_ctrl_helper *helpers = helper;
unsigned request = 0;
+ bool keep = false;
unsigned i, j;
int ret;
cs->error_idx = cs->count;
- if (V4L2_CTRL_ID2CLASS(cs->ctrl_class))
+ if (V4L2_CTRL_ID2CLASS(cs->ctrl_class)) {
cs->ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
- else
+ } else {
request = cs->request;
+ keep = set && (cs->request & V4L2_CTRL_REQ_FL_KEEP);
+ }
if (hdl == NULL || request > USHRT_MAX)
return -EINVAL;
@@ -3351,7 +3358,8 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct
v4l2_ctrl_handler *hdl,
} while (!ret && idx);
if (!ret)
- ret = try_or_set_cluster(fh, master, request, set, 0);
+ ret = try_or_set_cluster(fh, master, request,
+keep, set, 0);
/* Copy the new values back to userspace. */
if (!ret) {
@@ -3423,7 +3431,7 @@ static int set_ctrl(struct v4l2_fh *fh, unsigned request,
update_from_auto_cluster(master);
ctrl->is_new = 1;
- return try_or_set_cluster(fh, master, request, true, ch_flags);
+ return try_or_set_cluster(fh, master, request, false, true, ch_flags);
}
/* Helper function for VIDIOC_S_CTRL compatibility */
@@ -3517,6 +3525,7 @@ int v4l2_ctrl_apply_request(struct v4l2_ctrl_handler
*hdl, unsigned request)
list_for_each_entry(ref, &hdl->ctrl_refs, node) {
struct v4l2_ctrl *master;
bool apply_request = false;
+ bool keep = false;
if (ref->ctrl->max_reqs == 0)
continue;
@@ -3535,9 +3544,11 @@ int v4l2_ctrl_apply_request(struct v4l2_ctrl_handler
*hdl, unsigned request)
if (ctrl->request == NULL)
continue;
found_request = true;
- if (!ctrl->request->applied) {
+ if (ctrl->request->keep || !ctrl->request->applied) {
request_to_new(master->cluster[i]);
apply_request = true;
+ if (ctrl->request->keep)
+ keep = true;
ctrl->request->applied = 1;
}
}
@@ -3548,7 +3559,8 @@ int v4l2_ctrl_apply_request(struct v4l2_ctrl_handler
*hdl, unsigned request)
}
/*
-* Skip if it is a request that has already been applied.
+* Skip if it is a one-off request that has already been
+* applied.
*/
if (!apply_request)
goto unlock;
@@ -3569,7 +3581
[RFCv2 PATCH 07/15] v4l2-ctrls: implement delete request(s)
From: Hans Verkuil
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls.c | 42
include/media/v4l2-ctrls.h | 1 +
2 files changed, 43 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 93c51cc..43fb3c2 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3566,6 +3566,48 @@ unlock:
}
EXPORT_SYMBOL(v4l2_ctrl_apply_request);
+int v4l2_ctrl_delete_request(struct v4l2_ctrl_handler *hdl, unsigned request)
+{
+ struct v4l2_ctrl_ref *ref;
+ unsigned i;
+
+ if (hdl == NULL || request == 0)
+ return -EINVAL;
+
+ mutex_lock(hdl->lock);
+
+ list_for_each_entry(ref, &hdl->ctrl_refs, node) {
+ struct v4l2_ctrl *master;
+
+ if (ref->ctrl->max_reqs == 0)
+ continue;
+ master = ref->ctrl->cluster[0];
+ if (ref->ctrl != master)
+ continue;
+ if (master->handler != hdl)
+ v4l2_ctrl_lock(master);
+ for (i = 0; i < master->ncontrols; i++) {
+ struct v4l2_ctrl *ctrl = master->cluster[i];
+ struct v4l2_ctrl_req *req;
+
+ if (ctrl == NULL || ctrl->request_lists == NULL)
+ continue;
+
+ if (request == 0) {
+ free_requests(ctrl);
+ continue;
+ }
+ req = get_request(ctrl, request);
+ if (req)
+ del_request(ctrl, req);
+ }
+ if (master->handler != hdl)
+ v4l2_ctrl_unlock(master);
+ }
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_ctrl_delete_request);
+
void v4l2_ctrl_notify(struct v4l2_ctrl *ctrl, v4l2_ctrl_notify_fnc notify,
void *priv)
{
if (ctrl == NULL)
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 2d188a2..324db6d 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -810,6 +810,7 @@ static inline void v4l2_ctrl_s_max_reqs(struct v4l2_ctrl
*ctrl, u16 max_reqs)
}
int v4l2_ctrl_apply_request(struct v4l2_ctrl_handler *hdl, unsigned request);
+int v4l2_ctrl_delete_request(struct v4l2_ctrl_handler *hdl, unsigned request);
/* Internal helper functions that deal with control events. */
extern const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 12/15] v4l2-device: add v4l2_device_req_queue
From: Hans Verkuil
The v4l2_device_req_queue() function is a helper that can be used
as the req_queue callback in simple cases: it will walk over all
registered video_devices and call vb2_qbuf_request() for each video
device.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-device.c | 25 +
include/media/v4l2-device.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-device.c
b/drivers/media/v4l2-core/v4l2-device.c
index cdb2d72..5f073ad 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
{
@@ -295,3 +296,27 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
module_put(sd->owner);
}
EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
+
+int v4l2_device_req_queue(struct v4l2_device *v4l2_dev, u16 request)
+{
+ struct video_device *vdev;
+ struct video_device *tmp;
+ int err;
+
+ if (request == 0)
+ return -EINVAL;
+
+ list_for_each_entry_safe(vdev, tmp, &v4l2_dev->vdevs, list) {
+ if (vdev->queue == NULL || !vdev->queue->allow_requests)
+ continue;
+ if (vdev->lock && mutex_lock_interruptible(vdev->lock))
+ return -ERESTARTSYS;
+ err = vb2_qbuf_request(vdev->queue, request, NULL);
+ if (vdev->lock)
+ mutex_unlock(vdev->lock);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_device_req_queue);
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 6484e54..3595475 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -130,6 +130,9 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev
*sd,
sd->v4l2_dev->notify(sd, notification, arg);
}
+/* For each registered video_device struct call vb2_qbuf_request(). */
+int v4l2_device_req_queue(struct v4l2_device *v4l2_dev, u16 request);
+
/* Iterate over all subdevs. */
#define v4l2_device_for_each_subdev(sd, v4l2_dev) \
list_for_each_entry(sd, &(v4l2_dev)->subdevs, list)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 11/15] v4l2-device: keep track of registered video_devices
From: Hans Verkuil
In order to efficiently handle V4L2_REQ_CMD_QUEUE we need to know which
video_device structs are registered for the given v4l2_device struct.
So create a list of vdevs in v4l2_device and add/remove each video_device
there as it is registered/unregistered.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-dev.c| 8
drivers/media/v4l2-core/v4l2-device.c | 1 +
include/media/v4l2-dev.h | 3 +++
include/media/v4l2-device.h | 2 ++
4 files changed, 14 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c
b/drivers/media/v4l2-core/v4l2-dev.c
index ff206f1..a11e35d 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -768,6 +768,8 @@ int __video_register_device(struct video_device *vdev, int
type, int nr,
if (WARN_ON(!vdev->v4l2_dev))
return -EINVAL;
+ INIT_LIST_HEAD(&vdev->list);
+
/* v4l2_fh support */
spin_lock_init(&vdev->fh_lock);
INIT_LIST_HEAD(&vdev->fh_list);
@@ -927,6 +929,9 @@ int __video_register_device(struct video_device *vdev, int
type, int nr,
#endif
/* Part 6: Activate this minor. The char device can now be used. */
set_bit(V4L2_FL_REGISTERED, &vdev->flags);
+ spin_lock(&vdev->v4l2_dev->lock);
+ list_add_tail(&vdev->list, &vdev->v4l2_dev->vdevs);
+ spin_unlock(&vdev->v4l2_dev->lock);
return 0;
@@ -962,6 +967,9 @@ void video_unregister_device(struct video_device *vdev)
*/
clear_bit(V4L2_FL_REGISTERED, &vdev->flags);
mutex_unlock(&videodev_lock);
+ spin_lock(&vdev->v4l2_dev->lock);
+ list_del(&vdev->list);
+ spin_unlock(&vdev->v4l2_dev->lock);
device_unregister(&vdev->dev);
}
EXPORT_SYMBOL(video_unregister_device);
diff --git a/drivers/media/v4l2-core/v4l2-device.c
b/drivers/media/v4l2-core/v4l2-device.c
index 5b0a30b..cdb2d72 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -36,6 +36,7 @@ int v4l2_device_register(struct device *dev, struct
v4l2_device *v4l2_dev)
return -EINVAL;
INIT_LIST_HEAD(&v4l2_dev->subdevs);
+ INIT_LIST_HEAD(&v4l2_dev->vdevs);
spin_lock_init(&v4l2_dev->lock);
v4l2_prio_init(&v4l2_dev->prio);
kref_init(&v4l2_dev->ref);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index acbcd2f..a5a3401 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -84,6 +84,9 @@ struct v4l2_file_operations {
struct video_device
{
+ /* links into v4l2_device vdevs list */
+ struct list_head list;
+
#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_entity entity;
#endif
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 603e7f3..6484e54 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -46,6 +46,8 @@ struct v4l2_device {
#endif
/* used to keep track of the registered subdevs */
struct list_head subdevs;
+ /* used to keep track of the registered video_devices */
+ struct list_head vdevs;
/* lock this struct; can be used by the driver as well if this
struct is embedded into a larger struct. */
spinlock_t lock;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 06/15] v4l2-ctrls: add function to apply a request.
From: Hans Verkuil
Drivers need to be able to select a specific request. Add a new function that
can
be used to apply a given request.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls.c | 112 +--
include/media/v4l2-ctrls.h | 3 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 110 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d28035c..93c51cc 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1691,8 +1691,10 @@ static void new_to_cur(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 ch_flags)
/* Helper function: copy the new control value to the request */
static void new_to_request(struct v4l2_ctrl *ctrl)
{
- if (ctrl)
+ if (ctrl) {
ptr_to_ptr(ctrl, ctrl->p_new, ctrl->request->ptr);
+ ctrl->request->applied = 0;
+ }
}
/* Copy the current value to the new value */
@@ -1703,6 +1705,17 @@ static void cur_to_new(struct v4l2_ctrl *ctrl)
ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new);
}
+static void request_to_new(struct v4l2_ctrl *ctrl)
+{
+ if (ctrl == NULL)
+ return;
+ if (ctrl->request)
+ ptr_to_ptr(ctrl, ctrl->request->ptr, ctrl->p_new);
+ else
+ ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new);
+ ctrl->is_new = true;
+}
+
/* Return non-zero if one or more of the controls in the cluster has a new
value that differs from the current value. */
static int cluster_changed(struct v4l2_ctrl *master)
@@ -2599,6 +2612,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
u32 id = qc->id & V4L2_CTRL_ID_MASK;
u32 req = qc->request;
struct v4l2_ctrl_ref *ref;
+ struct v4l2_ctrl_req *ctrl_req = NULL;
struct v4l2_ctrl *ctrl;
if (hdl == NULL || req > USHRT_MAX)
@@ -2662,8 +2676,11 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
if (!ref)
return -EINVAL;
- if (req && get_request(ref->ctrl, req) == NULL)
- return -EINVAL;
+ if (req) {
+ ctrl_req = get_request(ref->ctrl, req);
+ if (ctrl_req == NULL)
+ return -EINVAL;
+ }
ctrl = ref->ctrl;
memset(qc, 0, sizeof(*qc));
@@ -2673,6 +2690,10 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
qc->id = ctrl->id;
strlcpy(qc->name, ctrl->name, sizeof(qc->name));
qc->flags = ctrl->flags;
+ if (req) {
+ if (ctrl_req->applied)
+ qc->flags |= V4L2_CTRL_FLAG_REQ_APPLIED;
+ }
qc->max_reqs = ctrl->max_reqs;
qc->type = ctrl->type;
if (ctrl->is_ptr)
@@ -2836,7 +2857,7 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler
*hdl,
bool get)
{
u32 ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
- unsigned request = cs->request & 0x;
+ unsigned request = cs->request & USHRT_MAX;
struct v4l2_ctrl_helper *h;
bool have_clusters = false;
u32 i;
@@ -3223,7 +3244,7 @@ static void update_from_auto_cluster(struct v4l2_ctrl
*master)
if (master->cluster[i] == NULL)
continue;
cur_to_new(master->cluster[i]);
- master->cluster[i]->request = 0;
+ master->cluster[i]->request = NULL;
}
if (!call_op(master, g_volatile_ctrl))
for (i = 1; i < master->ncontrols; i++)
@@ -3374,7 +3395,6 @@ static int set_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl
*ctrl, u32 ch_flags)
if (master->cluster[i] == NULL)
continue;
master->cluster[i]->is_new = 0;
- master->cluster[i]->request = NULL;
}
ret = validate_new(ctrl, ctrl->p_new);
@@ -3466,6 +3486,86 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl *ctrl,
const char *s)
}
EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string);
+int v4l2_ctrl_apply_request(struct v4l2_ctrl_handler *hdl, unsigned request)
+{
+ struct v4l2_ctrl_ref *ref;
+ bool found_request = false;
+ int ret = 0;
+ unsigned i;
+
+ if (hdl == NULL)
+ return -EINVAL;
+ if (request == 0)
+ return 0;
+
+ mutex_lock(hdl->lock);
+
+ list_for_each_entry(ref, &hdl->ctrl_refs, node) {
+ struct v4l2_ctrl *master;
+ bool apply_request = false;
+
+ if (ref->ctrl->max_reqs == 0)
+ continue;
+ master = ref->ctrl->cluster[0];
+ if (ref->ctrl != master)
+ continue;
+ if (master->handler != hdl)
+ v4l2_ctrl_lock(master);
+
[RFCv2 PATCH 05/15] v4l2-ctrls: add request support
From: Hans Verkuil
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls.c | 228 ++-
drivers/media/v4l2-core/v4l2-ioctl.c | 8 ++
include/media/v4l2-ctrls.h | 20 +++
3 files changed, 228 insertions(+), 28 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index e3a3468..d28035c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1235,6 +1235,96 @@ static void send_event(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev->fh, &ev);
}
+static int alloc_requests(struct v4l2_ctrl *ctrl)
+{
+ unsigned i;
+
+ ctrl->request_lists = kmalloc_array(VIDEO_MAX_FRAME,
+ sizeof(struct list_head), GFP_KERNEL);
+ if (ctrl->request_lists == NULL)
+ return -ENOMEM;
+ for (i = 0; i < VIDEO_MAX_FRAME; i++)
+ INIT_LIST_HEAD(ctrl->request_lists + i);
+ return 0;
+}
+
+static struct v4l2_ctrl_req *get_request(struct v4l2_ctrl *ctrl, unsigned
request)
+{
+ struct v4l2_ctrl_req *req;
+ struct list_head *head;
+
+ if (ctrl->request_lists == NULL)
+ return NULL;
+ head = ctrl->request_lists + (request % VIDEO_MAX_FRAME);
+ list_for_each_entry(req, head, node)
+ if (req->request == request)
+ return req;
+ return NULL;
+}
+
+static int add_request(struct v4l2_ctrl *ctrl, unsigned request,
+ struct v4l2_ctrl_req **p_req)
+{
+ struct v4l2_ctrl_req *req;
+ struct list_head *head;
+ unsigned idx;
+ int ret;
+
+ if (ctrl->nr_of_requests == ctrl->max_reqs)
+ return -ENOSPC;
+
+ if (ctrl->request_lists == NULL) {
+ ret = alloc_requests(ctrl);
+ if (ret)
+ return ret;
+ }
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (req == NULL)
+ return -ENOMEM;
+ req->ptr.p = kcalloc(ctrl->elems, ctrl->elem_size, GFP_KERNEL);
+ req->request = request;
+ if (req->ptr.p == NULL) {
+ kfree(req);
+ return -ENOMEM;
+ }
+ head = ctrl->request_lists + (request % VIDEO_MAX_FRAME);
+ for (idx = 0; idx < ctrl->elems; idx++)
+ ctrl->type_ops->init(ctrl, idx, req->ptr);
+ ctrl->nr_of_requests++;
+ list_add(&req->node, head);
+ if (p_req)
+ *p_req = req;
+ return 0;
+}
+
+static void del_request(struct v4l2_ctrl *ctrl, struct v4l2_ctrl_req *req)
+{
+ list_del(&req->node);
+ ctrl->nr_of_requests--;
+ kfree(req->ptr.p);
+ kfree(req);
+}
+
+static void free_requests(struct v4l2_ctrl *ctrl)
+{
+ unsigned idx;
+
+ if (!ctrl->request_lists)
+ return;
+
+ for (idx = 0; idx < VIDEO_MAX_FRAME; idx++) {
+ struct list_head *head = ctrl->request_lists + idx;
+
+ while (!list_empty(head)) {
+ struct v4l2_ctrl_req *req =
+ list_first_entry(head, struct v4l2_ctrl_req,
node);
+ del_request(ctrl, req);
+ }
+ }
+ kfree(ctrl->request_lists);
+ ctrl->request_lists = NULL;
+}
+
static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr1,
union v4l2_ctrl_ptr ptr2)
@@ -1482,6 +1572,15 @@ static int cur_to_user(struct v4l2_ext_control *c,
return ptr_to_user(c, ctrl, ctrl->p_cur);
}
+/* Helper function: copy the request's control value back to the caller */
+static int request_to_user(struct v4l2_ext_control *c,
+ struct v4l2_ctrl *ctrl)
+{
+ if (ctrl->request == NULL)
+ return ptr_to_user(c, ctrl, ctrl->p_new);
+ return ptr_to_user(c, ctrl, ctrl->request->ptr);
+}
+
/* Helper function: copy the new control value back to the caller */
static int new_to_user(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl)
@@ -1589,6 +1688,13 @@ static void new_to_cur(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 ch_flags)
}
}
+/* Helper function: copy the new control value to the request */
+static void new_to_request(struct v4l2_ctrl *ctrl)
+{
+ if (ctrl)
+ ptr_to_ptr(ctrl, ctrl->p_new, ctrl->request->ptr);
+}
+
/* Copy the current value to the new value */
static void cur_to_new(struct v4l2_ctrl *ctrl)
{
@@ -1750,6 +1856,7 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl)
list_del(&ctrl->node);
list_for_each_entry_safe(sev, next_sev, &ctrl->ev_subs, node)
list_del(&sev->node);
+ free_requests(ctrl);
kfree(ctrl);
}
kfree(hdl->buckets);
@@ -2095,8 +2202,10 @
[RFCv2 PATCH 03/15] videodev2.h: add request field to v4l2_buffer.
From: Hans Verkuil
When queuing buffers allow for passing the request ID that
should be associated with this buffer. Split the u32 reserved2 field
into two u16 fields, one for request, one with the old reserved2 name.
Signed-off-by: Hans Verkuil
---
drivers/media/usb/cpia2/cpia2_v4l.c | 1 +
drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +++-
drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++--
drivers/media/v4l2-core/videobuf2-core.c | 3 +++
include/uapi/linux/videodev2.h| 4 +++-
5 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c
b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea83..01c596a 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -952,6 +952,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct
v4l2_buffer *buf)
buf->sequence = cam->buffers[buf->index].seq;
buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
buf->length = cam->frame_size;
+ buf->request = 0;
buf->reserved2 = 0;
buf->reserved = 0;
memset(&buf->timecode, 0, sizeof(buf->timecode));
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index af63543..9d5c380 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -326,7 +326,8 @@ struct v4l2_buffer32 {
__s32 fd;
} m;
__u32 length;
- __u32 reserved2;
+ __u16 request;
+ __u16 reserved2;
__u32 reserved;
};
@@ -491,6 +492,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct
v4l2_buffer32 __user
put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
copy_to_user(&up->timecode, &kp->timecode, sizeof(struct
v4l2_timecode)) ||
put_user(kp->sequence, &up->sequence) ||
+ put_user(kp->request, &up->request) ||
put_user(kp->reserved2, &up->reserved2) ||
put_user(kp->reserved, &up->reserved))
return -EFAULT;
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 74af586..d2d1fb1 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -438,14 +438,14 @@ static void v4l_print_buffer(const void *arg, bool
write_only)
const struct v4l2_plane *plane;
int i;
- pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, "
+ pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request=%u, "
"flags=0x%08x, field=%s, sequence=%d, memory=%s",
p->timestamp.tv_sec / 3600,
(int)(p->timestamp.tv_sec / 60) % 60,
(int)(p->timestamp.tv_sec % 60),
(long)p->timestamp.tv_usec,
p->index,
- prt_names(p->type, v4l2_type_names),
+ prt_names(p->type, v4l2_type_names), p->request,
p->flags, prt_names(p->field, v4l2_field_names),
p->sequence, prt_names(p->memory, v4l2_memory_names));
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 1329dcc..a0e4e84 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -657,6 +657,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
struct v4l2_buffer *b)
/* Copy back data such as timestamp, flags, etc. */
memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
+ b->request = vb->v4l2_buf.request;
b->reserved2 = vb->v4l2_buf.reserved2;
b->reserved = vb->v4l2_buf.reserved;
@@ -682,6 +683,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
struct v4l2_buffer *b)
else if (q->memory == V4L2_MEMORY_DMABUF)
b->m.fd = vb->v4l2_planes[0].m.fd;
}
+ b->request = vb->v4l2_buf.request;
/*
* Clear any buffer state related flags.
@@ -1352,6 +1354,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
*/
vb->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
}
+ vb->v4l2_buf.request = b->request;
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
/*
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 388e376..e59a5e3 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -752,6 +752,7 @@ struct v4l2_plane {
* @length:size in bytes of the buffer (NOT its payload) for single-plane
* buffers (when type != *_MPLANE); number of elements in the
* planes array f
[RFCv2 PATCH 04/15] vb2: add allow_requests flag
From: Hans Verkuil
The driver has to set allow_requests explicitly in order to allow
queuing or preparing buffers for a specific request ID.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 5 +
include/media/videobuf2-core.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index a0e4e84..da513b1 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1703,6 +1703,11 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q,
struct v4l2_buffer *b,
return -EINVAL;
}
+ if (!q->allow_requests && b->request) {
+ dprintk(1, "%s: unsupported request ID\n", opname);
+ return -EINVAL;
+ }
+
return __verify_planes_array(q->bufs[b->index], b);
}
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index a5790fd..a3e3596 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -338,6 +338,7 @@ struct v4l2_fh;
* @fileio_read_once: report EOF after reading the first buffer
* @fileio_write_immediately: queue buffer after each write() call
* @allow_zero_bytesused: allow bytesused == 0 to be passed to the driver
+ * @allow_requests:allow request != 0 to be passed to the driver
* @lock: pointer to a mutex that protects the vb2_queue struct. The
* driver can set this to a mutex to let the v4l2 core serialize
* the queuing ioctls. If the driver wants to handle locking
@@ -390,6 +391,7 @@ struct vb2_queue {
unsignedfileio_read_once:1;
unsignedfileio_write_immediately:1;
unsignedallow_zero_bytesused:1;
+ unsignedallow_requests:1;
struct mutex*lock;
struct v4l2_fh *owner;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 01/15] videodev2.h: add max_reqs to struct v4l2_query_ext_ctrl
From: Hans Verkuil
struct v4l2_query_ext_ctrl is extended with a new 'max_reqs' field to store
the maximum number of outstanding requests that contain this control.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ioctl.c | 5 +++--
include/uapi/linux/videodev2.h | 4 +++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index aa407cb..119121f 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -535,12 +535,13 @@ static void v4l_print_query_ext_ctrl(const void *arg,
bool write_only)
pr_cont("id=0x%x, type=%d, name=%.*s, min/max=%lld/%lld, "
"step=%lld, default=%lld, flags=0x%08x, elem_size=%u, elems=%u,
"
- "nr_of_dims=%u, dims=%u,%u,%u,%u\n",
+ "nr_of_dims=%u, dims=%u,%u,%u,%u, max_reqs=%u, request=%u\n",
p->id, p->type, (int)sizeof(p->name), p->name,
p->minimum, p->maximum,
p->step, p->default_value, p->flags,
p->elem_size, p->elems, p->nr_of_dims,
- p->dims[0], p->dims[1], p->dims[2], p->dims[3]);
+ p->dims[0], p->dims[1], p->dims[2], p->dims[3],
+ p->max_reqs, p->request);
}
static void v4l_print_querymenu(const void *arg, bool write_only)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index fa376f7..2269152 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1439,7 +1439,9 @@ struct v4l2_query_ext_ctrl {
__u32elems;
__u32nr_of_dims;
__u32dims[V4L2_CTRL_MAX_DIMS];
- __u32reserved[32];
+ __u32max_reqs;
+ __u32request;
+ __u32reserved[30];
};
/* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 02/15] videodev2.h: add request to v4l2_ext_controls
From: Hans Verkuil
The ctrl_class is fairly pointless when used with drivers that use the control
framework: you can just fill in 0 and it will just work fine. There are still
some old unconverted drivers that do not support 0 and instead want the control
class there. The idea being that all controls in the list all belong to that
class. This was done to simplify drivers in the absence of the control
framework.
When using the control framework the framework itself is smart enough to allow
controls of any class to be included in the control list.
Since request IDs are in the range 1..65535 (or so, in any case a relatively
small non-zero positive integer) it makes sense to effectively rename ctrl_class
to request. Set it to 0 and you get the normal behavior (you change the current
control value), set it to a request ID and you get/set the control for
that request.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ioctl.c | 7 +--
include/uapi/linux/videodev2.h | 5 -
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 119121f..74af586 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -563,8 +563,11 @@ static void v4l_print_ext_controls(const void *arg, bool
write_only)
const struct v4l2_ext_controls *p = arg;
int i;
- pr_cont("class=0x%x, count=%d, error_idx=%d",
- p->ctrl_class, p->count, p->error_idx);
+ if (V4L2_CTRL_ID2CLASS(p->ctrl_class))
+ pr_cont("class=0x%x, ", p->ctrl_class);
+ else
+ pr_cont("request=%u, ", p->request);
+ pr_cont("count=%d, error_idx=%d", p->count, p->error_idx);
for (i = 0; i < p->count; i++) {
if (!p->controls[i].size)
pr_cont(", id/val=0x%x/0x%x",
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2269152..388e376 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1382,7 +1382,10 @@ struct v4l2_ext_control {
} __attribute__ ((packed));
struct v4l2_ext_controls {
- __u32 ctrl_class;
+ union {
+ __u32 ctrl_class;
+ __u32 request;
+ };
__u32 count;
__u32 error_idx;
__u32 reserved[2];
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv2 PATCH 00/15] Request API
From: Hans Verkuil This patch series adds support for the request API (formerly called the configuration store API: http://www.spinics.net/lists/linux-media/msg81024.html). This second version takes into account all the feedback I received from various people and from the discussions in Düsseldorf last year. Utilities that understand requests are available here: http://git.linuxtv.org/cgit.cgi/hverkuil/v4l-utils.git/log/?h=requests The git repo with these patches is here: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=requests Note: this will be rebased to the latest media_tree master on a regular basis. The last patch adds a document going into more detail of how everything works, so refer to that patch to get a good overview of this functionality. The patch adding request support to vivid is also useful to look at. DocBook patches are missing since I am waiting for a driver that will actually need this. If anyone is working on such a driver, then please let me know. If anyone has questions, or if anyone has ideas or thinks additional support for some functionality is needed in the core, then let me know as well. Regards, Hans Hans Verkuil (15): videodev2.h: add max_reqs to struct v4l2_query_ext_ctrl videodev2.h: add request to v4l2_ext_controls videodev2.h: add request field to v4l2_buffer. vb2: add allow_requests flag v4l2-ctrls: add request support v4l2-ctrls: add function to apply a request. v4l2-ctrls: implement delete request(s) v4l2-ctrls: add VIDIOC_REQUEST_CMD v4l2: add initial V4L2_REQ_CMD_QUEUE support vb2: add helper function to queue request-specific buffer. v4l2-device: keep track of registered video_devices v4l2-device: add v4l2_device_req_queue vivid: add request support for video capture. v4l2-ctrls: add REQ_KEEP flag Documentation: add v4l2-requests.txt Documentation/video4linux/v4l2-requests.txt | 233 drivers/media/platform/vivid/vivid-core.c| 2 + drivers/media/platform/vivid/vivid-ctrls.c | 4 + drivers/media/platform/vivid/vivid-kthread-cap.c | 2 + drivers/media/usb/cpia2/cpia2_v4l.c | 1 + drivers/media/v4l2-core/v4l2-compat-ioctl32.c| 4 +- drivers/media/v4l2-core/v4l2-ctrls.c | 440 --- drivers/media/v4l2-core/v4l2-dev.c | 9 + drivers/media/v4l2-core/v4l2-device.c| 26 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 123 ++- drivers/media/v4l2-core/v4l2-subdev.c| 78 +++- drivers/media/v4l2-core/videobuf2-core.c | 26 ++ include/media/v4l2-ctrls.h | 35 +- include/media/v4l2-dev.h | 3 + include/media/v4l2-device.h | 7 + include/media/v4l2-fh.h | 4 + include/media/videobuf2-core.h | 3 + include/uapi/linux/videodev2.h | 38 +- 18 files changed, 957 insertions(+), 81 deletions(-) create mode 100644 Documentation/video4linux/v4l2-requests.txt -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] v4l: xilinx: harmless buffer overflow
My static checker warns that the name of the port can be 15 characters
when you consider the NUL terminator and that's one more than the 14
characters in name[]. Maybe it's an off-by-one?
It's unlikely that we hit the limit and even if we do the overflow will
only affect one of the two bytes of padding so it's harmless. Still
let's fix it and also change the sprintf() to snprintf().
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c
b/drivers/media/platform/xilinx/xilinx-dma.c
index efde88a..98e50e4 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -653,7 +653,7 @@ static const struct v4l2_file_operations xvip_dma_fops = {
int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
enum v4l2_buf_type type, unsigned int port)
{
- char name[14];
+ char name[16];
int ret;
dma->xdev = xdev;
@@ -725,7 +725,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev,
struct xvip_dma *dma,
}
/* ... and the DMA channel. */
- sprintf(name, "port%u", port);
+ snprintf(name, sizeof(name), "port%u", port);
dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
if (dma->dma == NULL) {
dev_err(dma->xdev->dev, "no VDMA channel found\n");
--
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
