Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Hi, Lots of good stuff in this thread! It seems Mauro has answered most things, so I'm just going to respond to this bit. On 09/07/2011 05:37 AM, Devin Heitmueller wrote: We've added a parameter for that on xawtv3 (--alsa-latency). We've parametrized it at the alsa stream function call. So, all it needs is to add a new parameter at tvtime config file. Ugh. We really need some sort of heuristic to do this. It's unreasonable to expect users to know about some magic parameter buried in a config file which causes it to start working. Perhaps a counter that increments whenever an underrun is hit, and after a certain number it automatically restarts the stream with a higher latency. Or perhaps we're just making some poor choice in terms of the buffers/periods for a given rate. This may have something to do with usb versus pci capture, on my bttv card 30 ms works fine, but I can imagine it being a bit on the low side when doing video + audio capture over USB. So maybe should default to say 50 for usb capture devices and 30 for pci capture devices? In the end if people load there system enough / have a slow enough system our default will always be wrong for them. All we can do is try to get a default which is sane for most setups ... Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Compiling on 2.6.32-31-generic fails (nightly build server has same problem)
I'm using Mythbuntu 10.04 LTS with the 2.6.32-31-generic kernel. I tried to compile the latest v4l code, and I'm getting the following compile error: /home/lowmanator/media_build/v4l/tda18271-common.c: In function '_tda_printk': /home/lowmanator/media_build/v4l/tda18271-common.c:682: error: storage size of 'vaf' isn't known /home/lowmanator/media_build/v4l/tda18271-common.c:682: warning: unused variable 'vaf' make[3]: *** [/home/lowmanator/media_build/v4l/tda18271-common.o] Error 1 make[2]: *** [_module_/home/lowmanator/media_build/v4l] Error 2 make[2]: Leaving directory `/usr/src/linux-headers-2.6.32-31-generic' make[1]: *** [default] Error 2 make[1]: Leaving directory `/home/lowmanator/media_build/v4l' make: *** [all] Error 2 Just to try to workaround the error for now (and just because I'm a sadist for failure), I've removed the entire tda_printk method from that module, hoping that my hd-pvr isn't using the tda18271 chip :) When I do this and recontinue the make, I then fail on the following error: CC [M] /home/lowmanator/media_build/v4l/imon.o /home/lowmanator/media_build/v4l/imon.c: In function 'send_packet': /home/lowmanator/media_build/v4l/imon.c:521: error: implicit declaration of function 'pr_err_ratelimited' make[3]: *** [/home/lowmanator/media_build/v4l/imon.o] Error 1 make[2]: *** [_module_/home/lowmanator/media_build/v4l] Error 2 make[2]: Leaving directory `/usr/src/linux-headers-2.6.32-31-generic' make[1]: *** [default] Error 2 make[1]: Leaving directory `/home/lowmanator/media_build/v4l' make: *** [all] Error 2 imon.c sounds a little more centralized than tda18271, so I didn't feel like ripping out the "send_packet" method :) I've stopped for now. I've also noticed that these errors are reported in the nightly builds for the last week or so (I don't have nightly logs from before that). Any idea how I can workaround these two errors (without changing out my entire kernel)? I have a brand new shiny F2 revision HD-PVR and I'd really like to use it... Thanks, Lothsahn -- 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 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 11:42 PM, Devin Heitmueller wrote: > One more thing worth noting before I quit for the night: > > What audio processor is on your WinTV USB 2 device? The DVC-90 has an > emp202. Perhaps the WInTV uses a different audio processor (while > still using an em2820 as the bridge)? That might explain why your > device advertises effectively only one capture rate (32), while mine > advertises a whole range (8-48). Just took a look at the driver code. Seems we are calling em28xx_analog_audio_set() even if it's not using vendor audio. And that function actually hard-codes the rate to 48KHz. So here's the question: if using snd-usb-audio, should we really be poking at the AC97 registers at all? It seems that doing such can just get the audio processor state out of sync with however snd-usb-audio set it up. For example, the snd-usb-audio driver may very well be configuring the audio to 32 KHz, and then we reset the chip's state to 48Khz when we start streaming without the snd-usb-audio driver's knowledge. It seems like we should only be setting up the AC97 registers if it's an AC97 audio processor *and* it's using vendor audio. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 11:37 PM, Devin Heitmueller wrote: > On Tue, Sep 6, 2011 at 11:29 PM, Mauro Carvalho Chehab > wrote: >>> Basically the above starts at the *maximum* capture resolution and >>> works its way down. One might argue that this heuristic makes more >>> sense anyway - why *wouldn't* you want the highest quality audio >>> possible by default (rather than the lowest)? >> >> That change makes sense to me. Yet, you should try to disable pulseaudio >> and see what's the _real_ speed that the audio announces. On Fedora, >> just removing pulsaudio-oss-plugin (or something like that) is enough. >> >> It seems doubtful that my em2820 WinTV USB2 is different than yours. >> I suspect that pulseaudio is passing the "extra" range, offering to >> interpolate the data. > > I disabled pulseaudio and the capture device is advertising the exact > same range (8-48 KHz). Seems to be behaving the same way as well. > > So while I'm usually willing to blame things on Pulse, this doesn't > look like the case here. > >>> Even with that patch though, I hit severe underrun/overrun conditions >>> at 30ms of latency (to the point where the audio is interrupted dozens >>> of times per second). >> >> Yes, it is the same here: 30ms on my notebook is not enough for WinTV >> USB2 (it is OK with HVR-950). >> >>> Turned it up to 50ms and it's much better. >>> That said, of course such a change would impact lipsync, so perhaps we >>> need to be adjusting the periods instead. >> >> We've added a parameter for that on xawtv3 (--alsa-latency). We've >> parametrized >> it at the alsa stream function call. So, all it needs is to add a new >> parameter >> at tvtime config file. > > Ugh. We really need some sort of heuristic to do this. It's > unreasonable to expect users to know about some magic parameter buried > in a config file which causes it to start working. Perhaps a counter > that increments whenever an underrun is hit, and after a certain > number it automatically restarts the stream with a higher latency. Or > perhaps we're just making some poor choice in terms of the > buffers/periods for a given rate. > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com > One more thing worth noting before I quit for the night: What audio processor is on your WinTV USB 2 device? The DVC-90 has an emp202. Perhaps the WInTV uses a different audio processor (while still using an em2820 as the bridge)? That might explain why your device advertises effectively only one capture rate (32), while mine advertises a whole range (8-48). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 11:29 PM, Mauro Carvalho Chehab wrote: >> Basically the above starts at the *maximum* capture resolution and >> works its way down. One might argue that this heuristic makes more >> sense anyway - why *wouldn't* you want the highest quality audio >> possible by default (rather than the lowest)? > > That change makes sense to me. Yet, you should try to disable pulseaudio > and see what's the _real_ speed that the audio announces. On Fedora, > just removing pulsaudio-oss-plugin (or something like that) is enough. > > It seems doubtful that my em2820 WinTV USB2 is different than yours. > I suspect that pulseaudio is passing the "extra" range, offering to > interpolate the data. I disabled pulseaudio and the capture device is advertising the exact same range (8-48 KHz). Seems to be behaving the same way as well. So while I'm usually willing to blame things on Pulse, this doesn't look like the case here. >> Even with that patch though, I hit severe underrun/overrun conditions >> at 30ms of latency (to the point where the audio is interrupted dozens >> of times per second). > > Yes, it is the same here: 30ms on my notebook is not enough for WinTV > USB2 (it is OK with HVR-950). > >> Turned it up to 50ms and it's much better. >> That said, of course such a change would impact lipsync, so perhaps we >> need to be adjusting the periods instead. > > We've added a parameter for that on xawtv3 (--alsa-latency). We've > parametrized > it at the alsa stream function call. So, all it needs is to add a new > parameter > at tvtime config file. Ugh. We really need some sort of heuristic to do this. It's unreasonable to expect users to know about some magic parameter buried in a config file which causes it to start working. Perhaps a counter that increments whenever an underrun is hit, and after a certain number it automatically restarts the stream with a higher latency. Or perhaps we're just making some poor choice in terms of the buffers/periods for a given rate. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Em 07-09-2011 00:20, Devin Heitmueller escreveu:
> On Tue, Sep 6, 2011 at 10:58 PM, Devin Heitmueller
> wrote:
>> On Tue, Sep 6, 2011 at 11:29 AM, Mauro Carvalho Chehab
>> wrote:
>>> There are several issues with the original alsa_stream code that got
>>> fixed on xawtv3, made by me and by Hans de Goede. Basically, the
>>> code were re-written, in order to follow the alsa best practises.
>>>
>>> Backport the changes from xawtv, in order to make it to work on a
>>> wider range of V4L and sound adapters.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab
>>
>> Mauro,
>>
>> What tuners did you test this patch with? I went ahead and did a git
>> pull of your patch series into my local git tree, and now my DVC-90
>> (an em28xx device) is capturing at 32 KHz instead of 48 (this is one
>> of the snd-usb-audio based devices, not em28xx-alsa).
>>
>> Note I tested immediately before pulling your patch series and the
>> audio capture was working fine.
>>
>> I think this patch series is going in the right direction in general,
>> but this patch in particular seems to cause a regression. Is this
>> something you want to investigate? I think we need to hold off on
>> pulling this series into the new tvtime master until this problem is
>> resolved.
>>
>> Devin
>>
>> --
>> Devin J. Heitmueller - Kernel Labs
>> http://www.kernellabs.com
>>
>
> Spent a few minutes digging into this. Looks like the snd-usb-audio
> driver advertises 8-48KHz. However, it seems that it only captures
> successfully at 48 KHz.
>
> I made the following hack and it started working:
>
> diff --git a/src/alsa_stream.c b/src/alsa_stream.c
> index b6a41a5..57e3c3d 100644
> --- a/src/alsa_stream.c
> +++ b/src/alsa_stream.c
> @@ -261,7 +261,7 @@ static int setparams(snd_pcm_t *phandle, snd_pcm_t
> *chandle,
> fprintf(error_fp, "alsa: Will search a common rate between %u and
> %u\n",
> ratemin, ratemax);
>
> -for (i = ratemin; i <= ratemax; i+= 100) {
> +for (i = ratemax; i >= ratemin; i-= 100) {
> err = snd_pcm_hw_params_set_rate_near(chandle, c_hwparams, &i, 0);
> if (err)
> continue;
>
> Basically the above starts at the *maximum* capture resolution and
> works its way down. One might argue that this heuristic makes more
> sense anyway - why *wouldn't* you want the highest quality audio
> possible by default (rather than the lowest)?
That change makes sense to me. Yet, you should try to disable pulseaudio
and see what's the _real_ speed that the audio announces. On Fedora,
just removing pulsaudio-oss-plugin (or something like that) is enough.
It seems doubtful that my em2820 WinTV USB2 is different than yours.
I suspect that pulseaudio is passing the "extra" range, offering to
interpolate the data.
> Even with that patch though, I hit severe underrun/overrun conditions
> at 30ms of latency (to the point where the audio is interrupted dozens
> of times per second).
Yes, it is the same here: 30ms on my notebook is not enough for WinTV
USB2 (it is OK with HVR-950).
> Turned it up to 50ms and it's much better.
> That said, of course such a change would impact lipsync, so perhaps we
> need to be adjusting the periods instead.
We've added a parameter for that on xawtv3 (--alsa-latency). We've parametrized
it at the alsa stream function call. So, all it needs is to add a new parameter
at tvtime config file.
> ALSA has never been my area of expertise, so I look to you and Hans to
> offer some suggestions.
>
> Devin
>
--
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 01/10] alsa_stream: port changes made on xawtv3
Em 06-09-2011 23:58, Devin Heitmueller escreveu: > On Tue, Sep 6, 2011 at 11:29 AM, Mauro Carvalho Chehab > wrote: >> There are several issues with the original alsa_stream code that >> got fixed on xawtv3, made by me and by Hans de Goede. Basically, >> the code were re-written, in order to follow the alsa best >> practises. >> >> Backport the changes from xawtv, in order to make it to work on a >> wider range of V4L and sound adapters. >> >> Signed-off-by: Mauro Carvalho Chehab > > Mauro, > > What tuners did you test this patch with? I tested with some em28xx-based devices, like HVR-950 and WinTV USB2. > I went ahead and did a git pull of your patch series into my local > git tree, and now my DVC-90 (an em28xx device) is capturing at 32 KHz > instead of 48 (this is one of the snd-usb-audio based devices, not > em28xx-alsa). The new approach tries to match an speed that it is compatible between the audio and the video card. The algorithm tries first to not use software interpolation for audio, as it would reduce the audio quality. If it can't do it without interpolation, it will enable interpolation and seek again. By default, pulseaudio does interpolation, if you request it to use a different resolution. > Note I tested immediately before pulling your patch series and the > audio capture was working fine. Had you test to disable pulseaudio and see what speeds your boards accept? If you enable verbose mode, you'll see more details about the device detection. For example, this is what I get here with hvr-950, calling "tvtime -v": alsa: starting copying alsa stream from hw:1,0 to hw:0,0 videoinput: Using video4linux2 driver 'em28xx', card 'Hauppauge WinTV HVR 950' (bus usb-:00:1d.7-1). videoinput: Version is 196608, capabilities 5030051. alsa: Capture min rate is 48000 alsa: Capture max rate is 48000 alsa: Playback min rate is 44100 alsa: Playback max rate is 192000 alsa: Will search a common rate between 48000 and 48000 alsa: Using Rate 48000 alsa: capture periods range between 2 and 98. Want: 2 alsa: capture period time range between 333 and 65334. Want: 15000 alsa: playback periods range between 2 and 32. Want: 4 alsa: playback period time range between 666 and 10922667. Want: 15000 alsa: capture period set to 2 periods of 15000 time alsa: playback period set to 4 periods of 15333 time alsa: Negociated configuration: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat: STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 2944 period_size : 736 period_time : 15333 tstamp_mode : NONE period_step : 1 avail_min: 736 period_event : 0 start_threshold : 1440 stop_threshold : 2944 silence_threshold: 0 silence_size : 0 boundary : 1543503872 stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat: STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 1440 period_size : 720 period_time : 15000 tstamp_mode : NONE period_step : 1 avail_min: 720 period_event : 0 start_threshold : 720 stop_threshold : 1440 silence_threshold: 0 silence_size : 0 boundary : 1509949440 alsa: Parameters are 48000Hz, S16_LE, 2 channels alsa: Set bitrate to 48000, buffer size is 1440 alsa: stream started from hw:1,0 to hw:0,0 (48000 Hz, buffer delay = 30,00 ms) And those are the results with WinTV USB2: videoinput: Using video4linux2 driver 'em28xx', card 'Hauppauge WinTV USB 2' (bus usb-:00:1d.7-1). videoinput: Version is 196608, capabilities 5030041. alsa: starting copying alsa stream from hw:1,0 to hw:0,0 alsa: Capture min rate is 32000 alsa: Capture max rate is 32000 alsa: Playback min rate is 44100 alsa: Playback max rate is 192000 alsa: Will search a common rate between 44100 and 32000 alsa: Couldn't find a rate that it is supported by both playback and capture alsa: Trying plughw:0,0 for playback alsa: Resample enabled. alsa: Capture min rate is 32000 alsa: Capture max rate is 32000 alsa: Playback min rate is 4000 alsa: Playback max rate is 4294967295 alsa: Will search a common rate between 32000 and 32000 alsa: Using Rate 32000 alsa: capture periods range between 2 and 1024. Want: 2 alsa: capture period time range between 500 and 4096000. Want: 15000 alsa: playback period time range between 333 and 5461334. Want: 15000 alsa: capture period set to 2 periods of 15000 time alsa: playback period set to 4 periods of 15000 time alsa: Negociated configuration: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat: STD channels : 2 rate : 32000 exact rate : 32000 (32000/1) msbits : 16 buffer_size : 1920 period_size : 480 period_time : 15000 tstamp_mode : NONE period_step : 1 avail_min: 480 period_event : 0 start_threshold : 960 stop_
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 10:58 PM, Devin Heitmueller
wrote:
> On Tue, Sep 6, 2011 at 11:29 AM, Mauro Carvalho Chehab
> wrote:
>> There are several issues with the original alsa_stream code that got
>> fixed on xawtv3, made by me and by Hans de Goede. Basically, the
>> code were re-written, in order to follow the alsa best practises.
>>
>> Backport the changes from xawtv, in order to make it to work on a
>> wider range of V4L and sound adapters.
>>
>> Signed-off-by: Mauro Carvalho Chehab
>
> Mauro,
>
> What tuners did you test this patch with? I went ahead and did a git
> pull of your patch series into my local git tree, and now my DVC-90
> (an em28xx device) is capturing at 32 KHz instead of 48 (this is one
> of the snd-usb-audio based devices, not em28xx-alsa).
>
> Note I tested immediately before pulling your patch series and the
> audio capture was working fine.
>
> I think this patch series is going in the right direction in general,
> but this patch in particular seems to cause a regression. Is this
> something you want to investigate? I think we need to hold off on
> pulling this series into the new tvtime master until this problem is
> resolved.
>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com
>
Spent a few minutes digging into this. Looks like the snd-usb-audio
driver advertises 8-48KHz. However, it seems that it only captures
successfully at 48 KHz.
I made the following hack and it started working:
diff --git a/src/alsa_stream.c b/src/alsa_stream.c
index b6a41a5..57e3c3d 100644
--- a/src/alsa_stream.c
+++ b/src/alsa_stream.c
@@ -261,7 +261,7 @@ static int setparams(snd_pcm_t *phandle, snd_pcm_t *chandle,
fprintf(error_fp, "alsa: Will search a common rate between %u and %u\n",
ratemin, ratemax);
-for (i = ratemin; i <= ratemax; i+= 100) {
+for (i = ratemax; i >= ratemin; i-= 100) {
err = snd_pcm_hw_params_set_rate_near(chandle, c_hwparams, &i, 0);
if (err)
continue;
Basically the above starts at the *maximum* capture resolution and
works its way down. One might argue that this heuristic makes more
sense anyway - why *wouldn't* you want the highest quality audio
possible by default (rather than the lowest)?
Even with that patch though, I hit severe underrun/overrun conditions
at 30ms of latency (to the point where the audio is interrupted dozens
of times per second). Turned it up to 50ms and it's much better.
That said, of course such a change would impact lipsync, so perhaps we
need to be adjusting the periods instead.
ALSA has never been my area of expertise, so I look to you and Hans to
offer some suggestions.
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 11:29 AM, Mauro Carvalho Chehab wrote: > There are several issues with the original alsa_stream code that got > fixed on xawtv3, made by me and by Hans de Goede. Basically, the > code were re-written, in order to follow the alsa best practises. > > Backport the changes from xawtv, in order to make it to work on a > wider range of V4L and sound adapters. > > Signed-off-by: Mauro Carvalho Chehab Mauro, What tuners did you test this patch with? I went ahead and did a git pull of your patch series into my local git tree, and now my DVC-90 (an em28xx device) is capturing at 32 KHz instead of 48 (this is one of the snd-usb-audio based devices, not em28xx-alsa). Note I tested immediately before pulling your patch series and the audio capture was working fine. I think this patch series is going in the right direction in general, but this patch in particular seems to cause a regression. Is this something you want to investigate? I think we need to hold off on pulling this series into the new tvtime master until this problem is resolved. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] ISP:BUILD:FIX: Move media_entity_init() and
Em 06-09-2011 11:47, Laurent Pinchart escreveu: > Hi Vaibhav, > > On Tuesday 06 September 2011 16:12:35 Hiremath, Vaibhav wrote: >> On Monday, September 05, 2011 6:20 PM Laurent Pinchart wrote: >>> On Sunday 04 September 2011 15:32:28 Mauro Carvalho Chehab wrote: >> >> >> >>> I don't mind splitting the config option. An alternative would be to >>> compile media_entity_init() and media_entity_cleanup() based on >>> CONFIG_MEDIA_SUPPORT instead of CONFIG_MEDIA_CONTROLLER, but that looks a >>> bit hackish to me. >>> Also, I don't like the idea of increasing drivers complexity for the existing drivers that work properly without MC. All those core conversions that were done in the last two years caused already too much instability to them. We should really avoid touching on them again for something that won't be adding any new feature nor fixing any known bug. >>> >>> We don't have to convert them all in one go right now, we can implement >>> pad-level operations support selectively when a subdev driver becomes used >>> by an MC-enabled host/bridge driver. >> >> I completely agree that we should not be duplicating the code just for sake >> of it. >> >> Isn't the wrapper approach seems feasible here? > > As explained in a previous e-mail, a wrapper sounds like a good approach to > me, to emulate video::* operations based on pad::* operations. We want to > move > to pad::* operations, so we should not perform emulation the other way around. We have 300+ drivers under /drivers/media. Just a few of them need MC API. Good sense says that we shouldn't touch on 300+ just because of half dozen drivers. So, if a wrapper is needed, it should be done for the other side. Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] uvcvideo: Fix crash when linking entities
The uvc_mc_register_entity() function wrongfully selects the media_entity associated with a UVC entity when creating links. This results in access to uninitialized media_entity structures and can hit a BUG_ON statement in media_entity_create_link(). Fix it. Signed-off-by: Laurent Pinchart --- drivers/media/video/uvc/uvc_entity.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) This patch should fix a v3.0 regression that results in a kernel crash as reported in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=637740 and https://bugzilla.redhat.com/show_bug.cgi?id=735437. Test results will be welcome. diff --git a/drivers/media/video/uvc/uvc_entity.c b/drivers/media/video/uvc/uvc_entity.c index 48fea37..29e2399 100644 --- a/drivers/media/video/uvc/uvc_entity.c +++ b/drivers/media/video/uvc/uvc_entity.c @@ -49,7 +49,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain *chain, if (remote == NULL) return -EINVAL; - source = (UVC_ENTITY_TYPE(remote) != UVC_TT_STREAMING) + source = (UVC_ENTITY_TYPE(remote) == UVC_TT_STREAMING) ? (remote->vdev ? &remote->vdev->entity : NULL) : &remote->subdev.entity; if (source == NULL) -- 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 01/10] alsa_stream: port changes made on xawtv3
Em 06-09-2011 18:18, Devin Heitmueller escreveu: > On Tue, Sep 6, 2011 at 3:12 PM, Mauro Carvalho Chehab > wrote: >>> From a practical standpoint, the Ubuntu folks have the original tvtime >>> tarball and all their changes in one patch, which is clearly a bunch >>> of patches that are mashed together probably in their build system. I >>> need to reach out to them to find where they have an actual SCM tree >>> or the individual patches. They've got a bunch of patches which would >>> be good to get into a single tree (autobuild fixes, cross-compilation, >>> locale updates, etc). >> >> Yeah, it seems interesting. Maybe we can get something from this place: >>http://packages.qa.debian.org/t/tvtime.html >> >> The maintainer there seems to be: >>http://qa.debian.org/developer.php?login=ba...@debian.org > > I reached out to the Ubuntu maintainer; we'll see if he gets back to > me. From what I can tell it seems like Debian is actually taking the > patches from Ubuntu (yes, I realize this is backwards from their > typical process where Ubuntu bases their stuff on Debian). Good! >>> In the long term I have no real issue with the LinuxTV group being the >>> official maintainer of record. I've got lots of ideas and things I >>> would like to do to improve tvtime, but in practice I've done a pretty >>> crappy job of maintaining the source (merging patches, etc) at this >>> point. >> >> Putting it on a common place and giving permissions to a group of people >> is interesting, as none of us are focused on userspace, so we all >> have a very limited amount of time for dealing with userspace applications. >> >> By giving commit rights to a group of developers, it ends that more >> developers will contribute, speeding up the development. >> >> That was what happened with v4l-utils and, on a minor scale, with xawtv3. >> >> If you're ok with that, I can set a tvtime git repository at LinuxTV, >> cloning the tree I've created there already (it is a pure conversion >> of your tree from mercurial into git, if I remove the patches I've >> done so far from your clone), giving you the ownership of the new tree, >> and marking it as a shared repository. > > I have no problem with this. Let's set it up. Ok. The repository is here: http://git.linuxtv.org/tvtime.git In thesis, everything is set for group usage. Please let me know if you experience any troubles with it. >> I have already all set there to allow shared access to the repository >> (in opposite to -hg, git works really cool with shared repositories). > > I actually haven't hosted any git repos on linuxtv.org before. I'm > assuming my ssh public key got copied over from when I was hosting hg > repos there? The same key is used, whatever you're committing to cvs, hg or git. The maintenance application for git is called git-menu. >> We can later add permissions to the developers interested on helping >> the tvtime maintenance that you agree to add. > > Sounds good. >From my side, I'm interested on helping with it. When I have some time, I'd like to fix a few issues with it. For example, there's a local cable operator that broadcasts some channels with PAL/M and others with NTSC/M (not a big deal for STBs and TV sets, as almost all support both standards here). However, tvtime, needs to be restarted every time it changes from one to the other, and it is not possible to set a per-channel standard. To be worse, when tvtime is restarted, it doesn't honor the "-d" option, with means that it will open my laptop's webcam instead of the TV card. > > As said earlier, Kernel Labs never really wanted to be the maintainer > for tvtime - we did it because nobody else wanted to (and vektor never > responded to emails I sent him offering to help). That said, a > community oriented approach is probably the best for everybody > involved. > > I'll probably be looking in the next couple of weeks to write some > fresh content for a tvtime website. The stuff on > tvtime.sourceforge.net is so dated almost none of it still applies. Yeah, that makes sense. > Thanks, > > Devin > -- 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 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 3:12 PM, Mauro Carvalho Chehab wrote: >> From a practical standpoint, the Ubuntu folks have the original tvtime >> tarball and all their changes in one patch, which is clearly a bunch >> of patches that are mashed together probably in their build system. I >> need to reach out to them to find where they have an actual SCM tree >> or the individual patches. They've got a bunch of patches which would >> be good to get into a single tree (autobuild fixes, cross-compilation, >> locale updates, etc). > > Yeah, it seems interesting. Maybe we can get something from this place: > http://packages.qa.debian.org/t/tvtime.html > > The maintainer there seems to be: > http://qa.debian.org/developer.php?login=ba...@debian.org I reached out to the Ubuntu maintainer; we'll see if he gets back to me. From what I can tell it seems like Debian is actually taking the patches from Ubuntu (yes, I realize this is backwards from their typical process where Ubuntu bases their stuff on Debian). >> In the long term I have no real issue with the LinuxTV group being the >> official maintainer of record. I've got lots of ideas and things I >> would like to do to improve tvtime, but in practice I've done a pretty >> crappy job of maintaining the source (merging patches, etc) at this >> point. > > Putting it on a common place and giving permissions to a group of people > is interesting, as none of us are focused on userspace, so we all > have a very limited amount of time for dealing with userspace applications. > > By giving commit rights to a group of developers, it ends that more > developers will contribute, speeding up the development. > > That was what happened with v4l-utils and, on a minor scale, with xawtv3. > > If you're ok with that, I can set a tvtime git repository at LinuxTV, > cloning the tree I've created there already (it is a pure conversion > of your tree from mercurial into git, if I remove the patches I've > done so far from your clone), giving you the ownership of the new tree, > and marking it as a shared repository. I have no problem with this. Let's set it up. > I have already all set there to allow shared access to the repository > (in opposite to -hg, git works really cool with shared repositories). I actually haven't hosted any git repos on linuxtv.org before. I'm assuming my ssh public key got copied over from when I was hosting hg repos there? > We can later add permissions to the developers interested on helping > the tvtime maintenance that you agree to add. Sounds good. As said earlier, Kernel Labs never really wanted to be the maintainer for tvtime - we did it because nobody else wanted to (and vektor never responded to emails I sent him offering to help). That said, a community oriented approach is probably the best for everybody involved. I'll probably be looking in the next couple of weeks to write some fresh content for a tvtime website. The stuff on tvtime.sourceforge.net is so dated almost none of it still applies. Thanks, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] [media] at91: add code to initialize and manage the ISI_MCK for Atmel ISI driver.
On 09/06/2011 10:05 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> I'm not entirely sure on this one, but as we had a similar situation with >> clocks, we decided to extablish the clock hierarchy in the board code, and >> only deal with the actual device clocks in the driver itself. I.e., we >> moved all clk_set_parent() and setting up the parent clock into the board. >> And I do think, this makes more sense, than doing this in the driver, not >> all users of this driver will need to manage the parent clock, right? > > I don't like to manage the clock in the board except if it's manadatory > otherwise > we manage this at soc level > > the driver does not have to manage the clock hierachy or detail implementation > but manage the clock enable/disable and speed depending on it's need We had a similar problem in the past and we ended up having the boot loader setting up the parent clock for the device clock. The driver only controls clock gating and sets its clock frequency based on an internal IP version information, derived from the SoC revision. AFAIK there is also a generic API at the runtime PM core so the driver can register the clock(s) with it and only use pm_runtime_clk_* calls afterwards. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Reserved fields in v4l2_mbus_framefmt, v4l2_subdev_format alignment
On Tue, Sep 06, 2011 at 09:10:17PM +0200, Sylwester Nawrocki wrote:
> Hi Sakari,
Hi Sylwester,
> On 09/05/2011 05:55 PM, Sakari Ailus wrote:
> > Hi all,
> >
> > I recently came across a few issues in the definitions of v4l2_subdev_format
> > and v4l2_mbus_framefmt when I was working on sensor control that I wanted to
> > bring up here. The appropriate structure right now look like this:
> >
> > include/linux/v4l2-subdev.h:
> > ---8<---
> > /**
> > * struct v4l2_subdev_format - Pad-level media bus format
> > * @which: format type (from enum v4l2_subdev_format_whence)
> > * @pad: pad number, as reported by the media API
> > * @format: media bus format (format code and frame size)
> > */
> > struct v4l2_subdev_format {
> > __u32 which;
> > __u32 pad;
> > struct v4l2_mbus_framefmt format;
> > __u32 reserved[8];
> > };
> > ---8<---
> >
> > include/linux/v4l2-mediabus.h:
> > ---8<---
> > /**
> > * struct v4l2_mbus_framefmt - frame format on the media bus
> > * @width: frame width
> > * @height: frame height
> > * @code: data format code (from enum v4l2_mbus_pixelcode)
> > * @field: used interlacing type (from enum v4l2_field)
> > * @colorspace: colorspace of the data (from enum v4l2_colorspace)
> > */
> > struct v4l2_mbus_framefmt {
> > __u32 width;
> > __u32 height;
> > __u32 code;
> > __u32 field;
> > __u32 colorspace;
> > __u32 reserved[7];
> > };
> > ---8<---
> >
> > Offering a lower level interface for sensors which allows better control of
> > them from the user space involves providing the link frequency to the user
> > space. While the link frequency will be a control, together with the bus
> > type and number of lanes (on serial links), this will define the pixel
> > clock.
> >
> > http://www.spinics.net/lists/linux-media/msg36492.html>
> >
> > After adding pixel clock to v4l2_mbus_framefmt there will be six reserved
> > fields left, one of which will be further possibly consumed by maximum image
> > size:
> >
> > http://www.spinics.net/lists/linux-media/msg35949.html>
>
> Yes, thanks for remembering about it. I have done some experiments with a
> sensor
> producing JPEG data and I'd like to add '__u32 framesamples' field to struct
> v4l2_mbus_framefmt, even though it solves only part of the problem.
> I'm not sure when I'll be able to get this finished though. I've just attached
> the initial patch now.
>
> >
> > Frame blanking (horizontal and vertical) and number of lanes might be needed
> > in the struct as well in the future, bringing the reserved count down to
> > two. I find this alarmingly low for a relatively new structure definition
> > which will potentially have a few different uses in the future.
>
> Sorry, could you explain why we need to put the blanking information in struct
> v4l2_mbus_framefmt ? I thought it had been initially agreed that the control
> framework will be used for this.
Configuration of blanking will be implemented as controls, yes.
Bandwidth calculation in the ISP driver may well need to know more detailed
information than just the maximum pixel rate. Averge rate over certain
period may also be important.
For example, take a sensor which is able to produce pixel rate of 200 Mp/s.
In the OMAP 3 ISP only the CSI2 block will be able to process pixels at such
rate. The ISP driver needs this information to be able to decide whether
it's safe to start streaming or not.
Higher momentary pixel rates are still possible as there are buffers between
some of the blocks. When using downscaling on sensors this gets more tricky.
There may be bursts of data which may overflow these buffers since the
sensors do not output data at amortised rate. Information on the sensor
(bursts) and size of the buffers is at least required to assess this
question.
I have a vague feeling we may need some of this data as part of the
v4l2_mbus_framefmt before we have a solution.
> > The another issue is that the size of the v4l2_subdev_format struct is not
> > aligned to a power of two. Instead of the intended 32 u32's, the size is
> > actually 22 u32's.
>
> hmm, is this really an issue ? What is advantage of having the structure size
> being the power of 2 ? Isn't multiple of 4 just enough ?
A power of two has been considered a good practice. It's also how kmalloc
will allocate memory for the duration of the ioctl call. Typical sizes can
be found in /proc/slabinfo. For small sizes also some non-power of two sizes
appear available, at least on my machines.
I'm not sure about the allocation in user space.
> > The interface is present in the 3.0 and marked experimental. My proposal is
> > to add reserved fields to v4l2_mbus_framefmt to extend its size up to 32
> > u32's. I understand there are already few which use the interface right now
> > and
Re: [PATCH 0/19 v4] s5p-fimc driver conversion to media controller and control framework
Em 03-09-2011 13:32, Sylwester Nawrocki escreveu: > On 09/01/2011 05:30 PM, Sylwester Nawrocki wrote: >> Hello, >> >> following is a fourth version of the patchset converting s5p-fimc driver >> to the media controller API and the new control framework. >> >> Mauro, could you please have a look at the patches and let me know of any >> doubts? >> I tried to provide possibly detailed description of what each patch does and >> why. >> >> The changeset is available at: >>http://git.infradead.org/users/kmpark/linux-2.6-samsung >>branch: v4l_fimc_for_mauro >> >> on top of patches from Marek's 'Videobuf2& FIMC fixes" pull request >> which this series depends on. > ... >> >> Sylwester Nawrocki (19): >>s5p-fimc: Remove registration of video nodes from probe() >>s5p-fimc: Remove sclk_cam clock handling >>s5p-fimc: Limit number of available inputs to one >>s5p-fimc: Remove sensor management code from FIMC capture driver >>s5p-fimc: Remove v4l2_device from video capture and m2m driver >>s5p-fimc: Add the media device driver >>s5p-fimc: Conversion to use struct v4l2_fh >>s5p-fimc: Convert to the new control framework >>s5p-fimc: Add media operations in the capture entity driver >>s5p-fimc: Add PM helper function for streaming control >>s5p-fimc: Correct color format enumeration >>s5p-fimc: Convert to use media pipeline operations >>s5p-fimc: Add subdev for the FIMC processing block >>s5p-fimc: Add support for JPEG capture >>s5p-fimc: Add v4l2_device notification support for single frame >> capture >>s5p-fimc: Use consistent names for the buffer list functions >>s5p-fimc: Add runtime PM support in the camera capture driver >>s5p-fimc: Correct crop offset alignment on exynos4 >>s5p-fimc: Remove single-planar capability flags > > oops, I've done this posting wrong, the first patch is missing here :( > -> s5p-fimc: Add media entity initialization > > Still the patch set is complete at git repository as indicated above. > I'm sorry for the confusion. No problem. I always check from git. Patches applied, thanks! Mauro > > -- > Regards, > Sylwester > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] [media] at91: add code to initialize and manage the ISI_MCK for Atmel ISI driver.
On 08:54 Tue 06 Sep , Guennadi Liakhovetski wrote:
> On Tue, 6 Sep 2011, Josh Wu wrote:
>
> > This patch enable the configuration for ISI_MCK, which is provided by
> > programmable clock.
> >
> > Signed-off-by: Josh Wu
> > ---
> > drivers/media/video/atmel-isi.c | 60
> > ++-
> > include/media/atmel-isi.h |4 ++
> > 2 files changed, 63 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/media/video/atmel-isi.c
> > b/drivers/media/video/atmel-isi.c
> > index 7b89f00..768bf59 100644
> > --- a/drivers/media/video/atmel-isi.c
> > +++ b/drivers/media/video/atmel-isi.c
> > @@ -90,7 +90,10 @@ struct atmel_isi {
> > struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
> >
> > struct completion complete;
> > + /* ISI peripherial clock */
> > struct clk *pclk;
> > + /* ISI_MCK, provided by PCK */
> > + struct clk *mck;
> > unsigned intirq;
> >
> > struct isi_platform_data*pdata;
> > @@ -763,6 +766,10 @@ static int isi_camera_add_device(struct
> > soc_camera_device *icd)
> > if (ret)
> > return ret;
> >
> > + ret = clk_enable(isi->mck);
> > + if (ret)
> > + return ret;
> > +
>
> Don't you have to disable the pixel clock (isi->pclk), that you just have
> enabled above this hunk, on the above error path?
>
> > isi->icd = icd;
> > dev_dbg(icd->parent, "Atmel ISI Camera driver attached to camera %d\n",
> > icd->devnum);
> > @@ -776,6 +783,7 @@ static void isi_camera_remove_device(struct
> > soc_camera_device *icd)
> >
> > BUG_ON(icd != isi->icd);
> >
> > + clk_disable(isi->mck);
> > clk_disable(isi->pclk);
> > isi->icd = NULL;
> >
> > @@ -882,6 +890,49 @@ static struct soc_camera_host_ops
> > isi_soc_camera_host_ops = {
> > };
> >
> > /*
> > ---*/
> > +/* Initialize ISI_MCK clock, called by atmel_isi_probe() function */
> > +static int initialize_mck(struct platform_device *pdev,
> > + struct atmel_isi *isi)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct isi_platform_data *pdata = dev->platform_data;
> > + struct clk *pck_parent;
> > + int ret;
> > +
> > + if (!strlen(pdata->pck_name) || !strlen(pdata->pck_parent_name))
> > + return -EINVAL;
> > +
> > + /* ISI_MCK is provided by PCK clock */
> > + isi->mck = clk_get(dev, pdata->pck_name);
>
> I think, it's still not what Russell meant. Look at
or what I ask you too
> drivers/mmc/host/atmel-mci.c:
>
> host->mck = clk_get(&pdev->dev, "mci_clk");
>
> and in arch/arm/mach-at91/at91sam9g45.c they've got
>
> CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.0", &mmc0_clk),
> CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.1", &mmc1_clk),
>
> where
>
> #define CLKDEV_CON_DEV_ID(_con_id, _dev_id, _clk) \
> { \
> .con_id = _con_id, \
> .dev_id = _dev_id, \
> .clk = _clk,\
> }
>
> I.e., in the device driver (mmc in this case) you only use the (platform)
> device instance, whose dev_name(dev) is then matched against one of clock
> lookups above, and a connection ID, which is used in case your device is
> using more than one clock. In the ISI case, your pck1 clock, that you seem
> to need here, doesn't have a clock lookup object, so, you might have to
> add one, and then use its connection ID.
>
> > + if (IS_ERR(isi->mck)) {
> > + dev_err(dev, "Failed to get PCK: %s\n", pdata->pck_name);
> > + return PTR_ERR(isi->mck);
> > + }
> > +
> > + pck_parent = clk_get(dev, pdata->pck_parent_name);
> > + if (IS_ERR(pck_parent)) {
> > + ret = PTR_ERR(pck_parent);
> > + dev_err(dev, "Failed to get PCK parent: %s\n",
> > + pdata->pck_parent_name);
> > + goto err_init_mck;
> > + }
> > +
> > + ret = clk_set_parent(isi->mck, pck_parent);
>
> I'm not entirely sure on this one, but as we had a similar situation with
> clocks, we decided to extablish the clock hierarchy in the board code, and
> only deal with the actual device clocks in the driver itself. I.e., we
> moved all clk_set_parent() and setting up the parent clock into the board.
> And I do think, this makes more sense, than doing this in the driver, not
> all users of this driver will need to manage the parent clock, right?
I don't like to manage the clock in the board except if it's manadatory
otherwise
we manage this at soc level
the driver does not have to manage the clock hierachy or detail implementation
but manage the clock enable/disable and speed depending on it's need
Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the bod
Re: [PATCH v4] s5p-fimc: Add runtime PM support in the mem-to-mem driver
On 09/05/2011 11:20 PM, Sakari Ailus wrote:
+static int fimc_suspend(struct device *dev)
+{
+ struct fimc_dev *fimc = dev_get_drvdata(dev);
+
+ dbg("fimc%d: state: 0x%lx", fimc->id, fimc->state);
+
+ if (test_and_set_bit(ST_LPM,&fimc->state))
+ return 0;
+ if (fimc_capture_busy(fimc))
+ return fimc_capture_suspend(fimc);
>>>
>>> Now that fimc_capture_suspend() returns -EBUSY always, is this intended
>>> behavious or do you plan to change this later on?
>>
>> No, it's by no means the intended behaviour. This patch is only a part of the
>> whole picture, but I thought it's independent from the MC related patches
>> which are on hold and could be merged independently. Moreover the FIMC driver
>> is broken without this patch on Exynos4, if the boot loader doesn't enable
>> the related power domain permanently. So I thought it should be merged
>> regardless of the fate of the capture PM support patch which depends on the
>> MC related patches.
>
> Right, I agree the patch has enough merits for merging.
>
>> Here is the capture PM patch for your critics;) http://tinyurl.com/4yj8z4t
>
> I'll take a look at it once you post it on the list. ;)
It's been on the lists for some time already, this is the fourth version:
https://patchwork.kernel.org/patch/1119562/
However it doesn't include a small fix I have added after posting v4,
which is available in the above git repository.
>>>
>>> Not that it'd be really easy to do this properly; the sensors, for example,
>>> probably need a clock from the ISP and I2C before they can continue. The
>>> OMAP 3 ISP driver does attempt to do this but doesn't handle these
>>> dependencies.
>>
>> I'm not handling the device PM dependencies explicitly in this driver either.
>>
>> But it's assured the I2C bus device is registered first, then the camera host
>> device, and finally the I2C client devices.
>> AFAIU the PM core should call PM suspend helpers on the subdev/host drivers
>> in order: I2C clients, the camera host and I2C bus. And for the resume
>> helpers
>> the sequence should be reversed.
>
> In my understanding it is not ensured that the I2C bus driver starts before
> the media device parent does (as it is controlling the sensors' power
> state). The same goes for suspend. Or am I missing something?
AFAIU PM core maintains private list of its active devices which it then walks
when preparing, suspending, resuming and 'completing' subsystems.
Please check pm_device_add() and dpm_start_suspend() for instance. It looks like
the list can only be reordered through returning -EAGAIN from subsystem prepare
helper or by calling implicitly pm_device_move().
I might be missing some important details though, it would be best to clarify
these things on linux-pm ML.
>
>> The sensor drivers do not implement their standard PM helper callbacks,
>> their are just controlled directly through s_power op by the host driver.
>>
>>>
>>> I'm not suggesting this should be part of the patch, just thought of asking
>>> it. :)
>>
>> First of all I'm not entirely happy with this code. The are some issues in
>> the v4l2-mem2mem framework which I plan to address when time permits. I think
>> it wasn't designed in PM use cases in mind. Plus PM support in Exynos4
>> platform
>> (including drivers) is rather not yet stable in the mainline kernel. So I was
>> having hard time to make this PM code working in the mem-to-mem device.
>> But it's now done and only a per frame clock gating is still missing.
>> This is a quite complex topic, to get everything right, in line with all
>> frameworks involved.
>>
>>>
+ return fimc_m2m_suspend(fimc);
>>>
>>> Does pending mean there are further images to process in a queue, or just
>>> that driver is busy one?
>>
>> It means the driver got an ownership of a pair of buffers and is about to or
>> is already processing them. In any case fimc_m2m_suspend() will wait for
>> only those two buffers to be processed, without dequeuing them back to user
>> space. They will be returned back to user space when the driver's resume
>> helper
>> is called.
>
> I think this is a good approach. Processing the buffers takes a fraction of
> a second. If one would cancel this it would unnecessarily complicate the
> user space.
Yes, and applications should not really care much about device power state
transitions.
>
>>>
+#endif /* CONFIG_PM_SLEEP */
+
>> ...
diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c
b/drivers/media/video/s5p-fimc/fimc-reg.c
index 4893b2d..938dadf 100644
--- a/drivers/media/video/s5p-fimc/fimc-reg.c
+++ b/drivers/media/video/s5p-fimc/fimc-reg.c
@@ -30,7 +30,7 @@ void fimc_hw_reset(struct fimc_dev *dev)
cfg = readl(dev->regs + S5P_CIGCTRL);
cfg |= (S5P_CIGCTRL_SWRST | S5P_CIGCTRL_IRQ_LEVEL);
writel(cfg, dev->regs + S5P_CIGCTRL);
- udelay(1000);
+ udelay(10);
>>>
>>> Good c
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Em 06-09-2011 15:41, Devin Heitmueller escreveu: > On Tue, Sep 6, 2011 at 2:19 PM, Hans de Goede wrote: >> I think that what should be done is contact the debian / ubuntu maintainers, >> get any interesting fixes they have which the kl version misses merged, >> and then just declare the kl version as being the new official upstream >> (with the blessing of the debian / ubuntu guys, and if possible also >> with the blessing of the original authors). > > It has always been my intention to get the Debian/Ubuntu patches > merged (as well as other distros). My thoughts behind renaming were > oriented around the notion that that there are more distros out there > than just Fedora/Ubuntu/Debian, but that may be something that isn't > really a concern. Also, I had no idea whether the distros would > actually switch over to the Kernel Labs version as the official > upstream source, so providing it under a different name would in > theory allow both packages to be available in parallel. > > From a practical standpoint, the Ubuntu folks have the original tvtime > tarball and all their changes in one patch, which is clearly a bunch > of patches that are mashed together probably in their build system. I > need to reach out to them to find where they have an actual SCM tree > or the individual patches. They've got a bunch of patches which would > be good to get into a single tree (autobuild fixes, cross-compilation, > locale updates, etc). Yeah, it seems interesting. Maybe we can get something from this place: http://packages.qa.debian.org/t/tvtime.html The maintainer there seems to be: http://qa.debian.org/developer.php?login=ba...@debian.org >> This would require kl git to be open to others for pushing, or we >> could move the tree to git.linuxtv.org (which I assume may be >> easier then for you to make the necessary changes to give >> others push rights on kl.org). > > Kernel Labs has never really had any real interest in "owning" tvtime. > I just setup the hg tree in an effort to get all the distro patches > in one place and have something that builds against current kernels > (and on which I can add improvements/fixes without users having to > deal with patches). At the time there was also nobody who clearly had > the desire to serve as an official maintainer. > > In the long term I have no real issue with the LinuxTV group being the > official maintainer of record. I've got lots of ideas and things I > would like to do to improve tvtime, but in practice I've done a pretty > crappy job of maintaining the source (merging patches, etc) at this > point. Putting it on a common place and giving permissions to a group of people is interesting, as none of us are focused on userspace, so we all have a very limited amount of time for dealing with userspace applications. By giving commit rights to a group of developers, it ends that more developers will contribute, speeding up the development. That was what happened with v4l-utils and, on a minor scale, with xawtv3. If you're ok with that, I can set a tvtime git repository at LinuxTV, cloning the tree I've created there already (it is a pure conversion of your tree from mercurial into git, if I remove the patches I've done so far from your clone), giving you the ownership of the new tree, and marking it as a shared repository. I have already all set there to allow shared access to the repository (in opposite to -hg, git works really cool with shared repositories). We can later add permissions to the developers interested on helping the tvtime maintenance that you agree to add. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 2:43 PM, Hans de Goede wrote: > Hi, > > On 09/06/2011 08:35 PM, Michael Krufky wrote: >> >> On Tue, Sep 6, 2011 at 2:19 PM, Hans de Goede wrote: >>> >>> Hi, >>> >>> On 09/06/2011 06:24 PM, Devin Heitmueller wrote: >>> >>> >>> I've been thinking for a while that perhaps the project should be renamed (or I considered prepending "kl" onto the front resulting in it being called "kl-tvtime"). This isn't out of vanity but rather my concern that the fork will get confused with the original project (for example, I believe Ubuntu actually already calls their modified tree tvtime 1.0.3). I'm open to suggestions in this regards. >>> >>> I think that what should be done is contact the debian / ubuntu >>> maintainers, >>> get any interesting fixes they have which the kl version misses merged, >>> and then just declare the kl version as being the new official upstream >>> (with the blessing of the debian / ubuntu guys, and if possible also >>> with the blessing of the original authors). >>> >>> This would require kl git to be open to others for pushing, or we >>> could move the tree to git.linuxtv.org (which I assume may be >>> easier then for you to make the necessary changes to give >>> others push rights on kl.org). >> >> Hans, >> >> Everybody is welcome to contribute to open source projects, but global >> contribution doesn't mean that a given server be opened up to commits >> by the general public. > > I didn't write open to commits by the general public, now did I? I wrote > open to commits by others. For most upstream projects it is quite normal > that several people have push rights to the master tree. This actually > is quite a good idea, as it avoids adding a SPOF into the chain. It > means development can continue if one of the maintainers is on vacation > for a a few weeks, or just having a period in his/her life where he > is too busy to actively contribute to a spare time project. Hans, Now I understand -- that's completely reasonable. It looks like Devin is happy having the tree hosted on linuxtv.org anyway, so no worries :-) Sorry for the misunderstanding. Best Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Reserved fields in v4l2_mbus_framefmt, v4l2_subdev_format alignment
Hi Sakari,
On 09/05/2011 05:55 PM, Sakari Ailus wrote:
> Hi all,
>
> I recently came across a few issues in the definitions of v4l2_subdev_format
> and v4l2_mbus_framefmt when I was working on sensor control that I wanted to
> bring up here. The appropriate structure right now look like this:
>
> include/linux/v4l2-subdev.h:
> ---8<---
> /**
> * struct v4l2_subdev_format - Pad-level media bus format
> * @which: format type (from enum v4l2_subdev_format_whence)
> * @pad: pad number, as reported by the media API
> * @format: media bus format (format code and frame size)
> */
> struct v4l2_subdev_format {
> __u32 which;
> __u32 pad;
> struct v4l2_mbus_framefmt format;
> __u32 reserved[8];
> };
> ---8<---
>
> include/linux/v4l2-mediabus.h:
> ---8<---
> /**
> * struct v4l2_mbus_framefmt - frame format on the media bus
> * @width: frame width
> * @height: frame height
> * @code: data format code (from enum v4l2_mbus_pixelcode)
> * @field: used interlacing type (from enum v4l2_field)
> * @colorspace: colorspace of the data (from enum v4l2_colorspace)
> */
> struct v4l2_mbus_framefmt {
> __u32 width;
> __u32 height;
> __u32 code;
> __u32 field;
> __u32 colorspace;
> __u32 reserved[7];
> };
> ---8<---
>
> Offering a lower level interface for sensors which allows better control of
> them from the user space involves providing the link frequency to the user
> space. While the link frequency will be a control, together with the bus
> type and number of lanes (on serial links), this will define the pixel
> clock.
>
> http://www.spinics.net/lists/linux-media/msg36492.html>
>
> After adding pixel clock to v4l2_mbus_framefmt there will be six reserved
> fields left, one of which will be further possibly consumed by maximum image
> size:
>
> http://www.spinics.net/lists/linux-media/msg35949.html>
Yes, thanks for remembering about it. I have done some experiments with a sensor
producing JPEG data and I'd like to add '__u32 framesamples' field to struct
v4l2_mbus_framefmt, even though it solves only part of the problem.
I'm not sure when I'll be able to get this finished though. I've just attached
the initial patch now.
>
> Frame blanking (horizontal and vertical) and number of lanes might be needed
> in the struct as well in the future, bringing the reserved count down to
> two. I find this alarmingly low for a relatively new structure definition
> which will potentially have a few different uses in the future.
Sorry, could you explain why we need to put the blanking information in struct
v4l2_mbus_framefmt ? I thought it had been initially agreed that the control
framework will be used for this.
>
> The another issue is that the size of the v4l2_subdev_format struct is not
> aligned to a power of two. Instead of the intended 32 u32's, the size is
> actually 22 u32's.
hmm, is this really an issue ? What is advantage of having the structure size
being the power of 2 ? Isn't multiple of 4 just enough ?
>
> The interface is present in the 3.0 and marked experimental. My proposal is
> to add reserved fields to v4l2_mbus_framefmt to extend its size up to 32
> u32's. I understand there are already few which use the interface right now
> and thus this change must be done now or left as-is forever.
hmm, I feel a bit uncomfortable with increasing size of data structure which
is quite widely used, not only in sensors, also in TV capture cards, tuners,
etc.
So far struct v4l2_mbus_framefmt was quite generic. IMHO it might be good to try
to avoid extending it widely with properties specific to single subsystem.
--
Regards,
Sylwester
>From 18600fc32e65a6653491981b602af102f1dd52cb Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki
Date: Sun, 21 Aug 2011 00:51:59 +0200
Subject: [PATCH/RFC] v4l: Add framesamples field in struct v4l2_mbus_framefmt
The purpose of the new 'framesamples' field is to allow
the video pipeline elements to negotiate memory buffer size
for a transmitted frame.
This is mostly useful for compressed data formats where the
buffer size cannot be derived from pixel width and height
and the pixel code.
In case of the user space subdev API the applications must
assure the 'framesamples' value is consistent across the
pipeline. The drivers should validate those values before
the streaming is enabled.
Signed-off-by: Sylwester Nawrocki
---
Documentation/DocBook/media/v4l/dev-subdev.xml | 15 +++
Documentation/DocBook/media/v4l/subdev-formats.xml |7 ++-
include/linux/v4l2-mediabus.h |4 +++-
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/dev-subdev.xml b/Documentation/DocBook/media/v4l/dev-subdev.xml
index 05c8fef..48a1cdc 100644
--- a/Documentation/DocBook/media/v4l
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:Tue Sep 6 19:00:19 CEST 2011 git hash:1b19e42952963ae2a09a655f487de15b7c81c5b7 gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Hi, On 09/06/2011 08:35 PM, Michael Krufky wrote: On Tue, Sep 6, 2011 at 2:19 PM, Hans de Goede wrote: Hi, On 09/06/2011 06:24 PM, Devin Heitmueller wrote: I've been thinking for a while that perhaps the project should be renamed (or I considered prepending "kl" onto the front resulting in it being called "kl-tvtime"). This isn't out of vanity but rather my concern that the fork will get confused with the original project (for example, I believe Ubuntu actually already calls their modified tree tvtime 1.0.3). I'm open to suggestions in this regards. I think that what should be done is contact the debian / ubuntu maintainers, get any interesting fixes they have which the kl version misses merged, and then just declare the kl version as being the new official upstream (with the blessing of the debian / ubuntu guys, and if possible also with the blessing of the original authors). This would require kl git to be open to others for pushing, or we could move the tree to git.linuxtv.org (which I assume may be easier then for you to make the necessary changes to give others push rights on kl.org). Hans, Everybody is welcome to contribute to open source projects, but global contribution doesn't mean that a given server be opened up to commits by the general public. I didn't write open to commits by the general public, now did I? I wrote open to commits by others. For most upstream projects it is quite normal that several people have push rights to the master tree. This actually is quite a good idea, as it avoids adding a SPOF into the chain. It means development can continue if one of the maintainers is on vacation for a a few weeks, or just having a period in his/her life where he is too busy to actively contribute to a spare time project. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 2:19 PM, Hans de Goede wrote: > I think that what should be done is contact the debian / ubuntu maintainers, > get any interesting fixes they have which the kl version misses merged, > and then just declare the kl version as being the new official upstream > (with the blessing of the debian / ubuntu guys, and if possible also > with the blessing of the original authors). It has always been my intention to get the Debian/Ubuntu patches merged (as well as other distros). My thoughts behind renaming were oriented around the notion that that there are more distros out there than just Fedora/Ubuntu/Debian, but that may be something that isn't really a concern. Also, I had no idea whether the distros would actually switch over to the Kernel Labs version as the official upstream source, so providing it under a different name would in theory allow both packages to be available in parallel. >From a practical standpoint, the Ubuntu folks have the original tvtime tarball and all their changes in one patch, which is clearly a bunch of patches that are mashed together probably in their build system. I need to reach out to them to find where they have an actual SCM tree or the individual patches. They've got a bunch of patches which would be good to get into a single tree (autobuild fixes, cross-compilation, locale updates, etc). > This would require kl git to be open to others for pushing, or we > could move the tree to git.linuxtv.org (which I assume may be > easier then for you to make the necessary changes to give > others push rights on kl.org). Kernel Labs has never really had any real interest in "owning" tvtime. I just setup the hg tree in an effort to get all the distro patches in one place and have something that builds against current kernels (and on which I can add improvements/fixes without users having to deal with patches). At the time there was also nobody who clearly had the desire to serve as an official maintainer. In the long term I have no real issue with the LinuxTV group being the official maintainer of record. I've got lots of ideas and things I would like to do to improve tvtime, but in practice I've done a pretty crappy job of maintaining the source (merging patches, etc) at this point. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Em 06-09-2011 13:24, Devin Heitmueller escreveu: > On Tue, Sep 6, 2011 at 11:40 AM, Mauro Carvalho Chehab > wrote: >> Hi Devin, >> >> Em 06-09-2011 12:29, Mauro Carvalho Chehab escreveu: >>> There are several issues with the original alsa_stream code that got >>> fixed on xawtv3, made by me and by Hans de Goede. Basically, the >>> code were re-written, in order to follow the alsa best practises. >>> >>> Backport the changes from xawtv, in order to make it to work on a >>> wider range of V4L and sound adapters. >> >> FYI, just flooded your mailbox with 10 patches for tvtime. ;) >> >> I'm wanting to test some things with tvtime on one of my testboxes, but some >> of my cards weren't working with the alsa streaming, due to a few bugs that >> were solved on xawtv fork. >> >> So, I decided to backport it to tvtime and recompile the Fedora package for >> it. >> That's where the other 9 patches come ;) >> >> Basically, after applying this series of 10 patches, we can just remove all >> patches from Fedora, making life easier for distro maintainers (as the same >> thing is probably true on other distros - at least one of the Fedora patches >> came from Debian, from the fedora git logs). >> >> One important thing for distros is to have a tarball with the latest version >> hosted on a public site, so I've increased the version to 1.0.3 and I'm >> thinking on storing a copy of it at linuxtv, just like we do with xawtv3. >> >> If you prefer, all patches are also on my tvtime git tree, at: >> http://git.linuxtv.org/mchehab/tvtime.git >> >> Thanks, >> Mauro > > Hi Mauro, > > Funny you should send these along today. Last Friday I was actually > poking around at the Fedora tvtime repo because I was curious how they > had dealt with the V4L1 support issue (whether they were using my > patch removing v4l1 or some variant). Well, right time then ;) > > I've actually pulled in Fedora patches in the past (as you can see > from the hg repo), Yes, I saw it. Nice work! > and it has always been my intention to do it for > the other distros as well (e.g. debian/Ubuntu). So I appreciate your > having sent these along. It is a good idea to take a look at them. I looked into their repositories for the xawtv patches and I found some good stuff there. > I'll pull these in this week, do some testing to make sure nothing > serious got broken, and work to spin a 1.0.3 toward the end of the > week. Great! > Given the number of features/changes, and how long it's been > since the last formal release, I was considering calling it 1.1.0 > instead though. Seems fine for me. > I've been thinking for a while that perhaps the project should be > renamed (or I considered prepending "kl" onto the front resulting in > it being called "kl-tvtime"). This isn't out of vanity but rather my > concern that the fork will get confused with the original project (for > example, I believe Ubuntu actually already calls their modified tree > tvtime 1.0.3). I'm open to suggestions in this regards. IMO, I won't rename it. It is a well-known tool, and it is not a new version, but somebody's else took over its maintainership. I think you should touch the readme files in order to point to kl.com and to the places where the tree will be stored. Em 06-09-2011 15:19, Hans de Goede escreveu: > Hi, > I think that what should be done is contact the debian / ubuntu maintainers, > get any interesting fixes they have which the kl version misses merged, > and then just declare the kl version as being the new official upstream > (with the blessing of the debian / ubuntu guys, and if possible also > with the blessing of the original authors). Agree. I think Devin already tried to contact vektor about that. > This would require kl git to be open to others for pushing, or we > could move the tree to git.linuxtv.org (which I assume may be > easier then for you to make the necessary changes to give > others push rights on kl.org). I like this idea too. From my side, it proved to be very useful to be able to write on both Fedora and upstream repositories on xawtv3. I've made already the Fedora changes for tvtime 1.0.3 (in order to test it on my test boxes), so being able of adding a new release at both repos at the same time is a good idea. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 2:19 PM, Hans de Goede wrote: > Hi, > > On 09/06/2011 06:24 PM, Devin Heitmueller wrote: > > > >> I've been thinking for a while that perhaps the project should be >> renamed (or I considered prepending "kl" onto the front resulting in >> it being called "kl-tvtime"). This isn't out of vanity but rather my >> concern that the fork will get confused with the original project (for >> example, I believe Ubuntu actually already calls their modified tree >> tvtime 1.0.3). I'm open to suggestions in this regards. > > I think that what should be done is contact the debian / ubuntu maintainers, > get any interesting fixes they have which the kl version misses merged, > and then just declare the kl version as being the new official upstream > (with the blessing of the debian / ubuntu guys, and if possible also > with the blessing of the original authors). > > This would require kl git to be open to others for pushing, or we > could move the tree to git.linuxtv.org (which I assume may be > easier then for you to make the necessary changes to give > others push rights on kl.org). Hans, Everybody is welcome to contribute to open source projects, but global contribution doesn't mean that a given server be opened up to commits by the general public. You should feel free to push to your own git tree hosted on linuxtv.org (or any public git server, for that matter) and send pull requests to Devin Heitmueller, who is currently maintaining the kernellabs version of tvtime. Regards, Michael Krufky -- 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 01/10] alsa_stream: port changes made on xawtv3
Hi, On 09/06/2011 06:24 PM, Devin Heitmueller wrote: I've been thinking for a while that perhaps the project should be renamed (or I considered prepending "kl" onto the front resulting in it being called "kl-tvtime"). This isn't out of vanity but rather my concern that the fork will get confused with the original project (for example, I believe Ubuntu actually already calls their modified tree tvtime 1.0.3). I'm open to suggestions in this regards. I think that what should be done is contact the debian / ubuntu maintainers, get any interesting fixes they have which the kl version misses merged, and then just declare the kl version as being the new official upstream (with the blessing of the debian / ubuntu guys, and if possible also with the blessing of the original authors). This would require kl git to be open to others for pushing, or we could move the tree to git.linuxtv.org (which I assume may be easier then for you to make the necessary changes to give others push rights on kl.org). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On Tue, 2011-09-06 at 19:23 +0300, Antti Palosaari wrote: > hmm, lets see. Maybe I will add --format=email as keyboard shortcut button. or add [format] pretty = email to your .gitconfig or create and use a check_commitlog shell script or... -- 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 01/10] alsa_stream: port changes made on xawtv3
On Tue, Sep 6, 2011 at 11:40 AM, Mauro Carvalho Chehab wrote: > Hi Devin, > > Em 06-09-2011 12:29, Mauro Carvalho Chehab escreveu: >> There are several issues with the original alsa_stream code that got >> fixed on xawtv3, made by me and by Hans de Goede. Basically, the >> code were re-written, in order to follow the alsa best practises. >> >> Backport the changes from xawtv, in order to make it to work on a >> wider range of V4L and sound adapters. > > FYI, just flooded your mailbox with 10 patches for tvtime. ;) > > I'm wanting to test some things with tvtime on one of my testboxes, but some > of my cards weren't working with the alsa streaming, due to a few bugs that > were solved on xawtv fork. > > So, I decided to backport it to tvtime and recompile the Fedora package for > it. > That's where the other 9 patches come ;) > > Basically, after applying this series of 10 patches, we can just remove all > patches from Fedora, making life easier for distro maintainers (as the same > thing is probably true on other distros - at least one of the Fedora patches > came from Debian, from the fedora git logs). > > One important thing for distros is to have a tarball with the latest version > hosted on a public site, so I've increased the version to 1.0.3 and I'm > thinking on storing a copy of it at linuxtv, just like we do with xawtv3. > > If you prefer, all patches are also on my tvtime git tree, at: > http://git.linuxtv.org/mchehab/tvtime.git > > Thanks, > Mauro Hi Mauro, Funny you should send these along today. Last Friday I was actually poking around at the Fedora tvtime repo because I was curious how they had dealt with the V4L1 support issue (whether they were using my patch removing v4l1 or some variant). I've actually pulled in Fedora patches in the past (as you can see from the hg repo), and it has always been my intention to do it for the other distros as well (e.g. debian/Ubuntu). So I appreciate your having sent these along. I'll pull these in this week, do some testing to make sure nothing serious got broken, and work to spin a 1.0.3 toward the end of the week. Given the number of features/changes, and how long it's been since the last formal release, I was considering calling it 1.1.0 instead though. I've been thinking for a while that perhaps the project should be renamed (or I considered prepending "kl" onto the front resulting in it being called "kl-tvtime"). This isn't out of vanity but rather my concern that the fork will get confused with the original project (for example, I believe Ubuntu actually already calls their modified tree tvtime 1.0.3). I'm open to suggestions in this regards. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On 09/06/2011 07:10 PM, Joe Perches wrote: On Tue, 2011-09-06 at 18:30 +0300, Antti Palosaari wrote: On 09/06/2011 06:15 PM, Joe Perches wrote: On Tue, 2011-09-06 at 17:41 +0300, Antti Palosaari wrote: So what is recommended way to ensure patch is correct currently? a) before commit Use checkpatch. b) after commit Make the output of the commit log look like a patch. --format=email But still that sounds annoying, GIT is our default tool for handling patches and all the other tools like checkpatch.pl should honour that without any tricks. Why you don't add some detection logic to checkpatch.pl or even some new switch like --git. checkpatch is, as the name shows, for patches. I think using checkpatch on commit logs is not really useful. But that's what I have done every time I have added patches coming community. And also for my own patches. And when problem is found it is easy to git commit --amend and fix it. I think I am not the only maintainer who checks incoming patches like this way - you will got surely more feedback when that version of checkpatch will get more usage. If you're using checkpatch on commit logs, format the commit log output appropriately or use --ignore=BAD_SIGN_OFF or add that --ignore= to a .checkpatch.conf if you really must. hmm, lets see. Maybe I will add --format=email as keyboard shortcut button. Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On Tue, 2011-09-06 at 18:30 +0300, Antti Palosaari wrote: > On 09/06/2011 06:15 PM, Joe Perches wrote: > > On Tue, 2011-09-06 at 17:41 +0300, Antti Palosaari wrote: > >> So what is recommended way to ensure patch is correct currently? > >> a) before commit > > Use checkpatch. > >> b) after commit > > Make the output of the commit log look like a patch. > --format=email > But still that sounds annoying, GIT is our default tool for handling > patches and all the other tools like checkpatch.pl should honour that > without any tricks. Why you don't add some detection logic to > checkpatch.pl or even some new switch like --git. checkpatch is, as the name shows, for patches. I think using checkpatch on commit logs is not really useful. If you're using checkpatch on commit logs, format the commit log output appropriately or use --ignore=BAD_SIGN_OFF or add that --ignore= to a .checkpatch.conf if you really must. -- 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: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On 09/06/2011 06:15 PM, Joe Perches wrote: On Tue, 2011-09-06 at 17:41 +0300, Antti Palosaari wrote: So what is recommended way to ensure patch is correct currently? a) before commit Use checkpatch. b) after commit Make the output of the commit log look like a patch. --format=email But still that sounds annoying, GIT is our default tool for handling patches and all the other tools like checkpatch.pl should honour that without any tricks. Why you don't add some detection logic to checkpatch.pl or even some new switch like --git. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] alsa_stream: port changes made on xawtv3
Hi Devin,
Em 06-09-2011 12:29, Mauro Carvalho Chehab escreveu:
> There are several issues with the original alsa_stream code that got
> fixed on xawtv3, made by me and by Hans de Goede. Basically, the
> code were re-written, in order to follow the alsa best practises.
>
> Backport the changes from xawtv, in order to make it to work on a
> wider range of V4L and sound adapters.
FYI, just flooded your mailbox with 10 patches for tvtime. ;)
I'm wanting to test some things with tvtime on one of my testboxes, but some
of my cards weren't working with the alsa streaming, due to a few bugs that
were solved on xawtv fork.
So, I decided to backport it to tvtime and recompile the Fedora package for it.
That's where the other 9 patches come ;)
Basically, after applying this series of 10 patches, we can just remove all
patches from Fedora, making life easier for distro maintainers (as the same
thing is probably true on other distros - at least one of the Fedora patches
came from Debian, from the fedora git logs).
One important thing for distros is to have a tarball with the latest version
hosted on a public site, so I've increased the version to 1.0.3 and I'm
thinking on storing a copy of it at linuxtv, just like we do with xawtv3.
If you prefer, all patches are also on my tvtime git tree, at:
http://git.linuxtv.org/mchehab/tvtime.git
Thanks,
Mauro
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> src/alsa_stream.c | 629
> ++---
> src/alsa_stream.h |6 +-
> src/tvtime.c |6 +-
> 3 files changed, 363 insertions(+), 278 deletions(-)
>
> diff --git a/src/alsa_stream.c b/src/alsa_stream.c
> index 2243b02..b6a41a5 100644
> --- a/src/alsa_stream.c
> +++ b/src/alsa_stream.c
> @@ -6,6 +6,9 @@
>* Derived from the alsa-driver test tool latency.c:
>*Copyright (c) by Jaroslav Kysela
>*
> + * Copyright (c) 2011 - Mauro Carvalho Chehab
> + * Ported to xawtv, with bug fixes and improvements
> + *
>* 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
>* the Free Software Foundation; either version 2 of the License, or
> @@ -32,8 +35,16 @@
> #include
> #include
> #include
> +#include "alsa_stream.h"
> +
> +/* Private vars to control alsa thread status */
> +static int alsa_is_running = 0;
> +static int stop_alsa = 0;
>
> +/* Error handlers */
> snd_output_t *output = NULL;
> +FILE *error_fp;
> +int verbose = 0;
>
> struct final_params {
> int bufsize;
> @@ -42,403 +53,435 @@ struct final_params {
> int channels;
> };
>
> -int setparams_stream(snd_pcm_t *handle,
> - snd_pcm_hw_params_t *params,
> - snd_pcm_format_t format,
> - int channels,
> - int rate,
> - const char *id)
> +static int setparams_stream(snd_pcm_t *handle,
> + snd_pcm_hw_params_t *params,
> + snd_pcm_format_t format,
> + int channels,
> + const char *id)
> {
> int err;
> -unsigned int rrate;
>
> err = snd_pcm_hw_params_any(handle, params);
> if (err< 0) {
> - printf("Broken configuration for %s PCM: no configurations available:
> %s\n", snd_strerror(err), id);
> - return err;
> -}
> -err = snd_pcm_hw_params_set_rate_resample(handle, params, 1);
> -if (err< 0) {
> - printf("Resample setup failed for %s: %s\n", id, snd_strerror(err));
> + fprintf(error_fp,
> + "alsa: Broken configuration for %s PCM: no configurations
> available: %s\n",
> + snd_strerror(err), id);
> return err;
> }
> +
> err = snd_pcm_hw_params_set_access(handle, params,
> SND_PCM_ACCESS_RW_INTERLEAVED);
> if (err< 0) {
> - printf("Access type not available for %s: %s\n", id,
> -snd_strerror(err));
> + fprintf(error_fp, "alsa: Access type not available for %s: %s\n", id,
> + snd_strerror(err));
> return err;
> }
>
> err = snd_pcm_hw_params_set_format(handle, params, format);
> if (err< 0) {
> - printf("Sample format not available for %s: %s\n", id,
> + fprintf(error_fp, "alsa: Sample format not available for %s: %s\n", id,
> snd_strerror(err));
> return err;
> }
> err = snd_pcm_hw_params_set_channels(handle, params, channels);
> if (err< 0) {
> - printf("Channels count (%i) not available for %s: %s\n", channels, id,
> -snd_strerror(err));
> - return err;
> -}
> -rrate = rate;
> -err = snd_pcm_hw_params_set_rate_near(handle, params,&rrate, 0);
> -if (err< 0) {
> - printf("Rate %iHz not available for %s: %s\n", rate, id,
> -snd_strerror(err));
> + fprintf(error_fp, "alsa: Channels count (%i) not
Re: How to git and build HVR-2200 drivers from Kernel labs ?
>>> Eg should I use the source from "git clone >>> git://kernellabs.com/stoth/saa7164- >>> dev.git" or do you recommend something else that might be more stable ? >> >> pull a snapshot: >> >> >> http://www.kernellabs.com/gitweb/?p=stoth/saa7164-stable.git;a=snapshot;h=87e0c0378bf2068df5d0c43acd66aea9ba71bd89;sf=tgz >> > > I've now got my card working with the saa7164 driver from the > "87e0c0378bf2068df5d0c43acd66aea9ba71bd89" commit. Many thanks for your > help. Thanks for the followup Declan. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/10] tvtime: Bump to version 1.0.3
Need to update its version, in order to allow distros to use it. Signed-off-by: Mauro Carvalho Chehab --- ChangeLog|7 +++ NEWS | 10 +- configure.ac |4 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 147b822..c613bde 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +1.0.3 - Tue Sep 6 14:53:23 CEST 2011 + * djh: Conversion to Mercurial, compilation fixes, patch backports +from other places, more generic VBI handling, Alsa streaming +support, get rid of V4L1. + * mchehab/hdegoede: Improved alsa audio streaming code. + * mchehab: Backport the remaining patches found on Fedora. + 1.0.2 - Wed Nov 9 21:46:28 EST 2005 * vektor: Add a proper TVTIME_NOOP command so that you can remove keybindings. Thanks to Andrew Dalton for the fix. diff --git a/NEWS b/NEWS index 7fe5522..0279b1d 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,8 @@ - -For news and updates on tvtime, please visit our website at: - - http://tvtime.net/ - +News for 1.0.3 + * V4L1 removal + * Alsa streaming support + * Compilation fixes, patch backports from other places + * More generic VBI handling News for 1.0.2 diff --git a/configure.ac b/configure.ac index f102b5b..37c2871 100644 --- a/configure.ac +++ b/configure.ac @@ -1,8 +1,8 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.52) -AC_INIT(tvtime, 1.0.2, http://tvtime.net/) +AC_INIT(tvtime, 1.0.3, http://linuxtv.org/) AC_CONFIG_SRCDIR([src/tvtime.c]) -AM_INIT_AUTOMAKE(tvtime,1.0.2) +AM_INIT_AUTOMAKE(tvtime,1.0.3) AM_CONFIG_HEADER(config.h) AM_MAINTAINER_MODE AC_CANONICAL_HOST -- 1.7.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/10] Use a saner way to disable screensaver
Backport a Fedora patch that improves the way to disable
the screensaver:
commit 36cc9d2e1d762eddf5d8278fa58edd4680a7b449
Author: Tomas Smetana
Date: Mon Nov 8 22:01:57 2010 +0100
- fix #571339 use a saner way to disable screensaver, thanks to Debian folks
for the patch, namely Resul Cetin
Signed-off-by: Mauro Carvalho Chehab
---
configure.ac | 10 +-
src/xcommon.c | 50 +++---
2 files changed, 28 insertions(+), 32 deletions(-)
diff --git a/configure.ac b/configure.ac
index 6cdedfb..f102b5b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -140,11 +140,11 @@ if test x"$no_x" != x"yes"; then
X11_LIBS="$X11_LIBS -lXinerama"],,
[$X_PRE_LIBS $X_LIBS -lX11 $X_EXTRA_LIBS -lXext])
- dnl check for XTest
-AC_CHECK_LIB([Xtst],[XTestFakeKeyEvent],
-[AC_DEFINE([HAVE_XTESTEXTENSION],,[XTest support])
-X11_LIBS="$X11_LIBS -lXtst"],,
- [$X_PRE_LIBS $X_LIBS -lX11 $X_EXTRA_LIBS -lXext])
+ dnl check for XSs
+ PKG_CHECK_MODULES(XSS, xscrnsaver >= 1.2.0,
+ AC_DEFINE([HAVE_XSSEXTENSION],,[XScrnSaver support])
+ AC_SUBST(XSS_LIBS)
+ X11_LIBS="$X11_LIBS $XSS_LIBS",)
dnl check for Xvidmode
AC_CHECK_LIB([Xxf86vm],[XF86VidModeGetModeLine],
diff --git a/src/xcommon.c b/src/xcommon.c
index 8e3be4c..681b895 100644
--- a/src/xcommon.c
+++ b/src/xcommon.c
@@ -45,8 +45,8 @@
#include
#include
#include
-#ifdef HAVE_XTESTEXTENSION
-#include
+#ifdef HAVE_XSSEXTENSION
+#include
#endif
#include "xfullscreen.h"
@@ -67,7 +67,7 @@ static Window wm_window;
static Window fs_window;
static Window output_window;
static GC gc;
-static int have_xtest;
+static int have_xss;
static int output_width, output_height;
static int output_aspect;
static int output_on_root;
@@ -107,10 +107,6 @@ static Atom wm_delete_window;
static Atom xawtv_station;
static Atom xawtv_remote;
-#ifdef HAVE_XTESTEXTENSION
-static KeyCode kc_shift_l; /* Fake key to send. */
-#endif
-
static area_t video_area;
static area_t window_area;
static area_t scale_area;
@@ -248,12 +244,12 @@ static void x11_wait_mapped( Display *dpy, Window win )
} while ( (event.type != MapNotify) || (event.xmap.event != win) );
}
-static int have_xtestextention( void )
+static int have_xssextention( void )
{
-#ifdef HAVE_XTESTEXTENSION
-int dummy1, dummy2, dummy3, dummy4;
+#ifdef HAVE_XSSEXTENSION
+int dummy1, dummy2;
-return (XTestQueryExtension( display, &dummy1, &dummy2, &dummy3, &dummy4 )
== True);
+return (XScreenSaverQueryExtension( display, &dummy1, &dummy2 ) == True);
#endif
return 0;
}
@@ -843,7 +839,7 @@ int xcommon_open_display( const char *user_geometry, int
aspect, int verbose )
output_aspect = aspect;
output_height = 576;
-have_xtest = 0;
+have_xss = 0;
output_on_root = 0;
has_ewmh_state_fullscreen = 0;
has_ewmh_state_above = 0;
@@ -927,13 +923,16 @@ int xcommon_open_display( const char *user_geometry, int
aspect, int verbose )
xfullscreen_print_summary( xf );
}
-#ifdef HAVE_XTESTEXTENSION
-kc_shift_l = XKeysymToKeycode( display, XK_Shift_L );
-#endif
-have_xtest = have_xtestextention();
-if( have_xtest && xcommon_verbose ) {
-fprintf( stderr, "xcommon: Have XTest, will use it to ping the
screensaver.\n" );
+have_xss = have_xssextention();
+if( have_xss && xcommon_verbose ) {
+fprintf( stderr, "xcommon: Have XSS, will use it to disable the
screensaver.\n" );
+}
+
+#ifdef HAVE_XSSEXTENSION
+if ( have_xss ) {
+XScreenSaverSuspend( display, True );
}
+#endif
/* Initially, get the best width for our height. */
output_width = xv_get_width_for_height( output_height );
@@ -1112,15 +,7 @@ void xcommon_ping_screensaver( void )
gettimeofday( &curtime, 0 );
if( timediff( &curtime, &last_ping_time ) > SCREENSAVER_PING_TIME ) {
last_ping_time = curtime;
-#ifdef HAVE_XTESTEXTENSION
-if( have_xtest ) {
-XTestFakeKeyEvent( display, kc_shift_l, True, CurrentTime );
-XTestFakeKeyEvent( display, kc_shift_l, False, CurrentTime );
-} else
-#endif
-{
-XResetScreenSaver( display );
-}
+XResetScreenSaver( display );
}
}
@@ -1715,6 +1706,11 @@ void xcommon_poll_events( input_t *in )
void xcommon_close_display( void )
{
+#ifdef HAVE_XSSEXTENSION
+if ( have_xss ) {
+XScreenSaverSuspend( display, False );
+}
+#endif
XDestroyWindow( display, output_window );
XDestroyWindow( display, wm_window );
XDestroyWindow( display, fs_window );
--
1.7.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] Backport UVC fix from Fedora
>From Fedora logs:
fix #655038 - tvtime does not work with UVC webcams
Signed-off-by: Mauro Carvalho Chehab
---
src/videoinput.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/videoinput.c b/src/videoinput.c
index 2102b04..a8fd829 100644
--- a/src/videoinput.c
+++ b/src/videoinput.c
@@ -294,6 +294,7 @@ uint8_t *videoinput_next_frame( videoinput_t *vidin, int
*frameid )
wait_for_frame_v4l2( vidin );
cur_buf.type = vidin->capbuffers[ 0 ].vidbuf.type;
+cur_buf.memory = vidin->capbuffers[ 0 ].vidbuf.memory;
if( ioctl( vidin->grab_fd, VIDIOC_DQBUF, &cur_buf ) < 0 ) {
/* some drivers return EIO when there is no signal */
if( errno != EIO ) {
--
1.7.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/10] Add mkinstalldirs
This is needed for make distcheck to be happy. Not sure why this
script is not here already... tvtime 1.0.2 tarball has it.
Signed-off-by: Mauro Carvalho Chehab
---
mkinstalldirs | 111 +++
src/Makefile.am |3 +-
2 files changed, 113 insertions(+), 1 deletions(-)
create mode 100755 mkinstalldirs
diff --git a/mkinstalldirs b/mkinstalldirs
new file mode 100755
index 000..d2d5f21
--- /dev/null
+++ b/mkinstalldirs
@@ -0,0 +1,111 @@
+#! /bin/sh
+# mkinstalldirs --- make directory hierarchy
+# Author: Noah Friedman
+# Created: 1993-05-16
+# Public domain
+
+errstatus=0
+dirmode=""
+
+usage="\
+Usage: mkinstalldirs [-h] [--help] [-m mode] dir ..."
+
+# process command line arguments
+while test $# -gt 0 ; do
+ case $1 in
+-h | --help | --h*) # -h for help
+ echo "$usage" 1>&2
+ exit 0
+ ;;
+-m) # -m PERM arg
+ shift
+ test $# -eq 0 && { echo "$usage" 1>&2; exit 1; }
+ dirmode=$1
+ shift
+ ;;
+--) # stop option processing
+ shift
+ break
+ ;;
+-*) # unknown option
+ echo "$usage" 1>&2
+ exit 1
+ ;;
+*) # first non-opt arg
+ break
+ ;;
+ esac
+done
+
+for file
+do
+ if test -d "$file"; then
+shift
+ else
+break
+ fi
+done
+
+case $# in
+ 0) exit 0 ;;
+esac
+
+case $dirmode in
+ '')
+if mkdir -p -- . 2>/dev/null; then
+ echo "mkdir -p -- $*"
+ exec mkdir -p -- "$@"
+fi
+;;
+ *)
+if mkdir -m "$dirmode" -p -- . 2>/dev/null; then
+ echo "mkdir -m $dirmode -p -- $*"
+ exec mkdir -m "$dirmode" -p -- "$@"
+fi
+;;
+esac
+
+for file
+do
+ set fnord `echo ":$file" | sed -ne 's/^:\//#/;s/^://;s/\// /g;s/^#/\//;p'`
+ shift
+
+ pathcomp=
+ for d
+ do
+pathcomp="$pathcomp$d"
+case $pathcomp in
+ -*) pathcomp=./$pathcomp ;;
+esac
+
+if test ! -d "$pathcomp"; then
+ echo "mkdir $pathcomp"
+
+ mkdir "$pathcomp" || lasterr=$?
+
+ if test ! -d "$pathcomp"; then
+ errstatus=$lasterr
+ else
+ if test ! -z "$dirmode"; then
+ echo "chmod $dirmode $pathcomp"
+ lasterr=""
+ chmod "$dirmode" "$pathcomp" || lasterr=$?
+
+ if test ! -z "$lasterr"; then
+ errstatus=$lasterr
+ fi
+ fi
+ fi
+fi
+
+pathcomp="$pathcomp/"
+ done
+done
+
+exit $errstatus
+
+# Local Variables:
+# mode: shell-script
+# sh-indentation: 2
+# End:
+# mkinstalldirs ends here
diff --git a/src/Makefile.am b/src/Makefile.am
index 56e26a6..e48ef4c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -45,7 +45,8 @@ COMMON_SRCS = mixer.c videoinput.c rtctimer.c leetft.c
osdtools.c tvtimeconf.c \
utils.h utils.c pulldown.h pulldown.c hashtable.h hashtable.c \
cpuinfo.h cpuinfo.c menu.c menu.h \
outputfilter.h outputfilter.c xmltv.h xmltv.c gettext.h tvtimeglyphs.h \
- copyfunctions.h copyfunctions.c alsa_stream.c mixer-oss.c $(ALSA_SRCS)
+ copyfunctions.h copyfunctions.c alsa_stream.c alsa_stream.h \
+ mixer-oss.c $(ALSA_SRCS)
if ARCH_X86
DSCALER_SRCS = $(top_srcdir)/plugins/dscalerapi.h \
--
1.7.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/10] Ignore auto-generated files
Signed-off-by: Mauro Carvalho Chehab --- .gitignore | 17 + docs/.gitignore |3 +++ intl/.gitignore |2 ++ m4/.gitignore |8 po/.gitignore |7 +++ src/.gitignore |9 + 6 files changed, 46 insertions(+), 0 deletions(-) create mode 100644 .gitignore create mode 100644 docs/.gitignore create mode 100644 intl/.gitignore create mode 100644 m4/.gitignore create mode 100644 po/.gitignore create mode 100644 src/.gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 000..9bbc8df --- /dev/null +++ b/.gitignore @@ -0,0 +1,17 @@ +autom4te.cache/ +Makefile +Makefile.in +aclocal.m4 +config.guess +config.h +config.h.in +config.log +config.status +config.sub +configure +install-sh +libtool +ltmain.sh +missing +stamp-h1 +depcomp diff --git a/docs/.gitignore b/docs/.gitignore new file mode 100644 index 000..22a4e72 --- /dev/null +++ b/docs/.gitignore @@ -0,0 +1,3 @@ +Makefile +Makefile.in + diff --git a/intl/.gitignore b/intl/.gitignore new file mode 100644 index 000..5c1aa8c --- /dev/null +++ b/intl/.gitignore @@ -0,0 +1,2 @@ +plural.c + diff --git a/m4/.gitignore b/m4/.gitignore new file mode 100644 index 000..19aca97 --- /dev/null +++ b/m4/.gitignore @@ -0,0 +1,8 @@ +Makefile +Makefile.in +libtool.m4 +ltoptions.m4 +ltsugar.m4 +ltversion.m4 +lt~obsolete.m4 + diff --git a/po/.gitignore b/po/.gitignore new file mode 100644 index 000..d2fdb57 --- /dev/null +++ b/po/.gitignore @@ -0,0 +1,7 @@ +*.gmo +Makefile +Makefile.in +POTFILES +remove-potcdate.sed +stamp-po + diff --git a/src/.gitignore b/src/.gitignore new file mode 100644 index 000..3a6766d --- /dev/null +++ b/src/.gitignore @@ -0,0 +1,9 @@ +*.o +.deps/ +Makefile +Makefile.in +tvtime +tvtime-command +tvtime-configure +tvtime-scanner + -- 1.7.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/10] Properly document alsa mixer
Backported from Fedora: commit 16ee4edaccd1a6a6869e4abf07581a2f7c6df1fb Author: Tomas Smetana Date: Thu Jul 9 07:09:44 2009 + - fix a typo in the default config file commit a4c64442a7c94cd175bca661e88682ee8de87ce4 Author: Tomas Smetana Date: Sun Jun 28 10:01:27 2009 + - try to document the new ALSA mixer settings, make ALSA mixer the default one Signed-off-by: Mauro Carvalho Chehab --- docs/html/default.tvtime.xml |8 +--- docs/man/en/tvtime.xml.5 |5 - src/tvtimeconf.c | 18 +++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/docs/html/default.tvtime.xml b/docs/html/default.tvtime.xml index 29d939a..bc71d10 100644 --- a/docs/html/default.tvtime.xml +++ b/docs/html/default.tvtime.xml @@ -116,13 +116,15 @@ - +
[PATCH 03/10] Backport mixer-alsa patch from Fedora
commit e53212e85a71d831fff3bf61c792ed7235fa3a79
Author: Tomas Smetana
Date: Mon May 11 16:24:09 2009 +
Added first version of the ALSA mixer patch. Unused yet.
Signed-off-by: Mauro Carvalho Chehab
---
configure.ac | 12 ++-
src/Makefile.am | 15 ++-
src/commands.c |8 +-
src/mixer-alsa.c | 240 +++
src/mixer-oss.c | 261 +++
src/mixer.c | 272 +++---
src/mixer.h | 31 +--
src/tvtime.c | 10 +-
src/videoinput.c |6 +-
9 files changed, 628 insertions(+), 227 deletions(-)
create mode 100644 src/mixer-alsa.c
create mode 100644 src/mixer-oss.c
diff --git a/configure.ac b/configure.ac
index eac6c20..6cdedfb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -76,18 +76,26 @@ dnl -
dnl libxml2
dnl -
dnl Test for libxml2
-
AC_PATH_PROG(LIBXML2_CONFIG,xml2-config,no)
if test "$LIBXML2_CONFIG" = "no" ; then
AC_MSG_ERROR(libxml2 needed and xml2-config not found)
else
XML2_LIBS="`$LIBXML2_CONFIG --libs`"
XML2_FLAG="`$LIBXML2_CONFIG --cflags`"
- AC_DEFINE(HAVE_LIBXML2,,[LIBXML2 support])
+ AC_DEFINE(HAVE_LIBXML2,,[LIBXML2 support])
fi
AC_SUBST(XML2_LIBS)
AC_SUBST(XML2_FLAG)
+dnl -
+dnl libasound2
+dnl -
+dnl Test for ALSA
+AM_PATH_ALSA(1.0.9,
+ [ AC_DEFINE(HAVE_ALSA,1,[Define this if you have Alsa (libasound)
installed]) ],
+ AC_MSG_RESULT(libasound needed and not found))
+AM_CONDITIONAL(HAVE_ALSA, test x"$no_alsa" != "yes")
+
dnl -
dnl asound
diff --git a/src/Makefile.am b/src/Makefile.am
index 3a38aac..56e26a6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -29,6 +29,11 @@ OPT_CFLAGS = -Wall -pedantic -I.
-DDATADIR="\"$(pkgdatadir)\"" \
-DCONFDIR="\"$(pkgsysconfdir)\"" -DFIFODIR="\"$(tmpdir)\"" \
-D_LARGEFILE64_SOURCE -DLOCALEDIR="\"$(localedir)\""
+if HAVE_ALSA
+ALSA_SRCS =mixer-alsa.c
+else
+ALSA_SRCS =
+endif
COMMON_SRCS = mixer.c videoinput.c rtctimer.c leetft.c osdtools.c tvtimeconf.c
\
pngoutput.c tvtimeosd.c input.c cpu_accel.c speedy.c pnginput.c \
deinterlace.c videotools.c attributes.h deinterlace.h leetft.h \
@@ -40,7 +45,7 @@ COMMON_SRCS = mixer.c videoinput.c rtctimer.c leetft.c
osdtools.c tvtimeconf.c \
utils.h utils.c pulldown.h pulldown.c hashtable.h hashtable.c \
cpuinfo.h cpuinfo.c menu.c menu.h \
outputfilter.h outputfilter.c xmltv.h xmltv.c gettext.h tvtimeglyphs.h \
- copyfunctions.h copyfunctions.c alsa_stream.c
+ copyfunctions.h copyfunctions.c alsa_stream.c mixer-oss.c $(ALSA_SRCS)
if ARCH_X86
DSCALER_SRCS = $(top_srcdir)/plugins/dscalerapi.h \
@@ -74,10 +79,10 @@ bin_PROGRAMS = tvtime tvtime-command tvtime-configure
tvtime-scanner
tvtime_SOURCES = $(COMMON_SRCS) $(OUTPUT_SRCS) $(PLUGIN_SRCS) tvtime.c
tvtime_CFLAGS = $(TTF_CFLAGS) $(PNG_CFLAGS) $(OPT_CFLAGS) \
- $(PLUGIN_CFLAGS) $(X11_CFLAGS) $(XML2_FLAG) \
+ $(PLUGIN_CFLAGS) $(X11_CFLAGS) $(XML2_FLAG) $(ALSA_CFLAGS) \
$(FONT_CFLAGS) $(AM_CFLAGS)
tvtime_LDFLAGS = $(TTF_LIBS) $(ZLIB_LIBS) $(PNG_LIBS) \
- $(X11_LIBS) $(XML2_LIBS) $(ASOUND_LIBS) -lm -lstdc++
+ $(X11_LIBS) $(XML2_LIBS) $(ALSA_LIBS) -lm -lstdc++
tvtime_command_SOURCES = utils.h utils.c tvtimeconf.h tvtimeconf.c \
tvtime-command.c
@@ -90,6 +95,6 @@ tvtime_configure_LDFLAGS = $(ZLIB_LIBS) $(XML2_LIBS)
tvtime_scanner_SOURCES = utils.h utils.c videoinput.h videoinput.c \
tvtimeconf.h tvtimeconf.c station.h station.c tvtime-scanner.c \
mixer.h mixer.c
-tvtime_scanner_CFLAGS = $(OPT_CFLAGS) $(XML2_FLAG) $(AM_CFLAGS)
-tvtime_scanner_LDFLAGS = $(ZLIB_LIBS) $(XML2_LIBS)
+tvtime_scanner_CFLAGS = $(OPT_CFLAGS) $(XML2_FLAG) $(ALSA_CFLAGS) $(AM_CFLAGS)
+tvtime_scanner_LDFLAGS = $(ZLIB_LIBS) $(XML2_LIBS) $(ALSA_LIBS)
diff --git a/src/commands.c b/src/commands.c
index 964cab7..9141276 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -3012,10 +3012,10 @@ void commands_handle( commands_t *cmd, int tvtime_cmd,
const char *arg )
break;
case TVTIME_MIXER_TOGGLE_MUTE:
-mixer_mute( !mixer_ismute() );
+mixer->mute( !mixer->ismute() );
if( cmd->osd ) {
-tvtime_osd_show_data_bar( cmd->osd, _("Volume"),
(mixer_get_volume()) & 0xff );
+tvtime_osd_show_data_bar( cmd->osd, _("Volume"),
(mixer->get_volume()) & 0xff );
}
break;
@@ -3029,9 +3029,9 @@ void commands_handle( commands_t *cmd, int tvtime_cmd,
const char *arg )
/* Check to see if an argument was passed, if so, use it. */
if (atoi(arg) > 0) {
int perc = atoi(arg);
-volume = mixer_se
[PATCH 02/10] Fix make dist target
Signed-off-by: Mauro Carvalho Chehab --- src/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 50687c2..3a38aac 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -38,7 +38,7 @@ COMMON_SRCS = mixer.c videoinput.c rtctimer.c leetft.c osdtools.c tvtimeconf.c \ speedtools.h vbiscreen.h vbiscreen.c fifo.h fifo.c commands.h \ commands.c videofilter.h videofilter.c station.h station.c bands.h \ utils.h utils.c pulldown.h pulldown.c hashtable.h hashtable.c \ - cpuinfo.h cpuinfo.c videodev2.h menu.c menu.h \ + cpuinfo.h cpuinfo.c menu.c menu.h \ outputfilter.h outputfilter.c xmltv.h xmltv.c gettext.h tvtimeglyphs.h \ copyfunctions.h copyfunctions.c alsa_stream.c -- 1.7.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/10] alsa_stream: port changes made on xawtv3
There are several issues with the original alsa_stream code that got
fixed on xawtv3, made by me and by Hans de Goede. Basically, the
code were re-written, in order to follow the alsa best practises.
Backport the changes from xawtv, in order to make it to work on a
wider range of V4L and sound adapters.
Signed-off-by: Mauro Carvalho Chehab
---
src/alsa_stream.c | 629 ++---
src/alsa_stream.h |6 +-
src/tvtime.c |6 +-
3 files changed, 363 insertions(+), 278 deletions(-)
diff --git a/src/alsa_stream.c b/src/alsa_stream.c
index 2243b02..b6a41a5 100644
--- a/src/alsa_stream.c
+++ b/src/alsa_stream.c
@@ -6,6 +6,9 @@
* Derived from the alsa-driver test tool latency.c:
*Copyright (c) by Jaroslav Kysela
*
+ * Copyright (c) 2011 - Mauro Carvalho Chehab
+ * Ported to xawtv, with bug fixes and improvements
+ *
* 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
* the Free Software Foundation; either version 2 of the License, or
@@ -32,8 +35,16 @@
#include
#include
#include
+#include "alsa_stream.h"
+
+/* Private vars to control alsa thread status */
+static int alsa_is_running = 0;
+static int stop_alsa = 0;
+/* Error handlers */
snd_output_t *output = NULL;
+FILE *error_fp;
+int verbose = 0;
struct final_params {
int bufsize;
@@ -42,403 +53,435 @@ struct final_params {
int channels;
};
-int setparams_stream(snd_pcm_t *handle,
-snd_pcm_hw_params_t *params,
-snd_pcm_format_t format,
-int channels,
-int rate,
-const char *id)
+static int setparams_stream(snd_pcm_t *handle,
+ snd_pcm_hw_params_t *params,
+ snd_pcm_format_t format,
+ int channels,
+ const char *id)
{
int err;
-unsigned int rrate;
err = snd_pcm_hw_params_any(handle, params);
if (err < 0) {
- printf("Broken configuration for %s PCM: no configurations available:
%s\n", snd_strerror(err), id);
- return err;
-}
-err = snd_pcm_hw_params_set_rate_resample(handle, params, 1);
-if (err < 0) {
- printf("Resample setup failed for %s: %s\n", id, snd_strerror(err));
+ fprintf(error_fp,
+ "alsa: Broken configuration for %s PCM: no configurations
available: %s\n",
+ snd_strerror(err), id);
return err;
}
+
err = snd_pcm_hw_params_set_access(handle, params,
SND_PCM_ACCESS_RW_INTERLEAVED);
if (err < 0) {
- printf("Access type not available for %s: %s\n", id,
- snd_strerror(err));
+ fprintf(error_fp, "alsa: Access type not available for %s: %s\n", id,
+ snd_strerror(err));
return err;
}
err = snd_pcm_hw_params_set_format(handle, params, format);
if (err < 0) {
- printf("Sample format not available for %s: %s\n", id,
+ fprintf(error_fp, "alsa: Sample format not available for %s: %s\n", id,
snd_strerror(err));
return err;
}
err = snd_pcm_hw_params_set_channels(handle, params, channels);
if (err < 0) {
- printf("Channels count (%i) not available for %s: %s\n", channels, id,
- snd_strerror(err));
- return err;
-}
-rrate = rate;
-err = snd_pcm_hw_params_set_rate_near(handle, params, &rrate, 0);
-if (err < 0) {
- printf("Rate %iHz not available for %s: %s\n", rate, id,
- snd_strerror(err));
+ fprintf(error_fp, "alsa: Channels count (%i) not available for %s:
%s\n",
+ channels, id, snd_strerror(err));
return err;
}
-if ((int)rrate != rate) {
- printf("Rate doesn't match (requested %iHz, get %iHz)\n", rate, err);
- return -EINVAL;
-}
+
return 0;
}
-int setparams_bufsize(snd_pcm_t *handle,
+static void getparams_periods(snd_pcm_t *handle,
snd_pcm_hw_params_t *params,
- snd_pcm_hw_params_t *tparams,
- snd_pcm_uframes_t bufsize,
- int period_size,
+ unsigned int *usecs,
+ unsigned int *count,
+ const char *id)
+{
+unsigned min = 0, max = 0;
+
+snd_pcm_hw_params_get_periods_min(params, &min, 0);
+snd_pcm_hw_params_get_periods_max(params, &max, 0);
+if (min && max) {
+ if (verbose)
+ fprintf(error_fp, "alsa: %s periods range between %u and %u. Want:
%u\n",
+ id, min, max, *count);
+ if (*count < min)
+ *count = min;
+ if (*count > max)
+ *count = max;
+}
+
+min = max = 0;
+snd_pcm_hw_params_get_period_time_min(params, &min, 0);
+snd_pcm_hw_params_get_period_time_
Re: [RFC] Support for H.264/MPEG4 encoder (VPU) in i.MX27.
Hi Javier On Tue, 6 Sep 2011, javier Martin wrote: > Hi, > we are planning to add support to H.264/MPEG4 encoder inside the > i.MX27 to v4l2. It is mainly a hardware module that has the following > features: > > - It needs two input buffers (current frame and previous frame). > - It produces a third buffer as output, containing the encoded frame, > and generates an IRQ when that happens. > - Previous three buffers need contiguous physical memory addresses and > probably some alignment requirements. > - It needs an external firmware to be loaded in another contiguous > memory buffer. > > I would like to know what is your opinion on this, what v4l2 framework > should we use to deal with it, etc... I guess Multi Format Codec 5.1 > driver for s5pv210 and exynos4 SoC is the most similar piece of HW > I've found so far but it has not yet entered mainline [1] > > Note that mx2_camera driver is still using soc-camera framework and > soc-camera doesn't seem to be ready for integration with pad level API > [2]. For that reason we think we could develop this VPU driver > separately. (The following is my understanding and opinion, all corrections welcome) The MC API is important, when data can be passed directly between modules, using some internal busses. I.e., if you could just configure your VPU to receive the data directly from the camera capture module, without the use of memory buffers, then yes, you would want an MC-like API, and an ability to connect the camera module to the VPU using pads and links and configure each entity from the user-space separately, maintaining the format compatibility along the links. In your case, however, the VPU uses memory buffers. I.e., what you want is some zero-copy buffer passing from the camera module to the VPU. I don't think the MC API is very helpful in this situation. With the currently available options you want to use USERPTR buffers and pass them from the source to the sink in your user-space application. There are also some buffer-sharing options approaching, but as long as this is just one application, that tosses buffers between two devices, you, probably, don't need those very much either. So, this looks like a "simple" mem2mem driver case to me. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On Tue, 2011-09-06 at 17:41 +0300, Antti Palosaari wrote: > So what is recommended way to ensure patch is correct currently? > a) before commit Use checkpatch. > b) after commit Make the output of the commit log look like a patch. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] Support for H.264/MPEG4 encoder (VPU) in i.MX27.
Hi, we are planning to add support to H.264/MPEG4 encoder inside the i.MX27 to v4l2. It is mainly a hardware module that has the following features: - It needs two input buffers (current frame and previous frame). - It produces a third buffer as output, containing the encoded frame, and generates an IRQ when that happens. - Previous three buffers need contiguous physical memory addresses and probably some alignment requirements. - It needs an external firmware to be loaded in another contiguous memory buffer. I would like to know what is your opinion on this, what v4l2 framework should we use to deal with it, etc... I guess Multi Format Codec 5.1 driver for s5pv210 and exynos4 SoC is the most similar piece of HW I've found so far but it has not yet entered mainline [1] Note that mx2_camera driver is still using soc-camera framework and soc-camera doesn't seem to be ready for integration with pad level API [2]. For that reason we think we could develop this VPU driver separately. [1] http://www.spinics.net/lists/linux-media/msg35040.html [2] http://www.open-technology.de/index.php?/categories/2-SoC-camera -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: omap: Fix build error in ispccdc.c
Hi Joerg, On Tuesday 06 September 2011 16:02:15 Joerg Roedel wrote: > The following build error occurs with 3.1-rc5: > > CC drivers/media/video/omap3isp/ispccdc.o > /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c: In > function 'ccdc_lsc_config': > /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:42 > 7:2: error: implicit declaration of function 'kzalloc' > [-Werror=implicit-function-declaration] > /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:42 > 7:6: warning: assignment makes pointer from integer without a cast [enabled > by default] > > This patch adds the missing 'linux/slab.h' include to fix the problem. Thanks for the patch. > Cc: Laurent Pinchart > Cc: linux-media@vger.kernel.org > Cc: linux-o...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-ker...@vger.kernel.org > Signed-off-by: Joerg Roedel Acked-by: Laurent Pinchart Mauro, can you please pick this patch and push it to v3.1 ? > --- > drivers/media/video/omap3isp/ispccdc.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/video/omap3isp/ispccdc.c > b/drivers/media/video/omap3isp/ispccdc.c index 9d3459d..80796eb 100644 > --- a/drivers/media/video/omap3isp/ispccdc.c > +++ b/drivers/media/video/omap3isp/ispccdc.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > > #include "isp.h" -- 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: [PATCHv2] ISP:BUILD:FIX: Move media_entity_init() and
Hi Vaibhav, On Tuesday 06 September 2011 16:12:35 Hiremath, Vaibhav wrote: > On Monday, September 05, 2011 6:20 PM Laurent Pinchart wrote: > > On Sunday 04 September 2011 15:32:28 Mauro Carvalho Chehab wrote: > > > > > I don't mind splitting the config option. An alternative would be to > > compile media_entity_init() and media_entity_cleanup() based on > > CONFIG_MEDIA_SUPPORT instead of CONFIG_MEDIA_CONTROLLER, but that looks a > > bit hackish to me. > > > > > Also, I don't like the idea of increasing drivers complexity for the > > > existing drivers that work properly without MC. All those core > > > conversions that were done in the last two years caused already too much > > > instability to them. > > > > > > We should really avoid touching on them again for something that won't > > > be adding any new feature nor fixing any known bug. > > > > We don't have to convert them all in one go right now, we can implement > > pad-level operations support selectively when a subdev driver becomes used > > by an MC-enabled host/bridge driver. > > I completely agree that we should not be duplicating the code just for sake > of it. > > Isn't the wrapper approach seems feasible here? As explained in a previous e-mail, a wrapper sounds like a good approach to me, to emulate video::* operations based on pad::* operations. We want to move to pad::* operations, so we should not perform emulation the other way around. -- 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: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
On 09/06/2011 10:50 AM, Bjørn Mork wrote: Antti Palosaari writes: I am almost sure this have been working earlier, but now it seems like nothing is acceptable for checkpatch.pl! I did surely about 20 --amend and tried to remove everything, without luck. Could someone point out whats new acceptable logging format for checkpatch.pl ? [crope@localhost linux]$ git show 1b19e42952963ae2a09a655f487de15b7c81c5b7 |./scripts/checkpatch.pl - WARNING: Do not use whitespace before Signed-off-by: Don't know if checkpatch used to accept that, but you can use "--format=email" to make it work with git show: bjorn@canardo:/usr/local/src/git/linux$ git show --format=email 1b19e42952963ae2a09a655f487de15b7c81c5b7|./scripts/checkpatch.pl - total: 0 errors, 0 warnings, 48 lines checked Your patch has no obvious style problems and is ready for submission. Yes, I found it. It was rather new patch adding more checks. As a you pointed that can be workaround giving --format=email as a param for git show. But it is yet another "useless" param to remember... So what is recommended way to ensure patch is correct currently? a) before commit b) after commit commit 2011247550c1b903a9ecd68f6eb3e9e7b7b07f52 Author: Joe Perches Date: Mon Jul 25 17:13:23 2011 -0700 checkpatch: validate signature styles and To: and Cc: lines Signatures have many forms and can sometimes cause problems if not in the correct format when using git send-email or quilt. Try to verify the signature tags and email addresses to use the generally accepted "Signed-off-by: Full Name " form. Original idea by Anish Kumar regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2] ISP:BUILD:FIX: Move media_entity_init() and
> -Original Message- > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] > Sent: Monday, September 05, 2011 6:20 PM > To: Mauro Carvalho Chehab > Cc: Hiremath, Vaibhav; Ravi, Deepthy; linux-media@vger.kernel.org; linux- > ker...@vger.kernel.org; linux-o...@vger.kernel.org > Subject: Re: [PATCHv2] ISP:BUILD:FIX: Move media_entity_init() and > > Hi Mauro, > > On Sunday 04 September 2011 15:32:28 Mauro Carvalho Chehab wrote: > > Em 04-09-2011 06:01, Laurent Pinchart escreveu: > > > On Sunday 04 September 2011 00:21:38 Mauro Carvalho Chehab wrote: > > >> Em 24-08-2011 10:25, Laurent Pinchart escreveu: > > >>> On Wednesday 24 August 2011 14:19:01 Hiremath, Vaibhav wrote: > > On Wednesday, August 24, 2011 5:00 PM Laurent Pinchart wrote: > > > On Wednesday 24 August 2011 13:21:27 Ravi, Deepthy wrote: > > >> On Wed, Aug 24, 2011 at 4:47 PM, Laurent Pinchart wrote: > > >>> On Friday 19 August 2011 15:48:45 Deepthy Ravi wrote: > > From: Vaibhav Hiremath > > > > Fix the build break caused when CONFIG_MEDIA_CONTROLLER > > option is disabled and if any sensor driver has to be used > > between MC and non MC framework compatible devices. > > > > For example,if tvp514x video decoder driver migrated to > > MC framework is being built without CONFIG_MEDIA_CONTROLLER > > option enabled, the following error messages will result. > > drivers/built-in.o: In function `tvp514x_remove': > > drivers/media/video/tvp514x.c:1285: undefined reference to > > `media_entity_cleanup' > > drivers/built-in.o: In function `tvp514x_probe': > > drivers/media/video/tvp514x.c:1237: undefined reference to > > `media_entity_init' > I don't mind splitting the config option. An alternative would be to > compile > media_entity_init() and media_entity_cleanup() based on > CONFIG_MEDIA_SUPPORT > instead of CONFIG_MEDIA_CONTROLLER, but that looks a bit hackish to me. > > > Also, I don't like the idea of increasing drivers complexity for the > > existing drivers that work properly without MC. All those core > conversions > > that were done in the last two years caused already too much instability > > to them. > > > > We should really avoid touching on them again for something that won't > be > > adding any new feature nor fixing any known bug. > > We don't have to convert them all in one go right now, we can implement > pad- > level operations support selectively when a subdev driver becomes used by > an > MC-enabled host/bridge driver. > [Hiremath, Vaibhav] I completely agree that we should not be duplicating the code just for sake of it. Isn't the wrapper approach seems feasible here? Thanks, Vaibhav > > > This will result in no modification to the userspace. > > -- > 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
[PATCH] arm: omap: Fix build error in ispccdc.c
The following build error occurs with 3.1-rc5: CC drivers/media/video/omap3isp/ispccdc.o /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c: In function 'ccdc_lsc_config': /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:427:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration] /home/data/repos/linux.trees.git/drivers/media/video/omap3isp/ispccdc.c:427:6: warning: assignment makes pointer from integer without a cast [enabled by default] This patch adds the missing 'linux/slab.h' include to fix the problem. Cc: Laurent Pinchart Cc: linux-media@vger.kernel.org Cc: linux-o...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Joerg Roedel --- drivers/media/video/omap3isp/ispccdc.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c index 9d3459d..80796eb 100644 --- a/drivers/media/video/omap3isp/ispccdc.c +++ b/drivers/media/video/omap3isp/ispccdc.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "isp.h" -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l2: uvcvideo use after free bug fix
- Original message - > Hi, > > On Tuesday 06 September 2011 15:02:54 Hans Verkuil wrote: > > On Tuesday, September 06, 2011 14:12:14 Laurent Pinchart wrote: > > > On Tuesday 06 September 2011 14:08:08 Dave Young wrote: > > > > Reported-by: Sitsofe Wheeler > > > > Signed-off-by: Dave Young > > > > Tested-by: Sitsofe Wheeler > > > > Acked-by: Laurent Pinchart > > > > > > > > Unplugging uvc video camera trigger following oops: > > > > > > > > eeepc kernel: [ 1393.500719] usb 3-2: USB disconnect, device > > > > number 4 eeepc kernel: [ 1393.504351] uvcvideo: Failed to resubmit > > > > video URB (-19). eeepc kernel: [ 1495.428853] BUG: unable to > > > > handle kernel paging request at 6b6b6bcb eeepc kernel: [ > > > > 1495.429017] IP: [] > > > > dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] *pde = > > > > eeepc kernel: [ 1495.429017] Oops: [#1] > > > > DEBUG_PAGEALLOC eeepc kernel: [ 1495.429017] > > > > eeepc kernel: [ 1495.429017] Pid: 3476, comm: cheese Not tainted > > > > 3.1.0-rc3-00270-g7a54f5e-dirty #485 ASUSTeK Computer INC. 900/900 > > > > eeepc kernel: [ 1495.429017] EIP: 0060:[] EFLAGS: > > > > 00010202 CPU: 0 eeepc kernel: [ 1495.429017] EIP is at > > > > dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] EAX: > > > > 6b6b6b6b EBX: eb08d870 ECX: EDX: eb08d930 eeepc kernel: [ > > > > 1495.429017] ESI: eb08d870 EDI: eb08d870 EBP: d3249cac ESP: > > > > d3249cac eeepc kernel: [ 1495.429017] DS: 007b ES: 007b FS: > > > > GS: 00e0 SS: 0068 eeepc kernel: [ 1495.429017] Process cheese > > > > (pid: 3476, ti=d3248000 task=df46d870 task.ti=d3248000) eeepc > > > > kernel: [ 1495.429017] Stack: eeepc kernel: [ 1495.429017] > > > > d3249cb8 b03e77a1 d307b840 d3249ccc b03e77d1 d307b840 eb08d870 > > > > eb08d830 eeepc kernel: [ 1495.429017] d3249ce4 b03ed3b7 0246 > > > > d307b840 eb08d870 d3021b80 d3249cec b03ed565 eeepc kernel: [ > > > > 1495.429017] d3249cfc b03e044d e8323d10 b06e013c d3249d18 > > > > b0355fb9 fffe d3249d1c eeepc kernel: [ 1495.429017] Call Trace: > > > > eeepc kernel: [ 1495.429017] [] > > > > v4l2_device_disconnect+0x11/0x30 eeepc kernel: [ 1495.429017] > > > > [] v4l2_device_unregister+0x11/0x50 eeepc kernel: [ > > > > 1495.429017] [] uvc_delete+0x37/0x110 eeepc kernel: [ > > > > 1495.429017] [] uvc_release+0x25/0x30 eeepc kernel: [ > > > > 1495.429017] [] v4l2_device_release+0x9d/0xc0 eeepc > > > > kernel: [ 1495.429017] [] device_release+0x19/0x90 > > > > eeepc kernel: [ 1495.429017] [] ? > > > > usb_hcd_unlink_urb+0x7c/0x90 eeepc kernel: [ 1495.429017] > > > > [] kobject_release+0x3c/0x90 eeepc kernel: [ > > > > 1495.429017] [] ? kobject_del+0x30/0x30 eeepc kernel: [ > > > > 1495.429017] [] kref_put+0x2c/0x60 eeepc kernel: [ > > > > 1495.429017] [] kobject_put+0x1d/0x50 eeepc kernel: [ > > > > 1495.429017] [] ? usb_autopm_put_interface+0x25/0x30 > > > > eeepc kernel: [ 1495.429017] [] ? > > > > uvc_v4l2_release+0x5d/0xd0 eeepc kernel: [ 1495.429017] > > > > [] put_device+0xf/0x20 eeepc kernel: [ 1495.429017] > > > > [] v4l2_release+0x56/0x60 eeepc kernel: [ 1495.429017] > > > > [] fput+0xcc/0x220 eeepc kernel: [ 1495.429017] > > > > [] filp_close+0x44/0x70 eeepc kernel: [ 1495.429017] > > > > [] put_files_struct+0x158/0x180 eeepc kernel: [ > > > > 1495.429017] [] ? put_files_struct+0x20/0x180 eeepc > > > > kernel: [ 1495.429017] [] exit_files+0x40/0x50 eeepc > > > > kernel: [ 1495.429017] [] do_exit+0x5a7/0x660 eeepc > > > > kernel: [ 1495.429017] [] ? __dequeue_signal+0x12/0x120 > > > > eeepc kernel: [ 1495.429017] [] ? > > > > _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] > > > > [] do_group_exit+0x3c/0xb0 eeepc kernel: [ 1495.429017] > > > > [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ > > > > 1495.429017] [] get_signal_to_deliver+0x18f/0x570 eeepc > > > > kernel: [ 1495.429017] [] do_signal+0x47/0x9e0 > > > > eeepc kernel: [ 1495.429017] [] ? > > > > _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] > > > > [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ > > > > 1495.429017] [] ? T.1034+0x30/0xc0 > > > > eeepc kernel: [ 1495.429017] [] ? schedule+0x29f/0x640 > > > > eeepc kernel: [ 1495.429017] [] > > > > do_notify_resume+0x38/0x40 eeepc kernel: [ 1495.429017] > > > > [] work_notifysig+0x9/0x11 eeepc kernel: [ 1495.429017] > > > > Code: e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 c3 8d b4 26 00 00 00 00 > > > > 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 00 8b 40 04 85 c0 74 > > > > f0 <8b> 40 60 5d c3 8d 74 26 00 55 89 e5 53 89 c3 83 ec 04 8b 40 > > > > 04 eeepc kernel: [ 1495.429017] EIP: [] > > > > dev_get_drvdata+0x17/0x20 SS:ESP 0068:d3249cac eeepc kernel: [ > > > > 1495.429017] CR2: 6b6b6bcb eeepc kernel: [ 1495.466975] > > > > uvcvideo: Failed to resubmit video URB (-27). eeepc kernel: [ > > > > 1495.467860] uvcvideo: Failed to resubmit video URB (-27). eeepc > > > > kernel: last message repeated 3 times eeepc kernel: [ 1495.512610] > >
Re: [PATCH] media: Add camera controls for the ov5642 driver
On Tue, Sep 06, 2011 at 01:03:57PM +, Bastian Hecht wrote:
> Hello Sakari,
>
> 2011/9/6 Sakari Ailus :
> > On Tue, Sep 06, 2011 at 09:35:24AM +, Bastian Hecht wrote:
> >> Hello Sakari,
> >>
> >> 2011/9/6 Sakari Ailus :
> >> > On Tue, Sep 06, 2011 at 09:01:15AM +, Bastian Hecht wrote:
> >> >> 2011/9/6 Sakari Ailus :
> >> >> > On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
> >> >> >> Hello Sakari!
> >> >> >
> >> >> > Hi Bastian,
> >> >> >
> >> >> >> 2011/9/6 Sakari Ailus :
> >> >> >> > Hi Bastian,
> >> >> >> >
> >> >> >> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
> >> >> >> >> 2011/9/1 Sakari Ailus :
> >> >> >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki
> >> >> >> >> > wrote:
> >> >> >> >> >> Hi Sakari,
> >> >> >> >> >>
> >> >> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
> >> >> >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi
> >> >> >> >> >> > Liakhovetski wrote:
> >> >> >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
> >> >> >> >> >> >>
> >> >> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht
> >> >> >> >> >> >>> wrote:
> >> >> >> >> >> 2011/8/28 Laurent Pinchart
> >> >> >> >> >> :
> >> >> >> >> >> >>> [clip]
> >> >> >> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
> >> >> >> >> >>
> >> >> >> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm,
> >> >> >> >> >> googled
> >> >> >> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
> >> >> >> >> >> Documentation/feature-removal-schedule.txt. I couldn't
> >> >> >> >> >> find anything.
> >> >> >> >> >> >>>
> >> >> >> >> >> >>> Hmm. Did you happen to check when that has been written? :)
> >> >> >> >> >> >>>
> >> >> >> >> >> >>> Please use this one instead:
> >> >> >> >> >> >>>
> >> >> >> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
> >> >> >> >> >> >>
> >> >> >> >> >> >> "Drivers can also implement their own custom controls using
> >> >> >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
> >> >> >> >> >> >>
> >> >> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE
> >> >> >> >> >> >> differently there?
> >> >> >> >> >> >
> >> >> >> >> >> > That was a general comment, not related to the private base.
> >> >> >> >> >> > There's no
> >> >> >> >> >> > use for a three-year-old spec as a reference!
> >> >> >> >> >> >
> >> >> >> >> >> > The control framework does not support private controls, for
> >> >> >> >> >> > example. The
> >> >> >> >> >> > controls should be put to their own class in videodev2.h
> >> >> >> >> >> > nowadays, that's my
> >> >> >> >> >> > understanding. Cc Hans.
> >> >> >> >> >>
> >> >> >> >> >> Is this really the case that we close the door for private
> >> >> >> >> >> controls in
> >> >> >> >> >> the mainline kernel ? Or am I misunderstanding something ?
> >> >> >> >> >> How about v4l2_ctrl_new_custom() ?
> >> >> >> >> >>
> >> >> >> >> >> What if there are controls applicable to single driver only ?
> >> >> >> >> >> Do we really want to have plenty of such in videodev2.h ?
> >> >> >> >> >
> >> >> >> >> > We have some of those already in videodev2.h. I'm not certain
> >> >> >> >> > if I'm happy
> >> >> >> >> > with this myself, considering e.g. that we could get a few
> >> >> >> >> > truckloads of
> >> >> >> >> > only camera lens hardware specific controls in the near future.
> >> >> >> >>
> >> >> >> >> So in my case (as these are controls that might be used by others
> >> >> >> >> too)
> >> >> >> >> I should add something like
> >> >> >> >>
> >> >> >> >> #define V4L2_CID_BLUE_SATURATION
> >> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
> >> >> >> >> #define V4L2_CID_RED_SATURATION
> >> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
> >> >> >> >
> >> >> >> > What do these two controls do? Do they control gain or something
> >> >> >> > else?
> >> >> >>
> >> >> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
> >> >> >> Saturation. To me it looks like turning up the saturation in HSV
> >> >> >> space, but only for either the blue or the red channel. This would
> >> >> >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
> >> >> >> say it is "{Red,Blue} chroma balance".
> >> >> >>
> >> >> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
> >> >> >> These are gains. So in fact I should swap them in my code and the
> >> >> >> remaining question is, how to name the red and blue gain controls.
> >> >> >
> >> >> > I think Laurent had a similar issue in his Aptina sensor driver. In my
> >> >> > opinion we need a class for low level controls such as the gain ones.
> >> >> > Do I
> >> >> > understand correctly they control the red and blue pixel gain in the
> >> >> > sensor
> >> >> > pixel matrix? Do you also have gain controls for the two greens?
> >> >>
> >> >> Yes, I assume that this is done there. Either in the analog circuit by
> >> >>
Re: use soc-camera mt9m111 with omap3isp
Hi Lee, On Tuesday 06 September 2011 15:40:09 LBM wrote: > hi Laurent Pinchart > > you say "Everything is in the latest mainline kernel." but my kernel > just 2.6.32.so I need to migrate the new V4L2 subdev to the old kernel . > please show me the diff You will need to backport all that yourself, I have no patches readily available. My advice would be to upgrade your kernel though. -- 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] v4l2: uvcvideo use after free bug fix
Hi, On Tuesday 06 September 2011 15:02:54 Hans Verkuil wrote: > On Tuesday, September 06, 2011 14:12:14 Laurent Pinchart wrote: > > On Tuesday 06 September 2011 14:08:08 Dave Young wrote: > > > Reported-by: Sitsofe Wheeler > > > Signed-off-by: Dave Young > > > Tested-by: Sitsofe Wheeler > > > Acked-by: Laurent Pinchart > > > > > > Unplugging uvc video camera trigger following oops: > > > > > > eeepc kernel: [ 1393.500719] usb 3-2: USB disconnect, device number 4 > > > eeepc kernel: [ 1393.504351] uvcvideo: Failed to resubmit video URB > > > (-19). eeepc kernel: [ 1495.428853] BUG: unable to handle kernel > > > paging request at 6b6b6bcb eeepc kernel: [ 1495.429017] IP: > > > [] > > > dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] *pde = > > > eeepc kernel: [ 1495.429017] Oops: [#1] DEBUG_PAGEALLOC > > > eeepc kernel: [ 1495.429017] > > > eeepc kernel: [ 1495.429017] Pid: 3476, comm: cheese Not tainted > > > 3.1.0-rc3-00270-g7a54f5e-dirty #485 ASUSTeK Computer INC. 900/900 eeepc > > > kernel: [ 1495.429017] EIP: 0060:[] EFLAGS: 00010202 CPU: 0 > > > eeepc kernel: [ 1495.429017] EIP is at dev_get_drvdata+0x17/0x20 > > > eeepc kernel: [ 1495.429017] EAX: 6b6b6b6b EBX: eb08d870 ECX: > > > EDX: eb08d930 eeepc kernel: [ 1495.429017] ESI: eb08d870 EDI: eb08d870 > > > EBP: d3249cac ESP: d3249cac eeepc kernel: [ 1495.429017] DS: 007b ES: > > > 007b FS: GS: 00e0 SS: 0068 eeepc kernel: [ 1495.429017] Process > > > cheese (pid: 3476, ti=d3248000 task=df46d870 task.ti=d3248000) eeepc > > > kernel: [ 1495.429017] Stack: > > > eeepc kernel: [ 1495.429017] d3249cb8 b03e77a1 d307b840 d3249ccc > > > b03e77d1 d307b840 eb08d870 eb08d830 eeepc kernel: [ 1495.429017] > > > d3249ce4 b03ed3b7 0246 d307b840 eb08d870 d3021b80 d3249cec > > > b03ed565 eeepc kernel: [ 1495.429017] d3249cfc b03e044d e8323d10 > > > b06e013c d3249d18 b0355fb9 fffe d3249d1c eeepc kernel: [ > > > 1495.429017] Call Trace: > > > eeepc kernel: [ 1495.429017] [] > > > v4l2_device_disconnect+0x11/0x30 eeepc kernel: [ 1495.429017] > > > [] v4l2_device_unregister+0x11/0x50 eeepc kernel: [ > > > 1495.429017] [] uvc_delete+0x37/0x110 eeepc kernel: [ > > > 1495.429017] [] uvc_release+0x25/0x30 eeepc kernel: [ > > > 1495.429017] [] v4l2_device_release+0x9d/0xc0 eeepc kernel: > > > [ 1495.429017] [] device_release+0x19/0x90 eeepc kernel: [ > > > 1495.429017] [] ? usb_hcd_unlink_urb+0x7c/0x90 eeepc > > > kernel: [ 1495.429017] [] kobject_release+0x3c/0x90 eeepc > > > kernel: [ 1495.429017] [] ? kobject_del+0x30/0x30 eeepc > > > kernel: [ 1495.429017] [] kref_put+0x2c/0x60 > > > eeepc kernel: [ 1495.429017] [] kobject_put+0x1d/0x50 > > > eeepc kernel: [ 1495.429017] [] ? > > > usb_autopm_put_interface+0x25/0x30 eeepc kernel: [ 1495.429017] > > > [] ? uvc_v4l2_release+0x5d/0xd0 eeepc kernel: [ 1495.429017] > > > [] put_device+0xf/0x20 > > > eeepc kernel: [ 1495.429017] [] v4l2_release+0x56/0x60 > > > eeepc kernel: [ 1495.429017] [] fput+0xcc/0x220 > > > eeepc kernel: [ 1495.429017] [] filp_close+0x44/0x70 > > > eeepc kernel: [ 1495.429017] [] put_files_struct+0x158/0x180 > > > eeepc kernel: [ 1495.429017] [] ? > > > put_files_struct+0x20/0x180 eeepc kernel: [ 1495.429017] [] > > > exit_files+0x40/0x50 eeepc kernel: [ 1495.429017] [] > > > do_exit+0x5a7/0x660 eeepc kernel: [ 1495.429017] [] ? > > > __dequeue_signal+0x12/0x120 eeepc kernel: [ 1495.429017] [] > > > ? _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] > > > [] do_group_exit+0x3c/0xb0 eeepc kernel: [ 1495.429017] > > > [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ 1495.429017] > > > [] > > > get_signal_to_deliver+0x18f/0x570 eeepc kernel: [ 1495.429017] > > > [] do_signal+0x47/0x9e0 > > > eeepc kernel: [ 1495.429017] [] ? > > > _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] > > > [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ 1495.429017] > > > [] ? T.1034+0x30/0xc0 > > > eeepc kernel: [ 1495.429017] [] ? schedule+0x29f/0x640 > > > eeepc kernel: [ 1495.429017] [] do_notify_resume+0x38/0x40 > > > eeepc kernel: [ 1495.429017] [] work_notifysig+0x9/0x11 > > > eeepc kernel: [ 1495.429017] Code: e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 > > > c3 8d b4 26 00 00 00 00 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 > > > 00 8b 40 04 85 c0 74 f0 <8b> 40 60 5d c3 8d 74 26 00 55 89 e5 53 89 c3 > > > 83 ec 04 8b 40 04 eeepc kernel: [ 1495.429017] EIP: [] > > > dev_get_drvdata+0x17/0x20 SS:ESP 0068:d3249cac eeepc kernel: [ > > > 1495.429017] CR2: 6b6b6bcb > > > eeepc kernel: [ 1495.466975] uvcvideo: Failed to resubmit video URB > > > (-27). eeepc kernel: [ 1495.467860] uvcvideo: Failed to resubmit video > > > URB (-27). eeepc kernel: last message repeated 3 times > > > eeepc kernel: [ 1495.512610] ---[ end trace 73ec16848794e5a5 ]--- > > > > > > For uvc device, dev->vdev.dev is the &intf->dev, > > > > > > uvc_delete code is as below: > > > usb_put_intf(dev->intf); > > > usb_put_dev(dev->udev); > > > > > >
[PATCH/RFC] media: Add camera controls for the ov5642 driver
The driver now supports automatic/manual gain, automatic/manual white
balance, automatic/manual exposure control, vertical flip, brightness
control, contrast control and saturation control. Additionally the
following effects are available now: rotating the hue in the colorspace,
gray scale image and solarize effect.
Signed-off-by: Bastian Hecht
---
INCOMPLETE: There are some missing defines in videodev2.h that are
discussed currently. If something like V4L2_CID_{RED,BLUE}_GAIN is added
to them, my current V4L2_CID_{RED,BLUE}_BALANCE will become
V4L2_CID_{RED,BLUE}_GAIN and OV5642_CONTROL_{RED,BLUE}_SATURATION will
become V4L2_CID_{RED,BLUE}_BALANCE. The remaining code is complete.
diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
index 41b3f51..56459f2 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
@@ -30,6 +30,18 @@
#define REG_CHIP_ID_HIGH 0x300a
#define REG_CHIP_ID_LOW0x300b
+#define REG_RED_GAIN_HIGH 0x3400
+#define REG_RED_GAIN_LOW 0x3401
+#define REG_BLUE_GAIN_HIGH 0x3404
+#define REG_BLUE_GAIN_LOW 0x3405
+#define REG_AWB_MANUAL 0x3406
+#define REG_EXP_HIGH 0x3500
+#define REG_EXP_MIDDLE 0x3501
+#define REG_EXP_LOW0x3502
+#define REG_EXP_GAIN_CTRL 0x3503
+#define REG_GAIN 0x350b
+#define REG_EXTEND_FRAME_TIME_HIGH 0x350c
+#define REG_EXTEND_FRAME_TIME_LOW 0x350d
#define REG_WINDOW_START_X_HIGH0x3800
#define REG_WINDOW_START_X_LOW 0x3801
#define REG_WINDOW_START_Y_HIGH0x3802
@@ -46,13 +58,54 @@
#define REG_OUT_TOTAL_WIDTH_LOW0x380d
#define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e
#define REG_OUT_TOTAL_HEIGHT_LOW 0x380f
+#define REG_FLIP_SUBSAMPLE 0x3818
#define REG_OUTPUT_FORMAT 0x4300
#define REG_ISP_CTRL_010x5001
+#define REG_DIGITAL_EFFECTS0x5580
+#define REG_HUE_COS0x5581
+#define REG_HUE_SIN0x5582
+#define REG_BLUE_SATURATION0x5583
+#define REG_RED_SATURATION 0x5584
+#define REG_CONTRAST 0x5588
+#define REG_BRIGHTNESS 0x5589
+#define REG_D_E_AUXILLARY 0x558a
#define REG_AVG_WINDOW_END_X_HIGH 0x5682
#define REG_AVG_WINDOW_END_X_LOW 0x5683
#define REG_AVG_WINDOW_END_Y_HIGH 0x5686
#define REG_AVG_WINDOW_END_Y_LOW 0x5687
+/* default register initialisation */
+#define REG_EXP_GAIN_INIT 0x00
+#define REG_FLIP_SUBSAMPLE_INIT0xc1
+#define REG_DIGITAL_EFFECTS_INIT 0x06
+#define REG_D_E_AUXILLARY_INIT 0x01
+
+/* default values in native space */
+#define DEFAULT_RBBALANCE 0x400
+#define DEFAULT_CONTRAST 0x20
+#define DEFAULT_SATURATION 0x40
+
+#define MAX_EXP_NATIVE 0x01
+#define MAX_GAIN_NATIVE0x1f
+#define MAX_RBBALANCE_NATIVE 0x0fff
+#define MAX_EXP0x
+#define MAX_GAIN 0xff
+#define MAX_RBBALANCE 0xff
+#define MAX_HUE_TRIG_NATIVE0x80
+
+#define OV5642_CONTROL_BLUE_SATURATION (V4L2_CID_PRIVATE_BASE + 0)
+#define OV5642_CONTROL_RED_SATURATION (V4L2_CID_PRIVATE_BASE + 1)
+
+#define EXP_V4L2_TO_NATIVE(x) ((x) << 4)
+#define EXP_NATIVE_TO_V4L2(x) ((x) >> 4)
+#define GAIN_V4L2_TO_NATIVE(x) ((x) * MAX_GAIN_NATIVE / MAX_GAIN)
+#define GAIN_NATIVE_TO_V4L2(x) ((x) * MAX_GAIN / MAX_GAIN_NATIVE)
+#define RBBALANCE_V4L2_TO_NATIVE(x) ((x) * MAX_RBBALANCE_NATIVE /
MAX_RBBALANCE)
+#define RBBALANCE_NATIVE_TO_V4L2(x) ((x) * MAX_RBBALANCE /
MAX_RBBALANCE_NATIVE)
+
+/* flaw in the datasheet. we need some extra lines */
+#define MANUAL_LONG_EXP_SAFETY_DISTANCE20
+
/* active pixel array size */
#define OV5642_SENSOR_SIZE_X 2592
#define OV5642_SENSOR_SIZE_Y 1944
@@ -85,6 +138,9 @@
*/
#define BLANKING_MIN_HEIGHT1000
+static int ov5642_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl);
+static int ov5642_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl);
+
struct regval_list {
u16 reg_num;
u8 value;
@@ -132,10 +188,8 @@ static struct regval_list ov5642_default_regs_init[] = {
{ 0x471d, 0x5 },
{ 0x4708, 0x6 },
{ 0x370c, 0xa0 },
- { 0x5687, 0x94 },
{ 0x501f, 0x0 },
{ 0x5000, 0x4f },
- { 0x5001, 0xcf },
{ 0x4300, 0x30 },
{ 0x4300, 0x30 },
{ 0x460b, 0x35 },
@@ -148,11 +202,8 @@ static struct regval_list ov5642_default_regs_init[] = {
{ 0x4402, 0x90 },
{ 0x460c, 0x22 },
{ 0x3815, 0x44 },
- { 0x3503, 0x7 },
{ 0x3501, 0x73 },
{ 0x3502, 0x80 },
- { 0x350b, 0x0 },
- { 0x3818, 0xc8 },
Re: [RFC/PATCH 0/1] Ignore ctrl_class
On Tue, Sep 06, 2011 at 02:55:39PM +0200, Hans Verkuil wrote:
> On Tuesday, September 06, 2011 13:45:48 Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Tue, Sep 06, 2011 at 01:20:26PM +0200, Hans Verkuil wrote:
> > > On Tuesday, September 06, 2011 13:07:42 Sakari Ailus wrote:
> > > > Hi,
> > > >
> > > > I remember being in a discussion a while ago regarding the requirement
> > > > of
> > > > having all the controls belonging to the same class in
> > > > VIDIOC_{TRY,S,G}_EXT_CTRLS. The answer I remember was that there was a
> > > > historical reason for this and it no longer exists.
> > >
> > > The original rule was that all controls have to belong to the same class.
> > > This was
> > > done to simplify drivers. Drivers that use the control framework can
> > > handle a class
> > > of 0, which means that the controls can be of any class.
> > >
> > > But we still have drivers that implement S_EXT_CTRLS but do not use the
> > > control
> > > framework, and for those this restriction is still valid. Usually such
> > > drivers will only
> > > handle MPEG class controls through that API.
> > >
> > > So I don't think this restriction can be lifted as long as there are
> > > drivers that do not
> > > use the control framework.
> >
> > All the drivers which implement *_EXT_CTRLS and check for ctrl_class do the
> > check for a single class. All the references for ctrl_class in individual
> > drivers (which actually were only checks that the user has set the field
> > correctly) are removed by the patch I posted.
> >
> > So I don't see a reason why we couldn't just say "please set this to zero
> > from now on".
> >
> >
>
> From what I remember (and I may be wrong by now) the drivers that implement
> S_EXT_CTRLS
> by themselves typically only support ext_ctrls for controls of a specific
> class (MPEG usually).
That's what I also found out.
> Dropping the check means that: 1) applications may think they can use any
> control when they can't for a certain group of drivers, and 2)
Why? This doesn't change how VIDIOC_QUERYCTRL works. Or am I missing
something?
> applications can no longer detect up front whether a driver supports
> mixing of control classes or not.
I think we shouldn't accept new drivers which don't use the control
framework. The existing drivers only implement controls in a single class,
or ignore the ctrl_class field. The patch I sent removes the check which I
see has only been there to comply with the spec.
> The way you can do 2) is by setting the control class to 0 and calling
> G/S/TRY_EXT_CTRLS with
> 0 controls.
>
> Once everything is converted I don't mind dropping this check, but until then
> I believe it should
> stay.
The patch does everything that is required for this as far as I see.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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] v4l2: uvcvideo use after free bug fix
On Tuesday, September 06, 2011 14:12:14 Laurent Pinchart wrote: > Hans, > > On Tuesday 06 September 2011 14:08:08 Dave Young wrote: > > Reported-by: Sitsofe Wheeler > > Signed-off-by: Dave Young > > Tested-by: Sitsofe Wheeler > > Acked-by: Laurent Pinchart > > > > Unplugging uvc video camera trigger following oops: > > > > eeepc kernel: [ 1393.500719] usb 3-2: USB disconnect, device number 4 > > eeepc kernel: [ 1393.504351] uvcvideo: Failed to resubmit video URB (-19). > > eeepc kernel: [ 1495.428853] BUG: unable to handle kernel paging request at > > 6b6b6bcb eeepc kernel: [ 1495.429017] IP: [] > > dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] *pde = > > eeepc kernel: [ 1495.429017] Oops: [#1] DEBUG_PAGEALLOC > > eeepc kernel: [ 1495.429017] > > eeepc kernel: [ 1495.429017] Pid: 3476, comm: cheese Not tainted > > 3.1.0-rc3-00270-g7a54f5e-dirty #485 ASUSTeK Computer INC. 900/900 eeepc > > kernel: [ 1495.429017] EIP: 0060:[] EFLAGS: 00010202 CPU: 0 > > eeepc kernel: [ 1495.429017] EIP is at dev_get_drvdata+0x17/0x20 > > eeepc kernel: [ 1495.429017] EAX: 6b6b6b6b EBX: eb08d870 ECX: EDX: > > eb08d930 eeepc kernel: [ 1495.429017] ESI: eb08d870 EDI: eb08d870 EBP: > > d3249cac ESP: d3249cac eeepc kernel: [ 1495.429017] DS: 007b ES: 007b FS: > > GS: 00e0 SS: 0068 eeepc kernel: [ 1495.429017] Process cheese (pid: > > 3476, ti=d3248000 task=df46d870 task.ti=d3248000) eeepc kernel: [ > > 1495.429017] Stack: > > eeepc kernel: [ 1495.429017] d3249cb8 b03e77a1 d307b840 d3249ccc b03e77d1 > > d307b840 eb08d870 eb08d830 eeepc kernel: [ 1495.429017] d3249ce4 b03ed3b7 > > 0246 d307b840 eb08d870 d3021b80 d3249cec b03ed565 eeepc kernel: [ > > 1495.429017] d3249cfc b03e044d e8323d10 b06e013c d3249d18 b0355fb9 > > fffe d3249d1c eeepc kernel: [ 1495.429017] Call Trace: > > eeepc kernel: [ 1495.429017] [] v4l2_device_disconnect+0x11/0x30 > > eeepc kernel: [ 1495.429017] [] v4l2_device_unregister+0x11/0x50 > > eeepc kernel: [ 1495.429017] [] uvc_delete+0x37/0x110 > > eeepc kernel: [ 1495.429017] [] uvc_release+0x25/0x30 > > eeepc kernel: [ 1495.429017] [] v4l2_device_release+0x9d/0xc0 > > eeepc kernel: [ 1495.429017] [] device_release+0x19/0x90 > > eeepc kernel: [ 1495.429017] [] ? usb_hcd_unlink_urb+0x7c/0x90 > > eeepc kernel: [ 1495.429017] [] kobject_release+0x3c/0x90 > > eeepc kernel: [ 1495.429017] [] ? kobject_del+0x30/0x30 > > eeepc kernel: [ 1495.429017] [] kref_put+0x2c/0x60 > > eeepc kernel: [ 1495.429017] [] kobject_put+0x1d/0x50 > > eeepc kernel: [ 1495.429017] [] ? > > usb_autopm_put_interface+0x25/0x30 eeepc kernel: [ 1495.429017] > > [] ? uvc_v4l2_release+0x5d/0xd0 eeepc kernel: [ 1495.429017] > > [] put_device+0xf/0x20 > > eeepc kernel: [ 1495.429017] [] v4l2_release+0x56/0x60 > > eeepc kernel: [ 1495.429017] [] fput+0xcc/0x220 > > eeepc kernel: [ 1495.429017] [] filp_close+0x44/0x70 > > eeepc kernel: [ 1495.429017] [] put_files_struct+0x158/0x180 > > eeepc kernel: [ 1495.429017] [] ? put_files_struct+0x20/0x180 > > eeepc kernel: [ 1495.429017] [] exit_files+0x40/0x50 > > eeepc kernel: [ 1495.429017] [] do_exit+0x5a7/0x660 > > eeepc kernel: [ 1495.429017] [] ? __dequeue_signal+0x12/0x120 > > eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 > > eeepc kernel: [ 1495.429017] [] do_group_exit+0x3c/0xb0 > > eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 > > eeepc kernel: [ 1495.429017] [] > > get_signal_to_deliver+0x18f/0x570 eeepc kernel: [ 1495.429017] > > [] do_signal+0x47/0x9e0 > > eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 > > eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 > > eeepc kernel: [ 1495.429017] [] ? T.1034+0x30/0xc0 > > eeepc kernel: [ 1495.429017] [] ? schedule+0x29f/0x640 > > eeepc kernel: [ 1495.429017] [] do_notify_resume+0x38/0x40 > > eeepc kernel: [ 1495.429017] [] work_notifysig+0x9/0x11 > > eeepc kernel: [ 1495.429017] Code: e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 c3 > > 8d b4 26 00 00 00 00 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 00 8b 40 > > 04 85 c0 74 f0 <8b> 40 60 5d c3 8d 74 26 00 55 89 e5 53 89 c3 83 ec 04 8b > > 40 04 eeepc kernel: [ 1495.429017] EIP: [] > > dev_get_drvdata+0x17/0x20 SS:ESP 0068:d3249cac eeepc kernel: [ > > 1495.429017] CR2: 6b6b6bcb > > eeepc kernel: [ 1495.466975] uvcvideo: Failed to resubmit video URB (-27). > > eeepc kernel: [ 1495.467860] uvcvideo: Failed to resubmit video URB (-27). > > eeepc kernel: last message repeated 3 times > > eeepc kernel: [ 1495.512610] ---[ end trace 73ec16848794e5a5 ]--- > > > > For uvc device, dev->vdev.dev is the &intf->dev, > > uvc_delete code is as below: > > usb_put_intf(dev->intf); > > usb_put_dev(dev->udev); > > > > uvc_status_cleanup(dev); > > uvc_ctrl_cleanup_device(dev); > > > > ## the intf dev is released above, so below code will oops. > > > > if (dev->vdev.dev) > > v4l2_device_unregister(&dev->vdev); > > > > Fix it by get_device
Re: [PATCHv2] adp1653: make ->power() method optional
Hi Andy, On Thu, Aug 18, 2011 at 04:32:21PM +0300, Andy Shevchenko wrote: > On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: > > On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: > > > On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > > > > The ->power() could be absent or not used on some platforms. This patch > > > > makes > > > > its presence optional. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > Cc: Sakari Ailus > > > > --- > > > > drivers/media/video/adp1653.c |5 + > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/drivers/media/video/adp1653.c > > > > b/drivers/media/video/adp1653.c > > > > index 0fd9579..f830313 100644 > > > > --- a/drivers/media/video/adp1653.c > > > > +++ b/drivers/media/video/adp1653.c > > > > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int > > > > on) > > > > struct adp1653_flash *flash = to_adp1653_flash(subdev); > > > > int ret = 0; > > > > > > > > + /* There is no need to switch power in case of absence ->power() > > > > +* method. */ > > > > + if (flash->platform_data->power == NULL) > > > > + return 0; > > > > + > > > > mutex_lock(&flash->power_lock); > > > > > > > > /* If the power count is modified from 0 to != 0 or from != 0 > > > > to 0, > > > > > > He-h, I guess you are not going to apply this one. > > > The patch breaks init logic of the device. If we have no ->power(), we > > > still need to bring the device to the known state. I have no good idea > > > how to do this. > > > > I don't think it breaks anything actually. Albeit in practice one is still > > likely to put the adp1653 reset line to the board since that lowers its > > power > > consumption significantly. > Yeah, even in practice we might see various ways of a chip connection. That's true. But I think the bottom line is that these should be modelled in a generic way; the last resort is a board specific GPIO or regulator driver. This is the way ARM platform is moving in Linux and the sooner we adapt to that, the better. Otherwise we'll be in trouble. Of course we need to actively make our concerns known for others. Speaking of which, I don't think Intel (what comes to embedded CPUs) is necessarily in a very different position than the ARM vendors; ARM is exhibiting such symptoms not only because the vendors are very inventive with the hardware but also simply because these systems are embedded systems. Typically only few if any devices can be probed, for example, and there are various means for interacting with things like flash controllers. The same regulators and GPIOs are present there as well and the hardware description must be somehow available to the drivers. > > Instead of being in power-up state after opening the flash subdev, it will > > reach this state already when the system is powered up. At subdev open all > > the relevant registers are written to anyway, so I don't see an issue here. > You mean at first writing to the V4L2 value, do you? Because ->open() > uses set_power() which will be skipped in case of no ->power method > defined. > > > I think either this one, or one should check in probe() that the power() > > callback is non-NULL. > > The board code is going away in the near future so this callback will > > disappear eventually anyway. > So, it's up to you to include or not my last patch. My opinion is that instead of checking the power callback, the platform data needs to contain the GPIO number instead. The driver can then use the GPIO framework to toggle it. > > > The gpio code in the board file should likely > > be moved to the driver itself. > The line could be different, the hw could be used in environment w/o > gpio, but with (for example) external gate, and so on. I think current > generic driver is pretty okay. > > And what to do with limits? Pass them as the module parameters? > > > That assumes that there will be a gpio which > > can be used to enable and disable the device and I'm not fully certain this > > is generic enough. Hopefully it is, but I don't know where else the adp1653 > > would be used than on the N900. > Don't narrow a chip application to the one device. We don't, but we also don't generalise something that has no use (yet). -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
Hello Sakari,
2011/9/6 Sakari Ailus :
> On Tue, Sep 06, 2011 at 09:35:24AM +, Bastian Hecht wrote:
>> Hello Sakari,
>>
>> 2011/9/6 Sakari Ailus :
>> > On Tue, Sep 06, 2011 at 09:01:15AM +, Bastian Hecht wrote:
>> >> 2011/9/6 Sakari Ailus :
>> >> > On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
>> >> >> Hello Sakari!
>> >> >
>> >> > Hi Bastian,
>> >> >
>> >> >> 2011/9/6 Sakari Ailus :
>> >> >> > Hi Bastian,
>> >> >> >
>> >> >> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
>> >> >> >> 2011/9/1 Sakari Ailus :
>> >> >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki
>> >> >> >> > wrote:
>> >> >> >> >> Hi Sakari,
>> >> >> >> >>
>> >> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
>> >> >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi
>> >> >> >> >> > Liakhovetski wrote:
>> >> >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
>> >> >> >> >> >>
>> >> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht
>> >> >> >> >> >>> wrote:
>> >> >> >> >> 2011/8/28 Laurent Pinchart
>> >> >> >> >> :
>> >> >> >> >> >>> [clip]
>> >> >> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
>> >> >> >> >>
>> >> >> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm,
>> >> >> >> >> googled
>> >> >> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
>> >> >> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
>> >> >> >> >> anything.
>> >> >> >> >> >>>
>> >> >> >> >> >>> Hmm. Did you happen to check when that has been written? :)
>> >> >> >> >> >>>
>> >> >> >> >> >>> Please use this one instead:
>> >> >> >> >> >>>
>> >> >> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
>> >> >> >> >> >>
>> >> >> >> >> >> "Drivers can also implement their own custom controls using
>> >> >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
>> >> >> >> >> >>
>> >> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE
>> >> >> >> >> >> differently there?
>> >> >> >> >> >
>> >> >> >> >> > That was a general comment, not related to the private base.
>> >> >> >> >> > There's no
>> >> >> >> >> > use for a three-year-old spec as a reference!
>> >> >> >> >> >
>> >> >> >> >> > The control framework does not support private controls, for
>> >> >> >> >> > example. The
>> >> >> >> >> > controls should be put to their own class in videodev2.h
>> >> >> >> >> > nowadays, that's my
>> >> >> >> >> > understanding. Cc Hans.
>> >> >> >> >>
>> >> >> >> >> Is this really the case that we close the door for private
>> >> >> >> >> controls in
>> >> >> >> >> the mainline kernel ? Or am I misunderstanding something ?
>> >> >> >> >> How about v4l2_ctrl_new_custom() ?
>> >> >> >> >>
>> >> >> >> >> What if there are controls applicable to single driver only ?
>> >> >> >> >> Do we really want to have plenty of such in videodev2.h ?
>> >> >> >> >
>> >> >> >> > We have some of those already in videodev2.h. I'm not certain if
>> >> >> >> > I'm happy
>> >> >> >> > with this myself, considering e.g. that we could get a few
>> >> >> >> > truckloads of
>> >> >> >> > only camera lens hardware specific controls in the near future.
>> >> >> >>
>> >> >> >> So in my case (as these are controls that might be used by others
>> >> >> >> too)
>> >> >> >> I should add something like
>> >> >> >>
>> >> >> >> #define V4L2_CID_BLUE_SATURATION
>> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
>> >> >> >> #define V4L2_CID_RED_SATURATION
>> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
>> >> >> >
>> >> >> > What do these two controls do? Do they control gain or something
>> >> >> > else?
>> >> >>
>> >> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
>> >> >> Saturation. To me it looks like turning up the saturation in HSV
>> >> >> space, but only for either the blue or the red channel. This would
>> >> >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
>> >> >> say it is "{Red,Blue} chroma balance".
>> >> >>
>> >> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
>> >> >> These are gains. So in fact I should swap them in my code and the
>> >> >> remaining question is, how to name the red and blue gain controls.
>> >> >
>> >> > I think Laurent had a similar issue in his Aptina sensor driver. In my
>> >> > opinion we need a class for low level controls such as the gain ones.
>> >> > Do I
>> >> > understand correctly they control the red and blue pixel gain in the
>> >> > sensor
>> >> > pixel matrix? Do you also have gain controls for the two greens?
>> >>
>> >> Yes, I assume that this is done there. Either in the analog circuit by
>> >> decreasing the preload or digitally then. Don't know exactly. There
>> >> are registers for the green pixels as well. As I used the
>> >> V4L2_CID_{RED,BLUE}_BALANCE controls and there was no
>> >> V4L2_CID_GREEN_BALANCE, I just skipped green as one can
>> >> increase/decrease
Re: [RFC/PATCH 0/1] Ignore ctrl_class
On Tuesday, September 06, 2011 13:45:48 Sakari Ailus wrote:
> Hi Hans,
>
> On Tue, Sep 06, 2011 at 01:20:26PM +0200, Hans Verkuil wrote:
> > On Tuesday, September 06, 2011 13:07:42 Sakari Ailus wrote:
> > > Hi,
> > >
> > > I remember being in a discussion a while ago regarding the requirement of
> > > having all the controls belonging to the same class in
> > > VIDIOC_{TRY,S,G}_EXT_CTRLS. The answer I remember was that there was a
> > > historical reason for this and it no longer exists.
> >
> > The original rule was that all controls have to belong to the same class.
> > This was
> > done to simplify drivers. Drivers that use the control framework can handle
> > a class
> > of 0, which means that the controls can be of any class.
> >
> > But we still have drivers that implement S_EXT_CTRLS but do not use the
> > control
> > framework, and for those this restriction is still valid. Usually such
> > drivers will only
> > handle MPEG class controls through that API.
> >
> > So I don't think this restriction can be lifted as long as there are
> > drivers that do not
> > use the control framework.
>
> All the drivers which implement *_EXT_CTRLS and check for ctrl_class do the
> check for a single class. All the references for ctrl_class in individual
> drivers (which actually were only checks that the user has set the field
> correctly) are removed by the patch I posted.
>
> So I don't see a reason why we couldn't just say "please set this to zero
> from now on".
>
>
>From what I remember (and I may be wrong by now) the drivers that implement
>S_EXT_CTRLS
by themselves typically only support ext_ctrls for controls of a specific class
(MPEG usually).
Dropping the check means that: 1) applications may think they can use any
control when they
can't for a certain group of drivers, and 2) applications can no longer detect
up front whether
a driver supports mixing of control classes or not.
The way you can do 2) is by setting the control class to 0 and calling
G/S/TRY_EXT_CTRLS with
0 controls.
Once everything is converted I don't mind dropping this check, but until then I
believe it should
stay.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
On Tue, Sep 06, 2011 at 09:35:24AM +, Bastian Hecht wrote:
> Hello Sakari,
>
> 2011/9/6 Sakari Ailus :
> > On Tue, Sep 06, 2011 at 09:01:15AM +, Bastian Hecht wrote:
> >> 2011/9/6 Sakari Ailus :
> >> > On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
> >> >> Hello Sakari!
> >> >
> >> > Hi Bastian,
> >> >
> >> >> 2011/9/6 Sakari Ailus :
> >> >> > Hi Bastian,
> >> >> >
> >> >> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
> >> >> >> 2011/9/1 Sakari Ailus :
> >> >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
> >> >> >> >> Hi Sakari,
> >> >> >> >>
> >> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
> >> >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski
> >> >> >> >> > wrote:
> >> >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
> >> >> >> >> >>
> >> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
> >> >> >> >> 2011/8/28 Laurent Pinchart
> >> >> >> >> :
> >> >> >> >> >>> [clip]
> >> >> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
> >> >> >> >>
> >> >> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm,
> >> >> >> >> googled
> >> >> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
> >> >> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
> >> >> >> >> anything.
> >> >> >> >> >>>
> >> >> >> >> >>> Hmm. Did you happen to check when that has been written? :)
> >> >> >> >> >>>
> >> >> >> >> >>> Please use this one instead:
> >> >> >> >> >>>
> >> >> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
> >> >> >> >> >>
> >> >> >> >> >> "Drivers can also implement their own custom controls using
> >> >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
> >> >> >> >> >>
> >> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE
> >> >> >> >> >> differently there?
> >> >> >> >> >
> >> >> >> >> > That was a general comment, not related to the private base.
> >> >> >> >> > There's no
> >> >> >> >> > use for a three-year-old spec as a reference!
> >> >> >> >> >
> >> >> >> >> > The control framework does not support private controls, for
> >> >> >> >> > example. The
> >> >> >> >> > controls should be put to their own class in videodev2.h
> >> >> >> >> > nowadays, that's my
> >> >> >> >> > understanding. Cc Hans.
> >> >> >> >>
> >> >> >> >> Is this really the case that we close the door for private
> >> >> >> >> controls in
> >> >> >> >> the mainline kernel ? Or am I misunderstanding something ?
> >> >> >> >> How about v4l2_ctrl_new_custom() ?
> >> >> >> >>
> >> >> >> >> What if there are controls applicable to single driver only ?
> >> >> >> >> Do we really want to have plenty of such in videodev2.h ?
> >> >> >> >
> >> >> >> > We have some of those already in videodev2.h. I'm not certain if
> >> >> >> > I'm happy
> >> >> >> > with this myself, considering e.g. that we could get a few
> >> >> >> > truckloads of
> >> >> >> > only camera lens hardware specific controls in the near future.
> >> >> >>
> >> >> >> So in my case (as these are controls that might be used by others
> >> >> >> too)
> >> >> >> I should add something like
> >> >> >>
> >> >> >> #define V4L2_CID_BLUE_SATURATION
> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
> >> >> >> #define V4L2_CID_RED_SATURATION
> >> >> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
> >> >> >
> >> >> > What do these two controls do? Do they control gain or something else?
> >> >>
> >> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
> >> >> Saturation. To me it looks like turning up the saturation in HSV
> >> >> space, but only for either the blue or the red channel. This would
> >> >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
> >> >> say it is "{Red,Blue} chroma balance".
> >> >>
> >> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
> >> >> These are gains. So in fact I should swap them in my code and the
> >> >> remaining question is, how to name the red and blue gain controls.
> >> >
> >> > I think Laurent had a similar issue in his Aptina sensor driver. In my
> >> > opinion we need a class for low level controls such as the gain ones. Do
> >> > I
> >> > understand correctly they control the red and blue pixel gain in the
> >> > sensor
> >> > pixel matrix? Do you also have gain controls for the two greens?
> >>
> >> Yes, I assume that this is done there. Either in the analog circuit by
> >> decreasing the preload or digitally then. Don't know exactly. There
> >> are registers for the green pixels as well. As I used the
> >> V4L2_CID_{RED,BLUE}_BALANCE controls and there was no
> >> V4L2_CID_GREEN_BALANCE, I just skipped green as one can
> >> increase/decrease the global gain and get an arbitrary mix as well.
> >>
> >> So for these gain settings we should add these?
> >> V4L2_CID_RED_GAIN
> >> V4L2_CID_BLUE_GAIN
> >> V4L2_CID_GREEN_GAIN
> >
> > Do y
Re: [RFC] New class for low level sensors controls?
On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 06 September 2011 13:36:53 Sakari Ailus wrote: > > Hi all, > > > > We are beginning to have raw bayer image sensor drivers in the mainline. > > Typically such sensors are not controlled by general purpose applications > > but e.g. require a camera control algorithm framework in user space. This > > needs to be implemented in libv4l for general purpose applications to work > > properly on this kind of hardware. > > > > These sensors expose controls such as > > > > - Per-component gain controls. Red, blue, green (blue) and green (red) > > gains. > > > > - Link frequency. The frequency of the data link from the sensor to the > > bridge. > > > > - Horizontal and vertical blanking. > > Other controls often found in bayer sensors are black level compensation and > test pattern. > > > None of these controls are suitable for use of general purpose applications > > (let alone the end user!) but for the camera control algorithms. > > > > We have a control class called V4L2_CTRL_CLASS_CAMERA for camera controls. > > However, the controls in this class are relatively high level controls > > which are suitable for end user. The algorithms in the libv4l or a webcam > > could implement many of these controls whereas I see that only > > V4L2_CID_EXPOSURE_ABSOLUTE might be implemented by raw bayer sensors. > > > > My question is: would it make sense to create a new class of controls for > > the low level sensor controls in a similar fashion we have a control class > > for the flash controls? > > I think it would, but I'm not sure how we should name that class. > V4L2_CTRL_CLASS_SENSOR is tempting, but many of the controls that will be > found there (digital gains, black leverl compensation, test pattern, ...) can > also be found in ISPs or other hardware blocks. I don't think ISPs typically implement test patterns. Do you know of any? Should we separate controls which clearly apply to sensors only from the rest? For sensors only: - Analog gain(s) - Horizontal and vertical blanking - Link frequency - Test pattern The following can be implemented also on ISPs: - Per-component gains - Black level compensation Do we have more to add to the list? If we keep the two the same class, I could propose the following names: V4L2_CTRL_CLASS_LL_CAMERA (for low level camera) V4L2_CTRL_CLASS_SOURCE V4L2_CTRL_CLASS_IMAGE_SOURCE The last one would be a good name for the sensor control class, as far as I understand some are using tuners with the OMAP 3 ISP these days. For the another one, I propose V4L2_CTRL_CLASS_ISP. Better names are always welcome. :-) -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- 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] v4l2: uvcvideo use after free bug fix
Hans, On Tuesday 06 September 2011 14:08:08 Dave Young wrote: > Reported-by: Sitsofe Wheeler > Signed-off-by: Dave Young > Tested-by: Sitsofe Wheeler > Acked-by: Laurent Pinchart > > Unplugging uvc video camera trigger following oops: > > eeepc kernel: [ 1393.500719] usb 3-2: USB disconnect, device number 4 > eeepc kernel: [ 1393.504351] uvcvideo: Failed to resubmit video URB (-19). > eeepc kernel: [ 1495.428853] BUG: unable to handle kernel paging request at > 6b6b6bcb eeepc kernel: [ 1495.429017] IP: [] > dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] *pde = > eeepc kernel: [ 1495.429017] Oops: [#1] DEBUG_PAGEALLOC > eeepc kernel: [ 1495.429017] > eeepc kernel: [ 1495.429017] Pid: 3476, comm: cheese Not tainted > 3.1.0-rc3-00270-g7a54f5e-dirty #485 ASUSTeK Computer INC. 900/900 eeepc > kernel: [ 1495.429017] EIP: 0060:[] EFLAGS: 00010202 CPU: 0 > eeepc kernel: [ 1495.429017] EIP is at dev_get_drvdata+0x17/0x20 > eeepc kernel: [ 1495.429017] EAX: 6b6b6b6b EBX: eb08d870 ECX: EDX: > eb08d930 eeepc kernel: [ 1495.429017] ESI: eb08d870 EDI: eb08d870 EBP: > d3249cac ESP: d3249cac eeepc kernel: [ 1495.429017] DS: 007b ES: 007b FS: > GS: 00e0 SS: 0068 eeepc kernel: [ 1495.429017] Process cheese (pid: > 3476, ti=d3248000 task=df46d870 task.ti=d3248000) eeepc kernel: [ > 1495.429017] Stack: > eeepc kernel: [ 1495.429017] d3249cb8 b03e77a1 d307b840 d3249ccc b03e77d1 > d307b840 eb08d870 eb08d830 eeepc kernel: [ 1495.429017] d3249ce4 b03ed3b7 > 0246 d307b840 eb08d870 d3021b80 d3249cec b03ed565 eeepc kernel: [ > 1495.429017] d3249cfc b03e044d e8323d10 b06e013c d3249d18 b0355fb9 > fffe d3249d1c eeepc kernel: [ 1495.429017] Call Trace: > eeepc kernel: [ 1495.429017] [] v4l2_device_disconnect+0x11/0x30 > eeepc kernel: [ 1495.429017] [] v4l2_device_unregister+0x11/0x50 > eeepc kernel: [ 1495.429017] [] uvc_delete+0x37/0x110 > eeepc kernel: [ 1495.429017] [] uvc_release+0x25/0x30 > eeepc kernel: [ 1495.429017] [] v4l2_device_release+0x9d/0xc0 > eeepc kernel: [ 1495.429017] [] device_release+0x19/0x90 > eeepc kernel: [ 1495.429017] [] ? usb_hcd_unlink_urb+0x7c/0x90 > eeepc kernel: [ 1495.429017] [] kobject_release+0x3c/0x90 > eeepc kernel: [ 1495.429017] [] ? kobject_del+0x30/0x30 > eeepc kernel: [ 1495.429017] [] kref_put+0x2c/0x60 > eeepc kernel: [ 1495.429017] [] kobject_put+0x1d/0x50 > eeepc kernel: [ 1495.429017] [] ? > usb_autopm_put_interface+0x25/0x30 eeepc kernel: [ 1495.429017] > [] ? uvc_v4l2_release+0x5d/0xd0 eeepc kernel: [ 1495.429017] > [] put_device+0xf/0x20 > eeepc kernel: [ 1495.429017] [] v4l2_release+0x56/0x60 > eeepc kernel: [ 1495.429017] [] fput+0xcc/0x220 > eeepc kernel: [ 1495.429017] [] filp_close+0x44/0x70 > eeepc kernel: [ 1495.429017] [] put_files_struct+0x158/0x180 > eeepc kernel: [ 1495.429017] [] ? put_files_struct+0x20/0x180 > eeepc kernel: [ 1495.429017] [] exit_files+0x40/0x50 > eeepc kernel: [ 1495.429017] [] do_exit+0x5a7/0x660 > eeepc kernel: [ 1495.429017] [] ? __dequeue_signal+0x12/0x120 > eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 > eeepc kernel: [ 1495.429017] [] do_group_exit+0x3c/0xb0 > eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 > eeepc kernel: [ 1495.429017] [] > get_signal_to_deliver+0x18f/0x570 eeepc kernel: [ 1495.429017] > [] do_signal+0x47/0x9e0 > eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 > eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 > eeepc kernel: [ 1495.429017] [] ? T.1034+0x30/0xc0 > eeepc kernel: [ 1495.429017] [] ? schedule+0x29f/0x640 > eeepc kernel: [ 1495.429017] [] do_notify_resume+0x38/0x40 > eeepc kernel: [ 1495.429017] [] work_notifysig+0x9/0x11 > eeepc kernel: [ 1495.429017] Code: e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 c3 > 8d b4 26 00 00 00 00 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 00 8b 40 > 04 85 c0 74 f0 <8b> 40 60 5d c3 8d 74 26 00 55 89 e5 53 89 c3 83 ec 04 8b > 40 04 eeepc kernel: [ 1495.429017] EIP: [] > dev_get_drvdata+0x17/0x20 SS:ESP 0068:d3249cac eeepc kernel: [ > 1495.429017] CR2: 6b6b6bcb > eeepc kernel: [ 1495.466975] uvcvideo: Failed to resubmit video URB (-27). > eeepc kernel: [ 1495.467860] uvcvideo: Failed to resubmit video URB (-27). > eeepc kernel: last message repeated 3 times > eeepc kernel: [ 1495.512610] ---[ end trace 73ec16848794e5a5 ]--- > > For uvc device, dev->vdev.dev is the &intf->dev, > uvc_delete code is as below: > usb_put_intf(dev->intf); > usb_put_dev(dev->udev); > > uvc_status_cleanup(dev); > uvc_ctrl_cleanup_device(dev); > > ## the intf dev is released above, so below code will oops. > > if (dev->vdev.dev) > v4l2_device_unregister(&dev->vdev); > > Fix it by get_device in v4l2_device_register and put_device in > v4l2_device_disconnect --- > drivers/media/video/v4l2-device.c |2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/video/v4l2-device.c > b/drivers/media/video/v4l2-d
[PATCH] v4l2: uvcvideo use after free bug fix
Reported-by: Sitsofe Wheeler Signed-off-by: Dave Young Tested-by: Sitsofe Wheeler Acked-by: Laurent Pinchart Unplugging uvc video camera trigger following oops: eeepc kernel: [ 1393.500719] usb 3-2: USB disconnect, device number 4 eeepc kernel: [ 1393.504351] uvcvideo: Failed to resubmit video URB (-19). eeepc kernel: [ 1495.428853] BUG: unable to handle kernel paging request at 6b6b6bcb eeepc kernel: [ 1495.429017] IP: [] dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] *pde = eeepc kernel: [ 1495.429017] Oops: [#1] DEBUG_PAGEALLOC eeepc kernel: [ 1495.429017] eeepc kernel: [ 1495.429017] Pid: 3476, comm: cheese Not tainted 3.1.0-rc3-00270-g7a54f5e-dirty #485 ASUSTeK Computer INC. 900/900 eeepc kernel: [ 1495.429017] EIP: 0060:[] EFLAGS: 00010202 CPU: 0 eeepc kernel: [ 1495.429017] EIP is at dev_get_drvdata+0x17/0x20 eeepc kernel: [ 1495.429017] EAX: 6b6b6b6b EBX: eb08d870 ECX: EDX: eb08d930 eeepc kernel: [ 1495.429017] ESI: eb08d870 EDI: eb08d870 EBP: d3249cac ESP: d3249cac eeepc kernel: [ 1495.429017] DS: 007b ES: 007b FS: GS: 00e0 SS: 0068 eeepc kernel: [ 1495.429017] Process cheese (pid: 3476, ti=d3248000 task=df46d870 task.ti=d3248000) eeepc kernel: [ 1495.429017] Stack: eeepc kernel: [ 1495.429017] d3249cb8 b03e77a1 d307b840 d3249ccc b03e77d1 d307b840 eb08d870 eb08d830 eeepc kernel: [ 1495.429017] d3249ce4 b03ed3b7 0246 d307b840 eb08d870 d3021b80 d3249cec b03ed565 eeepc kernel: [ 1495.429017] d3249cfc b03e044d e8323d10 b06e013c d3249d18 b0355fb9 fffe d3249d1c eeepc kernel: [ 1495.429017] Call Trace: eeepc kernel: [ 1495.429017] [] v4l2_device_disconnect+0x11/0x30 eeepc kernel: [ 1495.429017] [] v4l2_device_unregister+0x11/0x50 eeepc kernel: [ 1495.429017] [] uvc_delete+0x37/0x110 eeepc kernel: [ 1495.429017] [] uvc_release+0x25/0x30 eeepc kernel: [ 1495.429017] [] v4l2_device_release+0x9d/0xc0 eeepc kernel: [ 1495.429017] [] device_release+0x19/0x90 eeepc kernel: [ 1495.429017] [] ? usb_hcd_unlink_urb+0x7c/0x90 eeepc kernel: [ 1495.429017] [] kobject_release+0x3c/0x90 eeepc kernel: [ 1495.429017] [] ? kobject_del+0x30/0x30 eeepc kernel: [ 1495.429017] [] kref_put+0x2c/0x60 eeepc kernel: [ 1495.429017] [] kobject_put+0x1d/0x50 eeepc kernel: [ 1495.429017] [] ? usb_autopm_put_interface+0x25/0x30 eeepc kernel: [ 1495.429017] [] ? uvc_v4l2_release+0x5d/0xd0 eeepc kernel: [ 1495.429017] [] put_device+0xf/0x20 eeepc kernel: [ 1495.429017] [] v4l2_release+0x56/0x60 eeepc kernel: [ 1495.429017] [] fput+0xcc/0x220 eeepc kernel: [ 1495.429017] [] filp_close+0x44/0x70 eeepc kernel: [ 1495.429017] [] put_files_struct+0x158/0x180 eeepc kernel: [ 1495.429017] [] ? put_files_struct+0x20/0x180 eeepc kernel: [ 1495.429017] [] exit_files+0x40/0x50 eeepc kernel: [ 1495.429017] [] do_exit+0x5a7/0x660 eeepc kernel: [ 1495.429017] [] ? __dequeue_signal+0x12/0x120 eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] [] do_group_exit+0x3c/0xb0 eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ 1495.429017] [] get_signal_to_deliver+0x18f/0x570 eeepc kernel: [ 1495.429017] [] do_signal+0x47/0x9e0 eeepc kernel: [ 1495.429017] [] ? _raw_spin_unlock_irq+0x22/0x30 eeepc kernel: [ 1495.429017] [] ? trace_hardirqs_on+0xb/0x10 eeepc kernel: [ 1495.429017] [] ? T.1034+0x30/0xc0 eeepc kernel: [ 1495.429017] [] ? schedule+0x29f/0x640 eeepc kernel: [ 1495.429017] [] do_notify_resume+0x38/0x40 eeepc kernel: [ 1495.429017] [] work_notifysig+0x9/0x11 eeepc kernel: [ 1495.429017] Code: e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 c3 8d b4 26 00 00 00 00 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 00 8b 40 04 85 c0 74 f0 <8b> 40 60 5d c3 8d 74 26 00 55 89 e5 53 89 c3 83 ec 04 8b 40 04 eeepc kernel: [ 1495.429017] EIP: [] dev_get_drvdata+0x17/0x20 SS:ESP 0068:d3249cac eeepc kernel: [ 1495.429017] CR2: 6b6b6bcb eeepc kernel: [ 1495.466975] uvcvideo: Failed to resubmit video URB (-27). eeepc kernel: [ 1495.467860] uvcvideo: Failed to resubmit video URB (-27). eeepc kernel: last message repeated 3 times eeepc kernel: [ 1495.512610] ---[ end trace 73ec16848794e5a5 ]--- For uvc device, dev->vdev.dev is the &intf->dev, uvc_delete code is as below: usb_put_intf(dev->intf); usb_put_dev(dev->udev); uvc_status_cleanup(dev); uvc_ctrl_cleanup_device(dev); ## the intf dev is released above, so below code will oops. if (dev->vdev.dev) v4l2_device_unregister(&dev->vdev); Fix it by get_device in v4l2_device_register and put_device in v4l2_device_disconnect --- drivers/media/video/v4l2-device.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c index c72856c..e6a2c3b 100644 --- a/drivers/media/video/v4l2-device.c +++ b/drivers/media/video/v4l2-device.c @@ -38,6 +38,7 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev) mutex
Re: [RFC/PATCH 0/1] Ignore ctrl_class
Hi Hans,
On Tue, Sep 06, 2011 at 01:20:26PM +0200, Hans Verkuil wrote:
> On Tuesday, September 06, 2011 13:07:42 Sakari Ailus wrote:
> > Hi,
> >
> > I remember being in a discussion a while ago regarding the requirement of
> > having all the controls belonging to the same class in
> > VIDIOC_{TRY,S,G}_EXT_CTRLS. The answer I remember was that there was a
> > historical reason for this and it no longer exists.
>
> The original rule was that all controls have to belong to the same class.
> This was
> done to simplify drivers. Drivers that use the control framework can handle a
> class
> of 0, which means that the controls can be of any class.
>
> But we still have drivers that implement S_EXT_CTRLS but do not use the
> control
> framework, and for those this restriction is still valid. Usually such
> drivers will only
> handle MPEG class controls through that API.
>
> So I don't think this restriction can be lifted as long as there are drivers
> that do not
> use the control framework.
All the drivers which implement *_EXT_CTRLS and check for ctrl_class do the
check for a single class. All the references for ctrl_class in individual
drivers (which actually were only checks that the user has set the field
correctly) are removed by the patch I posted.
So I don't see a reason why we couldn't just say "please set this to zero
from now on".
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] New class for low level sensors controls?
Hi Sakari, On Tuesday 06 September 2011 13:36:53 Sakari Ailus wrote: > Hi all, > > We are beginning to have raw bayer image sensor drivers in the mainline. > Typically such sensors are not controlled by general purpose applications > but e.g. require a camera control algorithm framework in user space. This > needs to be implemented in libv4l for general purpose applications to work > properly on this kind of hardware. > > These sensors expose controls such as > > - Per-component gain controls. Red, blue, green (blue) and green (red) > gains. > > - Link frequency. The frequency of the data link from the sensor to the > bridge. > > - Horizontal and vertical blanking. Other controls often found in bayer sensors are black level compensation and test pattern. > None of these controls are suitable for use of general purpose applications > (let alone the end user!) but for the camera control algorithms. > > We have a control class called V4L2_CTRL_CLASS_CAMERA for camera controls. > However, the controls in this class are relatively high level controls > which are suitable for end user. The algorithms in the libv4l or a webcam > could implement many of these controls whereas I see that only > V4L2_CID_EXPOSURE_ABSOLUTE might be implemented by raw bayer sensors. > > My question is: would it make sense to create a new class of controls for > the low level sensor controls in a similar fashion we have a control class > for the flash controls? I think it would, but I'm not sure how we should name that class. V4L2_CTRL_CLASS_SENSOR is tempting, but many of the controls that will be found there (digital gains, black leverl compensation, test pattern, ...) can also be found in ISPs or other hardware blocks. -- 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
[RFC] New class for low level sensors controls?
Hi all, We are beginning to have raw bayer image sensor drivers in the mainline. Typically such sensors are not controlled by general purpose applications but e.g. require a camera control algorithm framework in user space. This needs to be implemented in libv4l for general purpose applications to work properly on this kind of hardware. These sensors expose controls such as - Per-component gain controls. Red, blue, green (blue) and green (red) gains. - Link frequency. The frequency of the data link from the sensor to the bridge. - Horizontal and vertical blanking. None of these controls are suitable for use of general purpose applications (let alone the end user!) but for the camera control algorithms. We have a control class called V4L2_CTRL_CLASS_CAMERA for camera controls. However, the controls in this class are relatively high level controls which are suitable for end user. The algorithms in the libv4l or a webcam could implement many of these controls whereas I see that only V4L2_CID_EXPOSURE_ABSOLUTE might be implemented by raw bayer sensors. My question is: would it make sense to create a new class of controls for the low level sensor controls in a similar fashion we have a control class for the flash controls? -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- 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: [media-ctl][PATCHv5 1/5] libmediactl: restruct error path
Hi Andy, On Tuesday 06 September 2011 12:50:25 Andy Shevchenko wrote: > On Tue, 2011-09-06 at 13:46 +0300, Andy Shevchenko wrote: > > On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote: > > > I've slightly modified 1/5 and 3/5 (the first one returned -1 from > > > media_enum_entities(), which made media-ctl stop with a failure > > > message) and pushed the result to the repository. > > > > Okay. I looked at them. > > One minor comment: udef_unref is aware of NULL. I wasn't aware of that, thanks. > Ah, and another. I don't get why you split snprintf() to that suboptimal > strncpy + x[sizeof(x)-1] = 0? snprintf needs to parse the format argument, is strncpy really suboptimal ? -- 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: [RFC/PATCH 0/1] Ignore ctrl_class
On Tuesday, September 06, 2011 13:07:42 Sakari Ailus wrote:
> Hi,
>
> I remember being in a discussion a while ago regarding the requirement of
> having all the controls belonging to the same class in
> VIDIOC_{TRY,S,G}_EXT_CTRLS. The answer I remember was that there was a
> historical reason for this and it no longer exists.
The original rule was that all controls have to belong to the same class. This
was
done to simplify drivers. Drivers that use the control framework can handle a
class
of 0, which means that the controls can be of any class.
But we still have drivers that implement S_EXT_CTRLS but do not use the control
framework, and for those this restriction is still valid. Usually such drivers
will only
handle MPEG class controls through that API.
So I don't think this restriction can be lifted as long as there are drivers
that do not
use the control framework.
Regards,
Hans
> So here's the patch.
>
> The changes in drivers were really simple but have not been tested. The
> changes in the control framework have been tested for querying, getting and
> setting extended and non-extended controls.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/1] v4l: Ignore ctrl_class
Back in the old days there was probably a reason to require that controls
that are being used to access using VIDIOC_{TRY,G,S}_EXT_CTRLS belonged to
the same class. These days such reason does not exist, or at least cannot be
remembered, and concrete examples of the opposite can be seen: a single
(sub)device may well offer controls that belong to different classes and
there is no reason to deny changing them atomically.
Remove all checks of ctrl_class in existing drivers and the control
framework.
Signed-off-by: Sakari Ailus
---
.../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 41 +---
drivers/media/radio/si4713-i2c.c |6 --
drivers/media/video/cx231xx/cx231xx-417.c |6 --
drivers/media/video/cx23885/cx23885-417.c |8 --
drivers/media/video/cx88/cx88-blackbird.c |7 --
drivers/media/video/hdpvr/hdpvr-video.c| 70
drivers/media/video/saa7134/saa6752hs.c|6 --
drivers/media/video/saa7134/saa7134-empress.c |5 --
drivers/media/video/saa7164/saa7164-encoder.c | 70
drivers/media/video/saa7164/saa7164-vbi.c | 70
drivers/media/video/tlg2300/pd-radio.c |6 --
drivers/media/video/v4l2-ctrls.c | 18 ++
drivers/media/video/v4l2-ioctl.c | 12 ++--
13 files changed, 111 insertions(+), 214 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
index 5122ce8..d22a26e 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
@@ -55,9 +55,7 @@ VIDIOC_TRY_EXT_CTRLS
Description
These ioctls allow the caller to get or set multiple
-controls atomically. Control IDs are grouped into control classes (see
-) and all controls in the control array
-must belong to the same control class.
+controls atomically.
Applications must always fill in the
count,
@@ -69,10 +67,10 @@ initialize the &v4l2-ext-control; array pointed to by the
To get the current value of a set of controls applications
initialize the id,
-size and reserved2 fields
-of each &v4l2-ext-control; and call the
-VIDIOC_G_EXT_CTRLS ioctl. String controls controls
-must also set the string field.
+size, ctrl_class and
+reserved2 fields of each &v4l2-ext-control; and
+call the VIDIOC_G_EXT_CTRLS ioctl. String controls
+controls must also set the string field.
If the size is too small to
receive the control result (only relevant for pointer-type controls
@@ -87,7 +85,7 @@ value. It is guaranteed that that is sufficient memory.
To change the value of a set of controls applications
initialize the id, size,
-reserved2 and
+ctrl_class, reserved2 and
value/string fields of each &v4l2-ext-control; and
call the VIDIOC_S_EXT_CTRLS ioctl. The controls
will only be set if all control values are
@@ -95,19 +93,12 @@ valid.
To check if a set of controls have correct values applications
initialize the id, size,
-reserved2 and
+ctrl_class, reserved2 and
value/string fields of each &v4l2-ext-control; and
call the VIDIOC_TRY_EXT_CTRLS ioctl. It is up to
the driver whether wrong values are automatically adjusted to a valid
value or if an error is returned.
-When the id or
-ctrl_class is invalid drivers return an
-&EINVAL;. When the value is out of bounds drivers can choose to take
-the closest valid value or return an &ERANGE;, whatever seems more
-appropriate. In the first case the new value is set in
-&v4l2-ext-control;.
-
The driver will only set/get these controls if all control
values are correct. This prevents the situation where only some of the
controls were set/get. Only low-level errors (⪚ a failed i2c
@@ -182,8 +173,11 @@ applications must set the array to zero.
__u32
ctrl_class
- The control class to which all controls belong, see
-.
+
+ ctrl_class must be set to zero by
+ the applications.
+
+
__u32
@@ -270,12 +264,11 @@ These controls are described in
EINVAL
- The &v4l2-ext-control; id
-is invalid or the &v4l2-ext-controls;
-ctrl_class is invalid. This error code is
-also returned by the VIDIOC_S_EXT_CTRLS and
-VIDIOC_TRY_EXT_CTRLS ioctls if two or more
-control values are in conflict.
+ The &v4l2-ext-control; id is
+invalid. This error code is also returned by the
+VIDIOC_S_EXT_CTRLS and
+VIDIOC_TRY_EXT_CTRLS ioctls if two or more control
+values are in conflict.
diff --git a/drivers/media/radio/si4713-i2c.c b/drivers/media/radio/si4713-i2c.c
index c9f4a8e..b6417bb 100644
--- a/drivers/media/radio/si4713-i2c.c
+++ b/drivers/media/radio/si4713-i2c.c
@@ -1531,9 +1531,6 @@ static int si4713_s_ext_ctrls(struct v4l2_subdev
[RFC/PATCH 0/1] Ignore ctrl_class
Hi,
I remember being in a discussion a while ago regarding the requirement of
having all the controls belonging to the same class in
VIDIOC_{TRY,S,G}_EXT_CTRLS. The answer I remember was that there was a
historical reason for this and it no longer exists.
So here's the patch.
The changes in drivers were really simple but have not been tested. The
changes in the control framework have been tested for querying, getting and
setting extended and non-extended controls.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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: [media-ctl][PATCHv5 1/5] libmediactl: restruct error path
On Tue, 2011-09-06 at 13:46 +0300, Andy Shevchenko wrote: > On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote: > > I've slightly modified 1/5 and 3/5 (the first one returned -1 from > > media_enum_entities(), which made media-ctl stop with a failure message) > > and > > pushed the result to the repository. > Okay. I looked at them. > One minor comment: udef_unref is aware of NULL. Ah, and another. I don't get why you split snprintf() to that suboptimal strncpy + x[sizeof(x)-1] = 0? -- Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media-ctl][PATCHv5 1/5] libmediactl: restruct error path
On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote: > I've slightly modified 1/5 and 3/5 (the first one returned -1 from > media_enum_entities(), which made media-ctl stop with a failure message) and > pushed the result to the repository. Okay. I looked at them. One minor comment: udef_unref is aware of NULL. -- Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v5] media: Add support for arbitrary resolution
Hi Bastian, On Tuesday 06 September 2011 11:19:05 Bastian Hecht wrote: > This patch adds the ability to get arbitrary resolutions with a width > up to 2592 and a height up to 720 pixels instead of the standard 1280x720 > only. > > Signed-off-by: Bastian Hecht Acked-by: Laurent Pinchart Finally :-) Thank you for your work on this. -- 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] mt9p031: Do not use PLL if external frequency is the same as target frequency.
On 6 September 2011 12:27, Laurent Pinchart wrote: > Hi Javier, > > On Tuesday 06 September 2011 12:03:00 Javier Martin wrote: >> This patch adds a check to see whether ext_freq and target_freq are equal >> and, if true, PLL won't be used. > > Thanks for the patch. > > As you're touching PLL code, what about fixing PLL setup by computing > parameters dynamically instead of using a table of hardcoded values ? :-) Hi Laurent, I'm not exactly struggling with PLL code right now. I've just get a new prototype which provides an external 48MHz oscillator for the clock. So, no need to use PLL there and thus the purpose of this patch. However, as you said, dynamic configuration of PLL is one of the pending issues on the driver and I might address it myself in the future, but it depends on requirements of the project. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mt9p031: Do not use PLL if external frequency is the same as target frequency.
Hi Javier,
On Tuesday 06 September 2011 12:03:00 Javier Martin wrote:
> This patch adds a check to see whether ext_freq and target_freq are equal
> and, if true, PLL won't be used.
Thanks for the patch.
As you're touching PLL code, what about fixing PLL setup by computing
parameters dynamically instead of using a table of hardcoded values ? :-)
> Signed-off-by: Javier Martin
> ---
> drivers/media/video/mt9p031.c | 18 +++---
> 1 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c
> index 5cfa39f..42b5d18 100644
> --- a/drivers/media/video/mt9p031.c
> +++ b/drivers/media/video/mt9p031.c
> @@ -117,6 +117,7 @@ struct mt9p031 {
> u16 xskip;
> u16 yskip;
>
> + bool use_pll;
> const struct mt9p031_pll_divs *pll;
>
> /* Registers cache */
> @@ -201,10 +202,16 @@ static int mt9p031_pll_get_divs(struct mt9p031
> *mt9p031) struct i2c_client *client =
> v4l2_get_subdevdata(&mt9p031->subdev); int i;
>
> + if (mt9p031->pdata->ext_freq == mt9p031->pdata->target_freq) {
> + mt9p031->use_pll = false;
> + return 0;
> + }
> +
> for (i = 0; i < ARRAY_SIZE(mt9p031_divs); i++) {
> if (mt9p031_divs[i].ext_freq == mt9p031->pdata->ext_freq &&
> mt9p031_divs[i].target_freq == mt9p031->pdata->target_freq) {
> mt9p031->pll = &mt9p031_divs[i];
> + mt9p031->use_pll = true;
> return 0;
> }
> }
> @@ -385,8 +392,10 @@ static int mt9p031_s_stream(struct v4l2_subdev
> *subdev, int enable) MT9P031_OUTPUT_CONTROL_CEN, 0);
> if (ret < 0)
> return ret;
> -
> - return mt9p031_pll_disable(mt9p031);
> + if (mt9p031->use_pll)
> + return mt9p031_pll_disable(mt9p031);
> + else
> + return 0;
> }
>
> ret = mt9p031_set_params(mt9p031);
> @@ -399,7 +408,10 @@ static int mt9p031_s_stream(struct v4l2_subdev
> *subdev, int enable) if (ret < 0)
> return ret;
>
> - return mt9p031_pll_enable(mt9p031);
> + if (mt9p031->use_pll)
> + return mt9p031_pll_enable(mt9p031);
> + else
> + return 0;
> }
>
> static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
--
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: [media-ctl][PATCHv5 1/5] libmediactl: restruct error path
Hi Andy, Thank you for the patches. I've slightly modified 1/5 and 3/5 (the first one returned -1 from media_enum_entities(), which made media-ctl stop with a failure message) and pushed the result to the repository. I've also added another patch to fix autoconf malloc/realloc tests when cross- compiling that resulted in a compilation failure. -- 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
[PATCH] mt9p031: Do not use PLL if external frequency is the same as target frequency.
This patch adds a check to see whether ext_freq and target_freq are equal and,
if true, PLL won't be used.
Signed-off-by: Javier Martin
---
drivers/media/video/mt9p031.c | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c
index 5cfa39f..42b5d18 100644
--- a/drivers/media/video/mt9p031.c
+++ b/drivers/media/video/mt9p031.c
@@ -117,6 +117,7 @@ struct mt9p031 {
u16 xskip;
u16 yskip;
+ bool use_pll;
const struct mt9p031_pll_divs *pll;
/* Registers cache */
@@ -201,10 +202,16 @@ static int mt9p031_pll_get_divs(struct mt9p031 *mt9p031)
struct i2c_client *client = v4l2_get_subdevdata(&mt9p031->subdev);
int i;
+ if (mt9p031->pdata->ext_freq == mt9p031->pdata->target_freq) {
+ mt9p031->use_pll = false;
+ return 0;
+ }
+
for (i = 0; i < ARRAY_SIZE(mt9p031_divs); i++) {
if (mt9p031_divs[i].ext_freq == mt9p031->pdata->ext_freq &&
mt9p031_divs[i].target_freq == mt9p031->pdata->target_freq) {
mt9p031->pll = &mt9p031_divs[i];
+ mt9p031->use_pll = true;
return 0;
}
}
@@ -385,8 +392,10 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev,
int enable)
MT9P031_OUTPUT_CONTROL_CEN, 0);
if (ret < 0)
return ret;
-
- return mt9p031_pll_disable(mt9p031);
+ if (mt9p031->use_pll)
+ return mt9p031_pll_disable(mt9p031);
+ else
+ return 0;
}
ret = mt9p031_set_params(mt9p031);
@@ -399,7 +408,10 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev,
int enable)
if (ret < 0)
return ret;
- return mt9p031_pll_enable(mt9p031);
+ if (mt9p031->use_pll)
+ return mt9p031_pll_enable(mt9p031);
+ else
+ return 0;
}
static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
Hello Sakari,
2011/9/6 Sakari Ailus :
> On Tue, Sep 06, 2011 at 09:01:15AM +, Bastian Hecht wrote:
>> 2011/9/6 Sakari Ailus :
>> > On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
>> >> Hello Sakari!
>> >
>> > Hi Bastian,
>> >
>> >> 2011/9/6 Sakari Ailus :
>> >> > Hi Bastian,
>> >> >
>> >> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
>> >> >> 2011/9/1 Sakari Ailus :
>> >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
>> >> >> >> Hi Sakari,
>> >> >> >>
>> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
>> >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski
>> >> >> >> > wrote:
>> >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
>> >> >> >> >>
>> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
>> >> >> >> 2011/8/28 Laurent Pinchart :
>> >> >> >> >>> [clip]
>> >> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
>> >> >> >>
>> >> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled
>> >> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
>> >> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
>> >> >> >> anything.
>> >> >> >> >>>
>> >> >> >> >>> Hmm. Did you happen to check when that has been written? :)
>> >> >> >> >>>
>> >> >> >> >>> Please use this one instead:
>> >> >> >> >>>
>> >> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
>> >> >> >> >>
>> >> >> >> >> "Drivers can also implement their own custom controls using
>> >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
>> >> >> >> >>
>> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE
>> >> >> >> >> differently there?
>> >> >> >> >
>> >> >> >> > That was a general comment, not related to the private base.
>> >> >> >> > There's no
>> >> >> >> > use for a three-year-old spec as a reference!
>> >> >> >> >
>> >> >> >> > The control framework does not support private controls, for
>> >> >> >> > example. The
>> >> >> >> > controls should be put to their own class in videodev2.h
>> >> >> >> > nowadays, that's my
>> >> >> >> > understanding. Cc Hans.
>> >> >> >>
>> >> >> >> Is this really the case that we close the door for private controls
>> >> >> >> in
>> >> >> >> the mainline kernel ? Or am I misunderstanding something ?
>> >> >> >> How about v4l2_ctrl_new_custom() ?
>> >> >> >>
>> >> >> >> What if there are controls applicable to single driver only ?
>> >> >> >> Do we really want to have plenty of such in videodev2.h ?
>> >> >> >
>> >> >> > We have some of those already in videodev2.h. I'm not certain if I'm
>> >> >> > happy
>> >> >> > with this myself, considering e.g. that we could get a few
>> >> >> > truckloads of
>> >> >> > only camera lens hardware specific controls in the near future.
>> >> >>
>> >> >> So in my case (as these are controls that might be used by others too)
>> >> >> I should add something like
>> >> >>
>> >> >> #define V4L2_CID_BLUE_SATURATION
>> >> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
>> >> >> #define V4L2_CID_RED_SATURATION
>> >> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
>> >> >
>> >> > What do these two controls do? Do they control gain or something else?
>> >>
>> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
>> >> Saturation. To me it looks like turning up the saturation in HSV
>> >> space, but only for either the blue or the red channel. This would
>> >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
>> >> say it is "{Red,Blue} chroma balance".
>> >>
>> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
>> >> These are gains. So in fact I should swap them in my code and the
>> >> remaining question is, how to name the red and blue gain controls.
>> >
>> > I think Laurent had a similar issue in his Aptina sensor driver. In my
>> > opinion we need a class for low level controls such as the gain ones. Do I
>> > understand correctly they control the red and blue pixel gain in the sensor
>> > pixel matrix? Do you also have gain controls for the two greens?
>>
>> Yes, I assume that this is done there. Either in the analog circuit by
>> decreasing the preload or digitally then. Don't know exactly. There
>> are registers for the green pixels as well. As I used the
>> V4L2_CID_{RED,BLUE}_BALANCE controls and there was no
>> V4L2_CID_GREEN_BALANCE, I just skipped green as one can
>> increase/decrease the global gain and get an arbitrary mix as well.
>>
>> So for these gain settings we should add these?
>> V4L2_CID_RED_GAIN
>> V4L2_CID_BLUE_GAIN
>> V4L2_CID_GREEN_GAIN
>
> Do you have two or just one green gains? In all sensors I've seen there are
> two.
No, here is only one.
> I think I could send an RFC on this to the list and cc you and Laurent.
Ok fine, thanks! But hmmm - what do I do with my driver in the
meantime actually? Stall the upstream process or remove my controls
temporarily - or is there a better w
[PATCH 1/2 v5] media: Add support for arbitrary resolution
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard 1280x720
only.
Signed-off-by: Bastian Hecht
---
diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
index 6410bda..e52fdb1 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
@@ -14,8 +14,10 @@
* published by the Free Software Foundation.
*/
+#include
#include
#include
+#include
#include
#include
@@ -34,7 +36,7 @@
#define REG_WINDOW_START_Y_LOW 0x3803
#define REG_WINDOW_WIDTH_HIGH 0x3804
#define REG_WINDOW_WIDTH_LOW 0x3805
-#define REG_WINDOW_HEIGHT_HIGH 0x3806
+#define REG_WINDOW_HEIGHT_HIGH 0x3806
#define REG_WINDOW_HEIGHT_LOW 0x3807
#define REG_OUT_WIDTH_HIGH 0x3808
#define REG_OUT_WIDTH_LOW 0x3809
@@ -44,19 +46,44 @@
#define REG_OUT_TOTAL_WIDTH_LOW0x380d
#define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e
#define REG_OUT_TOTAL_HEIGHT_LOW 0x380f
+#define REG_OUTPUT_FORMAT 0x4300
+#define REG_ISP_CTRL_010x5001
+#define REG_AVG_WINDOW_END_X_HIGH 0x5682
+#define REG_AVG_WINDOW_END_X_LOW 0x5683
+#define REG_AVG_WINDOW_END_Y_HIGH 0x5686
+#define REG_AVG_WINDOW_END_Y_LOW 0x5687
+
+/* active pixel array size */
+#define OV5642_SENSOR_SIZE_X 2592
+#define OV5642_SENSOR_SIZE_Y 1944
/*
- * define standard resolution.
- * Works currently only for up to 720 lines
- * eg. 320x240, 640x480, 800x600, 1280x720, 2048x720
+ * About OV5642 resolution, cropping and binning:
+ * This sensor supports it all, at least in the feature description.
+ * Unfortunately, no combination of appropriate registers settings could make
+ * the chip work the intended way. As it works with predefined register lists,
+ * some undocumented registers are presumably changed there to achieve their
+ * goals.
+ * This driver currently only works for resolutions up to 720 lines with a
+ * 1:1 scale. Hopefully these restrictions will be removed in the future.
*/
+#define OV5642_MAX_WIDTH OV5642_SENSOR_SIZE_X
+#define OV5642_MAX_HEIGHT 720
-#define OV5642_WIDTH 1280
-#define OV5642_HEIGHT 720
-#define OV5642_TOTAL_WIDTH 3200
-#define OV5642_TOTAL_HEIGHT2000
-#define OV5642_SENSOR_SIZE_X 2592
-#define OV5642_SENSOR_SIZE_Y 1944
+/* default sizes */
+#define OV5642_DEFAULT_WIDTH 1280
+#define OV5642_DEFAULT_HEIGHT OV5642_MAX_HEIGHT
+
+/* minimum extra blanking */
+#define BLANKING_EXTRA_WIDTH 500
+#define BLANKING_EXTRA_HEIGHT 20
+
+/*
+ * the sensor's autoexposure is buggy when setting total_height low.
+ * It tries to expose longer than 1 frame period without taking care of it
+ * and this leads to weird output. So we set 1000 lines as minimum.
+ */
+#define BLANKING_MIN_HEIGHT1000
struct regval_list {
u16 reg_num;
@@ -581,6 +608,11 @@ struct ov5642_datafmt {
struct ov5642 {
struct v4l2_subdev subdev;
const struct ov5642_datafmt *fmt;
+ struct v4l2_rectcrop_rect;
+
+ /* blanking information */
+ int total_width;
+ int total_height;
};
static const struct ov5642_datafmt ov5642_colour_fmts[] = {
@@ -641,6 +673,21 @@ static int reg_write(struct i2c_client *client, u16 reg,
u8 val)
return 0;
}
+
+/*
+ * convenience function to write 16 bit register values that are split up
+ * into two consecutive high and low parts
+ */
+static int reg_write16(struct i2c_client *client, u16 reg, u16 val16)
+{
+ int ret;
+
+ ret = reg_write(client, reg, val16 >> 8);
+ if (ret)
+ return ret;
+ return reg_write(client, reg + 1, val16 & 0x00ff);
+}
+
#ifdef CONFIG_VIDEO_ADV_DEBUG
static int ov5642_get_register(struct v4l2_subdev *sd, struct
v4l2_dbg_register *reg)
{
@@ -684,58 +731,55 @@ static int ov5642_write_array(struct i2c_client *client,
return 0;
}
-static int ov5642_set_resolution(struct i2c_client *client)
+static int ov5642_set_resolution(struct v4l2_subdev *sd)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
+ int width = priv->crop_rect.width;
+ int height = priv->crop_rect.height;
+ int total_width = priv->total_width;
+ int total_height = priv->total_height;
+ int start_x = (OV5642_SENSOR_SIZE_X - width) / 2;
+ int start_y = (OV5642_SENSOR_SIZE_Y - height) / 2;
int ret;
- u8 start_x_high = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) >> 8;
- u8 start_x_low = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) & 0xff;
- u8 start_y_high = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) >> 8;
- u8 start_y_low = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) & 0xff;
-
- u8 width_high = OV5642_WIDTH >> 8;
- u8 width_low= OV5642_
Re: [PATCH 1/2 v4] media: Add support for arbitrary resolution for the ov5642 camera driver
I forgot the sign-off and just discovered checkpatch errors. I'll
repost the fixed version. Sorry for spam.
2011/9/6 Bastian Hecht :
> This patch adds the ability to get arbitrary resolutions with a width
> up to 2592 and a height up to 720 pixels instead of the standard 1280x720
> only.
>
> ---
> diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
> index 6410bda..3d7038c 100644
> --- a/drivers/media/video/ov5642.c
> +++ b/drivers/media/video/ov5642.c
> @@ -14,8 +14,10 @@
> * published by the Free Software Foundation.
> */
>
> +#include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -34,7 +36,7 @@
> #define REG_WINDOW_START_Y_LOW 0x3803
> #define REG_WINDOW_WIDTH_HIGH 0x3804
> #define REG_WINDOW_WIDTH_LOW 0x3805
> -#define REG_WINDOW_HEIGHT_HIGH 0x3806
> +#define REG_WINDOW_HEIGHT_HIGH 0x3806
> #define REG_WINDOW_HEIGHT_LOW 0x3807
> #define REG_OUT_WIDTH_HIGH 0x3808
> #define REG_OUT_WIDTH_LOW 0x3809
> @@ -44,19 +46,44 @@
> #define REG_OUT_TOTAL_WIDTH_LOW 0x380d
> #define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e
> #define REG_OUT_TOTAL_HEIGHT_LOW 0x380f
> +#define REG_OUTPUT_FORMAT 0x4300
> +#define REG_ISP_CTRL_01 0x5001
> +#define REG_AVG_WINDOW_END_X_HIGH 0x5682
> +#define REG_AVG_WINDOW_END_X_LOW 0x5683
> +#define REG_AVG_WINDOW_END_Y_HIGH 0x5686
> +#define REG_AVG_WINDOW_END_Y_LOW 0x5687
> +
> +/* active pixel array size */
> +#define OV5642_SENSOR_SIZE_X 2592
> +#define OV5642_SENSOR_SIZE_Y 1944
>
> /*
> - * define standard resolution.
> - * Works currently only for up to 720 lines
> - * eg. 320x240, 640x480, 800x600, 1280x720, 2048x720
> + * About OV5642 resolution, cropping and binning:
> + * This sensor supports it all, at least in the feature description.
> + * Unfortunately, no combination of appropriate registers settings could make
> + * the chip work the intended way. As it works with predefined register
> lists,
> + * some undocumented registers are presumably changed there to achieve their
> + * goals.
> + * This driver currently only works for resolutions up to 720 lines with a
> + * 1:1 scale. Hopefully these restrictions will be removed in the future.
> */
> +#define OV5642_MAX_WIDTH OV5642_SENSOR_SIZE_X
> +#define OV5642_MAX_HEIGHT 720
>
> -#define OV5642_WIDTH 1280
> -#define OV5642_HEIGHT 720
> -#define OV5642_TOTAL_WIDTH 3200
> -#define OV5642_TOTAL_HEIGHT 2000
> -#define OV5642_SENSOR_SIZE_X 2592
> -#define OV5642_SENSOR_SIZE_Y 1944
> +/* default sizes */
> +#define OV5642_DEFAULT_WIDTH 1280
> +#define OV5642_DEFAULT_HEIGHT OV5642_MAX_HEIGHT
> +
> +/* minimum extra blanking */
> +#define BLANKING_EXTRA_WIDTH 500
> +#define BLANKING_EXTRA_HEIGHT 20
> +
> +/*
> + * the sensor's autoexposure is buggy when setting total_height low.
> + * It tries to expose longer than 1 frame period without taking care of it
> + * and this leads to weird output. So we set 1000 lines as minimum.
> + */
> +#define BLANKING_MIN_HEIGHT 1000
>
> struct regval_list {
> u16 reg_num;
> @@ -581,6 +608,11 @@ struct ov5642_datafmt {
> struct ov5642 {
> struct v4l2_subdev subdev;
> const struct ov5642_datafmt *fmt;
> + struct v4l2_rect crop_rect;
> +
> + /* blanking information */
> + int total_width;
> + int total_height;
> };
>
> static const struct ov5642_datafmt ov5642_colour_fmts[] = {
> @@ -641,6 +673,21 @@ static int reg_write(struct i2c_client *client, u16 reg,
> u8 val)
>
> return 0;
> }
> +
> +/*
> + * convenience function to write 16 bit register values that are split up
> + * into two consecutive high and low parts
> + */
> +static int reg_write16(struct i2c_client *client, u16 reg, u16 val16)
> +{
> + int ret;
> +
> + ret = reg_write(client, reg, val16 >> 8);
> + if (ret)
> + return ret;
> + return reg_write(client, reg + 1, val16 & 0x00ff);
> +}
> +
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> static int ov5642_get_register(struct v4l2_subdev *sd, struct
> v4l2_dbg_register *reg)
> {
> @@ -684,58 +731,55 @@ static int ov5642_write_array(struct i2c_client *client,
> return 0;
> }
>
> -static int ov5642_set_resolution(struct i2c_client *client)
> +static int ov5642_set_resolution(struct v4l2_subdev *sd)
> {
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + struct ov5642 *priv = to_ov5642(client);
> + int width = priv->crop_rect.width;
> + int height = priv->crop_rect.height;
> + int total_width = priv->total_width;
> + int total_height = priv->total_height;
> + int start_x = (OV5642_SENSOR_SIZE_X - width) / 2;
> + int start_y = (OV5642_SENSOR_SIZE_Y - height) / 2;
> int ret;
> - u8 start_x_high = ((OV5642_SEN
Re: Getting started with OMAP3 ISP
On Tue, Sep 6, 2011 at 10:49 AM, Laurent Pinchart wrote: > On Monday 05 September 2011 18:37:04 you wrote: >> Yes that was the first thing i tried, anyway now i have it finally >> working. Well at least yavta doesn't hang, do you know some >> application to see raw yuv images? I made a typo since in fact it's uyvy ( so a tool to covert from yuv will not work ;) ), but if someone will ever need it: ffmpeg -f rawvideo -pix_fmt uyvy422 -s 720x628 -i frame-01.bin frame-1.png Enrico -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
On Tue, Sep 06, 2011 at 09:01:15AM +, Bastian Hecht wrote:
> 2011/9/6 Sakari Ailus :
> > On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
> >> Hello Sakari!
> >
> > Hi Bastian,
> >
> >> 2011/9/6 Sakari Ailus :
> >> > Hi Bastian,
> >> >
> >> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
> >> >> 2011/9/1 Sakari Ailus :
> >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
> >> >> >> Hi Sakari,
> >> >> >>
> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
> >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski
> >> >> >> > wrote:
> >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
> >> >> >> >>
> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
> >> >> >> 2011/8/28 Laurent Pinchart :
> >> >> >> >>> [clip]
> >> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
> >> >> >>
> >> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled
> >> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
> >> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
> >> >> >> anything.
> >> >> >> >>>
> >> >> >> >>> Hmm. Did you happen to check when that has been written? :)
> >> >> >> >>>
> >> >> >> >>> Please use this one instead:
> >> >> >> >>>
> >> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
> >> >> >> >>
> >> >> >> >> "Drivers can also implement their own custom controls using
> >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
> >> >> >> >>
> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE
> >> >> >> >> differently there?
> >> >> >> >
> >> >> >> > That was a general comment, not related to the private base.
> >> >> >> > There's no
> >> >> >> > use for a three-year-old spec as a reference!
> >> >> >> >
> >> >> >> > The control framework does not support private controls, for
> >> >> >> > example. The
> >> >> >> > controls should be put to their own class in videodev2.h nowadays,
> >> >> >> > that's my
> >> >> >> > understanding. Cc Hans.
> >> >> >>
> >> >> >> Is this really the case that we close the door for private controls
> >> >> >> in
> >> >> >> the mainline kernel ? Or am I misunderstanding something ?
> >> >> >> How about v4l2_ctrl_new_custom() ?
> >> >> >>
> >> >> >> What if there are controls applicable to single driver only ?
> >> >> >> Do we really want to have plenty of such in videodev2.h ?
> >> >> >
> >> >> > We have some of those already in videodev2.h. I'm not certain if I'm
> >> >> > happy
> >> >> > with this myself, considering e.g. that we could get a few truckloads
> >> >> > of
> >> >> > only camera lens hardware specific controls in the near future.
> >> >>
> >> >> So in my case (as these are controls that might be used by others too)
> >> >> I should add something like
> >> >>
> >> >> #define V4L2_CID_BLUE_SATURATION
> >> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
> >> >> #define V4L2_CID_RED_SATURATION
> >> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
> >> >
> >> > What do these two controls do? Do they control gain or something else?
> >>
> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
> >> Saturation. To me it looks like turning up the saturation in HSV
> >> space, but only for either the blue or the red channel. This would
> >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
> >> say it is "{Red,Blue} chroma balance".
> >>
> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
> >> These are gains. So in fact I should swap them in my code and the
> >> remaining question is, how to name the red and blue gain controls.
> >
> > I think Laurent had a similar issue in his Aptina sensor driver. In my
> > opinion we need a class for low level controls such as the gain ones. Do I
> > understand correctly they control the red and blue pixel gain in the sensor
> > pixel matrix? Do you also have gain controls for the two greens?
>
> Yes, I assume that this is done there. Either in the analog circuit by
> decreasing the preload or digitally then. Don't know exactly. There
> are registers for the green pixels as well. As I used the
> V4L2_CID_{RED,BLUE}_BALANCE controls and there was no
> V4L2_CID_GREEN_BALANCE, I just skipped green as one can
> increase/decrease the global gain and get an arbitrary mix as well.
>
> So for these gain settings we should add these?
> V4L2_CID_RED_GAIN
> V4L2_CID_BLUE_GAIN
> V4L2_CID_GREEN_GAIN
Do you have two or just one green gains? In all sensors I've seen there are
two.
I think I could send an RFC on this to the list and cc you and Laurent.
> >> >> #define V4L2_CID_GRAY_SCALE_IMAGE
> >> >> (V4L2_CID_CAMERA_CLASS_BASE+21)
> >> >
> >> > V4L2_CID_COLOR_KILLER looks like something which would fit for the
> >> > purpose.
> >>
> >> Oh great! So I just take this.
> >>
> >> >> #define V4L2_CID_SOLARIZE_EFFECT
> >> >> (V4L2_CID_CAMERA_CLASS_BASE+22)
> >
[PATCH 1/2 v4] media: Add support for arbitrary resolution for the ov5642 camera driver
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard 1280x720
only.
---
diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
index 6410bda..3d7038c 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
@@ -14,8 +14,10 @@
* published by the Free Software Foundation.
*/
+#include
#include
#include
+#include
#include
#include
@@ -34,7 +36,7 @@
#define REG_WINDOW_START_Y_LOW 0x3803
#define REG_WINDOW_WIDTH_HIGH 0x3804
#define REG_WINDOW_WIDTH_LOW 0x3805
-#define REG_WINDOW_HEIGHT_HIGH 0x3806
+#define REG_WINDOW_HEIGHT_HIGH 0x3806
#define REG_WINDOW_HEIGHT_LOW 0x3807
#define REG_OUT_WIDTH_HIGH 0x3808
#define REG_OUT_WIDTH_LOW 0x3809
@@ -44,19 +46,44 @@
#define REG_OUT_TOTAL_WIDTH_LOW0x380d
#define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e
#define REG_OUT_TOTAL_HEIGHT_LOW 0x380f
+#define REG_OUTPUT_FORMAT 0x4300
+#define REG_ISP_CTRL_010x5001
+#define REG_AVG_WINDOW_END_X_HIGH 0x5682
+#define REG_AVG_WINDOW_END_X_LOW 0x5683
+#define REG_AVG_WINDOW_END_Y_HIGH 0x5686
+#define REG_AVG_WINDOW_END_Y_LOW 0x5687
+
+/* active pixel array size */
+#define OV5642_SENSOR_SIZE_X 2592
+#define OV5642_SENSOR_SIZE_Y 1944
/*
- * define standard resolution.
- * Works currently only for up to 720 lines
- * eg. 320x240, 640x480, 800x600, 1280x720, 2048x720
+ * About OV5642 resolution, cropping and binning:
+ * This sensor supports it all, at least in the feature description.
+ * Unfortunately, no combination of appropriate registers settings could make
+ * the chip work the intended way. As it works with predefined register lists,
+ * some undocumented registers are presumably changed there to achieve their
+ * goals.
+ * This driver currently only works for resolutions up to 720 lines with a
+ * 1:1 scale. Hopefully these restrictions will be removed in the future.
*/
+#define OV5642_MAX_WIDTH OV5642_SENSOR_SIZE_X
+#define OV5642_MAX_HEIGHT 720
-#define OV5642_WIDTH 1280
-#define OV5642_HEIGHT 720
-#define OV5642_TOTAL_WIDTH 3200
-#define OV5642_TOTAL_HEIGHT2000
-#define OV5642_SENSOR_SIZE_X 2592
-#define OV5642_SENSOR_SIZE_Y 1944
+/* default sizes */
+#define OV5642_DEFAULT_WIDTH 1280
+#define OV5642_DEFAULT_HEIGHT OV5642_MAX_HEIGHT
+
+/* minimum extra blanking */
+#define BLANKING_EXTRA_WIDTH 500
+#define BLANKING_EXTRA_HEIGHT 20
+
+/*
+ * the sensor's autoexposure is buggy when setting total_height low.
+ * It tries to expose longer than 1 frame period without taking care of it
+ * and this leads to weird output. So we set 1000 lines as minimum.
+ */
+#define BLANKING_MIN_HEIGHT1000
struct regval_list {
u16 reg_num;
@@ -581,6 +608,11 @@ struct ov5642_datafmt {
struct ov5642 {
struct v4l2_subdev subdev;
const struct ov5642_datafmt *fmt;
+ struct v4l2_rectcrop_rect;
+
+ /* blanking information */
+ int total_width;
+ int total_height;
};
static const struct ov5642_datafmt ov5642_colour_fmts[] = {
@@ -641,6 +673,21 @@ static int reg_write(struct i2c_client *client, u16 reg,
u8 val)
return 0;
}
+
+/*
+ * convenience function to write 16 bit register values that are split up
+ * into two consecutive high and low parts
+ */
+static int reg_write16(struct i2c_client *client, u16 reg, u16 val16)
+{
+ int ret;
+
+ ret = reg_write(client, reg, val16 >> 8);
+ if (ret)
+ return ret;
+ return reg_write(client, reg + 1, val16 & 0x00ff);
+}
+
#ifdef CONFIG_VIDEO_ADV_DEBUG
static int ov5642_get_register(struct v4l2_subdev *sd, struct
v4l2_dbg_register *reg)
{
@@ -684,58 +731,55 @@ static int ov5642_write_array(struct i2c_client *client,
return 0;
}
-static int ov5642_set_resolution(struct i2c_client *client)
+static int ov5642_set_resolution(struct v4l2_subdev *sd)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
+ int width = priv->crop_rect.width;
+ int height = priv->crop_rect.height;
+ int total_width = priv->total_width;
+ int total_height = priv->total_height;
+ int start_x = (OV5642_SENSOR_SIZE_X - width) / 2;
+ int start_y = (OV5642_SENSOR_SIZE_Y - height) / 2;
int ret;
- u8 start_x_high = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) >> 8;
- u8 start_x_low = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) & 0xff;
- u8 start_y_high = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) >> 8;
- u8 start_y_low = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) & 0xff;
-
- u8 width_high = OV5642_WIDTH >> 8;
- u8 width_low= OV5642_WIDTH & 0xff;
-
Re: Getting started with OMAP3 ISP
On Tue, Sep 6, 2011 at 10:48 AM, Laurent Pinchart wrote: > On Monday 05 September 2011 18:37:04 Enrico wrote: >> Now the problem is that the fix is weird...as you suggested you must >> use half height values for VD0 and VD1 (2/3) interrupts, problem is >> that it only works if you DISABLE vd1 interrupt. >> If it is enabled the vd1_isr is run (once) and nothing else happens. > > Have you set VD0 at half height and VD1 at 1/3 height ? Yes, i also tried some "offset" on vd1 / 3 to see if the vd0 interrupt was just being lost but with no success. Maybe disabling the ccdc ( __cdc_enable(.. , 0) ) in vd1_isr makes the vd0 interrupt to not be triggered, i don't know... Enrico -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
2011/9/6 Sakari Ailus :
> On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
>> Hello Sakari!
>
> Hi Bastian,
>
>> 2011/9/6 Sakari Ailus :
>> > Hi Bastian,
>> >
>> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
>> >> 2011/9/1 Sakari Ailus :
>> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
>> >> >> Hi Sakari,
>> >> >>
>> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
>> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski
>> >> >> > wrote:
>> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
>> >> >> >>
>> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
>> >> >> 2011/8/28 Laurent Pinchart :
>> >> >> >>> [clip]
>> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
>> >> >>
>> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled
>> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
>> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
>> >> >> anything.
>> >> >> >>>
>> >> >> >>> Hmm. Did you happen to check when that has been written? :)
>> >> >> >>>
>> >> >> >>> Please use this one instead:
>> >> >> >>>
>> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
>> >> >> >>
>> >> >> >> "Drivers can also implement their own custom controls using
>> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
>> >> >> >>
>> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE differently
>> >> >> >> there?
>> >> >> >
>> >> >> > That was a general comment, not related to the private base. There's
>> >> >> > no
>> >> >> > use for a three-year-old spec as a reference!
>> >> >> >
>> >> >> > The control framework does not support private controls, for
>> >> >> > example. The
>> >> >> > controls should be put to their own class in videodev2.h nowadays,
>> >> >> > that's my
>> >> >> > understanding. Cc Hans.
>> >> >>
>> >> >> Is this really the case that we close the door for private controls in
>> >> >> the mainline kernel ? Or am I misunderstanding something ?
>> >> >> How about v4l2_ctrl_new_custom() ?
>> >> >>
>> >> >> What if there are controls applicable to single driver only ?
>> >> >> Do we really want to have plenty of such in videodev2.h ?
>> >> >
>> >> > We have some of those already in videodev2.h. I'm not certain if I'm
>> >> > happy
>> >> > with this myself, considering e.g. that we could get a few truckloads of
>> >> > only camera lens hardware specific controls in the near future.
>> >>
>> >> So in my case (as these are controls that might be used by others too)
>> >> I should add something like
>> >>
>> >> #define V4L2_CID_BLUE_SATURATION
>> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
>> >> #define V4L2_CID_RED_SATURATION
>> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
>> >
>> > What do these two controls do? Do they control gain or something else?
>>
>> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
>> Saturation. To me it looks like turning up the saturation in HSV
>> space, but only for either the blue or the red channel. This would
>> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
>> say it is "{Red,Blue} chroma balance".
>>
>> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
>> These are gains. So in fact I should swap them in my code and the
>> remaining question is, how to name the red and blue gain controls.
>
> I think Laurent had a similar issue in his Aptina sensor driver. In my
> opinion we need a class for low level controls such as the gain ones. Do I
> understand correctly they control the red and blue pixel gain in the sensor
> pixel matrix? Do you also have gain controls for the two greens?
Yes, I assume that this is done there. Either in the analog circuit by
decreasing the preload or digitally then. Don't know exactly. There
are registers for the green pixels as well. As I used the
V4L2_CID_{RED,BLUE}_BALANCE controls and there was no
V4L2_CID_GREEN_BALANCE, I just skipped green as one can
increase/decrease the global gain and get an arbitrary mix as well.
So for these gain settings we should add these?
V4L2_CID_RED_GAIN
V4L2_CID_BLUE_GAIN
V4L2_CID_GREEN_GAIN
>> >> #define V4L2_CID_GRAY_SCALE_IMAGE
>> >> (V4L2_CID_CAMERA_CLASS_BASE+21)
>> >
>> > V4L2_CID_COLOR_KILLER looks like something which would fit for the purpose.
>>
>> Oh great! So I just take this.
>>
>> >> #define V4L2_CID_SOLARIZE_EFFECT
>> >> (V4L2_CID_CAMERA_CLASS_BASE+22)
>> >
>> > Sounds interesting for a sensor. I wonder if this would fall under a menu
>> > control, V4L2_CID_COLORFX.
>>
>> When I read the the possible enums for V4L2_CID_COLORFX, it indeed
>> sounds very much like my solarize effect should be added there too. I
>> found V4L2_COLORFX_BW there, too. Isn't that a duplicate of the color
>> killer control then?
>
> In my opinion V4L2_CID_COLORFX should never be implemented in drivers for
> which the hardware doesn't im
Re: use soc-camera mt9m111 with omap3isp
Hi Lee, On Tuesday 06 September 2011 10:23:30 LBM wrote: > hi Laurent Pinchart > thank you very much! > > now i find the "Sub-device pad-level operations " come from you > http://lwn.net/Articles/427934/ > > so if you can give me the full codes about this patchs? Everything is in the latest mainline kernel. -- 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: Getting started with OMAP3 ISP
Hi Enrico, (CC'ing Hans de Goede) On Monday 05 September 2011 18:37:04 Enrico wrote: > On Fri, Sep 2, 2011 at 1:27 PM, Laurent Pinchart wrote: > > On Friday 02 September 2011 11:02:23 Enrico wrote: > >> Right now my problem is that i can't get the isp to generate > >> interrupts, i think there is some isp configuration error. > > > > If your device generates interlaced images that's not surprising, as the > > CCDC will only receive half the number of lines it expects. > > Yes that was the first thing i tried, anyway now i have it finally > working. Well at least yavta doesn't hang, do you know some > application to see raw yuv images? Hans, could libv4lconvert be used to implement a command line format conversion tool ? From a quick look at it it requires a V4L2 device, could that limitation be easily lifted ? > Now the problem is that the fix is weird...as you suggested you must > use half height values for VD0 and VD1 (2/3) interrupts, problem is > that it only works if you DISABLE vd1 interrupt. > If it is enabled the vd1_isr is run (once) and nothing else happens. Have you set VD0 at half height and VD1 at 1/3 height ? -- 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] at91: add Atmel ISI and ov2640 support on m10/g45 board.
On 16:55 Mon 05 Sep , Wu, Josh wrote:
>
>
> On 09/03/2011 2:22 AM Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> >>
> >> #include
> >> #include
> >> @@ -194,6 +197,95 @@ static void __init ek_add_device_nand(void)
> >>at91_add_device_nand(&ek_nand_data);
> >> }
> >>
> >> +/*
> >> + * ISI
> >> + */
> >> +#if defined(CONFIG_VIDEO_ATMEL_ISI) ||
> >> defined(CONFIG_VIDEO_ATMEL_ISI_MODULE)
> >> +static struct isi_platform_data __initdata isi_data = {
> >> + .frate = ISI_CFG1_FRATE_CAPTURE_ALL,
> >> + .has_emb_sync = 0,
> >> + .emb_crc_sync = 0,
> >> + .hsync_act_low = 0,
> >> + .vsync_act_low = 0,
> >> + .pclk_act_falling = 0,
> >> + /* to use codec and preview path simultaneously */
> >> + .isi_full_mode = 1,
> >> + .data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10,
> >> +};
> >> +
> >> +static void __init isi_set_clk(void)
> >> +{
> >> + struct clk *pck1;
> >> + struct clk *plla;
> >> +
> >> + pck1 = clk_get(NULL, "pck1");
> >> + plla = clk_get(NULL, "plla");
> >> +
> >> + clk_set_parent(pck1, plla);
> >> + clk_set_rate(pck1, 2500);
> >> + clk_enable(pck1);
>
> > you must not enable the clock always
>
> > you must enable it just when you need it
>
> > and manage the clock at the board level really so so
>
> I see, I will move such clock code to atmel_isi.c driver and add clock name,
> clock frequence to isi_platform_data structure in next version.
no you miss the idea bind the clkdev
you manage the clock at soc level and then only if it's mandatory at board
level
for the clock rate you pass it to the driver
and let the driver manage when it want to enable/disable the clock
the driver need to have a abtraction of the clock constraint and just request
it and use it
Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
On Tue, Sep 06, 2011 at 07:56:40AM +, Bastian Hecht wrote:
> Hello Sakari!
Hi Bastian,
> 2011/9/6 Sakari Ailus :
> > Hi Bastian,
> >
> > On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
> >> 2011/9/1 Sakari Ailus :
> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
> >> >> Hi Sakari,
> >> >>
> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski wrote:
> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
> >> >> >>
> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
> >> >> 2011/8/28 Laurent Pinchart :
> >> >> >>> [clip]
> >> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
> >> >>
> >> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled
> >> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
> >> >> Documentation/feature-removal-schedule.txt. I couldn't find
> >> >> anything.
> >> >> >>>
> >> >> >>> Hmm. Did you happen to check when that has been written? :)
> >> >> >>>
> >> >> >>> Please use this one instead:
> >> >> >>>
> >> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
> >> >> >>
> >> >> >> "Drivers can also implement their own custom controls using
> >> >> >> V4L2_CID_PRIVATE_BASE and higher values."
> >> >> >>
> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE differently
> >> >> >> there?
> >> >> >
> >> >> > That was a general comment, not related to the private base. There's
> >> >> > no
> >> >> > use for a three-year-old spec as a reference!
> >> >> >
> >> >> > The control framework does not support private controls, for example.
> >> >> > The
> >> >> > controls should be put to their own class in videodev2.h nowadays,
> >> >> > that's my
> >> >> > understanding. Cc Hans.
> >> >>
> >> >> Is this really the case that we close the door for private controls in
> >> >> the mainline kernel ? Or am I misunderstanding something ?
> >> >> How about v4l2_ctrl_new_custom() ?
> >> >>
> >> >> What if there are controls applicable to single driver only ?
> >> >> Do we really want to have plenty of such in videodev2.h ?
> >> >
> >> > We have some of those already in videodev2.h. I'm not certain if I'm
> >> > happy
> >> > with this myself, considering e.g. that we could get a few truckloads of
> >> > only camera lens hardware specific controls in the near future.
> >>
> >> So in my case (as these are controls that might be used by others too)
> >> I should add something like
> >>
> >> #define V4L2_CID_BLUE_SATURATION
> >> (V4L2_CID_CAMERA_CLASS_BASE+19)
> >> #define V4L2_CID_RED_SATURATION
> >> (V4L2_CID_CAMERA_CLASS_BASE+20)
> >
> > What do these two controls do? Do they control gain or something else?
>
> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
> Saturation. To me it looks like turning up the saturation in HSV
> space, but only for either the blue or the red channel. This would
> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
> say it is "{Red,Blue} chroma balance".
>
> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
> These are gains. So in fact I should swap them in my code and the
> remaining question is, how to name the red and blue gain controls.
I think Laurent had a similar issue in his Aptina sensor driver. In my
opinion we need a class for low level controls such as the gain ones. Do I
understand correctly they control the red and blue pixel gain in the sensor
pixel matrix? Do you also have gain controls for the two greens?
> >> #define V4L2_CID_GRAY_SCALE_IMAGE
> >> (V4L2_CID_CAMERA_CLASS_BASE+21)
> >
> > V4L2_CID_COLOR_KILLER looks like something which would fit for the purpose.
>
> Oh great! So I just take this.
>
> >> #define V4L2_CID_SOLARIZE_EFFECT
> >> (V4L2_CID_CAMERA_CLASS_BASE+22)
> >
> > Sounds interesting for a sensor. I wonder if this would fall under a menu
> > control, V4L2_CID_COLORFX.
>
> When I read the the possible enums for V4L2_CID_COLORFX, it indeed
> sounds very much like my solarize effect should be added there too. I
> found V4L2_COLORFX_BW there, too. Isn't that a duplicate of the color
> killer control then?
In my opinion V4L2_CID_COLORFX should never be implemented in drivers for
which the hardware doesn't implement these effects in a non-parametrisable
way. This control was originally added for the OMAP 3 ISP driver but the
driver never implemented it.
I think you have a valid case using this control. I think the main
difference between the two is that V4L2_COLORFX_BW is something that you
can't use with other effects while V4L2_CID_COLOR_KILLER can be used with
any of the effects.
Based on your original proposal the black/white should stay as a separate
control but the solarise should be configurable through V4L2_CID_COLORFX
menu control. So it boils down to the question whether you can use them at
the same time.
--
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
On Tuesday 06 September 2011 00:31:03 Sitsofe Wheeler wrote:
> On Mon, Sep 05, 2011 at 12:16:42PM +0200, Hans Verkuil wrote:
> > On Monday, September 05, 2011 12:13:26 Hans Verkuil wrote:
> > > The original order is correct, but what I missed is that for drivers
> > > that release (free) everything in the videodev release callback the
> > > v4l2_device struct is also freed and v4l2_device_put will fail.
> > >
> > > To fix this, add this code just before the vdev->release call:
> > > /* Do not call v4l2_device_put if there is no release callback set. */
> > > if (v4l2_dev->release == NULL)
> > >
> > > v4l2_dev = NULL;
> > >
> > > If there is no release callback, then the refcounting is pointless
> > > anyway.
> > >
> > > This should work.
> >
> > Note that in the long run using the v4l2_device release callback
> > instead of the videodev release is better. But it's a lot of work to
> > convert everything so that's long term. I'm quite surprised BTW that
> > this bug wasn't found much earlier.
>
> This inline patch fixes the second "poison overwritten" problem so:
> Tested-by: Sitsofe Wheeler
>
> However, it does not prevent the original oops that was reported in the
> original message. Yang Ruirui's patch in
> https://lkml.org/lkml/2011/9/1/74 seems to be required to resolve
> that initial problem - can it be ACK'd? Yang's patch is reproduced
> inline below:
>
> For uvc device, dev->vdev.dev is the &intf->dev,
> uvc_delete code is as below:
> usb_put_intf(dev->intf);
> usb_put_dev(dev->udev);
>
> uvc_status_cleanup(dev);
> uvc_ctrl_cleanup_device(dev);
>
> ## the intf dev is released above, so below code will oops.
>
> if (dev->vdev.dev)
> v4l2_device_unregister(&dev->vdev);
> Fix it by get_device in v4l2_device_register and put_device in
> v4l2_device_disconnect
Acked-by: Laurent Pinchart
> ---
> drivers/media/video/v4l2-device.c |2 ++
> 1 file changed, 2 insertions(+)
> diff --git a/drivers/media/video/v4l2-device.c
> b/drivers/media/video/v4l2-device.c index c72856c..e6a2c3b 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -38,6 +38,7 @@ int v4l2_device_register(struct device *dev, struct
> v4l2_device *v4l2_dev) mutex_init(&v4l2_dev->ioctl_lock);
> v4l2_prio_init(&v4l2_dev->prio);
> kref_init(&v4l2_dev->ref);
> + get_device(dev);
> v4l2_dev->dev = dev;
> if (dev == NULL) {
> /* If dev == NULL, then name must be filled in by the caller */
> @@ -93,6 +94,7 @@ void v4l2_device_disconnect(struct v4l2_device *v4l2_dev)
>
> if (dev_get_drvdata(v4l2_dev->dev) == v4l2_dev)
> dev_set_drvdata(v4l2_dev->dev, NULL);
> + put_device(v4l2_dev->dev);
> v4l2_dev->dev = NULL;
> }
> EXPORT_SYMBOL_GPL(v4l2_device_disconnect);
--
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: [media-ctl][PATCHv4 3/3] libmediactl: get the device name via udev
On Mon, 2011-09-05 at 16:57 +0200, Laurent Pinchart wrote: > > > This will break binary compatibility if an application creates a struct > > > media_device instances itself. On the other hand applications are not > > > supposed to do that. > > > > > > As the struct udev pointer is only used internally, what about passing it > > > around between functions explicitly instead ? > > > > That we will break the API in media_close(). > > Might be I am a blind, but I can't see the way how to do both 1) don't > > provide static global variable and 2) don't break the API/ABI. > > What about passing the udev pointer explictly to media_enum_entities() (which > is static), and calling media_udev_close() in media_open() after the > media_enum_entities() call ? I sent the patch series that incorporates your last comments. -- Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: use soc-camera mt9m111 with omap3isp
Hi Lee, On Tuesday 06 September 2011 07:07:34 LBM wrote: > hi my friend > i use the omap3530 board from "ema-tech",which is the third of TI. > Now i use the mt9m111,it's very difficulty for me to get the images > from this sensor ,becaus it just a soc-camera. can you tell me how to use > this sensor with our omap3isp? You will need to implement the subdev pad-level operation in the mt9m111 driver and make the soc-camera dependencies optional. > if you did something about it,can you give me the codes? I'm not aware of any patch that implements this. -- 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: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
On Tue, Sep 6, 2011 at 6:31 AM, Sitsofe Wheeler wrote:
> On Mon, Sep 05, 2011 at 12:16:42PM +0200, Hans Verkuil wrote:
>> On Monday, September 05, 2011 12:13:26 Hans Verkuil wrote:
>> >
>> > The original order is correct, but what I missed is that for drivers
>> > that release (free) everything in the videodev release callback the
>> > v4l2_device struct is also freed and v4l2_device_put will fail.
>> >
>> > To fix this, add this code just before the vdev->release call:
>> >
>> > /* Do not call v4l2_device_put if there is no release callback set. */
>> > if (v4l2_dev->release == NULL)
>> > v4l2_dev = NULL;
>> >
>> > If there is no release callback, then the refcounting is pointless
>> > anyway.
>> >
>> > This should work.
>>
>> Note that in the long run using the v4l2_device release callback
>> instead of the videodev release is better. But it's a lot of work to
>> convert everything so that's long term. I'm quite surprised BTW that
>> this bug wasn't found much earlier.
>
> This inline patch fixes the second "poison overwritten" problem so:
> Tested-by: Sitsofe Wheeler
Confirmed
>
> However, it does not prevent the original oops that was reported in the
> original message. Yang Ruirui's patch in
> https://lkml.org/lkml/2011/9/1/74 seems to be required to resolve
> that initial problem - can it be ACK'd? Yang's patch is reproduced
> inline below:
Thanks, If it's ok, I can resend it as inline.
>
> For uvc device, dev->vdev.dev is the &intf->dev,
> uvc_delete code is as below:
> usb_put_intf(dev->intf);
> usb_put_dev(dev->udev);
>
> uvc_status_cleanup(dev);
> uvc_ctrl_cleanup_device(dev);
>
> ## the intf dev is released above, so below code will oops.
>
> if (dev->vdev.dev)
> v4l2_device_unregister(&dev->vdev);
> Fix it by get_device in v4l2_device_register and put_device in
> v4l2_device_disconnect
> ---
> drivers/media/video/v4l2-device.c | 2 ++
> 1 file changed, 2 insertions(+)
> diff --git a/drivers/media/video/v4l2-device.c
> b/drivers/media/video/v4l2-device.c
> index c72856c..e6a2c3b 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -38,6 +38,7 @@ int v4l2_device_register(struct device *dev, struct
> v4l2_device *v4l2_dev)
> mutex_init(&v4l2_dev->ioctl_lock);
> v4l2_prio_init(&v4l2_dev->prio);
> kref_init(&v4l2_dev->ref);
> + get_device(dev);
> v4l2_dev->dev = dev;
> if (dev == NULL) {
> /* If dev == NULL, then name must be filled in by the caller */
> @@ -93,6 +94,7 @@ void v4l2_device_disconnect(struct v4l2_device *v4l2_dev)
>
> if (dev_get_drvdata(v4l2_dev->dev) == v4l2_dev)
> dev_set_drvdata(v4l2_dev->dev, NULL);
> + put_device(v4l2_dev->dev);
> v4l2_dev->dev = NULL;
> }
> EXPORT_SYMBOL_GPL(v4l2_device_disconnect);
>
> --
> Sitsofe | http://sucs.org/~sits/
>
--
Regards
Yang RuiRui
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
Hello Sakari!
2011/9/6 Sakari Ailus :
> Hi Bastian,
>
> On Mon, Sep 05, 2011 at 09:32:55AM +, Bastian Hecht wrote:
>> 2011/9/1 Sakari Ailus :
>> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote:
>> >> Hi Sakari,
>> >>
>> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote:
>> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski wrote:
>> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote:
>> >> >>
>> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote:
>> >> 2011/8/28 Laurent Pinchart :
>> >> >>> [clip]
>> >> > If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated.
>> >>
>> >> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled
>> >> "V4L2_CID_PRIVATE_BASE deprecated" and read
>> >> Documentation/feature-removal-schedule.txt. I couldn't find anything.
>> >> >>>
>> >> >>> Hmm. Did you happen to check when that has been written? :)
>> >> >>>
>> >> >>> Please use this one instead:
>> >> >>>
>> >> >>> http://hverkuil.home.xs4all.nl/spec/media.html>
>> >> >>
>> >> >> "Drivers can also implement their own custom controls using
>> >> >> V4L2_CID_PRIVATE_BASE and higher values."
>> >> >>
>> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE differently
>> >> >> there?
>> >> >
>> >> > That was a general comment, not related to the private base. There's no
>> >> > use for a three-year-old spec as a reference!
>> >> >
>> >> > The control framework does not support private controls, for example.
>> >> > The
>> >> > controls should be put to their own class in videodev2.h nowadays,
>> >> > that's my
>> >> > understanding. Cc Hans.
>> >>
>> >> Is this really the case that we close the door for private controls in
>> >> the mainline kernel ? Or am I misunderstanding something ?
>> >> How about v4l2_ctrl_new_custom() ?
>> >>
>> >> What if there are controls applicable to single driver only ?
>> >> Do we really want to have plenty of such in videodev2.h ?
>> >
>> > We have some of those already in videodev2.h. I'm not certain if I'm happy
>> > with this myself, considering e.g. that we could get a few truckloads of
>> > only camera lens hardware specific controls in the near future.
>>
>> So in my case (as these are controls that might be used by others too)
>> I should add something like
>>
>> #define V4L2_CID_BLUE_SATURATION (V4L2_CID_CAMERA_CLASS_BASE+19)
>> #define V4L2_CID_RED_SATURATION (V4L2_CID_CAMERA_CLASS_BASE+20)
>
> What do these two controls do? Do they control gain or something else?
Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V
Saturation. To me it looks like turning up the saturation in HSV
space, but only for either the blue or the red channel. This would
correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They
say it is "{Red,Blue} chroma balance".
I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE.
These are gains. So in fact I should swap them in my code and the
remaining question is, how to name the red and blue gain controls.
>> #define V4L2_CID_GRAY_SCALE_IMAGE (V4L2_CID_CAMERA_CLASS_BASE+21)
>
> V4L2_CID_COLOR_KILLER looks like something which would fit for the purpose.
Oh great! So I just take this.
>> #define V4L2_CID_SOLARIZE_EFFECT (V4L2_CID_CAMERA_CLASS_BASE+22)
>
> Sounds interesting for a sensor. I wonder if this would fall under a menu
> control, V4L2_CID_COLORFX.
When I read the the possible enums for V4L2_CID_COLORFX, it indeed
sounds very much like my solarize effect should be added there too. I
found V4L2_COLORFX_BW there, too. Isn't that a duplicate of the color
killer control then?
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
>
Thanks for your help,
Bastian
--
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: checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
Antti Palosaari writes: > I am almost sure this have been working earlier, but now it seems like > nothing is acceptable for checkpatch.pl! I did surely about 20 --amend > and tried to remove everything, without luck. Could someone point out > whats new acceptable logging format for checkpatch.pl ? > > [crope@localhost linux]$ git show > 1b19e42952963ae2a09a655f487de15b7c81c5b7 |./scripts/checkpatch.pl - > WARNING: Do not use whitespace before Signed-off-by: Don't know if checkpatch used to accept that, but you can use "--format=email" to make it work with git show: bjorn@canardo:/usr/local/src/git/linux$ git show --format=email 1b19e42952963ae2a09a655f487de15b7c81c5b7|./scripts/checkpatch.pl - total: 0 errors, 0 warnings, 48 lines checked Your patch has no obvious style problems and is ready for submission. Bjørn -- 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: migrate soc-camera to V4L2
On Tue, 6 Sep 2011, LBM wrote: > hi Guennadi >thank you very much! > but,how can i port mt9m111 to work with omap3?can you give me some > examples? I'm hoping to get a fixed version of Hans' patches by the end of the day today. After that the use of struct soc_camera_device has to be eliminated in the driver, I might be able to do that too. Then you should be able to more or less just add mt9m111 platform data to your board file, including struct soc_camera_link, and start experimenting. Thanks Guennadi > thanks > LEE > > -- Original -- > From: "g.liakhovetski"; > Date: Tue, Sep 6, 2011 03:00 PM > To: "LBM"; > Cc: "linux-media"; > Subject: Re: migrate soc-camera to V4L2 > > > Hi Lee > > On Tue, 6 Sep 2011, LBM wrote: > > > hi Guennadi > > I used Hans's codes about "migrate soc-camera to the new V4L2 > > control framework".There is a problem,the programe can't go to exec the > > function "soc_camera_probe()". > > You don't need it. > > > I find some information,that say it need > > to use the function "soc_camera_host_register()". > > Nor you need this one. > > > but i don't know why > > and how to use it. My system is "omap3530+mt9m111" and kernel is > > linux2.6.32. > > And now i find the oamp1_camera.c,maby i must fill codes in the > > struct soc_camera_host for my omap3.or if you did this thing already?if > > that can you give me the codes? > > You shouldn't need to touch the SoC drivers (omap). omap3isp is the > correct driver for you. You only need to port mt9m111 to work with omap3. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: migrate soc-camera to V4L2
Hi Lee On Tue, 6 Sep 2011, LBM wrote: > hi Guennadi > I used Hans's codes about "migrate soc-camera to the new V4L2 > control framework".There is a problem,the programe can't go to exec the > function "soc_camera_probe()". You don't need it. > I find some information,that say it need > to use the function "soc_camera_host_register()". Nor you need this one. > but i don't know why > and how to use it. My system is "omap3530+mt9m111" and kernel is > linux2.6.32. > And now i find the oamp1_camera.c,maby i must fill codes in the > struct soc_camera_host for my omap3.or if you did this thing already?if > that can you give me the codes? You shouldn't need to touch the SoC drivers (omap). omap3isp is the correct driver for you. You only need to port mt9m111 to work with omap3. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
