Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

2013-01-08 Thread Michael Olbrich
On Tue, Jan 08, 2013 at 07:31:30AM -0700, Jonathan Corbet wrote:
> On Tue, 08 Jan 2013 07:50:41 +0100
> Marek Szyprowski  wrote:
> 
> > > Couldn't this performance difference be due to the usage of GFP_DMA inside
> > > the VB2 code, like Federico's new patch series is proposing?
> > >
> > > If not, why are there a so large performance penalty?  
> > 
> > Nope, this was caused rather by a very poor CPU access to non-cached (aka
> > 'coherent') memory and the way the video data has been accessed/read 
> > with CPU.
> 
> Exactly.  Uncached memory *hurts*, especially if you're having to touch it
> all with the CPU.

Even worse, on ARMv7 (at least) the cache implements or is necessary for
(I'm not an expert here) unaligned access. I've seen applications crash
on non-cached memory with a bus error because gcc assumes unaligned access
works. And there isn't even a exception handler in the kernel, probably for
the same reason.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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


[media] dvb-usb: reading before start of array

2013-01-08 Thread Dan Carpenter
This is a static checker fix.  In the ttusb_process_muxpack() we do:

cc = (muxpack[len - 4] << 8) | muxpack[len - 3];

That means if we pass a number less than 4 then we will either trigger a
checksum error message or read before the start of the array.

Signed-off-by: Dan Carpenter 
---
I can't test this.

This patch doesn't introduce any bugs, but I'm not positive this is the
right thing to do.  Perhaps it's better to print an error message?

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 5b682cc..99a2fd1 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -709,7 +709,7 @@ static void ttusb_process_frame(struct ttusb *ttusb, u8 * 
data, int len)
 * if length is valid and we reached the end:
 * goto next muxpack
 */
-   if ((ttusb->muxpack_ptr >= 2) &&
+   if ((ttusb->muxpack_ptr >= 4) &&
(ttusb->muxpack_ptr ==
 ttusb->muxpack_len)) {
ttusb_process_muxpack(ttusb,
--
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 5/5] kfifo: log based kfifo API

2013-01-08 Thread Yuanhan Liu
On Tue, Jan 08, 2013 at 10:16:46AM -0800, Dmitry Torokhov wrote:
> Hi Yuanhan,
> 
> On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
> > The current kfifo API take the kfifo size as input, while it rounds
> >  _down_ the size to power of 2 at __kfifo_alloc. This may introduce
> > potential issue.
> > 
> > Take the code at drivers/hid/hid-logitech-dj.c as example:
> > 
> > if (kfifo_alloc(&djrcv_dev->notif_fifo,
> >DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct 
> > dj_report),
> >GFP_KERNEL)) {
> > 
> > Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct dj_report)
> > is 15.
> > 
> > Which means it wants to allocate a kfifo buffer which can store 8
> > dj_report entries at once. The expected kfifo buffer size would be
> > 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
> > size to rounddown_power_of_2(120) =  64, and then allocate a buf
> > with 64 bytes, which I don't think this is the original author want.
> > 
> > With the new log API, we can do like following:
> > 
> > int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
> > sizeof(struct dj_report));
> > 
> > if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, GFP_KERNEL)) {
> > 
> > This make sure we will allocate enough kfifo buffer for holding
> > DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.
> 
> Why don't you simply change __kfifo_alloc to round the allocation up
> instead of down?

Hi Dmitry,

Yes, it would be neat and that was my first reaction as well. I then
sent out a patch, but it was NACKed by Stefani(the original kfifo
author). Here is the link:

https://lkml.org/lkml/2012/10/26/144

Then Stefani proposed to change the API to take log of size as input to
root fix this kind of issues. And here it is.

Thanks.

--yliu
--
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 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices

2013-01-08 Thread Simon Horman
On Tue, Jan 08, 2013 at 11:35:21PM +0100, Guennadi Liakhovetski wrote:
> Hi Simon
> 
> On Tue, 8 Jan 2013, Simon Horman wrote:
> 
> > On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote:
> > > Register the imx074 camera I2C and the CSI-2 platform devices directly
> > > in board platform data instead of letting the sh_mobile_ceu_camera driver
> > > and the soc-camera framework register them at their run-time. This uses
> > > the V4L2 asynchronous subdevice probing capability.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski 
> > 
> > Hi Guennadi,
> > 
> > could you let me know what if any dependencies this patch has.
> > And the status of any dependencies.
> 
> This patch depends on the other 5 patches in this series. Since the other 
> patches are still in work, this patch cannot be applied either yet. Sorry, 
> I should have marked it as RFC.

Thanks, got it.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service

2013-01-08 Thread Devin Heitmueller
On Tue, Jan 8, 2013 at 6:18 PM, Simon Farnsworth
 wrote:
> The wireless folk use dBm (reference point 1 milliwatt), as that's the
> reference point used in the 802.11 standard.
>
> Perhaps we need an extra FE_SCALE constant; FE_SCALE_DECIBEL has no reference
> point (so suitable for carrier to noise etc, or for when the reference point
> is unknown), and FE_SCALE_DECIBEL_MILLIWATT for when the reference point is
> 1mW, so that frontends report in dBm?
>
> Note that if the frontend internally uses a different reference point, the
> conversion is always going to be adding or subtracting a constant.

Hi Simon,

Probably the biggest issue you're going to have is that very few of
the consumer-grade demodulators actually report data in that format.
And only a small subset of those actually provide the documentation in
their datasheet.

The goal behind the "strength" indicator is to provide a field even an
idiot can understand.  While I appreciate the desire to be able to
access the data in it's an "advanced" format, in reality the number of
demodulator drivers that would actually be able to report such is very
small -- and the approach prevents a "lowest common denominator",
which is what 99% of the users *actually* care about.

For that matter, even the SNR field being reported in dB isn't going
to allow you to reliably compare across different demodulator chips.
If demod X says 28.3 dB and demod Y says 29.2 dB, that doesn't
*really* mean demod Y performs better - just that it's reporting a
better number.  However it *does* allow you to compare the demod
against itself either across multiple frequencies or under different
signal conditions - which is what typical users really care about.

And while I realize in your case, Simon, that your requirements may be
different from typical consumer-grade applications, I worry that
adding yet more complexity to the solution will just result in getting
*NOTHING*, as we have for all these years every time the issue of
signal statistics has been raised.

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 RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service

2013-01-08 Thread Simon Farnsworth
On Tuesday 8 January 2013 19:00:11 Frank Schäfer wrote:
> Am 08.01.2013 12:45, schrieb Simon Farnsworth:
> > On Monday 7 January 2013 22:25:47 Mauro Carvalho Chehab wrote:
> > 
> >> +  
> >> +  
> >> FE_SCALE_NOT_AVAILABLE - If it is not 
> >> possible to collect a given parameter (could be a transitory or permanent 
> >> condition)
> >> +  
> >> FE_SCALE_DECIBEL - parameter is a 
> >> signed value, measured in 0.1 dB
> >> +  
> >> FE_SCALE_RELATIVE - parameter is a 
> >> unsigned value, where 0 means 0% and 65535 means 100%.
> >> +  
> >> FE_SCALE_COUNTER - parameter is a 
> >> unsigned value that counts the occurrence of an event, like bit error, 
> >> block error, or lapsed time.
> >> +  
> > 
> >> +  
> >> +  DTV_QOS_SIGNAL_STRENGTH
> >> +  Indicates the signal strength level at the analog part of 
> >> the tuner.
> >> +  
> > Signal strength is traditionally an absolute field strength; there's no way 
> > in
> > this API for me to provide my reference point, so two different front ends
> > could represent the same signal strength as "0 dB" (where the reference 
> > point
> > is one microwatt), "-30 dB" (where the reference point is one milliwatt), or
> > "17 dB" (using a reference point of 1 millivolt on a 50 ohm impedance).
> >
> > Could you choose a reference point for signal strength, and specify that if
> > you're using FE_SCALE_DECIBEL, you're referenced against that point?
> >
> > My preference would be to reference against 1 microwatt, as (on the DVB-T 
> > and
> > ATSC cards I use) that leads to the signal measure being 0 dBµW if you've 
> > got
> > perfect signal, negative number if your signal is weak, and positive numbers
> > if your signal is strong. However, referenced against 1 milliwatt also works
> > well for me, as the conversion is trivial.
> 
> Yeah, that's one of the most popular mistakes in the technical world.
> Decibel is a relative unit. X dB says nothing about the absolute value
> without a reference value.
> Hence these reference values must be specified in the document.
> Otherwise the reported signal strengths are meaningless / not comparable.
> 
> It might be worth to take a look at what the wireles network people have
> done.
> IIRC, they had the same discussion about signal strength reporting a
> (longer) while ago.
> 
The wireless folk use dBm (reference point 1 milliwatt), as that's the
reference point used in the 802.11 standard.

Perhaps we need an extra FE_SCALE constant; FE_SCALE_DECIBEL has no reference
point (so suitable for carrier to noise etc, or for when the reference point
is unknown), and FE_SCALE_DECIBEL_MILLIWATT for when the reference point is
1mW, so that frontends report in dBm?

Note that if the frontend internally uses a different reference point, the
conversion is always going to be adding or subtracting a constant.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.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] omap3isp: Add support for interlaced input data

2013-01-08 Thread William Swanson

On 01/07/2013 04:20 AM, Laurent Pinchart wrote:

What do you get in the memory buffers ? Are fields captured in separate
buffers or combined in a single buffer ? If they're combined, are they
interleaved or sequential in memory ?


I believe the data is combined in a single buffer, with alternate fields 
interleaved.


-William
--
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 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices

2013-01-08 Thread Guennadi Liakhovetski
Hi Simon

On Tue, 8 Jan 2013, Simon Horman wrote:

> On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote:
> > Register the imx074 camera I2C and the CSI-2 platform devices directly
> > in board platform data instead of letting the sh_mobile_ceu_camera driver
> > and the soc-camera framework register them at their run-time. This uses
> > the V4L2 asynchronous subdevice probing capability.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> 
> Hi Guennadi,
> 
> could you let me know what if any dependencies this patch has.
> And the status of any dependencies.

This patch depends on the other 5 patches in this series. Since the other 
patches are still in work, this patch cannot be applied either yet. Sorry, 
I should have marked it as RFC.

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: 15a4:1001 fails. (Afatech AF9035)

2013-01-08 Thread Antti Palosaari

On 01/08/2013 11:54 PM, Eranga Jayasundera wrote:

Dear All,

I wasn't able to install the driver for Afatech AF9035 (15a4:1001) on
my mythbuntu (kernel v3.2.0) box.

I used the bellow commands to install the driver. It was compiled and
installed without errors.

git clone git://linuxtv.org/media_build.git
cd media_build
./build
sudo make install


dmesg output:

[  138.998628] input: Afa Technologies Inc. AF9035A USB Device as
/devices/pci:00/:00:1d.0/usb2/2-1/2-1.6/2-1.6.2/2-1.6.2:1.1/input/input13
[  138.998826] generic-usb 0003:15A4:1001.0004: input,hidraw3: USB HID
v1.01 Keyboard [Afa Technologies Inc. AF9035A USB Device] on
usb-:00:1d.0-1.6.2/input1
[  139.015499] usb 2-1.6.2: dvb_usb_v2: found a 'Afatech AF9035
reference design' in cold state
[  139.016174] usbcore: registered new interface driver dvb_usb_af9035
[  139.018104] usb 2-1.6.2: dvb_usb_v2: Did not find the firmware file
'dvb-usb-af9035-02.fw'. Please see linux/Documentation/dvb/ for more
details on firmware-problems. Status -2
[  139.018109] usb 2-1.6.2: dvb_usb_v2: 'Afatech AF9035 reference
design' error while loading driver (-2)
[  139.018116] usb 2-1.6.2: dvb_usb_v2: 'Afatech AF9035 reference
design' successfully deinitialized and disconnected
[  180.666110] usb 2-1.6.2: USB disconnect, device number 8


Any thoughts?


Firmware is missing. Take one from there:
http://palosaari.fi/linux/
and test. You should rename it to the dvb-usb-af9035-02.fw and install 
location /lib/firmware/


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: No Signal with TerraTec Cinergy T PCIe dual

2013-01-08 Thread Cédric Girard
On Tue, Jan 8, 2013 at 10:46 PM, Antti Palosaari wrote:
> On 01/08/2013 11:02 PM, Cédric Girard wrote:
>> Meaningful bits of an strace of the same command
>> ###
>> ioctl(3, FE_READ_STATUS, 0x7fff69172ef0) = 0
>> ioctl(3, FE_READ_BER, 0x7fff69173018)   = -1 EAGAIN (Resource
>> temporarily unavailable)
>> ioctl(3, FE_READ_SIGNAL_STRENGTH, 0x7fff6917301c) = -1 EAGAIN
>> (Resource temporarily unavailable)
>> ioctl(3, FE_READ_SNR, 0x7fff6917301e)   = -1 EAGAIN (Resource
>> temporarily unavailable)
>> ioctl(3, FE_READ_UNCORRECTED_BLOCKS, 0x7fff69173020) = -1 EAGAIN
>> (Resource temporarily unavailable)
>> ###
>
>
> I could guess these errors are coming because you query statistics but
> device is sleeping and statistics cannot be offered. -EAGAIN == device is
> sleeping, -ENOTTY device does not support given statistic at all.
>
> Could you ensure that? Use some app, like w_scan, tzap, czap, etc. to tune
> and recheck if it shows these or not. femon could not tune, it queries only
> statistics. You will need to leave tuning on and then use other terminal for
> femon.

You were right. Now I get this while running w_scan in another terminal:
###
status SCV   | signal   5% | snr   0% | ber 0 | unc 0 |
status SCV   | signal   5% | snr   0% | ber 0 | unc 0 |
status SCV   | signal   5% | snr   0% | ber 0 | unc 0 |
[...]
###

This is coherent with the "no signal" result.

>
>
>> w_scan give "no signal" result.
>> dvbscan give "Unable to query frontend status"
>
>
> hmm, that dvbscan results sounds crazy. I am not sure about these scanning
> apps as there is scan, dvbscan, scandvb. Maybe some of those, or even all,
> are just same app but renamed as "scan" is too general. My Fedora 17 has
> scandvb. I just tested against one DRX-K device and it worked fine.

According to the wiki [1] scandvb is a distro-side renaming of dvbscan.


>
>
>> Any hint to where I should look would be welcome!
>
>
> Try to downgrade Kernels until you find working one.

Guess it is probably the only way. I will try.

[1] http://linuxtv.org/wiki/index.php/Dvbscan

--
Cédric Girard
--
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: No Signal with TerraTec Cinergy T PCIe dual

2013-01-08 Thread Antti Palosaari

On 01/08/2013 11:02 PM, Cédric Girard wrote:

Hi,

Since a few weeks, I am unable to use my TerraTec Cinergy T PCIe dual
card anymore. On the kernel and drivers side, everything seems to be
working as expected but I get no signal from any channel.

It used to work and I have done too many updates recently to be able
to pinpoint to from which point it stopped to work.
Plugging the same cable to my TV tuner I get perfect reception so no
change in signal quality.

I am running Arch Linux with linux 3.7.1 on an x86_64 architecture.

Following are some logs.

Kernel logs when loading cx23885 module:
###
[93752.708088] cx23885 driver version 0.0.3 loaded
[93752.708954] CORE cx23885[0]: subsystem: 153b:117e, board: TerraTec
Cinergy T PCIe Dual [card=34,autodetected]
[93752.847421] cx25840 12-0044: cx23885 A/V decoder found @ 0x88 (cx23885[0])
[93753.502291] cx25840 12-0044: loaded v4l-cx23885-avcore-01.fw
firmware (16382 bytes)
[93753.518236] cx23885_dvb_register() allocating 1 frontend(s)
[93753.518242] cx23885[0]: cx23885 based dvb card
[93753.533378] drxk: status = 0x639160d9
[93753.533383] drxk: detected a drx-3916k, spin A3, xtal 20.250 MHz
[93753.606391] DRXK driver version 0.9.4300
[93753.630635] drxk: frontend initialized.
[93753.667152] mt2063_attach: Attaching MT2063
[93753.667156] DVB: registering new adapter (cx23885[0])
[93753.667161] cx23885 :03:00.0: DVB: registering adapter 0
frontend 0 (DRXK DVB-T)...
[93753.670728] cx23885_dvb_register() allocating 1 frontend(s)
[93753.670732] cx23885[0]: cx23885 based dvb card
[93753.686602] drxk: status = 0x639130d9
[93753.686605] drxk: detected a drx-3913k, spin A3, xtal 20.250 MHz
[93753.759577] DRXK driver version 0.9.4300
[93753.783845] drxk: frontend initialized.
[93753.783852] mt2063_attach: Attaching MT2063
[93753.783854] DVB: registering new adapter (cx23885[0])
[93753.783858] cx23885 :03:00.0: DVB: registering adapter 1
frontend 0 (DRXK DVB-C DVB-T)...
[93753.784284] cx23885_dev_checkrevision() Hardware revision = 0xa5
[93753.784290] cx23885[0]/0: found at :03:00.0, rev: 4, irq: 18,
latency: 0, mmio: 0xfb80
###

Kernel logs when scanning
###
[93853.705675] mt2063: detected a mt2063 B3
###

"femon -H -a0" output:
###
FE: DRXK DVB-T (DVBT)
Problem retrieving frontend information: Resource temporarily unavailable
status SCV   | signal  49% | snr   0% | ber 2014507767 | unc 1 |
###

Meaningful bits of an strace of the same command
###
ioctl(3, FE_READ_STATUS, 0x7fff69172ef0) = 0
ioctl(3, FE_READ_BER, 0x7fff69173018)   = -1 EAGAIN (Resource
temporarily unavailable)
ioctl(3, FE_READ_SIGNAL_STRENGTH, 0x7fff6917301c) = -1 EAGAIN
(Resource temporarily unavailable)
ioctl(3, FE_READ_SNR, 0x7fff6917301e)   = -1 EAGAIN (Resource
temporarily unavailable)
ioctl(3, FE_READ_UNCORRECTED_BLOCKS, 0x7fff69173020) = -1 EAGAIN
(Resource temporarily unavailable)
###


I could guess these errors are coming because you query statistics but 
device is sleeping and statistics cannot be offered. -EAGAIN == device 
is sleeping, -ENOTTY device does not support given statistic at all.


Could you ensure that? Use some app, like w_scan, tzap, czap, etc. to 
tune and recheck if it shows these or not. femon could not tune, it 
queries only statistics. You will need to leave tuning on and then use 
other terminal for femon.



w_scan give "no signal" result.
dvbscan give "Unable to query frontend status"


hmm, that dvbscan results sounds crazy. I am not sure about these 
scanning apps as there is scan, dvbscan, scandvb. Maybe some of those, 
or even all, are just same app but renamed as "scan" is too general. My 
Fedora 17 has scandvb. I just tested against one DRX-K device and it 
worked fine.



Any hint to where I should look would be welcome!


Try to downgrade Kernels until you find working one.

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


cron job: media_tree daily build: ERRORS

2013-01-08 Thread Hans Verkuil
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 Jan  8 19:00:17 CET 2013
git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

linux-git-arm-eabi-davinci: WARNINGS
linux-git-arm-eabi-exynos: WARNINGS
linux-git-arm-eabi-omap: ERRORS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.12-i686: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-i686: WARNINGS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: WARNINGS
linux-3.0-i686: WARNINGS
linux-3.1-i686: WARNINGS
linux-3.2.1-i686: WARNINGS
linux-3.3-i686: WARNINGS
linux-3.4-i686: WARNINGS
linux-3.5-i686: WARNINGS
linux-3.6-i686: WARNINGS
linux-3.7-i686: WARNINGS
linux-3.8-rc1-i686: OK
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-x86_64: WARNINGS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: WARNINGS
linux-3.0-x86_64: WARNINGS
linux-3.1-x86_64: WARNINGS
linux-3.2.1-x86_64: WARNINGS
linux-3.3-x86_64: WARNINGS
linux-3.4-x86_64: WARNINGS
linux-3.5-x86_64: WARNINGS
linux-3.6-x86_64: WARNINGS
linux-3.7-x86_64: WARNINGS
linux-3.8-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Sylwester,

On Tuesday 08 January 2013 15:52:21 Sylwester Nawrocki wrote:
> On 01/08/2013 09:10 AM, Laurent Pinchart wrote:
> >> +/*
> >> + * If subdevice probing fails any time after v4l2_async_subdev_bind(),
> >> + * no clean up must be called. This function is only a message of
> >> + * intention.
> >> + */
> >> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> >> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > 
> > Could you please explain why you need both a bind notifier and a bound
> > notifier ? I was expecting a single v4l2_async_subdev_register() call in
> > subdev drivers (and, thinking about it, I would probably name it
> > v4l2_subdev_register()).
> 
> I expected it to be done this way too, and I also used
> v4l2_subdev_register() name in my early version of the subdev registration
> code where subdevs were registering themselves to the v4l2 core.

I think we can switch back to v4l2_subdev_register() if we can solve the clock 
name issue. This doesn't seem impossible at first sight.

> BTW, this might not be most important thing here, but do we need separate
> file, i.e. v4l2-async.c, instead of for example putting it in v4l2-device.c
> ?

I'm fine with both, but I tend to try and keep source files not too large for 
ease of reading. Depending on the amount of code we end up adding, moving the 
functions to v4l2-device.c might be a good idea.

> >> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> >> +#endif

-- 
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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Sylwester,

On Tuesday 08 January 2013 15:27:09 Sylwester Nawrocki wrote:
> On 01/08/2013 01:41 PM, Laurent Pinchart wrote:
> >> Subdev names are exposed to user space by the media controller API.
> >> So they are really part of an ABI, aren't they ?
> > 
> > They're used to construct the name exposed to userspace, but the media
> > controller core could probably handle that internally by concatenating the
> > driver name and the subdev name.
> > 
> >> Also having I2C bus number or I2C slave address as part of the subdev
> >> name makes it more difficult to write portable applications. Hence
> >> in sensor drivers I used to overwrite subdev name to remove I2C bus
> >> and slave address, as the format used v4l2_i2c_subdev_init() seemed
> >> highly unsuitable..
> > 
> > This clearly shows that we need to discuss the matter and agree on a
> > common mode of operation.
> > 
> > Aren't applications that use the subdev name directly inherently
> > non-portable anyway ? If you want your application to support different
> > boards/sensors/SoCs you should discover the pipeline and find the sensor
> > by iterating over entities, instead of using the sensor entity name.
> 
> It depends on how we define the entity names :) It the names change from
> board to board and are completely unreliable then user space applications
> using them have no any chance to be generic. Nevertheless, struct
> media_entity_desc::name [1] has currently no specific semantics defined,
> e.g. for V4L2.
> 
> It's likely way better for the kernel space to be no constrained by the
> subdev user space name requirement. But having no clear definition of the
> entity names brings more trouble to user space. E.g. when a sensor exposes
> multiple subdevs. User space library/application could then reference them
> by entity name. It seems difficult to me to handle such multiple subdev
> devices without somehow reliable subdev names.

I agree, I think naming rules are required.

> I imagine a system with multiple sensors of same type sitting on different
> I2C busses, then appending I2C bus number/slave address to the name would be
> useful.

And an application can always strip off the I2C bus number and address and use 
the sensor name only if needed (or use the sensor name in addition to the I2C 
information).

> And is it always possible to discover the pipeline, as opposite to e.g.
> using configuration file to activate all required links ? Configurations
> could be of course per board file, but then at least we should keep subdev
> names constant through the kernel releases.

I'm fine with applications more or less hardcoding the pipeline, and I agree 
that subdev names need to be kept constant across kernel releases (and, very 
importantly, well-defined). Now we "just" need to agree on naming rules :-)

> [1] http://linuxtv.org/downloads/v4l-dvb-apis/media-ioc-enum-entities.html

-- 
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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Sakari,

On Tuesday 08 January 2013 15:15:44 Sakari Ailus wrote:
> On Tue, Jan 08, 2013 at 01:41:59PM +0100, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> > > On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > > > If we need a workaround, I'd rather pass the device name in
> > > > addition to the I2C adapter number and address, instead of
> > > > embedding the workaround in this new API.
> > >  
> > >  ...or we can change the I2C subdevice name format. The actual need
> > >  to do
> > >  
> > >   snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > >    asdl->dev->driver->name,
> > >    i2c_adapter_id(client->adapter), client->addr);
> > >  
> > >  in soc-camera now to exactly match the subdevice name, as created
> > >  by v4l2_i2c_subdev_init(), doesn't make me specifically happy
> > >  either. What if the latter changes at some point? Or what if one
> > >  driver wishes to create several subdevices for one I2C device?
> > > >>> 
> > > >>> The common clock framework uses %d-%04x, maybe we could use that as
> > > >>> well for clock names ?
> > > >> 
> > > >> And preserve the subdevice names? Then matching would be more
> > > >> difficult and less precise. Or change subdevice names too? I think,
> > > >> we can do the latter, since anyway at any time only one driver can be
> > > >> attached to an I2C device.
> > > > 
> > > > That's right. Where else is the subdev name used ?
> > > 
> > > Subdev names are exposed to user space by the media controller API.
> > > So they are really part of an ABI, aren't they ?
> > 
> > They're used to construct the name exposed to userspace, but the media
> > controller core could probably handle that internally by concatenating the
> > driver name and the subdev name.
> > 
> > > Also having I2C bus number or I2C slave address as part of the subdev
> > > name makes it more difficult to write portable applications. Hence
> > > in sensor drivers I used to overwrite subdev name to remove I2C bus
> > > and slave address, as the format used v4l2_i2c_subdev_init() seemed
> > > highly unsuitable..
> > 
> > This clearly shows that we need to discuss the matter and agree on a
> > common mode of operation.
> > 
> > Aren't applications that use the subdev name directly inherently
> > non-portable anyway ? If you want your application to support different
> > boards/sensors/SoCs
>
> Well, the name could come from a configuration file to distinguish e.g.
> between primary and secondary sensors.

In that case having the I2C bus number and address in the name doesn't create 
an extra portability issue, does it ?
 
> For what it's worth, the SMIA++ driver uses the actual name of the sensor
> since there are about 10 sensors supported at the moment, and calling them
> all smiapp- looks a bit insipid. So one has to talk to the sensor to
> know what it's called.
> 
> This isn't strictly mandatory but a nice feature.
> 
> > you should discover the pipeline and find the sensor by iterating over
> > entities, instead of using the sensor entity name.
> 
> To be fully generic, yes.

-- 
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 5/5] kfifo: log based kfifo API

2013-01-08 Thread Andy Walls
Dmitry Torokhov  wrote:

>Hi Yuanhan,
>
>On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
>> The current kfifo API take the kfifo size as input, while it rounds
>>  _down_ the size to power of 2 at __kfifo_alloc. This may introduce
>> potential issue.
>> 
>> Take the code at drivers/hid/hid-logitech-dj.c as example:
>> 
>>  if (kfifo_alloc(&djrcv_dev->notif_fifo,
>>DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct
>dj_report),
>>GFP_KERNEL)) {
>> 
>> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct
>dj_report)
>> is 15.
>> 
>> Which means it wants to allocate a kfifo buffer which can store 8
>> dj_report entries at once. The expected kfifo buffer size would be
>> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
>> size to rounddown_power_of_2(120) =  64, and then allocate a buf
>> with 64 bytes, which I don't think this is the original author want.
>> 
>> With the new log API, we can do like following:
>> 
>>  int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
>>  sizeof(struct dj_report));
>> 
>>  if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order,
>GFP_KERNEL)) {
>> 
>> This make sure we will allocate enough kfifo buffer for holding
>> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.
>
>Why don't you simply change __kfifo_alloc to round the allocation up
>instead of down?
>
>Thanks.
>
>-- 
>Dmitry
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media"
>in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Dmitry,

I agree.   I don't see the benefit in pushing up the change to a kfifo internal 
decision/problem to many different places in the kernel.

Regards,
Andy

 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


No Signal with TerraTec Cinergy T PCIe dual

2013-01-08 Thread Cédric Girard
Hi,

Since a few weeks, I am unable to use my TerraTec Cinergy T PCIe dual
card anymore. On the kernel and drivers side, everything seems to be
working as expected but I get no signal from any channel.

It used to work and I have done too many updates recently to be able
to pinpoint to from which point it stopped to work.
Plugging the same cable to my TV tuner I get perfect reception so no
change in signal quality.

I am running Arch Linux with linux 3.7.1 on an x86_64 architecture.

Following are some logs.

Kernel logs when loading cx23885 module:
###
[93752.708088] cx23885 driver version 0.0.3 loaded
[93752.708954] CORE cx23885[0]: subsystem: 153b:117e, board: TerraTec
Cinergy T PCIe Dual [card=34,autodetected]
[93752.847421] cx25840 12-0044: cx23885 A/V decoder found @ 0x88 (cx23885[0])
[93753.502291] cx25840 12-0044: loaded v4l-cx23885-avcore-01.fw
firmware (16382 bytes)
[93753.518236] cx23885_dvb_register() allocating 1 frontend(s)
[93753.518242] cx23885[0]: cx23885 based dvb card
[93753.533378] drxk: status = 0x639160d9
[93753.533383] drxk: detected a drx-3916k, spin A3, xtal 20.250 MHz
[93753.606391] DRXK driver version 0.9.4300
[93753.630635] drxk: frontend initialized.
[93753.667152] mt2063_attach: Attaching MT2063
[93753.667156] DVB: registering new adapter (cx23885[0])
[93753.667161] cx23885 :03:00.0: DVB: registering adapter 0
frontend 0 (DRXK DVB-T)...
[93753.670728] cx23885_dvb_register() allocating 1 frontend(s)
[93753.670732] cx23885[0]: cx23885 based dvb card
[93753.686602] drxk: status = 0x639130d9
[93753.686605] drxk: detected a drx-3913k, spin A3, xtal 20.250 MHz
[93753.759577] DRXK driver version 0.9.4300
[93753.783845] drxk: frontend initialized.
[93753.783852] mt2063_attach: Attaching MT2063
[93753.783854] DVB: registering new adapter (cx23885[0])
[93753.783858] cx23885 :03:00.0: DVB: registering adapter 1
frontend 0 (DRXK DVB-C DVB-T)...
[93753.784284] cx23885_dev_checkrevision() Hardware revision = 0xa5
[93753.784290] cx23885[0]/0: found at :03:00.0, rev: 4, irq: 18,
latency: 0, mmio: 0xfb80
###

Kernel logs when scanning
###
[93853.705675] mt2063: detected a mt2063 B3
###

"femon -H -a0" output:
###
FE: DRXK DVB-T (DVBT)
Problem retrieving frontend information: Resource temporarily unavailable
status SCV   | signal  49% | snr   0% | ber 2014507767 | unc 1 |
###

Meaningful bits of an strace of the same command
###
ioctl(3, FE_READ_STATUS, 0x7fff69172ef0) = 0
ioctl(3, FE_READ_BER, 0x7fff69173018)   = -1 EAGAIN (Resource
temporarily unavailable)
ioctl(3, FE_READ_SIGNAL_STRENGTH, 0x7fff6917301c) = -1 EAGAIN
(Resource temporarily unavailable)
ioctl(3, FE_READ_SNR, 0x7fff6917301e)   = -1 EAGAIN (Resource
temporarily unavailable)
ioctl(3, FE_READ_UNCORRECTED_BLOCKS, 0x7fff69173020) = -1 EAGAIN
(Resource temporarily unavailable)
###

w_scan give "no signal" result.
dvbscan give "Unable to query frontend status"

Any hint to where I should look would be welcome!

Regards,

--
Cédric Girard
--
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] Initial scan files troubles and brainstorming

2013-01-08 Thread Johannes Stezenbach
On Mon, Jan 07, 2013 at 01:53:25PM +0100, Christoph Pfister wrote:
> 
> @Johannes: As I don't need the account anymore, please delete it.

Done.

Thanks for the work you put into it!

Johannes
--
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] Initial scan files troubles and brainstorming

2013-01-08 Thread Johannes Stezenbach
On Mon, Jan 07, 2013 at 01:48:29PM +0100, Oliver Schinagl wrote:
> On 07-01-13 11:46, Jiri Slaby wrote:
> >On 12/18/2012 11:01 PM, Oliver Schinagl wrote:
> >>Unfortunatly, I have had zero replies.
> >Hmm, it's sad there is a silence in this thread from linux-media guys :/.
> In their defense, they are very very busy people ;) chatter on this
> thread does bring it up however.

This is such a nice thing to say :-)
But it might be that Mauro thinks the dvb-apps maintainer should
respond, but apparently there is no dvb-apps maintainer...
Maybe you should ask Mauro directly to create an account for you
to implement what you proposed.

Thanks,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Sylwester Nawrocki

Hi Guennadi,

Cc: LKML

On 01/08/2013 11:26 AM, Guennadi Liakhovetski wrote:

On Tue, 8 Jan 2013, Laurent Pinchart wrote:

On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:

On Tue, 8 Jan 2013, Laurent Pinchart wrote:

On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:

On Tue, 8 Jan 2013, Laurent Pinchart wrote:

On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:

> From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00

2001


[snip]


+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+struct v4l2_async_notifier *notifier);
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier
*notifier);
+/*
+ * If subdevice probing fails any time after
v4l2_async_subdev_bind(),
no
+ * clean up must be called. This function is only a message of
intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);


Could you please explain why you need both a bind notifier and a bound
notifier ? I was expecting a single v4l2_async_subdev_register() call
in subdev drivers (and, thinking about it, I would probably name it
v4l2_subdev_register()).


I think I can, yes. Because between .bind() and .bound() the subdevice
driver does the actual hardware probing. So, .bind() is used to make
sure the hardware can be accessed, most importantly to provide a clock
to the subdevice. You can look at soc_camera_async_bind(). There I'm
registering the clock for the subdevice, about to bind. Why I cannot do
it before, is because I need subdevice name for clock matching. With I2C
subdevices the subdevice name contains the name of the driver, adapter
number and i2c address. The latter 2 I've got from host subdevice list.
But not the driver name. I thought about also passing the driver name
there, but that seemed too limiting to me. I also request regulators
there, because before ->bound() the sensor driver, but that could be
done on the first call to soc_camera_power_on(), although doing this
"first call" thingie is kind of hackish too. I could add one more soc-
camera-power helper like soc_camera_prepare() or similar too.


I think a soc_camera_power_init() function (or similar) would be a good
idea, yes.


So, the main problem is the clock

subdevice name. Also see the comment in soc_camera.c:
/*
 * It is ok to keep the clock for the whole soc_camera_device
 life-time,
 * in principle it would be more logical to register the clock on icd
 * creation, the only problem is, that at that time we don't know the
 * driver name yet.
 */


I think we should fix that problem instead of shaping the async API around
a workaround :-)

 From the subdevice point of view, the probe function should request
resources, perform whatever initialization is needed (including verifying
that the hardware is functional when possible), and the register the
subdev with the code if everything succeeded. Splitting registration into
bind() and bound() appears a bit as a workaround to me.

If we need a workaround, I'd rather pass the device name in addition to
the I2C adapter number and address, instead of embedding the workaround in
this new API.


...or we can change the I2C subdevice name format. The actual need to do

snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
 asdl->dev->driver->name,
 i2c_adapter_id(client->adapter), client->addr);

in soc-camera now to exactly match the subdevice name, as created by
v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
the latter changes at some point? Or what if one driver wishes to create
several subdevices for one I2C device?


The common clock framework uses %d-%04x, maybe we could use that as well for
clock names ?


And preserve the subdevice names? Then matching would be more difficult
and less precise. Or change subdevice names too? I think, we can do the
latter, since anyway at any time only one driver can be attached to an I2C
device.


I'm just wondering why we can't associate the clock with relevant device,
rather than its driver ? This could eliminate the problem of unknown
sub-device name at the host driver, before sub-device driver is actually
probed, couldn't it ?

--

Thanks,
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 v3] media: V4L2: add temporary clock helpers

2013-01-08 Thread Sylwester Nawrocki

Hi Guennadi,

Just few minor remarks below...

On 12/04/2012 11:42 AM, Guennadi Liakhovetski wrote:

+struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
+  const char *dev_id,
+  const char *id, void *priv)
+{
+   struct v4l2_clk *clk;
+   int ret;
+
+   if (!ops || !dev_id)
+   return ERR_PTR(-EINVAL);
+
+   clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
+   if (!clk)
+   return ERR_PTR(-ENOMEM);
+
+   clk->id = kstrdup(id, GFP_KERNEL);
+   clk->dev_id = kstrdup(dev_id, GFP_KERNEL);
+   if ((id&&  !clk->id) || !clk->dev_id) {
+   ret = -ENOMEM;
+   goto ealloc;
+   }
+   clk->ops = ops;
+   clk->priv = priv;
+   atomic_set(&clk->use_count, 0);
+   mutex_init(&clk->lock);
+
+   mutex_lock(&clk_lock);
+   if (!IS_ERR(v4l2_clk_find(dev_id, id))) {
+   mutex_unlock(&clk_lock);
+   ret = -EEXIST;
+   goto eexist;
+   }
+   list_add_tail(&clk->list,&clk_list);
+   mutex_unlock(&clk_lock);
+
+   return clk;
+
+eexist:
+ealloc:


These multiple labels could be avoided by naming labels after what
happens on next lines, rather than after the location we start from.


+   kfree(clk->id);
+   kfree(clk->dev_id);
+   kfree(clk);
+   return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(v4l2_clk_register);
+
+void v4l2_clk_unregister(struct v4l2_clk *clk)
+{
+   if (unlikely(atomic_read(&clk->use_count))) {


I don't think unlikely() is significant here, it doesn't seem to be really
a fast path.


+   pr_err("%s(): Unregistering ref-counted %s:%s clock!\n",
+  __func__, clk->dev_id, clk->id);
+   BUG();


Hmm, I wouldn't certainly like, e.g. my phone to crash completely only
because camera drivers are buggy. Camera clocks likely aren't essential
resources for system operation... I would just use WARN() here and return
without actually freeing the clock. Not sure if changing signature of
this function and returning an error would be any useful.

Is it indeed such an unrecoverable error we need to resort to BUG() ?

And here is Linus' opinion on how many BUG_ON()s we have in the kernel:
https://lkml.org/lkml/2012/9/27/461
http://permalink.gmane.org/gmane.linux.kernel/1347333

:)


+   }
+
+   mutex_lock(&clk_lock);
+   list_del(&clk->list);
+   mutex_unlock(&clk_lock);
+
+   kfree(clk->id);
+   kfree(clk->dev_id);
+   kfree(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_unregister);


--

Thanks,
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 15/15] em28xx: convert to videobuf2

2013-01-08 Thread Devin Heitmueller
On Tue, Jan 8, 2013 at 1:40 PM, Frank Schäfer
 wrote:
> Bad news. :(
> The patch seems to break USB bulk transfer mode. The framerate is zero.
> I've tested with the Silvercrest webcam and the Cinergy 200. Both
> devices work fine when selecting isoc transfers.

I'll take a look.  I cannot actively debug it since I don't have any
devices which do bulk, but I'll at least see if anything jumps out at
me.

If I had to guess, probably something related to the setting up of the
USB alternate.

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 15/15] em28xx: convert to videobuf2

2013-01-08 Thread Frank Schäfer
Am 04.01.2013 21:59, schrieb Devin Heitmueller:
> This patch converts the em28xx driver over to videobuf2.  It is
> likely that em28xx_fh can go away entirely, but that will come in
> a separate patch.
>
> [mche...@redhat.com: fix a non-trivial merge conflict with some VBI
>  patches]
>
> Signed-off-by: Devin Heitmueller 
> ---
>  drivers/media/usb/em28xx/Kconfig|3 +-
>  drivers/media/usb/em28xx/em28xx-cards.c |7 +-
>  drivers/media/usb/em28xx/em28xx-dvb.c   |4 +-
>  drivers/media/usb/em28xx/em28xx-vbi.c   |  123 ++---
>  drivers/media/usb/em28xx/em28xx-video.c |  743 
> +++
>  drivers/media/usb/em28xx/em28xx.h   |   30 +-
>  6 files changed, 327 insertions(+), 583 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/Kconfig 
> b/drivers/media/usb/em28xx/Kconfig
> index 094c4ec..c754a80 100644
> --- a/drivers/media/usb/em28xx/Kconfig
> +++ b/drivers/media/usb/em28xx/Kconfig
> @@ -3,7 +3,7 @@ config VIDEO_EM28XX
>   depends on VIDEO_DEV && I2C
>   select VIDEO_TUNER
>   select VIDEO_TVEEPROM
> - select VIDEOBUF_VMALLOC
> + select VIDEOBUF2_VMALLOC
>   select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
>   select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
>   select VIDEO_MSP3400 if MEDIA_SUBDRV_AUTOSELECT
> @@ -48,7 +48,6 @@ config VIDEO_EM28XX_DVB
>   select DVB_S5H1409 if MEDIA_SUBDRV_AUTOSELECT
>   select MEDIA_TUNER_QT1010 if MEDIA_SUBDRV_AUTOSELECT
>   select MEDIA_TUNER_TDA18271 if MEDIA_SUBDRV_AUTOSELECT
> - select VIDEOBUF_DVB
>   ---help---
> This adds support for DVB cards based on the
> Empiatech em28xx chips.
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
> b/drivers/media/usb/em28xx/em28xx-cards.c
> index 4117d38..0a4c868 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -57,7 +57,7 @@ module_param(disable_usb_speed_check, int, 0444);
>  MODULE_PARM_DESC(disable_usb_speed_check,
>"override min bandwidth requirement of 480M bps");
>  
> -static unsigned int card[] = {[0 ... (EM28XX_MAXBOARDS - 1)] = UNSET };
> +static unsigned int card[] = {[0 ... (EM28XX_MAXBOARDS - 1)] = -1U };
>  module_param_array(card,  int, NULL, 0444);
>  MODULE_PARM_DESC(card, "card type");
>  
> @@ -2965,6 +2965,8 @@ static int em28xx_init_dev(struct em28xx *dev, struct 
> usb_device *udev,
>   const char *chip_name = default_chip_name;
>  
>   dev->udev = udev;
> + mutex_init(&dev->vb_queue_lock);
> + mutex_init(&dev->vb_vbi_queue_lock);
>   mutex_init(&dev->ctrl_urb_lock);
>   spin_lock_init(&dev->slock);
>  
> @@ -3411,6 +3413,9 @@ static int em28xx_usb_probe(struct usb_interface 
> *interface,
>   /* save our data pointer in this interface device */
>   usb_set_intfdata(interface, dev);
>  
> + /* initialize videobuf2 stuff */
> + em28xx_vb2_setup(dev);
> +
>   /* allocate device struct */
>   mutex_init(&dev->lock);
>   mutex_lock(&dev->lock);
> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
> b/drivers/media/usb/em28xx/em28xx-dvb.c
> index a70b19e..01bb800 100644
> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -27,7 +27,9 @@
>  
>  #include "em28xx.h"
>  #include 
> -#include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include "tuner-simple.h"
>  #include 
> diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c 
> b/drivers/media/usb/em28xx/em28xx-vbi.c
> index d74713b..9fcfc910 100644
> --- a/drivers/media/usb/em28xx/em28xx-vbi.c
> +++ b/drivers/media/usb/em28xx/em28xx-vbi.c
> @@ -41,105 +41,72 @@ MODULE_PARM_DESC(vbi_debug, "enable debug messages 
> [vbi]");
>  
>  /* -- */
>  
> -static void
> -free_buffer(struct videobuf_queue *vq, struct em28xx_buffer *buf)
> +static int vbi_queue_setup(struct vb2_queue *vq, const struct v4l2_format 
> *fmt,
> +   unsigned int *nbuffers, unsigned int *nplanes,
> +   unsigned int sizes[], void *alloc_ctxs[])
>  {
> - struct em28xx_fh *fh  = vq->priv_data;
> - struct em28xx*dev = fh->dev;
> - unsigned long flags = 0;
> - if (in_interrupt())
> - BUG();
> -
> - /* We used to wait for the buffer to finish here, but this didn't work
> -because, as we were keeping the state as VIDEOBUF_QUEUED,
> -videobuf_queue_cancel marked it as finished for us.
> -(Also, it could wedge forever if the hardware was misconfigured.)
> -
> -This should be safe; by the time we get here, the buffer isn't
> -queued anymore. If we ever start marking the buffers as
> -VIDEOBUF_ACTIVE, it won't be, though.
> - */
> - spin_lock_irqsave(&dev->slock, flags);
> - if (dev->usb_ctl.vbi_buf == buf)
> - dev->usb_ctl.vbi_buf = NULL;
> - spin_unlock_irqrestore(&dev->s

Re: [PATCH 5/5] kfifo: log based kfifo API

2013-01-08 Thread Dmitry Torokhov
Hi Yuanhan,

On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
> The current kfifo API take the kfifo size as input, while it rounds
>  _down_ the size to power of 2 at __kfifo_alloc. This may introduce
> potential issue.
> 
> Take the code at drivers/hid/hid-logitech-dj.c as example:
> 
>   if (kfifo_alloc(&djrcv_dev->notif_fifo,
>DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report),
>GFP_KERNEL)) {
> 
> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct dj_report)
> is 15.
> 
> Which means it wants to allocate a kfifo buffer which can store 8
> dj_report entries at once. The expected kfifo buffer size would be
> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
> size to rounddown_power_of_2(120) =  64, and then allocate a buf
> with 64 bytes, which I don't think this is the original author want.
> 
> With the new log API, we can do like following:
> 
>   int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
>   sizeof(struct dj_report));
> 
>   if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, GFP_KERNEL)) {
> 
> This make sure we will allocate enough kfifo buffer for holding
> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.

Why don't you simply change __kfifo_alloc to round the allocation up
instead of down?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: em28xx, sound problems, STV40, linux 3.7.1

2013-01-08 Thread Lluís Batlle i Rossell
Hello,

On Tue, Jan 08, 2013 at 06:54:11PM +0100, Frank Schäfer wrote:
> Am 01.01.2013 21:02, schrieb Lluís Batlle i Rossell:
> > Hello,
> >
> > I'm testing a STV40 usb card, and I've some problems that disables me from
> > capturing audio+video. I'm on linux 3.7.1.
> >
> > 1) Capturing the video with ffmpeg mutes the audio. Simply doing.
> >   $ ffmpeg -f video4linux2 -i /dev/video1 out.flv
> >
> >   Previous to running ffmpeg, I've arecord with "-V mono", and the
> >   vumeter goes to 0% once running that ffmpeg line.
> >
> > 2) The card does not advertise audio controls (like mute), through
> >   "v4l2-ctl --list-ctrls"
> >
> > 3) The card muting and unmuting works fine using V4L2_CID_AUDIO_MUTE
> >   But "v4l2-ctl -c mute=0" can't be used because 'mute' isn't advertised.
> >   Once the card is muted, any call to arecord records all samples zero.
> >   I mention that, because snd_em28xx_capture_open() looks like meant to
> >   unmute.
> >
> > 4) If something captures the sound (muted), and still capturing, I
> >   unmute using V4L2_CID_AUDIO_MUTE, the samples arrive broken. I don't
> >   know how to describe the noisy effect: 
> > http://viric.name/~viric/tmp/noise.wav
> >   (only one channel was connected, of the stereo input)
> >
> > Due to 1) and 4), trying to capture audio+video with ffmpeg results in
> > no-sound (muted), but if enabling it with a program apart while capturing 
> > using
> > V4L2_CID_AUDIO_MUTE, the sound recorded by ffmpeg is crippled.
> >
> > Of course, I've no idea why the audio goes muted at video capture start.
> >
> > As a final note apart, the implementation of vidioc_g_audio fills a->name
> > based on the incoming structure, instead of the later initialized a->index. 
> > I
> > think it's wrong. That's what makes "v4l2-ctl --get-audio-input" report "1
> > (Television)", while it should show "1 (Line In)".
> >
> > Additionally, "v4l2-ctl --list-audio-inputs" doesn't show any input either.
> >
> > As a note, here is the kernel information about the card I have:
> > [45161.345491] em28xx: New device  USB 2861 Device (SVEON STV40) @ 480 Mbps 
> > (1b80:e309, interface 0, class 0)
> > [45161.345500] em28xx: Video interface 0 found
> > [45161.345504] em28xx: DVB interface 0 found
> > [45161.345592] em28xx #0: chip ID is em2860
> > [45161.456962] em28xx #0: i2c eeprom 00: 1a eb 67 95 80 1b 09 e3 50 00 20 
> > 03 6a 3c 00 00
> > [45161.456988] em28xx #0: i2c eeprom 10: 00 00 04 57 06 02 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457029] em28xx #0: i2c eeprom 20: 02 00 01 00 f0 00 01 00 00 00 00 
> > 00 5b 00 00 00
> > [45161.457049] em28xx #0: i2c eeprom 30: 00 00 20 40 20 80 02 20 01 01 02 
> > 01 00 00 00 00
> > [45161.457068] em28xx #0: i2c eeprom 40: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457086] em28xx #0: i2c eeprom 50: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457105] em28xx #0: i2c eeprom 60: 00 00 00 00 00 00 00 00 00 00 3c 
> > 03 55 00 53 00
> > [45161.457154] em28xx #0: i2c eeprom 70: 42 00 20 00 32 00 38 00 36 00 31 
> > 00 20 00 44 00
> > [45161.457173] em28xx #0: i2c eeprom 80: 65 00 76 00 69 00 63 00 65 00 20 
> > 00 28 00 53 00
> > [45161.457192] em28xx #0: i2c eeprom 90: 56 00 45 00 4f 00 4e 00 20 00 53 
> > 00 54 00 56 00
> > [45161.457210] em28xx #0: i2c eeprom a0: 34 00 30 00 29 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457229] em28xx #0: i2c eeprom b0: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457247] em28xx #0: i2c eeprom c0: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457265] em28xx #0: i2c eeprom d0: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457283] em28xx #0: i2c eeprom e0: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457302] em28xx #0: i2c eeprom f0: 00 00 00 00 00 00 00 00 00 00 00 
> > 00 00 00 00 00
> > [45161.457327] em28xx #0: EEPROM ID= 0x9567eb1a, EEPROM hash = 0xa3963040
> > [45161.457332] em28xx #0: EEPROM info:
> > [45161.457335] em28xx #0:   AC97 audio (5 sample rates)
> > [45161.457338] em28xx #0:   500mA max power
> > [45161.457343] em28xx #0:   Table at 0x04, strings=0x3c6a, 0x, 
> > 0x
> > [45161.457350] em28xx #0: Identified as Easy Cap Capture DC-60 (card=64)
> > [45161.661731] saa7115 9-0025: saa7113 found (1f7113d0e10) @ 0x4a 
> > (em28xx #0)
> > [45162.049366] em28xx #0: Config register raw data: 0x50
> > [45162.061248] em28xx #0: AC97 vendor ID = 0x83847650
> > [45162.067100] em28xx #0: AC97 features = 0x6a90
> > [45162.067110] em28xx #0: Empia 202 AC97 audio processor detected
> > [45162.301273] em28xx #0: v4l2 driver version 0.1.3
> > [45162.822556] em28xx #0: V4L2 video device registered as video1
> > [45162.822565] em28xx #0: V4L2 VBI device registered as vbi0
> >
> > Regards,
> > Lluís.
> >
> 
> Thank you for reporting this issue.
> Is there any known kernel version where this has been working ?

Sorry, I wanted to send a new letter about all my new findings, with grea

Re: em28xx, sound problems, STV40, linux 3.7.1

2013-01-08 Thread Devin Heitmueller
On Tue, Jan 8, 2013 at 12:54 PM, Frank Schäfer
 wrote:
> Thank you for reporting this issue.
> Is there any known kernel version where this has been working ?
>
> Regards,
> Frank

Frank,

Just an FYI:  I'm already actively looking into this.

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 RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service

2013-01-08 Thread Frank Schäfer
Am 08.01.2013 12:45, schrieb Simon Farnsworth:
> On Monday 7 January 2013 22:25:47 Mauro Carvalho Chehab wrote:
> 
>> +
>> +
>> FE_SCALE_NOT_AVAILABLE - If it is not 
>> possible to collect a given parameter (could be a transitory or permanent 
>> condition)
>> +
>> FE_SCALE_DECIBEL - parameter is a 
>> signed value, measured in 0.1 dB
>> +
>> FE_SCALE_RELATIVE - parameter is a 
>> unsigned value, where 0 means 0% and 65535 means 100%.
>> +
>> FE_SCALE_COUNTER - parameter is a 
>> unsigned value that counts the occurrence of an event, like bit error, block 
>> error, or lapsed time.
>> +
> 
>> +
>> +DTV_QOS_SIGNAL_STRENGTH
>> +Indicates the signal strength level at the analog part of 
>> the tuner.
>> +
> Signal strength is traditionally an absolute field strength; there's no way in
> this API for me to provide my reference point, so two different front ends
> could represent the same signal strength as "0 dB" (where the reference point
> is one microwatt), "-30 dB" (where the reference point is one milliwatt), or
> "17 dB" (using a reference point of 1 millivolt on a 50 ohm impedance).
>
> Could you choose a reference point for signal strength, and specify that if
> you're using FE_SCALE_DECIBEL, you're referenced against that point?
>
> My preference would be to reference against 1 microwatt, as (on the DVB-T and
> ATSC cards I use) that leads to the signal measure being 0 dBµW if you've got
> perfect signal, negative number if your signal is weak, and positive numbers
> if your signal is strong. However, referenced against 1 milliwatt also works
> well for me, as the conversion is trivial.

Yeah, that's one of the most popular mistakes in the technical world.
Decibel is a relative unit. X dB says nothing about the absolute value
without a reference value.
Hence these reference values must be specified in the document.
Otherwise the reported signal strengths are meaningless / not comparable.

It might be worth to take a look at what the wireles network people have
done.
IIRC, they had the same discussion about signal strength reporting a
(longer) while ago.

Just my two cents.

Regards,
Frank
--
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: em28xx, sound problems, STV40, linux 3.7.1

2013-01-08 Thread Frank Schäfer
Hi,

Am 01.01.2013 21:02, schrieb Lluís Batlle i Rossell:
> Hello,
>
> I'm testing a STV40 usb card, and I've some problems that disables me from
> capturing audio+video. I'm on linux 3.7.1.
>
> 1) Capturing the video with ffmpeg mutes the audio. Simply doing.
>   $ ffmpeg -f video4linux2 -i /dev/video1 out.flv
>
>   Previous to running ffmpeg, I've arecord with "-V mono", and the
>   vumeter goes to 0% once running that ffmpeg line.
>
> 2) The card does not advertise audio controls (like mute), through
>   "v4l2-ctl --list-ctrls"
>
> 3) The card muting and unmuting works fine using V4L2_CID_AUDIO_MUTE
>   But "v4l2-ctl -c mute=0" can't be used because 'mute' isn't advertised.
>   Once the card is muted, any call to arecord records all samples zero.
>   I mention that, because snd_em28xx_capture_open() looks like meant to
>   unmute.
>
> 4) If something captures the sound (muted), and still capturing, I
>   unmute using V4L2_CID_AUDIO_MUTE, the samples arrive broken. I don't
>   know how to describe the noisy effect: 
> http://viric.name/~viric/tmp/noise.wav
>   (only one channel was connected, of the stereo input)
>
> Due to 1) and 4), trying to capture audio+video with ffmpeg results in
> no-sound (muted), but if enabling it with a program apart while capturing 
> using
> V4L2_CID_AUDIO_MUTE, the sound recorded by ffmpeg is crippled.
>
> Of course, I've no idea why the audio goes muted at video capture start.
>
> As a final note apart, the implementation of vidioc_g_audio fills a->name
> based on the incoming structure, instead of the later initialized a->index. I
> think it's wrong. That's what makes "v4l2-ctl --get-audio-input" report "1
> (Television)", while it should show "1 (Line In)".
>
> Additionally, "v4l2-ctl --list-audio-inputs" doesn't show any input either.
>
> As a note, here is the kernel information about the card I have:
> [45161.345491] em28xx: New device  USB 2861 Device (SVEON STV40) @ 480 Mbps 
> (1b80:e309, interface 0, class 0)
> [45161.345500] em28xx: Video interface 0 found
> [45161.345504] em28xx: DVB interface 0 found
> [45161.345592] em28xx #0: chip ID is em2860
> [45161.456962] em28xx #0: i2c eeprom 00: 1a eb 67 95 80 1b 09 e3 50 00 20 03 
> 6a 3c 00 00
> [45161.456988] em28xx #0: i2c eeprom 10: 00 00 04 57 06 02 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457029] em28xx #0: i2c eeprom 20: 02 00 01 00 f0 00 01 00 00 00 00 00 
> 5b 00 00 00
> [45161.457049] em28xx #0: i2c eeprom 30: 00 00 20 40 20 80 02 20 01 01 02 01 
> 00 00 00 00
> [45161.457068] em28xx #0: i2c eeprom 40: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457086] em28xx #0: i2c eeprom 50: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457105] em28xx #0: i2c eeprom 60: 00 00 00 00 00 00 00 00 00 00 3c 03 
> 55 00 53 00
> [45161.457154] em28xx #0: i2c eeprom 70: 42 00 20 00 32 00 38 00 36 00 31 00 
> 20 00 44 00
> [45161.457173] em28xx #0: i2c eeprom 80: 65 00 76 00 69 00 63 00 65 00 20 00 
> 28 00 53 00
> [45161.457192] em28xx #0: i2c eeprom 90: 56 00 45 00 4f 00 4e 00 20 00 53 00 
> 54 00 56 00
> [45161.457210] em28xx #0: i2c eeprom a0: 34 00 30 00 29 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457229] em28xx #0: i2c eeprom b0: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457247] em28xx #0: i2c eeprom c0: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457265] em28xx #0: i2c eeprom d0: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457283] em28xx #0: i2c eeprom e0: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457302] em28xx #0: i2c eeprom f0: 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00
> [45161.457327] em28xx #0: EEPROM ID= 0x9567eb1a, EEPROM hash = 0xa3963040
> [45161.457332] em28xx #0: EEPROM info:
> [45161.457335] em28xx #0:   AC97 audio (5 sample rates)
> [45161.457338] em28xx #0:   500mA max power
> [45161.457343] em28xx #0:   Table at 0x04, strings=0x3c6a, 0x, 0x
> [45161.457350] em28xx #0: Identified as Easy Cap Capture DC-60 (card=64)
> [45161.661731] saa7115 9-0025: saa7113 found (1f7113d0e10) @ 0x4a (em28xx 
> #0)
> [45162.049366] em28xx #0: Config register raw data: 0x50
> [45162.061248] em28xx #0: AC97 vendor ID = 0x83847650
> [45162.067100] em28xx #0: AC97 features = 0x6a90
> [45162.067110] em28xx #0: Empia 202 AC97 audio processor detected
> [45162.301273] em28xx #0: v4l2 driver version 0.1.3
> [45162.822556] em28xx #0: V4L2 video device registered as video1
> [45162.822565] em28xx #0: V4L2 VBI device registered as vbi0
>
> Regards,
> Lluís.
>

Thank you for reporting this issue.
Is there any known kernel version where this has been working ?

Regards,
Frank


--
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: bttv does not load module ir-kbd-i2c for Hauppauge model 37284, rev B421

2013-01-08 Thread Frank Schäfer

>
>>> It probably makes sense to add a "has_ir_i2c" field at bttv, add a flag
>>> there at modprobe to prevent the autoload, and start tagging the boards
>>> with I2C IR with such tag.
>> Without having looked into the code, it seems that the driver detects
>> the i2c rc already without a board flag.
>> Otherwise it wouldn't register the i2c device. Unfortunately, it doesn't
>> display a message.
> No. In the past (kernel 2.4 and upper), I2C bus used to work with 0-len
> reads to scan the used I2C addresses. The I2C drivers like tuners, demods,
> IR's etc used to register to the I2C core saying that they were to be used
> on TV boards. The I2C logic binds them to the I2C bus driver when they were
> detected, during the scanning process.
>
> That's why it is so hard to know what boards are using I2C remotes, on
> those older drivers.

Hmmm... so the I2C subystem probes a list of addresses and just
registers all devices it finds, but the driver itself doesn't know if
one of them is a RC device ?
Sounds odd. Will have to check the code to understand what's going on...

Regards,
Frank

--
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 6/6] ir-kbd-i2c: fix get_key_knc1()

2013-01-08 Thread Frank Schäfer

Am 07.01.2013 17:40, schrieb Mauro Carvalho Chehab:
>>> Also, this was widely discussed several years ago, when we decided to 
>>> cleanup
>>> the I2C code. We then invested lot of efforts to move those get_keys away
>>> from ir-i2c-kbd. The only things left there are the ones we identified that
>>> were needed by auto-detection mode on old devices that we don't have.
>>>
>>> What was decided is to move everything that we know to the *-input driver,
>>> keeping there only the legacy stuff.
>> Uhm... ok.
>> My assumption was, that the goal is the opposite (move as much common
>> code as possible to i2c-ir-kbd).
>> I'm a bit puzzled about this decision...
>>
>> Okay but then... why do we still use ir-kbd-i2c in em28xx ?
>> We can easily get rid of it. Everything we need is already on board.
>>
>> I can send a patch if you want.
> No. We don't want to mix I2C client code inside the I2C bus drivers.

Well, you can't have both at the same time. :D
Either do it in a separate module (ir-kbd-i2c) or in the em28xx driver !? ;)

> If we had started to code it right now, we would likely have used a
> different approach for those I2C IR's, but it is not valuable enough
> to change it right now, as it works, and there's nothing new happening
> here.
>
> The RC trend is to not use hardware decoding anymore, as this doesn't
> follow Microsoft's MCE architecture. All newer chipsets I'm aware of
> just sends a sequence of pulse/space duration, and let the kernel to
> decode. Empia seems to be late on following this market trend. Even so,
> I don't see any new board design with an IR I2C hardware for years.

True, but doesn't change the fact that the current code can be improved.
I think it's valuable enough, as we can get rid of a module dependency here.

> So, the better is to just not re-engineer the things that are working,
> in order to avoid breaking them.

Yeah, we  should _always_  be careful.
But it's definitely not re-engineering. The key polling and decoding
functions are already there.
The main change is, that em28xx-input would call them itself instead of
ir-kbd-i2c, which is what it already does with the built-in decoders.
For maximum safety, we can use the same key handling function as ir-kbd-i2c.
If we don't do it now (yet we have devices for testing), it will likely
never happen.


>
>>> In any case, I don't see any need for patches 4/6 or 6/6.
>>>
 The second thing is the small fix for the key code debug output. Don't
 you think it makes sense ?
>>> Now that we have "ir-keycode -t", all those key/scancode printk's inside
>>> the driver are pretty much useless, as both are reported as events.
>>>
>>> In the past, when most of the RC code were written, those prints were
>> Then we should remove them.
> That makes sense on my eyes. 
>
> Yet, writing/reviewing such cleanup patches would have a very low priority. 
> Btw, all drivers have a lot of printk's message from the time they were 
> written that can be cleaned up, especially nowadays, as several of those 
> printk's can be replaced by Kernel tracing facilities.
>
> If one is willing to do it, I expect that it would be reviewing all
> of them and not just those ones.
Sure.

Regards,
Frank

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx

2013-01-08 Thread Frank Schäfer
Am 07.01.2013 18:04, schrieb Mauro Carvalho Chehab:
> Em Sun, 06 Jan 2013 21:20:31 +0100
> Frank Schäfer  escreveu:
>
>> Am 05.01.2013 16:06, schrieb Mauro Carvalho Chehab:
>>> Em Sat, 05 Jan 2013 14:22:08 +0100
>>> Frank Schäfer  escreveu:
>>>
 Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab:
> Frank pointed that IR was not working with I2C devices. So, I took some
> time to fix them.
>
> Tested with Hauppauge WinTV USB2.
>
> Mauro Carvalho Chehab (4):
>   [media] em28xx: initialize button/I2C IR earlier
>   [media] em28xx: autoload em28xx-rc if the device has an I2C IR
>   [media] em28xx: simplify IR names on I2C devices
>   [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol
>
>  drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
>  drivers/media/usb/em28xx/em28xx-input.c | 29 
> -
>  2 files changed, 17 insertions(+), 14 deletions(-)
>
 While these patches make I2C IR remote controls working again, they
 leave several issues unaddressed which should really be fixed:
 1) the i2c client isn't unregistered on module unload. This was the
 reason for patch 2 in my series. There is also a FIXME comment about
 this in em28xx_release_resources() (although this is the wrong place to
 do it).
>>> AFAIKT, this is not really needed, as the I2C clients are unregistered
>>> when the I2C bus is unregistered.
>>>
>>> So, a device disconnect will release it. Also, an em28xx driver unload.
>>>
>>> The only difference might be if just ir-kbd-i2c and em28xx-rc are
>>> unloaded, but em28xx is still loaded, but I think that, even on this
>>> case, calling the .release code for an I2C bus will release it.
>>>
>>> So, I don't see any need for such patch. I might be wrong, of course, but,
>>> in order to proof that a release code is needed, you'll need to check if
>>> some memory are lost after module load/unload.
>> Mauro, just because code luckily 'works' in the current constellation,
>> it isn't necessarily good code.
> It doesn't luckly 'works'. It is rock solid. 

I disagree here.

> There were really few bug
> reports for ir-kbd-i2c during all those years.

Yeah, but not because the code is so good. ;)
Getting no bug reports doesn't mean that the code is good quality / bug
free (in fact is was broken for a long time).
The 3 main reasons are
1) noone uses the hardware (because it's old, because it's completely
broken, ...)
2) noone want's to report bugs (because they don't know where, think
it's a waste of time, ...)
3) the bugs are reported somewhere else (distros) but not processed /
forwared properly

>> It's this kind of issues that can easily cause regressions if the code
>> changes somewhere else.
> Nah. What causes regression is touching on a code that works for no
> good reason and without enough testing.
>
> Btw, I was told that audio on HVR-950 seemed to stop working.

Yes, and there was another similar bug report recently...

> Not sure what broke it, but, as I tested it some time ago, I suspect
> was due to one of those recent patches (v4l2-compliance, vb2 or your
> patches - we need to bisect to discover what broke it). 

I don't think it was one of those changes.
AFAICS, the bug reports arrived before thes patches were applied.

> None of those
> touched on em28xx-alsa directly, but perhaps one of the patches had
> a bad side effect.

I've done a quick test with my devices but can't reproduce it. And I
didn't look into the audio code too deep yet.
We should ask for further details.


[...]

 3) All RC maps should be assigned at the same place, no matter if the
 receiver/demodulator is built in or external. Spreading them over the
 code is inconsistent and makes the code bug prone.
>>> I don't agree. It is better to keep RC maps for those devices together
>>> with the RC protocol setting, get_key config, etc. At boards config,
>>> it is very easy to identify I2C IR's, as there's an special field there
>>> to mark those devices (has_ir_i2c). So, if the board has_ir_i2c, the
>>> IR config is inside em28xx-input. 
>> ... which is exactly what made it so easy to cause this regression !!!
>>
>> It's not obvious for programmers that no RC map has to be specified for
>> i2c RCs in the board data.
>> It's also not obvious that em28xx-input silently overwrites the rc-map
>> assigned at board level.
>> In general, it's not obvious that two completely different code areas
>> have to be touched for these devices.
>> That's why we really should avoid those board specific code parts spread
>> all over the driver as much as possible.
>> In case of the RC map it's really easy.
>>
>> I also fail to see what you would loose in em28xx-input. We would still
>> assign the RC map to dev->init_data.
>> If you prefer seeing the used RC map in the em28xx-input code directly,
>> then the same should apply for devices with built-in IR
>> recceiver/decoder (which means moving all rc-map assignme

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Marcus Lorentzon

On 01/08/2013 05:36 PM, Tomasz Figa wrote:

On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote:

On 01/08/2013 09:18 AM, Laurent Pinchart wrote:

On Thursday 27 December 2012 15:43:34 Tomasz Figa wrote:

  On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:

  >   On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:

  >   >   On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan

wrote:

  >   >   >   On 17 December 2012 20:55, Laurent Pinchart wrote:

  >   >   >   >   Hi Vikas,
  >   >   >   >
  >   >   >   >   Sorry for the late reply. I now have more time to
  >   >   >   >   work on CDF, so
  >   >   >   >   delays should be much shorter.
  >   >   >   >
  >   >   >   >   On Thursday 06 December 2012 10:51:15 Vikas Sajjan

wrote:

  >   >   >   >   >   Hi Laurent,
  >   >   >   >   >
  >   >   >   >   >   I was thinking of porting CDF to samsung
  >   >   >   >   >   EXYNOS 5250 platform,
  >   >   >   >   >   what I found is that, the exynos display
  >   >   >   >   >   controller is MIPI DSI
  >   >   >   >   >   based controller.
  >   >   >   >   >
  >   >   >   >   >   But if I look at CDF patches, it has only
  >   >   >   >   >   support for MIPI DBI
  >   >   >   >   >   based Display controller.
  >   >   >   >   >
  >   >   >   >   >   So my question is, do we have any generic
  >   >   >   >   >   framework for MIPI DSI
  >   >   >   >   >   based display controller? basically I wanted
  >   >   >   >   >   to know, how to go
  >   >   >   >   >   about porting CDF for such kind of display
  >   >   >   >   >   controller.

  >   >   >   >
  >   >   >   >   MIPI DSI support is not available yet. The only
  >   >   >   >   reason for that is
  >   >   >   >   that I don't have any MIPI DSI hardware to write
  >   >   >   >   and test the code
  >   >   >   >   with:-)
  >   >   >   >
  >   >   >   >   The common display framework should definitely
  >   >   >   >   support MIPI DSI. I
  >   >   >   >   think the existing MIPI DBI code could be used as
  >   >   >   >   a base, so the
  >   >   >   >   implementation shouldn't be too high.
  >   >   >   >
  >   >   >   >   Yeah, i was also thinking in similar lines, below
  >   >   >   >   is my though for
  >   >   >   >   MIPI DSI support in CDF.

  >   >   >
  >   >   >   o   MIPI DSI support as part of CDF framework will
  >   >   >   expose
  >   >   >   §  mipi_dsi_register_device(mpi_device) (will be
  >   >   >   called mach-xxx-dt.c
  >   >   >   file )
  >   >   >   §  mipi_dsi_register_driver(mipi_driver, bus ops)
  >   >   >   (will be called
  >   >   >   from platform specific init driver call )
  >   >   >   ·bus ops will be
  >   >   >   o   read data
  >   >   >   o   write data
  >   >   >   o   write command
  >   >   >   §  MIPI DSI will be registered as bus_register()
  >   >   >
  >   >   >   When MIPI DSI probe is called, it (e.g., Exynos or
  >   >   >   OMAP MIPI DSI)
  >   >   >   will initialize the MIPI DSI HW IP.
  >   >   >
  >   >   >   This probe will also parse the DT file for MIPI DSI
  >   >   >   based panel, add
  >   >   >   the panel device (device_add() ) to kernel and
  >   >   >   register the display
  >   >   >   entity with its control and  video ops with CDF.
  >   >   >
  >   >   >   I can give this a try.

  >   >
  >   >   I am currently in progress of reworking Exynos MIPI DSIM
  >   >   code and
  >   >   s6e8ax0 LCD driver to use the v2 RFC of Common Display
  >   >   Framework. I
  >   >   have most of the work done, I have just to solve several
  >   >   remaining
  >   >   problems.

  >
  >   Do you already have code that you can publish ? I'm
  >   particularly
  >   interested (and I think Tomi Valkeinen would be as well) in
  >   looking at
  >   the DSI operations you expose to DSI sinks (panels,
  >   transceivers, ...).


  Well, I'm afraid this might be little below your expectations, but
  here's an initial RFC of the part defining just the DSI bus. I
  need a bit more time for patches for Exynos MIPI DSI master and
  s6e8ax0 LCD.

No worries. I was particularly interested in the DSI operations you
needed to export, they seem pretty simple. Thank you for sharing the
code.

FYI,
here is STE "DSI API":
http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f
=include/video/mcde.h;h=499ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD
#l361

But it is not perfect. After a couple of products we realized that most
panel drivers want an easy way to send a bunch of init commands in one
go. So I think it should be an op for sending an array of commands at
once. Something like

struct dsi_cmd {
  enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */
  u8 cmd;
  int dataLen;
  u8 *data;
}
struct dsi_ops {
  int dsi_write(source, int num_cmds, struct dsi_cmd *cmds);
  ...
}

Yes, this should be flexible enough to cover most of (or even whole) DSI
specification.

However I'm not sure whether the dsi_write name would 

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Tomasz Figa
On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote:
> On 01/08/2013 09:18 AM, Laurent Pinchart wrote:
> > On Thursday 27 December 2012 15:43:34 Tomasz Figa wrote:
> >> >  On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:
> >>> >  >  On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:
>  >  >  >  On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan 
wrote:
> > >  >  >  >  On 17 December 2012 20:55, Laurent Pinchart wrote:
> >> >  >  >  >  >  Hi Vikas,
> >> >  >  >  >  >  
> >> >  >  >  >  >  Sorry for the late reply. I now have more time to
> >> >  >  >  >  >  work on CDF, so
> >> >  >  >  >  >  delays should be much shorter.
> >> >  >  >  >  >  
> >> >  >  >  >  >  On Thursday 06 December 2012 10:51:15 Vikas Sajjan 
wrote:
> >>> >  >  >  >  >  >  Hi Laurent,
> >>> >  >  >  >  >  >  
> >>> >  >  >  >  >  >  I was thinking of porting CDF to samsung
> >>> >  >  >  >  >  >  EXYNOS 5250 platform,
> >>> >  >  >  >  >  >  what I found is that, the exynos display
> >>> >  >  >  >  >  >  controller is MIPI DSI
> >>> >  >  >  >  >  >  based controller.
> >>> >  >  >  >  >  >  
> >>> >  >  >  >  >  >  But if I look at CDF patches, it has only
> >>> >  >  >  >  >  >  support for MIPI DBI
> >>> >  >  >  >  >  >  based Display controller.
> >>> >  >  >  >  >  >  
> >>> >  >  >  >  >  >  So my question is, do we have any generic
> >>> >  >  >  >  >  >  framework for MIPI DSI
> >>> >  >  >  >  >  >  based display controller? basically I wanted
> >>> >  >  >  >  >  >  to know, how to go
> >>> >  >  >  >  >  >  about porting CDF for such kind of display
> >>> >  >  >  >  >  >  controller.
> >> >  >  >  >  >  
> >> >  >  >  >  >  MIPI DSI support is not available yet. The only
> >> >  >  >  >  >  reason for that is
> >> >  >  >  >  >  that I don't have any MIPI DSI hardware to write
> >> >  >  >  >  >  and test the code
> >> >  >  >  >  >  with:-)
> >> >  >  >  >  >  
> >> >  >  >  >  >  The common display framework should definitely
> >> >  >  >  >  >  support MIPI DSI. I
> >> >  >  >  >  >  think the existing MIPI DBI code could be used as
> >> >  >  >  >  >  a base, so the
> >> >  >  >  >  >  implementation shouldn't be too high.
> >> >  >  >  >  >  
> >> >  >  >  >  >  Yeah, i was also thinking in similar lines, below
> >> >  >  >  >  >  is my though for
> >> >  >  >  >  >  MIPI DSI support in CDF.
> > >  >  >  >  
> > >  >  >  >  o   MIPI DSI support as part of CDF framework will
> > >  >  >  >  expose
> > >  >  >  >  §  mipi_dsi_register_device(mpi_device) (will be
> > >  >  >  >  called mach-xxx-dt.c
> > >  >  >  >  file )
> > >  >  >  >  §  mipi_dsi_register_driver(mipi_driver, bus ops)
> > >  >  >  >  (will be called
> > >  >  >  >  from platform specific init driver call )
> > >  >  >  >  ·bus ops will be
> > >  >  >  >  o   read data
> > >  >  >  >  o   write data
> > >  >  >  >  o   write command
> > >  >  >  >  §  MIPI DSI will be registered as bus_register()
> > >  >  >  >  
> > >  >  >  >  When MIPI DSI probe is called, it (e.g., Exynos or
> > >  >  >  >  OMAP MIPI DSI)
> > >  >  >  >  will initialize the MIPI DSI HW IP.
> > >  >  >  >  
> > >  >  >  >  This probe will also parse the DT file for MIPI DSI
> > >  >  >  >  based panel, add
> > >  >  >  >  the panel device (device_add() ) to kernel and
> > >  >  >  >  register the display
> > >  >  >  >  entity with its control and  video ops with CDF.
> > >  >  >  >  
> > >  >  >  >  I can give this a try.
>  >  >  >  
>  >  >  >  I am currently in progress of reworking Exynos MIPI DSIM
>  >  >  >  code and
>  >  >  >  s6e8ax0 LCD driver to use the v2 RFC of Common Display
>  >  >  >  Framework. I
>  >  >  >  have most of the work done, I have just to solve several
>  >  >  >  remaining
>  >  >  >  problems.
> >>> >  >  
> >>> >  >  Do you already have code that you can publish ? I'm
> >>> >  >  particularly
> >>> >  >  interested (and I think Tomi Valkeinen would be as well) in
> >>> >  >  looking at
> >>> >  >  the DSI operations you expose to DSI sinks (panels,
> >>> >  >  transceivers, ...).
> >> >  
> >> >  Well, I'm afraid this might be little below your expectations, but
> >> >  here's an initial RFC of the part defining just the DSI bus. I
> >> >  need a bit more time for patches for Exynos MIPI DSI master and
> >> >  s6e8ax0 LCD.
> > 
> > No worries. I was particularly interested in the DSI operations you
> > needed to export, they seem pretty simple. Thank you for sharing the
> > code.
> FYI,
> here is STE "DSI API":
> http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f
> =include/video/mcde.h;h=499ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD
> #l361
> 
> But it is not perfect. After a couple of pro

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Rob Clark
On Tue, Jan 8, 2013 at 2:25 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Thursday 27 December 2012 09:54:55 Rob Clark wrote:
>> What I've done to avoid that so far is that the master device registers the
>> drivers for it's output sub-devices before registering it's own device.
>
> I'm not sure to follow you here. The master device doesn't register anything,
> do you mean the master device driver ? If so, how does the master device
> driver register its own device ? Devices are not registered by their driver.

sorry, that should have read "master driver registers drivers for it's
sub-devices.."

BR,
-R
--
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: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Mauro Carvalho Chehab
Em Tue, 8 Jan 2013 14:49:54 +0100
Hans Verkuil  escreveu:

> On Tue 8 January 2013 14:05:10 Mauro Carvalho Chehab wrote:
> > Em Tue, 8 Jan 2013 13:43:43 +0100
> > Hans Verkuil  escreveu:
> > 
> > > On Tue 8 January 2013 12:01:51 Gianluca Gennari wrote:
> > > > Il 08/01/2013 10:58, Hans Verkuil ha scritto:
> > > > > On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
> > > > >> 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:Mon Jan  7 19:00:18 CET 2013
> > > > >> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
> > > > >> gcc version:  i686-linux-gcc (GCC) 4.7.1
> > > > >> host hardware:x86_64
> > > > >> host os:  3.4.07-marune
> > > > >>
> > > > >> linux-git-arm-eabi-davinci: WARNINGS
> > > > >> linux-git-arm-eabi-exynos: WARNINGS
> > > > >> linux-git-arm-eabi-omap: ERRORS
> > > > >> linux-git-i686: OK
> > > > >> linux-git-m32r: OK
> > > > >> linux-git-mips: WARNINGS
> > > > >> linux-git-powerpc64: OK
> > > > >> linux-git-sh: OK
> > > > >> linux-git-x86_64: OK
> > > > >> linux-2.6.31.12-i686: WARNINGS
> > > > >> linux-2.6.32.6-i686: WARNINGS
> > > > >> linux-2.6.33-i686: WARNINGS
> > > > >> linux-2.6.34-i686: WARNINGS
> > > > >> linux-2.6.35.3-i686: WARNINGS
> > > > >> linux-2.6.36-i686: WARNINGS
> > > > >> linux-2.6.37-i686: WARNINGS
> > > > >> linux-2.6.38.2-i686: WARNINGS
> > > > >> linux-2.6.39.1-i686: WARNINGS
> > > > >> linux-3.0-i686: WARNINGS
> > > > >> linux-3.1-i686: WARNINGS
> > > > >> linux-3.2.1-i686: WARNINGS
> > > > >> linux-3.3-i686: WARNINGS
> > > > >> linux-3.4-i686: WARNINGS
> > > > >> linux-3.5-i686: WARNINGS
> > > > >> linux-3.6-i686: WARNINGS
> > > > >> linux-3.7-i686: WARNINGS
> > > > >> linux-3.8-rc1-i686: WARNINGS
> > > > >> linux-2.6.31.12-x86_64: WARNINGS
> > > > >> linux-2.6.32.6-x86_64: WARNINGS
> > > > >> linux-2.6.33-x86_64: WARNINGS
> > > > >> linux-2.6.34-x86_64: WARNINGS
> > > > >> linux-2.6.35.3-x86_64: WARNINGS
> > > > >> linux-2.6.36-x86_64: WARNINGS
> > > > >> linux-2.6.37-x86_64: WARNINGS
> > > > >> linux-2.6.38.2-x86_64: WARNINGS
> > > > >> linux-2.6.39.1-x86_64: WARNINGS
> > > > >> linux-3.0-x86_64: WARNINGS
> > > > >> linux-3.1-x86_64: WARNINGS
> > > > >> linux-3.2.1-x86_64: WARNINGS
> > > > >> linux-3.3-x86_64: WARNINGS
> > > > >> linux-3.4-x86_64: WARNINGS
> > > > >> linux-3.5-x86_64: WARNINGS
> > > > >> linux-3.6-x86_64: WARNINGS
> > > > >> linux-3.7-x86_64: WARNINGS
> > > > >> linux-3.8-rc1-x86_64: WARNINGS
> > > > >> apps: WARNINGS
> > > > >> spec-git: OK
> > > > >> sparse: ERRORS
> > > > >>
> > > > >> Detailed results are available here:
> > > > >>
> > > > >> http://www.xs4all.nl/~hverkuil/logs/Monday.log
> > > > > 
> > > > > There were a lot of new 'redefined' warnings that I have fixed.
> > > > > 
> > > > > In addition, it turned out that any driver using vb2 wasn't compiled 
> > > > > for
> > > > > kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. 
> > > > > That's fixed
> > > > > as well, so drivers like em28xx and vivi will now compile on those 
> > > > > older
> > > > > kernels. This also was the reason I never saw that the 
> > > > > usb_translate_error
> > > > > function needed to be added to compat.h: it's used in em28xx but that 
> > > > > driver
> > > > > was never compiled on kernels without usb_translate_error.
> > > > > 
> > > > > Hopefully everything works now.
> > > > > 
> > > > > Regards,
> > > > > 
> > > > >   Hans
> > > > 
> > > > Hi Hans,
> > > > on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
> > > > just a few remaining warnings.
> > > > 
> > > > In particular, there are several new warnings related to 
> > > > DMA_SHARED_BUFFER:
> > > > 
> > > > WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] 
> > > > undefined!
> > > > WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] 
> > > > undefined!
> > > > WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
> > > > WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
> > > > WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!
> > > 
> > > Gianluca,
> > > 
> > > Can you patch media_build with the patch below and try again? If it 
> > > doesn't
> > > work, then replace '#ifdef CONFIG_DMA_SHARED_BUFFER' by '#if 0' in the 
> > > patch
> > > below and try that instead.
> > > 
> > > Let me know what works.
> > 
> > You don't need to write a patch that replaces CONFIG_DMA_SHARED_BUFFER by 0.
> > you can patch, instead, v4l/scripts/make_kconfig.pl, like those:
> > 
> > 
> > # Kernel < 2.6.22 is missing the HAS_IOMEM option
> > if (!defined $kernopts{HAS_IOMEM} && cmp_ver($kernver, '2.6.22') < 0) {
> > $kernopts{HAS_IOMEM} = 2;
> > }
> > 
> > # Kernel < 2.6.22 is missing the HAS_DMA option
> > if (!defined $kernopts{HAS_DMA} && cmp_ver($kernver, '2.6.22') < 0) {
> >

Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Sylwester Nawrocki

Hi,

On 01/08/2013 09:10 AM, Laurent Pinchart wrote:

+/*
+ * If subdevice probing fails any time after v4l2_async_subdev_bind(), no
+ * clean up must be called. This function is only a message of intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);


Could you please explain why you need both a bind notifier and a bound
notifier ? I was expecting a single v4l2_async_subdev_register() call in
subdev drivers (and, thinking about it, I would probably name it
v4l2_subdev_register()).


I expected it to be done this way too, and I also used 
v4l2_subdev_register()

name in my early version of the subdev registration code where subdevs
were registering themselves to the v4l2 core.

BTW, this might not be most important thing here, but do we need separate
file, i.e. v4l2-async.c, instead of for example putting it in 
v4l2-device.c ?



+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
+#endif


--

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


[PATCH 5/5] kfifo: log based kfifo API

2013-01-08 Thread Yuanhan Liu
The current kfifo API take the kfifo size as input, while it rounds
 _down_ the size to power of 2 at __kfifo_alloc. This may introduce
potential issue.

Take the code at drivers/hid/hid-logitech-dj.c as example:

if (kfifo_alloc(&djrcv_dev->notif_fifo,
   DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report),
   GFP_KERNEL)) {

Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct dj_report)
is 15.

Which means it wants to allocate a kfifo buffer which can store 8
dj_report entries at once. The expected kfifo buffer size would be
8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
size to rounddown_power_of_2(120) =  64, and then allocate a buf
with 64 bytes, which I don't think this is the original author want.

With the new log API, we can do like following:

int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
sizeof(struct dj_report));

if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, GFP_KERNEL)) {

This make sure we will allocate enough kfifo buffer for holding
DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.

Cc: Stefani Seibold 
Cc: Andrew Morton 
Cc: linux-o...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: platform-driver-...@vger.kernel.org
Cc: linux-in...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-r...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-...@lists.infradead.org
Cc: libertas-...@lists.infradead.org
Cc: linux-wirel...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: open-is...@googlegroups.com
Cc: linux-s...@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Cc: linux-ser...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux...@kvack.org
Cc: d...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Signed-off-by: Yuanhan Liu 
---
 arch/arm/plat-omap/Kconfig  |2 +-
 arch/arm/plat-omap/mailbox.c|6 +++-
 arch/powerpc/sysdev/fsl_rmu.c   |2 +-
 drivers/char/sonypi.c   |9 ---
 drivers/hid/hid-logitech-dj.c   |7 +++--
 drivers/iio/industrialio-event.c|2 +-
 drivers/iio/kfifo_buf.c |3 +-
 drivers/infiniband/hw/cxgb3/cxio_resource.c |8 --
 drivers/media/i2c/cx25840/cx25840-ir.c  |9 +--
 drivers/media/pci/cx23885/cx23888-ir.c  |9 +--
 drivers/media/pci/meye/meye.c   |6 +---
 drivers/media/pci/meye/meye.h   |2 +
 drivers/media/rc/ir-raw.c   |7 +++--
 drivers/memstick/host/r592.h|2 +-
 drivers/mmc/card/sdio_uart.c|4 ++-
 drivers/mtd/sm_ftl.c|5 +++-
 drivers/net/wireless/libertas/main.c|4 ++-
 drivers/net/wireless/rt2x00/rt2x00dev.c |5 +--
 drivers/pci/pcie/aer/aerdrv_core.c  |3 +-
 drivers/platform/x86/fujitsu-laptop.c   |5 ++-
 drivers/platform/x86/sony-laptop.c  |6 ++--
 drivers/rapidio/devices/tsi721.c|5 ++-
 drivers/scsi/libiscsi_tcp.c |6 +++-
 drivers/staging/omapdrm/omap_plane.c|5 +++-
 drivers/tty/n_gsm.c |4 ++-
 drivers/tty/nozomi.c|5 +--
 drivers/tty/serial/ifx6x60.c|2 +-
 drivers/tty/serial/ifx6x60.h|3 +-
 drivers/tty/serial/kgdb_nmi.c   |7 +++--
 drivers/usb/host/fhci.h |4 ++-
 drivers/usb/serial/cypress_m8.c |4 +-
 drivers/usb/serial/io_ti.c  |4 +-
 drivers/usb/serial/ti_usb_3410_5052.c   |7 +++--
 drivers/usb/serial/usb-serial.c |2 +-
 include/linux/kfifo.h   |   31 +--
 include/linux/rio.h |1 +
 include/media/lirc_dev.h|4 ++-
 kernel/kfifo.c  |9 +--
 mm/memory-failure.c |3 +-
 net/dccp/probe.c|6 +++-
 net/sctp/probe.c|6 +++-
 samples/kfifo/bytestream-example.c  |8 +++---
 samples/kfifo/dma-example.c |5 ++-
 samples/kfifo/inttype-example.c |7 +++--
 samples/kfifo/record-example.c  |6 ++--
 45 files changed, 142 insertions(+), 108 deletions(-)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 665870d..7eda02c 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -124,7 +124,7 @@ config OMAP_MBOX_FWK
  DSP, IVA1.0 and IVA2 in OMAP1/2/3.
 
 config OMAP_MBOX_KFIFO_SIZE
-   int "Mailbox kfifo default buffer size (bytes)"
+   int "Mailbox kfifo default buffer size (bytes, should be power of 2. If 
not, will roundup to power of 2"

Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

2013-01-08 Thread Federico Vaga
> Ok, then I would simply pass the flags from the driver without any
> alternation
> in the allocator itself, so drivers can pass 'GFP_KERNEL' or
> 'GFP_KERNEL | GFP_DMA' depending on their preference. Please also update
> all
> the existing clients of vb2_dma_dc allocator.

I taked a look at drivers that use dma-contig. They use the structure 
vb2_alloc_ctx which is just a name, there is not a real vb2_alloc_ctx 
structure implementation. "Now" driver must gain access to vb2_dc_conf to set 
the correct flags.

I have the following ideas:

  1.  replace all the names and expose vb2_dc_conf to all drivers (like dma-
sg, it export vb2_dma_sg_desc to all its users)

  2.  create an helper which configure flags. This maintain the vb2_dc_conf 
private
  vb2_set_mem_flags(struct vb2_alloc_ctx *alloc_ctx, gfp_t flags)

  3.  rename vb2_dc_conf to vb2_alloc_ctx because it is an implementation 
vb2_alloc_ctx and (at the moment) it is used only by dma-contig

-- 
Federico Vaga
--
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 v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

2013-01-08 Thread Jonathan Corbet
On Tue, 08 Jan 2013 07:50:41 +0100
Marek Szyprowski  wrote:

> > Couldn't this performance difference be due to the usage of GFP_DMA inside
> > the VB2 code, like Federico's new patch series is proposing?
> >
> > If not, why are there a so large performance penalty?  
> 
> Nope, this was caused rather by a very poor CPU access to non-cached (aka
> 'coherent') memory and the way the video data has been accessed/read 
> with CPU.

Exactly.  Uncached memory *hurts*, especially if you're having to touch it
all with the CPU.

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Sylwester Nawrocki

Hi Laurent,

On 01/08/2013 01:41 PM, Laurent Pinchart wrote:

Subdev names are exposed to user space by the media controller API.
So they are really part of an ABI, aren't they ?


They're used to construct the name exposed to userspace, but the media
controller core could probably handle that internally by concatenating the
driver name and the subdev name.


Also having I2C bus number or I2C slave address as part of the subdev
name makes it more difficult to write portable applications. Hence
in sensor drivers I used to overwrite subdev name to remove I2C bus
and slave address, as the format used v4l2_i2c_subdev_init() seemed
highly unsuitable..


This clearly shows that we need to discuss the matter and agree on a common
mode of operation.

Aren't applications that use the subdev name directly inherently non-portable
anyway ? If you want your application to support different boards/sensors/SoCs
you should discover the pipeline and find the sensor by iterating over
entities, instead of using the sensor entity name.


It depends on how we define the entity names :) It the names change from 
board
to board and are completely unreliable then user space applications 
using them
have no any chance to be generic. Nevertheless, struct 
media_entity_desc::name

[1] has currently no specific semantics defined, e.g. for V4L2.

It's likely way better for the kernel space to be no constrained by the 
subdev
user space name requirement. But having no clear definition of the 
entity names
brings more trouble to user space. E.g. when a sensor exposes multiple 
subdevs.

User space library/application could then reference them by entity name. It
seems difficult to me to handle such multiple subdev devices without 
somehow

reliable subdev names.

I imagine a system with multiple sensors of same type sitting on 
different I2C
busses, then appending I2C bus number/slave address to the name would be 
useful.


And is it always possible to discover the pipeline, as opposite to e.g. 
using

configuration file to activate all required links ? Configurations could be
of course per board file, but then at least we should keep subdev names
constant through the kernel releases.


[1] http://linuxtv.org/downloads/v4l-dvb-apis/media-ioc-enum-entities.html

--

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: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Hans Verkuil
On Tue 8 January 2013 14:05:10 Mauro Carvalho Chehab wrote:
> Em Tue, 8 Jan 2013 13:43:43 +0100
> Hans Verkuil  escreveu:
> 
> > On Tue 8 January 2013 12:01:51 Gianluca Gennari wrote:
> > > Il 08/01/2013 10:58, Hans Verkuil ha scritto:
> > > > On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
> > > >> 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:Mon Jan  7 19:00:18 CET 2013
> > > >> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
> > > >> gcc version:  i686-linux-gcc (GCC) 4.7.1
> > > >> host hardware:x86_64
> > > >> host os:  3.4.07-marune
> > > >>
> > > >> linux-git-arm-eabi-davinci: WARNINGS
> > > >> linux-git-arm-eabi-exynos: WARNINGS
> > > >> linux-git-arm-eabi-omap: ERRORS
> > > >> linux-git-i686: OK
> > > >> linux-git-m32r: OK
> > > >> linux-git-mips: WARNINGS
> > > >> linux-git-powerpc64: OK
> > > >> linux-git-sh: OK
> > > >> linux-git-x86_64: OK
> > > >> linux-2.6.31.12-i686: WARNINGS
> > > >> linux-2.6.32.6-i686: WARNINGS
> > > >> linux-2.6.33-i686: WARNINGS
> > > >> linux-2.6.34-i686: WARNINGS
> > > >> linux-2.6.35.3-i686: WARNINGS
> > > >> linux-2.6.36-i686: WARNINGS
> > > >> linux-2.6.37-i686: WARNINGS
> > > >> linux-2.6.38.2-i686: WARNINGS
> > > >> linux-2.6.39.1-i686: WARNINGS
> > > >> linux-3.0-i686: WARNINGS
> > > >> linux-3.1-i686: WARNINGS
> > > >> linux-3.2.1-i686: WARNINGS
> > > >> linux-3.3-i686: WARNINGS
> > > >> linux-3.4-i686: WARNINGS
> > > >> linux-3.5-i686: WARNINGS
> > > >> linux-3.6-i686: WARNINGS
> > > >> linux-3.7-i686: WARNINGS
> > > >> linux-3.8-rc1-i686: WARNINGS
> > > >> linux-2.6.31.12-x86_64: WARNINGS
> > > >> linux-2.6.32.6-x86_64: WARNINGS
> > > >> linux-2.6.33-x86_64: WARNINGS
> > > >> linux-2.6.34-x86_64: WARNINGS
> > > >> linux-2.6.35.3-x86_64: WARNINGS
> > > >> linux-2.6.36-x86_64: WARNINGS
> > > >> linux-2.6.37-x86_64: WARNINGS
> > > >> linux-2.6.38.2-x86_64: WARNINGS
> > > >> linux-2.6.39.1-x86_64: WARNINGS
> > > >> linux-3.0-x86_64: WARNINGS
> > > >> linux-3.1-x86_64: WARNINGS
> > > >> linux-3.2.1-x86_64: WARNINGS
> > > >> linux-3.3-x86_64: WARNINGS
> > > >> linux-3.4-x86_64: WARNINGS
> > > >> linux-3.5-x86_64: WARNINGS
> > > >> linux-3.6-x86_64: WARNINGS
> > > >> linux-3.7-x86_64: WARNINGS
> > > >> linux-3.8-rc1-x86_64: WARNINGS
> > > >> apps: WARNINGS
> > > >> spec-git: OK
> > > >> sparse: ERRORS
> > > >>
> > > >> Detailed results are available here:
> > > >>
> > > >> http://www.xs4all.nl/~hverkuil/logs/Monday.log
> > > > 
> > > > There were a lot of new 'redefined' warnings that I have fixed.
> > > > 
> > > > In addition, it turned out that any driver using vb2 wasn't compiled for
> > > > kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's 
> > > > fixed
> > > > as well, so drivers like em28xx and vivi will now compile on those older
> > > > kernels. This also was the reason I never saw that the 
> > > > usb_translate_error
> > > > function needed to be added to compat.h: it's used in em28xx but that 
> > > > driver
> > > > was never compiled on kernels without usb_translate_error.
> > > > 
> > > > Hopefully everything works now.
> > > > 
> > > > Regards,
> > > > 
> > > > Hans
> > > 
> > > Hi Hans,
> > > on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
> > > just a few remaining warnings.
> > > 
> > > In particular, there are several new warnings related to 
> > > DMA_SHARED_BUFFER:
> > > 
> > > WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] 
> > > undefined!
> > > WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
> > > WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
> > > WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
> > > WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!
> > 
> > Gianluca,
> > 
> > Can you patch media_build with the patch below and try again? If it doesn't
> > work, then replace '#ifdef CONFIG_DMA_SHARED_BUFFER' by '#if 0' in the patch
> > below and try that instead.
> > 
> > Let me know what works.
> 
> You don't need to write a patch that replaces CONFIG_DMA_SHARED_BUFFER by 0.
> you can patch, instead, v4l/scripts/make_kconfig.pl, like those:
> 
> 
> # Kernel < 2.6.22 is missing the HAS_IOMEM option
> if (!defined $kernopts{HAS_IOMEM} && cmp_ver($kernver, '2.6.22') < 0) {
> $kernopts{HAS_IOMEM} = 2;
> }
> 
> # Kernel < 2.6.22 is missing the HAS_DMA option
> if (!defined $kernopts{HAS_DMA} && cmp_ver($kernver, '2.6.22') < 0) {
> $kernopts{HAS_DMA} = 2;
> }
> 
> # Kernel < 2.6.23 is missing the VIRT_TO_BUS option
> if (!defined $kernopts{VIRT_TO_BUS} && cmp_ver($kernver, '2.6.23') < 0) {
>   # VIRT_TO_BUS -> !PPC64
>   $kernopts{VIRT_TO_BUS} = 2 - $kernopts{PPC64};
> }
> 
> # Kernel < 2.6.37 is missing the BKL option
> if (!defined $kernopts{BKL} && cmp_ver($ke

[PATCH 2/2] omap3isp: Set cam_mclk rate directly

2013-01-08 Thread Laurent Pinchart
Now that the cam_mclk rate changes are back-propagated to dpll4_m5_ck we
can set the cam_mclk rate directly instead of manually setting the rate
of the parent clock.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/omap3isp/isp.c |   18 ++
 drivers/media/platform/omap3isp/isp.h |8 +++-
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 07eea5b..63a583f 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1338,28 +1338,15 @@ static int isp_enable_clocks(struct isp_device *isp)
 {
int r;
unsigned long rate;
-   int divisor;
-
-   /*
-* cam_mclk clock chain:
-*   dpll4 -> dpll4_m5 -> dpll4_m5x2 -> cam_mclk
-*
-* In OMAP3630 dpll4_m5x2 != 2 x dpll4_m5 but both are
-* set to the same value. Hence the rate set for dpll4_m5
-* has to be twice of what is set on OMAP3430 to get
-* the required value for cam_mclk
-*/
-   divisor = isp->revision == ISP_REVISION_15_0 ? 1 : 2;
 
r = clk_prepare_enable(isp->clock[ISP_CLK_CAM_ICK]);
if (r) {
dev_err(isp->dev, "failed to enable cam_ick clock\n");
goto out_clk_enable_ick;
}
-   r = clk_set_rate(isp->clock[ISP_CLK_DPLL4_M5_CK],
-CM_CAM_MCLK_HZ/divisor);
+   r = clk_set_rate(isp->clock[ISP_CLK_CAM_MCLK], CM_CAM_MCLK_HZ);
if (r) {
-   dev_err(isp->dev, "clk_set_rate for dpll4_m5_ck failed\n");
+   dev_err(isp->dev, "clk_set_rate for cam_mclk failed\n");
goto out_clk_enable_mclk;
}
r = clk_prepare_enable(isp->clock[ISP_CLK_CAM_MCLK]);
@@ -1401,7 +1388,6 @@ static void isp_disable_clocks(struct isp_device *isp)
 static const char *isp_clocks[] = {
"cam_ick",
"cam_mclk",
-   "dpll4_m5_ck",
"csi2_96m_fck",
"l3_ick",
 };
diff --git a/drivers/media/platform/omap3isp/isp.h 
b/drivers/media/platform/omap3isp/isp.h
index 517d348..c77e1f2 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -147,7 +147,6 @@ struct isp_platform_callback {
  * @ref_count: Reference count for handling multiple ISP requests.
  * @cam_ick: Pointer to camera interface clock structure.
  * @cam_mclk: Pointer to camera functional clock structure.
- * @dpll4_m5_ck: Pointer to DPLL4 M5 clock structure.
  * @csi2_fck: Pointer to camera CSI2 complexIO clock structure.
  * @l3_ick: Pointer to OMAP3 L3 bus interface clock.
  * @irq: Currently attached ISP ISR callbacks information structure.
@@ -189,10 +188,9 @@ struct isp_device {
u32 xclk_divisor[2];/* Two clocks, a and b. */
 #define ISP_CLK_CAM_ICK0
 #define ISP_CLK_CAM_MCLK   1
-#define ISP_CLK_DPLL4_M5_CK2
-#define ISP_CLK_CSI2_FCK   3
-#define ISP_CLK_L3_ICK 4
-   struct clk *clock[5];
+#define ISP_CLK_CSI2_FCK   2
+#define ISP_CLK_L3_ICK 3
+   struct clk *clock[4];
 
/* ISP modules */
struct ispstat isp_af;
-- 
1.7.8.6

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] OMAP3 ISP: Simplify clock usage

2013-01-08 Thread Laurent Pinchart
Hello,

Now that the OMAP3 supports the common clock framework, clock rate
back-propagation is available for the ISP clocks. Instead of setting the
cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark the
dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting back-propagation, and set
the cam_mclk rate directly. This simplifies the ISP clocks configuration.

Laurent Pinchart (2):
  ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to
dpll4_m5
  omap3isp: Set cam_mclk rate directly

 arch/arm/mach-omap2/cclock3xxx_data.c |   10 +-
 drivers/media/platform/omap3isp/isp.c |   18 ++
 drivers/media/platform/omap3isp/isp.h |8 +++-
 3 files changed, 14 insertions(+), 22 deletions(-)

-- 
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 1/2] ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to dpll4_m5

2013-01-08 Thread Laurent Pinchart
The cam_mclk clock is generated through the following clocks chain:

dpll4 -> dpll4_m5 -> dpll4_m5x2 -> cam_mclk

As dpll4_m5 and dpll4_m5x2 do not driver any clock other than cam_mclk,
back-propagate the cam_clk rate changes up to dpll4_m5.

Signed-off-by: Laurent Pinchart 
---
 arch/arm/mach-omap2/cclock3xxx_data.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c 
b/arch/arm/mach-omap2/cclock3xxx_data.c
index bdf3948..326b6ad 100644
--- a/arch/arm/mach-omap2/cclock3xxx_data.c
+++ b/arch/arm/mach-omap2/cclock3xxx_data.c
@@ -426,6 +426,7 @@ static struct clk dpll4_m5x2_ck_3630 = {
.parent_names   = dpll4_m5x2_ck_parent_names,
.num_parents= ARRAY_SIZE(dpll4_m5x2_ck_parent_names),
.ops= &dpll4_m5x2_ck_3630_ops,
+   .flags  = CLK_SET_RATE_PARENT,
 };
 
 static struct clk cam_mclk;
@@ -443,7 +444,14 @@ static struct clk_hw_omap cam_mclk_hw = {
.clkdm_name = "cam_clkdm",
 };
 
-DEFINE_STRUCT_CLK(cam_mclk, cam_mclk_parent_names, aes2_ick_ops);
+static struct clk cam_mclk = {
+   .name   = "cam_mclk",
+   .hw = &cam_mclk_hw.hw,
+   .parent_names   = cam_mclk_parent_names,
+   .num_parents= ARRAY_SIZE(cam_mclk_parent_names),
+   .ops= &aes2_ick_ops,
+   .flags  = CLK_SET_RATE_PARENT,
+};
 
 static const struct clksel_rate clkout2_src_core_rates[] = {
{ .div = 1, .val = 0, .flags = RATE_IN_3XXX },
-- 
1.7.8.6

--
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: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Gianluca Gennari
Il 08/01/2013 13:43, Hans Verkuil ha scritto:
> On Tue 8 January 2013 12:01:51 Gianluca Gennari wrote:
>> Il 08/01/2013 10:58, Hans Verkuil ha scritto:
>>> On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
 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:Mon Jan  7 19:00:18 CET 2013
 git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
 gcc version:  i686-linux-gcc (GCC) 4.7.1
 host hardware:x86_64
 host os:  3.4.07-marune

 linux-git-arm-eabi-davinci: WARNINGS
 linux-git-arm-eabi-exynos: WARNINGS
 linux-git-arm-eabi-omap: ERRORS
 linux-git-i686: OK
 linux-git-m32r: OK
 linux-git-mips: WARNINGS
 linux-git-powerpc64: OK
 linux-git-sh: OK
 linux-git-x86_64: OK
 linux-2.6.31.12-i686: WARNINGS
 linux-2.6.32.6-i686: WARNINGS
 linux-2.6.33-i686: WARNINGS
 linux-2.6.34-i686: WARNINGS
 linux-2.6.35.3-i686: WARNINGS
 linux-2.6.36-i686: WARNINGS
 linux-2.6.37-i686: WARNINGS
 linux-2.6.38.2-i686: WARNINGS
 linux-2.6.39.1-i686: WARNINGS
 linux-3.0-i686: WARNINGS
 linux-3.1-i686: WARNINGS
 linux-3.2.1-i686: WARNINGS
 linux-3.3-i686: WARNINGS
 linux-3.4-i686: WARNINGS
 linux-3.5-i686: WARNINGS
 linux-3.6-i686: WARNINGS
 linux-3.7-i686: WARNINGS
 linux-3.8-rc1-i686: WARNINGS
 linux-2.6.31.12-x86_64: WARNINGS
 linux-2.6.32.6-x86_64: WARNINGS
 linux-2.6.33-x86_64: WARNINGS
 linux-2.6.34-x86_64: WARNINGS
 linux-2.6.35.3-x86_64: WARNINGS
 linux-2.6.36-x86_64: WARNINGS
 linux-2.6.37-x86_64: WARNINGS
 linux-2.6.38.2-x86_64: WARNINGS
 linux-2.6.39.1-x86_64: WARNINGS
 linux-3.0-x86_64: WARNINGS
 linux-3.1-x86_64: WARNINGS
 linux-3.2.1-x86_64: WARNINGS
 linux-3.3-x86_64: WARNINGS
 linux-3.4-x86_64: WARNINGS
 linux-3.5-x86_64: WARNINGS
 linux-3.6-x86_64: WARNINGS
 linux-3.7-x86_64: WARNINGS
 linux-3.8-rc1-x86_64: WARNINGS
 apps: WARNINGS
 spec-git: OK
 sparse: ERRORS

 Detailed results are available here:

 http://www.xs4all.nl/~hverkuil/logs/Monday.log
>>>
>>> There were a lot of new 'redefined' warnings that I have fixed.
>>>
>>> In addition, it turned out that any driver using vb2 wasn't compiled for
>>> kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's fixed
>>> as well, so drivers like em28xx and vivi will now compile on those older
>>> kernels. This also was the reason I never saw that the usb_translate_error
>>> function needed to be added to compat.h: it's used in em28xx but that driver
>>> was never compiled on kernels without usb_translate_error.
>>>
>>> Hopefully everything works now.
>>>
>>> Regards,
>>>
>>> Hans
>>
>> Hi Hans,
>> on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
>> just a few remaining warnings.
>>
>> In particular, there are several new warnings related to DMA_SHARED_BUFFER:
>>
>> WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
>> WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
>> WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
>> WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
>> WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!
> 
> Gianluca,
> 
> Can you patch media_build with the patch below and try again? If it doesn't
> work, then replace '#ifdef CONFIG_DMA_SHARED_BUFFER' by '#if 0' in the patch
> below and try that instead.
> 
> Let me know what works.
> 
> Thanks,
> 
>   Hans
> 

Hi Hans,
your patch works perfectly fine in both ways.

BTW, those are the few extra warnings remaining:

2 random warnings:

media_build/v4l/anysee.c: In function 'anysee_frontend_attach':
media_build/v4l/anysee.c:641: warning: 'ret' may be used uninitialized
in this function

media_build/v4l/ngene-cards.c:813: warning: initialization discards
qualifiers from pointer target type

and a redefinition of the "err" macro:

media_build/v4l/mxl111sf.c:58:1: warning: "err" redefined
In file included from include/linux/usb/input.h:12,
 from media_build/v4l/dvb_usb.h:25,
 from media_build/v4l/mxl111sf.h:18,
 from media_build/v4l/mxl111sf.c:14:
include/linux/usb.h:1593:1: warning: this is the location of the
previous definition

media_build/v4l/mxl111sf-tuner.c:34:1: warning: "err" redefined
In file included from include/linux/usb/input.h:12,
 from media_build/v4l/dvb_usb.h:25,
 from media_build/v4l/mxl111sf.h:18,
 from media_build/v4l/mxl111sf-tuner.h:26,
 from media_build/v4l/mxl111sf-tuner.c:21:
include/linux/usb.h:1593:1: warning: this is the location of the
previous definition

Thanks and regards,
Gianluca

> 
> diff --git a/backports/backports.txt 

Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Sakari Ailus
Hi Laurent,

On Tue, Jan 08, 2013 at 01:41:59PM +0100, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> > On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > > If we need a workaround, I'd rather pass the device name in addition
> > > to the I2C adapter number and address, instead of embedding the
> > > workaround in this new API.
> >  
> >  ...or we can change the I2C subdevice name format. The actual need to
> >  do
> >  
> > snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> >  asdl->dev->driver->name,
> >  i2c_adapter_id(client->adapter), client->addr);
> >  
> >  in soc-camera now to exactly match the subdevice name, as created by
> >  v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
> >  if the latter changes at some point? Or what if one driver wishes to
> >  create several subdevices for one I2C device?
> > >>> 
> > >>> The common clock framework uses %d-%04x, maybe we could use that as well
> > >>> for clock names ?
> > >> 
> > >> And preserve the subdevice names? Then matching would be more difficult
> > >> and less precise. Or change subdevice names too? I think, we can do the
> > >> latter, since anyway at any time only one driver can be attached to an
> > >> I2C device.
> > > 
> > > That's right. Where else is the subdev name used ?
> > 
> > Subdev names are exposed to user space by the media controller API.
> > So they are really part of an ABI, aren't they ?
> 
> They're used to construct the name exposed to userspace, but the media 
> controller core could probably handle that internally by concatenating the 
> driver name and the subdev name.
> 
> > Also having I2C bus number or I2C slave address as part of the subdev
> > name makes it more difficult to write portable applications. Hence
> > in sensor drivers I used to overwrite subdev name to remove I2C bus
> > and slave address, as the format used v4l2_i2c_subdev_init() seemed
> > highly unsuitable..
> 
> This clearly shows that we need to discuss the matter and agree on a common 
> mode of operation.
> 
> Aren't applications that use the subdev name directly inherently non-portable 
> anyway ? If you want your application to support different 
> boards/sensors/SoCs 

Well, the name could come from a configuration file to distinguish e.g.
between primary and secondary sensors.

For what it's worth, the SMIA++ driver uses the actual name of the sensor
since there are about 10 sensors supported at the moment, and calling them
all smiapp- looks a bit insipid. So one has to talk to the sensor to
know what it's called.

This isn't strictly mandatory but a nice feature.

> you should discover the pipeline and find the sensor by iterating over 
> entities, instead of using the sensor entity name.

To be fully generic, yes.

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: 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: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Mauro Carvalho Chehab
Em Tue, 8 Jan 2013 13:43:43 +0100
Hans Verkuil  escreveu:

> On Tue 8 January 2013 12:01:51 Gianluca Gennari wrote:
> > Il 08/01/2013 10:58, Hans Verkuil ha scritto:
> > > On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
> > >> 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:Mon Jan  7 19:00:18 CET 2013
> > >> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
> > >> gcc version:  i686-linux-gcc (GCC) 4.7.1
> > >> host hardware:x86_64
> > >> host os:  3.4.07-marune
> > >>
> > >> linux-git-arm-eabi-davinci: WARNINGS
> > >> linux-git-arm-eabi-exynos: WARNINGS
> > >> linux-git-arm-eabi-omap: ERRORS
> > >> linux-git-i686: OK
> > >> linux-git-m32r: OK
> > >> linux-git-mips: WARNINGS
> > >> linux-git-powerpc64: OK
> > >> linux-git-sh: OK
> > >> linux-git-x86_64: OK
> > >> linux-2.6.31.12-i686: WARNINGS
> > >> linux-2.6.32.6-i686: WARNINGS
> > >> linux-2.6.33-i686: WARNINGS
> > >> linux-2.6.34-i686: WARNINGS
> > >> linux-2.6.35.3-i686: WARNINGS
> > >> linux-2.6.36-i686: WARNINGS
> > >> linux-2.6.37-i686: WARNINGS
> > >> linux-2.6.38.2-i686: WARNINGS
> > >> linux-2.6.39.1-i686: WARNINGS
> > >> linux-3.0-i686: WARNINGS
> > >> linux-3.1-i686: WARNINGS
> > >> linux-3.2.1-i686: WARNINGS
> > >> linux-3.3-i686: WARNINGS
> > >> linux-3.4-i686: WARNINGS
> > >> linux-3.5-i686: WARNINGS
> > >> linux-3.6-i686: WARNINGS
> > >> linux-3.7-i686: WARNINGS
> > >> linux-3.8-rc1-i686: WARNINGS
> > >> linux-2.6.31.12-x86_64: WARNINGS
> > >> linux-2.6.32.6-x86_64: WARNINGS
> > >> linux-2.6.33-x86_64: WARNINGS
> > >> linux-2.6.34-x86_64: WARNINGS
> > >> linux-2.6.35.3-x86_64: WARNINGS
> > >> linux-2.6.36-x86_64: WARNINGS
> > >> linux-2.6.37-x86_64: WARNINGS
> > >> linux-2.6.38.2-x86_64: WARNINGS
> > >> linux-2.6.39.1-x86_64: WARNINGS
> > >> linux-3.0-x86_64: WARNINGS
> > >> linux-3.1-x86_64: WARNINGS
> > >> linux-3.2.1-x86_64: WARNINGS
> > >> linux-3.3-x86_64: WARNINGS
> > >> linux-3.4-x86_64: WARNINGS
> > >> linux-3.5-x86_64: WARNINGS
> > >> linux-3.6-x86_64: WARNINGS
> > >> linux-3.7-x86_64: WARNINGS
> > >> linux-3.8-rc1-x86_64: WARNINGS
> > >> apps: WARNINGS
> > >> spec-git: OK
> > >> sparse: ERRORS
> > >>
> > >> Detailed results are available here:
> > >>
> > >> http://www.xs4all.nl/~hverkuil/logs/Monday.log
> > > 
> > > There were a lot of new 'redefined' warnings that I have fixed.
> > > 
> > > In addition, it turned out that any driver using vb2 wasn't compiled for
> > > kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's 
> > > fixed
> > > as well, so drivers like em28xx and vivi will now compile on those older
> > > kernels. This also was the reason I never saw that the usb_translate_error
> > > function needed to be added to compat.h: it's used in em28xx but that 
> > > driver
> > > was never compiled on kernels without usb_translate_error.
> > > 
> > > Hopefully everything works now.
> > > 
> > > Regards,
> > > 
> > >   Hans
> > 
> > Hi Hans,
> > on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
> > just a few remaining warnings.
> > 
> > In particular, there are several new warnings related to DMA_SHARED_BUFFER:
> > 
> > WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
> > WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
> > WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
> > WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
> > WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!
> 
> Gianluca,
> 
> Can you patch media_build with the patch below and try again? If it doesn't
> work, then replace '#ifdef CONFIG_DMA_SHARED_BUFFER' by '#if 0' in the patch
> below and try that instead.
> 
> Let me know what works.

You don't need to write a patch that replaces CONFIG_DMA_SHARED_BUFFER by 0.
you can patch, instead, v4l/scripts/make_kconfig.pl, like those:


# Kernel < 2.6.22 is missing the HAS_IOMEM option
if (!defined $kernopts{HAS_IOMEM} && cmp_ver($kernver, '2.6.22') < 0) {
$kernopts{HAS_IOMEM} = 2;
}

# Kernel < 2.6.22 is missing the HAS_DMA option
if (!defined $kernopts{HAS_DMA} && cmp_ver($kernver, '2.6.22') < 0) {
$kernopts{HAS_DMA} = 2;
}

# Kernel < 2.6.23 is missing the VIRT_TO_BUS option
if (!defined $kernopts{VIRT_TO_BUS} && cmp_ver($kernver, '2.6.23') < 0) {
# VIRT_TO_BUS -> !PPC64
$kernopts{VIRT_TO_BUS} = 2 - $kernopts{PPC64};
}

# Kernel < 2.6.37 is missing the BKL option
if (!defined $kernopts{BKL} && cmp_ver($kernver, '2.6.37') < 0) {
$kernopts{BKL} = 2;
}

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Hans Verkuil
On Tue 8 January 2013 12:01:51 Gianluca Gennari wrote:
> Il 08/01/2013 10:58, Hans Verkuil ha scritto:
> > On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
> >> 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:Mon Jan  7 19:00:18 CET 2013
> >> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
> >> gcc version:  i686-linux-gcc (GCC) 4.7.1
> >> host hardware:x86_64
> >> host os:  3.4.07-marune
> >>
> >> linux-git-arm-eabi-davinci: WARNINGS
> >> linux-git-arm-eabi-exynos: WARNINGS
> >> linux-git-arm-eabi-omap: ERRORS
> >> linux-git-i686: OK
> >> linux-git-m32r: OK
> >> linux-git-mips: WARNINGS
> >> linux-git-powerpc64: OK
> >> linux-git-sh: OK
> >> linux-git-x86_64: OK
> >> linux-2.6.31.12-i686: WARNINGS
> >> linux-2.6.32.6-i686: WARNINGS
> >> linux-2.6.33-i686: WARNINGS
> >> linux-2.6.34-i686: WARNINGS
> >> linux-2.6.35.3-i686: WARNINGS
> >> linux-2.6.36-i686: WARNINGS
> >> linux-2.6.37-i686: WARNINGS
> >> linux-2.6.38.2-i686: WARNINGS
> >> linux-2.6.39.1-i686: WARNINGS
> >> linux-3.0-i686: WARNINGS
> >> linux-3.1-i686: WARNINGS
> >> linux-3.2.1-i686: WARNINGS
> >> linux-3.3-i686: WARNINGS
> >> linux-3.4-i686: WARNINGS
> >> linux-3.5-i686: WARNINGS
> >> linux-3.6-i686: WARNINGS
> >> linux-3.7-i686: WARNINGS
> >> linux-3.8-rc1-i686: WARNINGS
> >> linux-2.6.31.12-x86_64: WARNINGS
> >> linux-2.6.32.6-x86_64: WARNINGS
> >> linux-2.6.33-x86_64: WARNINGS
> >> linux-2.6.34-x86_64: WARNINGS
> >> linux-2.6.35.3-x86_64: WARNINGS
> >> linux-2.6.36-x86_64: WARNINGS
> >> linux-2.6.37-x86_64: WARNINGS
> >> linux-2.6.38.2-x86_64: WARNINGS
> >> linux-2.6.39.1-x86_64: WARNINGS
> >> linux-3.0-x86_64: WARNINGS
> >> linux-3.1-x86_64: WARNINGS
> >> linux-3.2.1-x86_64: WARNINGS
> >> linux-3.3-x86_64: WARNINGS
> >> linux-3.4-x86_64: WARNINGS
> >> linux-3.5-x86_64: WARNINGS
> >> linux-3.6-x86_64: WARNINGS
> >> linux-3.7-x86_64: WARNINGS
> >> linux-3.8-rc1-x86_64: WARNINGS
> >> apps: WARNINGS
> >> spec-git: OK
> >> sparse: ERRORS
> >>
> >> Detailed results are available here:
> >>
> >> http://www.xs4all.nl/~hverkuil/logs/Monday.log
> > 
> > There were a lot of new 'redefined' warnings that I have fixed.
> > 
> > In addition, it turned out that any driver using vb2 wasn't compiled for
> > kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's fixed
> > as well, so drivers like em28xx and vivi will now compile on those older
> > kernels. This also was the reason I never saw that the usb_translate_error
> > function needed to be added to compat.h: it's used in em28xx but that driver
> > was never compiled on kernels without usb_translate_error.
> > 
> > Hopefully everything works now.
> > 
> > Regards,
> > 
> > Hans
> 
> Hi Hans,
> on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
> just a few remaining warnings.
> 
> In particular, there are several new warnings related to DMA_SHARED_BUFFER:
> 
> WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
> WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
> WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
> WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
> WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!

Gianluca,

Can you patch media_build with the patch below and try again? If it doesn't
work, then replace '#ifdef CONFIG_DMA_SHARED_BUFFER' by '#if 0' in the patch
below and try that instead.

Let me know what works.

Thanks,

Hans


diff --git a/backports/backports.txt b/backports/backports.txt
index f2d08b9..73ecbf6 100644
--- a/backports/backports.txt
+++ b/backports/backports.txt
@@ -26,6 +26,7 @@ add pr_fmt.patch
 
 [3.1.255]
 add v3.1_no_export_h.patch
+add v3.1_no_dma_buf_h.patch
 add v3.1_no_pm_qos.patch
 
 [3.0.255]
diff --git a/backports/v3.1_no_dma_buf_h.patch 
b/backports/v3.1_no_dma_buf_h.patch
new file mode 100644
index 000..5a7a7fb
--- /dev/null
+++ b/backports/v3.1_no_dma_buf_h.patch
@@ -0,0 +1,116 @@
+diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
+index bd2e52c..eb48f38 100644
+--- a/include/linux/dma-buf.h
 b/include/linux/dma-buf.h
+@@ -156,6 +156,7 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)
+   get_file(dmabuf->file);
+ }
+ 
++#ifdef CONFIG_DMA_SHARED_BUFFER
+ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
+   struct device *dev);
+ void dma_buf_detach(struct dma_buf *dmabuf,
+@@ -183,5 +184,103 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct 
*,
+unsigned long);
+ void *dma_buf_vmap(struct dma_buf *);
+ void dma_buf_vunmap(struct dma_buf *, void *vaddr);
++#else
++
++static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf 
*dmabuf,
++   

Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Sylwester,

On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > If we need a workaround, I'd rather pass the device name in addition
> > to the I2C adapter number and address, instead of embedding the
> > workaround in this new API.
>  
>  ...or we can change the I2C subdevice name format. The actual need to
>  do
>  
>   snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
>    asdl->dev->driver->name,
>    i2c_adapter_id(client->adapter), client->addr);
>  
>  in soc-camera now to exactly match the subdevice name, as created by
>  v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
>  if the latter changes at some point? Or what if one driver wishes to
>  create several subdevices for one I2C device?
> >>> 
> >>> The common clock framework uses %d-%04x, maybe we could use that as well
> >>> for clock names ?
> >> 
> >> And preserve the subdevice names? Then matching would be more difficult
> >> and less precise. Or change subdevice names too? I think, we can do the
> >> latter, since anyway at any time only one driver can be attached to an
> >> I2C device.
> > 
> > That's right. Where else is the subdev name used ?
> 
> Subdev names are exposed to user space by the media controller API.
> So they are really part of an ABI, aren't they ?

They're used to construct the name exposed to userspace, but the media 
controller core could probably handle that internally by concatenating the 
driver name and the subdev name.

> Also having I2C bus number or I2C slave address as part of the subdev
> name makes it more difficult to write portable applications. Hence
> in sensor drivers I used to overwrite subdev name to remove I2C bus
> and slave address, as the format used v4l2_i2c_subdev_init() seemed
> highly unsuitable..

This clearly shows that we need to discuss the matter and agree on a common 
mode of operation.

Aren't applications that use the subdev name directly inherently non-portable 
anyway ? If you want your application to support different boards/sensors/SoCs 
you should discover the pipeline and find the sensor by iterating over 
entities, instead of using the sensor entity name.

-- 
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 RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service

2013-01-08 Thread Mauro Carvalho Chehab
Em Mon,  7 Jan 2013 22:25:47 -0200
Mauro Carvalho Chehab  escreveu:

> The DVBv3 quality parameters are limited on several ways:
> 
> - Doesn't provide any way to indicate the used measure,
> so userspace need to guess how to calculate the measure;
> 
> - Only a limited set of stats are supported;
> 
> - Can't be called in a way to require them to be filled
>   all at once (atomic reads from the hardware), with may
>   cause troubles on interpreting them on userspace;
> 
> - On some OFDM delivery systems, the carriers can be
>   independently modulated, having different properties.
>   Currently, there's no way to report per-layer stats.
> 
> To address the above issues, adding a new DVBv5-based stats
> API.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 

...

> +struct dtv_stats {
> + __u8 scale; /* enum fecap_scale_params type */
> + union {
> + __u32 uvalue;   /* for counters and relative scales */
> + __s32 svalue;   /* for 0.1 dB measures */

32 bits for total bit count is not enough, as it can be truncated too
early (~1 seg on ISDB-T, ~0.5 seg on DVB-C). I think we need to use
64 bits here, and put at the API that the drivers should monotonically
increment.

As struct buffer inside struct dtv_property has 48 bytes, we can do such
change here without breaking userspace, as struct dtv_stats will have
37 bytes.

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: add parameters to V4L controls

2013-01-08 Thread Andrzej Hajda

On 07.01.2013 21:02, Laurent Pinchart wrote:

Hi Hans,

On Monday 07 January 2013 13:10:54 Hans Verkuil wrote:

On Mon January 7 2013 11:46:38 Andrzej Hajda wrote:

Hi,

I have included this proposition already in the post "[PATCH RFC 0/2]
V4L: Add auto focus area control and selection" but it left unanswered.
I repost it again in a separate e-mail, I hope this way it will be
easier to attract attention.

Problem description

Currently V4L2 controls can have only single value (of type int, int64,
string). Some hardware controls require more than single int parameter,
for example to set auto-focus (AF) rectangle four coordinates should be
passed, to set auto-focus spot two coordinates should be passed.

Current solution

In case of AF rectangle we can reuse selection API as in "[PATCH RFC
0/2] V4L: Add auto focus area control and selection" post.
Pros:
- reuse existing API,
Cons:
- two IOCTL's to perform one action,
- non-atomic operation,

Is it really an issue in your use cases, or is this only a theoretical problem
?
Currently it is not a blocker, but it shows additional issues which we 
should deal with, for example:

- which ioctls should trigger the AF action,
- default values for AF selection rectangle/spot, which should be 
(probably) reset after each change of format on device/pad.
Documenting it will make the situation clear for app/driver developers, 
but with atomic ioctl we could just forget about it.
Btw. there is already discussion about it in '[PATCH RFC 2/2] V4L: Add 
V4L2_CID_AUTO_FOCUS_AREA control'.



- fits well only for rectangles and spots (but with unused fields width,
height), in case of other parameters we should find a different way.

Proposed solution

The solution takes an advantage of the fact VIDIOC_(G/S/TRY)_EXT_CTRLS
ioctls can be called with multiple controls per call.

I will present it using AF area control example.

There could be added four pseudo-controls, lets call them for short:
LEFT, TOP, WIDTH, HEIGHT.
Those controls could be passed together with
V4L2_AUTO_FOCUS_AREA_RECTANGLE
control in one ioctl as a kind of parameters.

For example setting auto-focus spot would require calling
VIDIOC_S_EXT_CTRLS with the following controls:
- V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE
- LEFT = ...
- RIGHT = ...

Setting AF rectangle:
- V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE
- LEFT = ...
- TOP = ...
- WIDTH = ...
- HEIGHT = ...

Setting  AF object detection (no parameters required):
- V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION

If you want to do this, then you have to make LEFT/TOP/WIDTH/HEIGHT real
controls. There is no such thing as a pseudo control. So you need five
new controls in total:

V4L2_CID_AUTO_FOCUS_AREA
V4L2_CID_AUTO_FOCUS_LEFT
V4L2_CID_AUTO_FOCUS_RIGHT
V4L2_CID_AUTO_FOCUS_WIDTH
V4L2_CID_AUTO_FOCUS_HEIGHT

I have no problem with this from the point of view of the control API, but
whether this is the best solution for implementing auto-focus is a different
issue and input from sensor specialists is needed as well (added Laurent
and Sakari to the CC list).

The primary concern I have is that this does not scale to multiple focus
rectangles. This might not be relevant to auto focus, though.

Time to implement a rectangle control type ? Or to make it possible to issue
atomic transactions across ioctls ? We've been postponing this discussion for
ages.


I have presented all three cases to show the advantages of this solution:
- atomicity - control and its parameters are passed in one call,
- flexibility - we are not limited by a fixed number of parameters,
- no-redundancy - we can pass only required parameters

(no need to pass null width and height in case of spot selection),

- extensibility - it is possible to extend parameters in the future,
for example add parameters to V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION,
without breaking API,
- backward compatibility,
- re-usability - this schema could be used in other controls,

pseudo-controls could be re-used in other controls as well.

- API backward compatibility.

Regards,
Andrzej

--
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 RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service

2013-01-08 Thread Simon Farnsworth
On Monday 7 January 2013 22:25:47 Mauro Carvalho Chehab wrote:

> + 
> + 
> FE_SCALE_NOT_AVAILABLE - If it is not 
> possible to collect a given parameter (could be a transitory or permanent 
> condition)
> + 
> FE_SCALE_DECIBEL - parameter is a signed 
> value, measured in 0.1 dB
> + 
> FE_SCALE_RELATIVE - parameter is a 
> unsigned value, where 0 means 0% and 65535 means 100%.
> + 
> FE_SCALE_COUNTER - parameter is a 
> unsigned value that counts the occurrence of an event, like bit error, block 
> error, or lapsed time.
> + 

> + 
> + DTV_QOS_SIGNAL_STRENGTH
> + Indicates the signal strength level at the analog part of 
> the tuner.
> + 

Signal strength is traditionally an absolute field strength; there's no way in
this API for me to provide my reference point, so two different front ends
could represent the same signal strength as "0 dB" (where the reference point
is one microwatt), "-30 dB" (where the reference point is one milliwatt), or
"17 dB" (using a reference point of 1 millivolt on a 50 ohm impedance).

Could you choose a reference point for signal strength, and specify that if
you're using FE_SCALE_DECIBEL, you're referenced against that point?

My preference would be to reference against 1 microwatt, as (on the DVB-T and
ATSC cards I use) that leads to the signal measure being 0 dBµW if you've got
perfect signal, negative number if your signal is weak, and positive numbers
if your signal is strong. However, referenced against 1 milliwatt also works
well for me, as the conversion is trivial.
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Sylwester Nawrocki

Hi,

On 01/08/2013 11:35 AM, Laurent Pinchart wrote:

If we need a workaround, I'd rather pass the device name in addition
to the I2C adapter number and address, instead of embedding the
workaround in this new API.


...or we can change the I2C subdevice name format. The actual need to do

snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",

 asdl->dev->driver->name,
 i2c_adapter_id(client->adapter), client->addr);

in soc-camera now to exactly match the subdevice name, as created by
v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
if the latter changes at some point? Or what if one driver wishes to
create several subdevices for one I2C device?


The common clock framework uses %d-%04x, maybe we could use that as well
for clock names ?


And preserve the subdevice names? Then matching would be more difficult
and less precise. Or change subdevice names too? I think, we can do the
latter, since anyway at any time only one driver can be attached to an I2C
device.


That's right. Where else is the subdev name used ?


Subdev names are exposed to user space by the media controller API.
So they are really part of an ABI, aren't they ?

Also having I2C bus number or I2C slave address as part of the subdev
name makes it more difficult to write portable applications. Hence
in sensor drivers I used to overwrite subdev name to remove I2C bus
and slave address, as the format used v4l2_i2c_subdev_init() seemed
highly unsuitable..

--

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 1/3] [media] winbond-cir: only enable higher sample resolution if needed

2013-01-08 Thread David Härdeman
On Sun, Jan 06, 2013 at 05:19:43PM +, Sean Young wrote:
>A sample resolution of 2us generates more than 300 interrupts per key
>and this resolution is not needed unless carrier reports are enabled.
>
>Revert to a resolution of 10us unless carrier reports are needed. This
>generates up to a fifth of the interrupts.
>
>Signed-off-by: Sean Young 

Thanks Sean!

Acked-by: David Härdeman 

>---
> drivers/media/rc/winbond-cir.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
>index 553d1cd..66f543c 100644
>--- a/drivers/media/rc/winbond-cir.c
>+++ b/drivers/media/rc/winbond-cir.c
>@@ -154,6 +154,8 @@
> #define WBCIR_CNTR_R  0x02
> /* Invert TX */
> #define WBCIR_IRTX_INV0x04
>+/* Receiver oversampling */
>+#define WBCIR_RX_T_OV 0x40
> 
> /* Valid banks for the SP3 UART */
> enum wbcir_bank {
>@@ -394,7 +396,8 @@ wbcir_irq_rx(struct wbcir_data *data, struct pnp_dev 
>*device)
>   if (data->rxstate == WBCIR_RXSTATE_ERROR)
>   continue;
> 
>-  duration = ((irdata & 0x7F) + 1) * 2;
>+  duration = ((irdata & 0x7F) + 1) *
>+  (data->carrier_report_enabled ? 2 : 10);
>   rawir.pulse = irdata & 0x80 ? false : true;
>   rawir.duration = US_TO_NS(duration);
> 
>@@ -550,6 +553,17 @@ wbcir_set_carrier_report(struct rc_dev *dev, int enable)
>   wbcir_set_bits(data->ebase + WBCIR_REG_ECEIR_CCTL,
>   WBCIR_CNTR_EN, WBCIR_CNTR_EN | WBCIR_CNTR_R);
> 
>+  /* Set a higher sampling resolution if carrier reports are enabled */
>+  wbcir_select_bank(data, WBCIR_BANK_2);
>+  data->dev->rx_resolution = US_TO_NS(enable ? 2 : 10);
>+  outb(enable ? 0x03 : 0x0f, data->sbase + WBCIR_REG_SP3_BGDL);
>+  outb(0x00, data->sbase + WBCIR_REG_SP3_BGDH);
>+
>+  /* Enable oversampling if carrier reports are enabled */
>+  wbcir_select_bank(data, WBCIR_BANK_7);
>+  wbcir_set_bits(data->sbase + WBCIR_REG_SP3_RCCFG,
>+  enable ? WBCIR_RX_T_OV : 0, WBCIR_RX_T_OV);
>+
>   data->carrier_report_enabled = enable;
>   spin_unlock_irqrestore(&data->spinlock, flags);
> 
>@@ -931,8 +945,8 @@ wbcir_init_hw(struct wbcir_data *data)
>   /* prescaler 1.0, tx/rx fifo lvl 16 */
>   outb(0x30, data->sbase + WBCIR_REG_SP3_EXCR2);
> 
>-  /* Set baud divisor to sample every 2 ns */
>-  outb(0x03, data->sbase + WBCIR_REG_SP3_BGDL);
>+  /* Set baud divisor to sample every 10 us */
>+  outb(0x0f, data->sbase + WBCIR_REG_SP3_BGDL);
>   outb(0x00, data->sbase + WBCIR_REG_SP3_BGDH);
> 
>   /* Set CEIR mode */
>@@ -941,12 +955,9 @@ wbcir_init_hw(struct wbcir_data *data)
>   inb(data->sbase + WBCIR_REG_SP3_LSR); /* Clear LSR */
>   inb(data->sbase + WBCIR_REG_SP3_MSR); /* Clear MSR */
> 
>-  /*
>-   * Disable RX demod, enable run-length enc/dec, set freq span and
>-   * enable over-sampling
>-   */
>+  /* Disable RX demod, enable run-length enc/dec, set freq span */
>   wbcir_select_bank(data, WBCIR_BANK_7);
>-  outb(0xd0, data->sbase + WBCIR_REG_SP3_RCCFG);
>+  outb(0x90, data->sbase + WBCIR_REG_SP3_RCCFG);
> 
>   /* Disable timer */
>   wbcir_select_bank(data, WBCIR_BANK_4);
>-- 
>1.7.11.7
>

-- 
David Härdeman
--
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: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Gianluca Gennari
Il 08/01/2013 10:58, Hans Verkuil ha scritto:
> On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
>> 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:Mon Jan  7 19:00:18 CET 2013
>> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
>> gcc version:  i686-linux-gcc (GCC) 4.7.1
>> host hardware:x86_64
>> host os:  3.4.07-marune
>>
>> linux-git-arm-eabi-davinci: WARNINGS
>> linux-git-arm-eabi-exynos: WARNINGS
>> linux-git-arm-eabi-omap: ERRORS
>> linux-git-i686: OK
>> linux-git-m32r: OK
>> linux-git-mips: WARNINGS
>> linux-git-powerpc64: OK
>> linux-git-sh: OK
>> linux-git-x86_64: OK
>> linux-2.6.31.12-i686: WARNINGS
>> linux-2.6.32.6-i686: WARNINGS
>> linux-2.6.33-i686: WARNINGS
>> linux-2.6.34-i686: WARNINGS
>> linux-2.6.35.3-i686: WARNINGS
>> linux-2.6.36-i686: WARNINGS
>> linux-2.6.37-i686: WARNINGS
>> linux-2.6.38.2-i686: WARNINGS
>> linux-2.6.39.1-i686: WARNINGS
>> linux-3.0-i686: WARNINGS
>> linux-3.1-i686: WARNINGS
>> linux-3.2.1-i686: WARNINGS
>> linux-3.3-i686: WARNINGS
>> linux-3.4-i686: WARNINGS
>> linux-3.5-i686: WARNINGS
>> linux-3.6-i686: WARNINGS
>> linux-3.7-i686: WARNINGS
>> linux-3.8-rc1-i686: WARNINGS
>> linux-2.6.31.12-x86_64: WARNINGS
>> linux-2.6.32.6-x86_64: WARNINGS
>> linux-2.6.33-x86_64: WARNINGS
>> linux-2.6.34-x86_64: WARNINGS
>> linux-2.6.35.3-x86_64: WARNINGS
>> linux-2.6.36-x86_64: WARNINGS
>> linux-2.6.37-x86_64: WARNINGS
>> linux-2.6.38.2-x86_64: WARNINGS
>> linux-2.6.39.1-x86_64: WARNINGS
>> linux-3.0-x86_64: WARNINGS
>> linux-3.1-x86_64: WARNINGS
>> linux-3.2.1-x86_64: WARNINGS
>> linux-3.3-x86_64: WARNINGS
>> linux-3.4-x86_64: WARNINGS
>> linux-3.5-x86_64: WARNINGS
>> linux-3.6-x86_64: WARNINGS
>> linux-3.7-x86_64: WARNINGS
>> linux-3.8-rc1-x86_64: WARNINGS
>> apps: WARNINGS
>> spec-git: OK
>> sparse: ERRORS
>>
>> Detailed results are available here:
>>
>> http://www.xs4all.nl/~hverkuil/logs/Monday.log
> 
> There were a lot of new 'redefined' warnings that I have fixed.
> 
> In addition, it turned out that any driver using vb2 wasn't compiled for
> kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's fixed
> as well, so drivers like em28xx and vivi will now compile on those older
> kernels. This also was the reason I never saw that the usb_translate_error
> function needed to be added to compat.h: it's used in em28xx but that driver
> was never compiled on kernels without usb_translate_error.
> 
> Hopefully everything works now.
> 
> Regards,
> 
>   Hans

Hi Hans,
on kernel 2.6.32 (Ubuntu 10.04) the media_build tree compiles fine, with
just a few remaining warnings.

In particular, there are several new warnings related to DMA_SHARED_BUFFER:

WARNING: "dma_buf_vunmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
WARNING: "dma_buf_vmap" [media_build/v4l/videobuf2-vmalloc.ko] undefined!
WARNING: "dma_buf_fd" [media_build/v4l/videobuf2-core.ko] undefined!
WARNING: "dma_buf_put" [media_build/v4l/videobuf2-core.ko] undefined!
WARNING: "dma_buf_get" [media_build/v4l/videobuf2-core.ko] undefined!

Regards,
Gianluca
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

2013-01-08 Thread Marek Szyprowski

Hello,

On 1/8/2013 11:15 AM, Federico Vaga wrote:

> > @@ -165,7 +161,8 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> > long size)>
> >   /* align image size to PAGE_SIZE */
> >   size = PAGE_ALIGN(size);
> >
> > - buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
GFP_KERNEL);
> > + buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
> > + 
GFP_KERNEL | conf->mem_flags);
>
> I think we can add GFP_DMA flag unconditionally to the vb2_dc_contig
> allocator.
> It won't hurt existing clients as most of nowadays platforms doesn't
> have DMA
> zone (GFP_DMA is ignored in such case), but it should fix the issues
> with some
> older and non-standard systems.

I did not set GFP_DMA fixed in the allocator because I do not want to brake
something in the future. On x86 platform GFP_DMA allocates under 16MB and this
limit can be too strict. When many other drivers use GFP_DMA we can saturate
this tiny zone.
As you said, this fix the issue with _older_ and _non-standard_ (like sta2x11)
systems. But this fix has effect on every other standard and new systems.
That's why I preferred to set the flag optionally.


Ok, then I would simply pass the flags from the driver without any 
alternation

in the allocator itself, so drivers can pass 'GFP_KERNEL' or
'GFP_KERNEL | GFP_DMA' depending on their preference. Please also update 
all

the existing clients of vb2_dma_dc allocator.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
On Tuesday 08 January 2013 11:26:57 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:

[snip]

> > > > > > Could you please explain why you need both a bind notifier and a
> > > > > > bound notifier ? I was expecting a single
> > > > > > v4l2_async_subdev_register() call in subdev drivers (and, thinking
> > > > > > about it, I would probably name it v4l2_subdev_register()).
> > > > > 
> > > > > I think I can, yes. Because between .bind() and .bound() the
> > > > > subdevice driver does the actual hardware probing. So, .bind() is
> > > > > used to make sure the hardware can be accessed, most importantly to
> > > > > provide a clock to the subdevice. You can look at
> > > > > soc_camera_async_bind(). There I'm registering the clock for the
> > > > > subdevice, about to bind. Why I cannot do it before, is because I
> > > > > need subdevice name for clock matching. With I2C subdevices the
> > > > > subdevice name contains the name of the driver, adapter number and
> > > > > i2c address. The latter 2 I've got from host subdevice list. But not
> > > > > the driver name. I thought about also passing the driver name there,
> > > > > but that seemed too limiting to me. I also request regulators there,
> > > > > because before ->bound() the sensor driver, but that could be done
> > > > > on the first call to soc_camera_power_on(), although doing this
> > > > > "first call" thingie is kind of hackish too. I could add one more
> > > > > soc-camera-power helper like soc_camera_prepare() or similar too.
> > > > 
> > > > I think a soc_camera_power_init() function (or similar) would be a
> > > > good idea, yes.
> > > > 
> > > > > So, the main problem is the clock
> > > > > 
> > > > > subdevice name. Also see the comment in soc_camera.c:
> > > > >   /*
> > > > >* It is ok to keep the clock for the whole soc_camera_device
> > > > >* life-time, in principle it would be more logical to register
> > > > >* the clock on icd creation, the only problem is, that at that
> > > > >* time we don't know the driver name yet.
> > > > >*/
> > > > 
> > > > I think we should fix that problem instead of shaping the async API
> > > > around a workaround :-)
> > > > 
> > > > From the subdevice point of view, the probe function should request
> > > > resources, perform whatever initialization is needed (including
> > > > verifying that the hardware is functional when possible), and the
> > > > register the subdev with the code if everything succeeded. Splitting
> > > > registration into bind() and bound() appears a bit as a workaround to
> > > > me.
> > > > 
> > > > If we need a workaround, I'd rather pass the device name in addition
> > > > to the I2C adapter number and address, instead of embedding the
> > > > workaround in this new API.
> > > 
> > > ...or we can change the I2C subdevice name format. The actual need to do
> > > 
> > >   snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > >   
> > >asdl->dev->driver->name,
> > >i2c_adapter_id(client->adapter), client->addr);
> > > 
> > > in soc-camera now to exactly match the subdevice name, as created by
> > > v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
> > > if the latter changes at some point? Or what if one driver wishes to
> > > create several subdevices for one I2C device?
> > 
> > The common clock framework uses %d-%04x, maybe we could use that as well
> > for clock names ?
> 
> And preserve the subdevice names? Then matching would be more difficult
> and less precise. Or change subdevice names too? I think, we can do the
> latter, since anyway at any time only one driver can be attached to an I2C
> device.

That's right. Where else is the subdev name used ?

> > > > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list
> > > > > > > *asdl);
> > > > > > > +#endif

-- 
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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Guennadi Liakhovetski
On Tue, 8 Jan 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
> > > > > > >2001
> > 
> > [snip]
> > 
> > > > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > > > +struct v4l2_async_notifier *notifier);
> > > > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > > > *notifier);
> > > > > > +/*
> > > > > > + * If subdevice probing fails any time after
> > > > > > v4l2_async_subdev_bind(),
> > > > > > no
> > > > > > + * clean up must be called. This function is only a message of
> > > > > > intention.
> > > > > > + */
> > > > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > > > > 
> > > > > Could you please explain why you need both a bind notifier and a bound
> > > > > notifier ? I was expecting a single v4l2_async_subdev_register() call
> > > > > in subdev drivers (and, thinking about it, I would probably name it
> > > > > v4l2_subdev_register()).
> > > > 
> > > > I think I can, yes. Because between .bind() and .bound() the subdevice
> > > > driver does the actual hardware probing. So, .bind() is used to make
> > > > sure the hardware can be accessed, most importantly to provide a clock
> > > > to the subdevice. You can look at soc_camera_async_bind(). There I'm
> > > > registering the clock for the subdevice, about to bind. Why I cannot do
> > > > it before, is because I need subdevice name for clock matching. With I2C
> > > > subdevices the subdevice name contains the name of the driver, adapter
> > > > number and i2c address. The latter 2 I've got from host subdevice list.
> > > > But not the driver name. I thought about also passing the driver name
> > > > there, but that seemed too limiting to me. I also request regulators
> > > > there, because before ->bound() the sensor driver, but that could be
> > > > done on the first call to soc_camera_power_on(), although doing this
> > > > "first call" thingie is kind of hackish too. I could add one more soc-
> > > > camera-power helper like soc_camera_prepare() or similar too.
> > > 
> > > I think a soc_camera_power_init() function (or similar) would be a good
> > > idea, yes.
> > > 
> > > > So, the main problem is the clock
> > > > 
> > > > subdevice name. Also see the comment in soc_camera.c:
> > > > /*
> > > >  * It is ok to keep the clock for the whole soc_camera_device
> > > >  life-time,
> > > >  * in principle it would be more logical to register the clock 
> > > > on icd
> > > >  * creation, the only problem is, that at that time we don't 
> > > > know the
> > > >  * driver name yet.
> > > >  */
> > > 
> > > I think we should fix that problem instead of shaping the async API around
> > > a workaround :-)
> > > 
> > > From the subdevice point of view, the probe function should request
> > > resources, perform whatever initialization is needed (including verifying
> > > that the hardware is functional when possible), and the register the
> > > subdev with the code if everything succeeded. Splitting registration into
> > > bind() and bound() appears a bit as a workaround to me.
> > > 
> > > If we need a workaround, I'd rather pass the device name in addition to
> > > the I2C adapter number and address, instead of embedding the workaround in
> > > this new API.
> > 
> > ...or we can change the I2C subdevice name format. The actual need to do
> > 
> > snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> >  asdl->dev->driver->name,
> >  i2c_adapter_id(client->adapter), client->addr);
> > 
> > in soc-camera now to exactly match the subdevice name, as created by
> > v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
> > the latter changes at some point? Or what if one driver wishes to create
> > several subdevices for one I2C device?
> 
> The common clock framework uses %d-%04x, maybe we could use that as well for 
> clock names ?

And preserve the subdevice names? Then matching would be more difficult 
and less precise. Or change subdevice names too? I think, we can do the 
latter, since anyway at any time only one driver can be attached to an I2C 
device.

> > > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > > > +#endif

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...@vg

Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Guennadi,

On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
> > > > > >2001
> 
> [snip]
> 
> > > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > > +  struct v4l2_async_notifier *notifier);
> > > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > > *notifier);
> > > > > +/*
> > > > > + * If subdevice probing fails any time after
> > > > > v4l2_async_subdev_bind(),
> > > > > no
> > > > > + * clean up must be called. This function is only a message of
> > > > > intention.
> > > > > + */
> > > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > > > 
> > > > Could you please explain why you need both a bind notifier and a bound
> > > > notifier ? I was expecting a single v4l2_async_subdev_register() call
> > > > in subdev drivers (and, thinking about it, I would probably name it
> > > > v4l2_subdev_register()).
> > > 
> > > I think I can, yes. Because between .bind() and .bound() the subdevice
> > > driver does the actual hardware probing. So, .bind() is used to make
> > > sure the hardware can be accessed, most importantly to provide a clock
> > > to the subdevice. You can look at soc_camera_async_bind(). There I'm
> > > registering the clock for the subdevice, about to bind. Why I cannot do
> > > it before, is because I need subdevice name for clock matching. With I2C
> > > subdevices the subdevice name contains the name of the driver, adapter
> > > number and i2c address. The latter 2 I've got from host subdevice list.
> > > But not the driver name. I thought about also passing the driver name
> > > there, but that seemed too limiting to me. I also request regulators
> > > there, because before ->bound() the sensor driver, but that could be
> > > done on the first call to soc_camera_power_on(), although doing this
> > > "first call" thingie is kind of hackish too. I could add one more soc-
> > > camera-power helper like soc_camera_prepare() or similar too.
> > 
> > I think a soc_camera_power_init() function (or similar) would be a good
> > idea, yes.
> > 
> > > So, the main problem is the clock
> > > 
> > > subdevice name. Also see the comment in soc_camera.c:
> > >   /*
> > >* It is ok to keep the clock for the whole soc_camera_device
> > >life-time,
> > >* in principle it would be more logical to register the clock on icd
> > >* creation, the only problem is, that at that time we don't know the
> > >* driver name yet.
> > >*/
> > 
> > I think we should fix that problem instead of shaping the async API around
> > a workaround :-)
> > 
> > From the subdevice point of view, the probe function should request
> > resources, perform whatever initialization is needed (including verifying
> > that the hardware is functional when possible), and the register the
> > subdev with the code if everything succeeded. Splitting registration into
> > bind() and bound() appears a bit as a workaround to me.
> > 
> > If we need a workaround, I'd rather pass the device name in addition to
> > the I2C adapter number and address, instead of embedding the workaround in
> > this new API.
> 
> ...or we can change the I2C subdevice name format. The actual need to do
> 
>   snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
>asdl->dev->driver->name,
>i2c_adapter_id(client->adapter), client->addr);
> 
> in soc-camera now to exactly match the subdevice name, as created by
> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
> the latter changes at some point? Or what if one driver wishes to create
> several subdevices for one I2C device?

The common clock framework uses %d-%04x, maybe we could use that as well for 
clock names ?

> > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > > +#endif

-- 
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 1/5] capemgr: Beaglebone DT overlay based cape manager

2013-01-08 Thread Pantelis Antoniou
Hi Guennadi,

On Jan 8, 2013, at 11:51 AM, Guennadi Liakhovetski wrote:

> (adding linux-media ML to cc)
> 
> Hi Pantelis
> 
> On Tue, 8 Jan 2013, Pantelis Antoniou wrote:
> 
>> Hi Arnd,
>> 
>> On Jan 7, 2013, at 11:35 PM, Arnd Bergmann wrote:
>> 
>>> (Adding Sascha Hauer, Linus Walleij, Lee Jones to Cc)
>>> 
>>> On Monday 07 January 2013, Tony Lindgren wrote:
> 
> At the end of the line, some kind of hardware glue is going to be needed.
> 
> I just feel that drawing from a sample size of 1 (maybe 2 if I get to 
> throw
> in the beagleboard), it is a bit premature to think about making it overly
> general, besides the part that are obviously part of the infrastructure 
> (like the DT overlay stuff).
> 
> What I'm getting at, is that we need some user experience about this, 
> before
> going away and creating structure out of possible misconception about the 
> uses. 
 
 IMHO stuff like this will be needed by many SoCs. Some examples of similar
 things for omaps that have eventually become generic frameworks have been
 the clock framework, USB OTG support, runtime PM, pinmux framework and
 so on.
 
 So I suggest a minimal generic API from the start as that will make things
 a lot easier in the long run.
>>> 
>>> I agree. The ux500 platform already has the concept of "user interface 
>>> boards",
>>> which currently is not well integrated into devicetree. I believe Sascha
>>> mentioned that Pengutronix had been shipping some other systems with add-on
>>> boards and generating device tree binaries from source for each combination.
>>> 
>>> Ideally, both of the above should be able to use the same DT overlay logic
>>> as BeagleBone, and I'm sure there are more of those.
>>> 
>>> Arnd
>> 
>> Hmm, I see. 
>> 
>> I will need some more information about the interface of the 'user interface 
>> boards'.
>> I.e. how is the board identified, what is typically present on those boards, 
>> etc.
>> 
>> Can we get some input by the owner of other similar hardware? I know the FPGA
>> people have similar requirements for example. There are other people that 
>> are hitting
>> problems getting DT to work with their systems, like the V4L people with the 
>> order
>> of initialization; see http://lwn.net/Articles/531068/. I think the V4L 
>> problem is
>> cleanly solved by the overlay being contained in the V4L device node and 
>> applied just before
>> the device is probed.
> 
> You probably mean these related V4L patches: 
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/58646 
> that base upon of asynchronous V4L2 subdevice probing, referenced above. 
> Yes, V4L DT nodes, as documented in 
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/58646/focus=58648
>  
> contain "port" and "endpoint" nodes, that describe the configuration of 
> the hardware port and link to devices, connected to it. Not sure how well 
> this would work with DT overlays, because endpoint nodes on both sides of 
> the video data bus contain references to the other side and I don't know 
> whether and how these can be created and / or updated at run-time. 
> Otherwise, yes, the approach that we're currently developing on V4L allows 
> us to build video data pipelines independent of (sub)device driver probing 
> order.
> 

I'm not very well versed at the V4L intricacies, and correct me if I got it 
wrong,
but it seems like you have the  problem on needing to probe a bunch of devices 
only after 
the parent device has been probed successfully. 
Your async subdevice probing method seems to be doing this.

It might be simpler to do something like this:

v4ldevice {
compatible = "foo,v4ldev";
...
overlay {
fragment@0 {
target = <&i2c0>;
__overlay__ {
...
/* i2c devices */
};
};
fragment@0 {
target = <&ocp>;
__overlay__ {
..
/* platform devices */
};
};
...
}:
};

And in the probe of the v4ldev apply the overlay. The i2c devices will end up 
in the
i2c node, the platform devices to the ocp node, etc, and will be registered.

There is nothing preventing the overlay being in a part of the board dts file. 
You will need to do a resolve pass for the phandles, but it's not 
insurmountable IMO.

Regards

-- Pantelis

> Thanks
> Guennadi
> 
>> In the meantime it would be better to wait until we have some ack from the 
>> maintainers
>> of the core subsystems about what they think.
>> 
>> Regards
>> 
>> -- Pantelis
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

--
To unsubscribe from thi

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Marcus Lorentzon

On 01/08/2013 09:18 AM, Laurent Pinchart wrote:

On Thursday 27 December 2012 15:43:34 Tomasz Figa wrote:

>  On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:

>  >  On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:

>  >  >  On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan wrote:

>  >  >  >  On 17 December 2012 20:55, Laurent Pinchart wrote:

>  >  >  >  >  Hi Vikas,
>  >  >  >  >  
>  >  >  >  >  Sorry for the late reply. I now have more time to work on CDF, so

>  >  >  >  >  delays should be much shorter.
>  >  >  >  >  
>  >  >  >  >  On Thursday 06 December 2012 10:51:15 Vikas Sajjan wrote:

>  >  >  >  >  >  Hi Laurent,
>  >  >  >  >  >  
>  >  >  >  >  >  I was thinking of porting CDF to samsung EXYNOS 5250 platform,

>  >  >  >  >  >  what I found is that, the exynos display controller is MIPI 
DSI
>  >  >  >  >  >  based controller.
>  >  >  >  >  >  
>  >  >  >  >  >  But if I look at CDF patches, it has only support for MIPI DBI

>  >  >  >  >  >  based Display controller.
>  >  >  >  >  >  
>  >  >  >  >  >  So my question is, do we have any generic framework for MIPI DSI

>  >  >  >  >  >  based display controller? basically I wanted to know, how to 
go
>  >  >  >  >  >  about porting CDF for such kind of display controller.
>  >  >  >  >  
>  >  >  >  >  MIPI DSI support is not available yet. The only reason for that is

>  >  >  >  >  that I don't have any MIPI DSI hardware to write and test the 
code
>  >  >  >  >  with:-)
>  >  >  >  >  
>  >  >  >  >  The common display framework should definitely support MIPI DSI. I

>  >  >  >  >  think the existing MIPI DBI code could be used as a base, so the
>  >  >  >  >  implementation shouldn't be too high.
>  >  >  >  >  
>  >  >  >  >  Yeah, i was also thinking in similar lines, below is my though for

>  >  >  >  >  MIPI DSI support in CDF.
>  >  >  >  
>  >  >  >  o   MIPI DSI support as part of CDF framework will expose

>  >  >  >  §  mipi_dsi_register_device(mpi_device) (will be called 
mach-xxx-dt.c
>  >  >  >  file )
>  >  >  >  §  mipi_dsi_register_driver(mipi_driver, bus ops) (will be called
>  >  >  >  from platform specific init driver call )
>  >  >  >  ·bus ops will be
>  >  >  >  o   read data
>  >  >  >  o   write data
>  >  >  >  o   write command
>  >  >  >  §  MIPI DSI will be registered as bus_register()
>  >  >  >  
>  >  >  >  When MIPI DSI probe is called, it (e.g., Exynos or OMAP MIPI DSI)

>  >  >  >  will initialize the MIPI DSI HW IP.
>  >  >  >  
>  >  >  >  This probe will also parse the DT file for MIPI DSI based panel, add

>  >  >  >  the panel device (device_add() ) to kernel and register the display
>  >  >  >  entity with its control and  video ops with CDF.
>  >  >  >  
>  >  >  >  I can give this a try.
>  >  >  
>  >  >  I am currently in progress of reworking Exynos MIPI DSIM code and

>  >  >  s6e8ax0 LCD driver to use the v2 RFC of Common Display Framework. I
>  >  >  have most of the work done, I have just to solve several remaining
>  >  >  problems.
>  >  
>  >  Do you already have code that you can publish ? I'm particularly

>  >  interested (and I think Tomi Valkeinen would be as well) in looking at
>  >  the DSI operations you expose to DSI sinks (panels, transceivers, ...).
>  
>  Well, I'm afraid this might be little below your expectations, but here's

>  an initial RFC of the part defining just the DSI bus. I need a bit more
>  time for patches for Exynos MIPI DSI master and s6e8ax0 LCD.

No worries. I was particularly interested in the DSI operations you needed to
export, they seem pretty simple. Thank you for sharing the code.


FYI,
here is STE "DSI API":
http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f=include/video/mcde.h;h=499ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD#l361

But it is not perfect. After a couple of products we realized that most 
panel drivers want an easy way to send a bunch of init commands in one 
go. So I think it should be an op for sending an array of commands at 
once. Something like


struct dsi_cmd {
enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */
u8 cmd;
int dataLen;
u8 *data;
}
struct dsi_ops {
int dsi_write(source, int num_cmds, struct dsi_cmd *cmds);
...
}

The rest of "DSI write API" could be made helpers on top of this one op. 
This grouping also allows driver to describe intent to send a bunch of 
commands together which might be of interest with mode set (if you need 
to synchronize a bunch of commands with a mode set, like setting smart 
panel rotation in synch with new framebuffer in dsi video mode).


I also looked at the video source in Tomi's git tree 
(http://gitorious.org/linux-omap-dss2/linux/blobs/work/dss-dev-model-cdf/include/video/display.h). 
I think I would prefer a single "setup" op taking a "struct dsi_config" 
as argument. Then each DSI formatter/encoder driver could decide best 
way to set that up. We have something similar at 
http://www.igloo

Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

2013-01-08 Thread Federico Vaga
> > @@ -165,7 +161,8 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> > long size)> 
> > /* align image size to PAGE_SIZE */
> > size = PAGE_ALIGN(size);
> > 
> > -   buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, 
GFP_KERNEL);
> > +   buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
> > +   
GFP_KERNEL | conf->mem_flags);
> 
> I think we can add GFP_DMA flag unconditionally to the vb2_dc_contig
> allocator.
> It won't hurt existing clients as most of nowadays platforms doesn't
> have DMA
> zone (GFP_DMA is ignored in such case), but it should fix the issues
> with some
> older and non-standard systems.

I did not set GFP_DMA fixed in the allocator because I do not want to brake 
something in the future. On x86 platform GFP_DMA allocates under 16MB and this 
limit can be too strict. When many other drivers use GFP_DMA we can saturate 
this tiny zone.
As you said, this fix the issue with _older_ and _non-standard_ (like sta2x11) 
systems. But this fix has effect on every other standard and new systems. 
That's why I preferred to set the flag optionally.

-- 
Federico Vaga
--
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 1/6 v5] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Guennadi Liakhovetski
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
is attached to a video bridge device, the bridge driver will create an I2C
device and wait for the respective I2C driver to probe. This makes linking
of devices straight forward, but this approach cannot be used with
intrinsically asynchronous and unordered device registration systems like
the Flattened Device Tree. To support such systems this patch adds an
asynchronous subdevice registration framework to V4L2. To use it respective
(e.g. I2C) subdevice drivers must request deferred probing as long as their
bridge driver hasn't probed. The bridge driver during its probing submits a
an arbitrary number of subdevice descriptor groups to the framework to
manage. After that it can add callbacks to each of those groups to be
called at various stages during subdevice probing, e.g. after completion.
Then the bridge driver can request single groups to be probed, finish its
own probing and continue its video subsystem configuration from its
callbacks.

Signed-off-by: Guennadi Liakhovetski 
---
v5: Now really fix the case, when subdevices probe successfully before the 
bridge, thanks to Prabhakar for testing and reporting

 drivers/media/v4l2-core/Makefile |3 +-
 drivers/media/v4l2-core/v4l2-async.c |  294 ++
 include/media/v4l2-async.h   |  113 +
 3 files changed, 409 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-async.c
 create mode 100644 include/media/v4l2-async.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index d065c01..b667ced 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,8 @@
 tuner-objs :=  tuner-core.o
 
 videodev-objs  :=  v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
-   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
+   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
+   v4l2-async.o
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
new file mode 100644
index 000..434c53d
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -0,0 +1,294 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   return hw_dev->bus_type == V4L2_ASYNC_BUS_I2C &&
+   hw_dev->match.i2c.adapter_id == client->adapter->nr &&
+   hw_dev->match.i2c.address == client->addr;
+}
+
+static bool match_platform(struct device *dev, struct v4l2_async_hw_device 
*hw_dev)
+{
+   return hw_dev->bus_type == V4L2_ASYNC_BUS_PLATFORM &&
+   !strcmp(hw_dev->match.platform.name, dev_name(dev));
+}
+
+static LIST_HEAD(subdev_list);
+static LIST_HEAD(notifier_list);
+static DEFINE_MUTEX(list_lock);
+
+static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier 
*notifier,
+   struct 
v4l2_async_subdev_list *asdl)
+{
+   struct v4l2_async_subdev *asd = NULL;
+   bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+
+   list_for_each_entry (asd, ¬ifier->waiting, list) {
+   struct v4l2_async_hw_device *hw = &asd->hw;
+   switch (hw->bus_type) {
+   case V4L2_ASYNC_BUS_SPECIAL:
+   match = hw->match.special.match;
+   if (!match)
+   /* Match always */
+   return asd;
+   break;
+   case V4L2_ASYNC_BUS_PLATFORM:
+   match = match_platform;
+   break;
+   case V4L2_ASYNC_BUS_I2C:
+   match = match_i2c;
+   break;
+   default:
+   /* Oops */
+   match = NULL;
+   dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : 
NULL,
+   "Invalid bus-type %u on %p\n", hw->bus_type, 
asd);
+   }
+
+   if (match && match(asdl->dev, hw))
+   break;
+   }
+
+   return asd;
+}
+
+static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
+  

Re: cron job: media_tree daily build: ERRORS

2013-01-08 Thread Hans Verkuil
On Mon 7 January 2013 22:38:23 Hans Verkuil wrote:
> 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:Mon Jan  7 19:00:18 CET 2013
> git hash:73ec66c000e9816806c7380ca3420f4e0638c40e
> gcc version:  i686-linux-gcc (GCC) 4.7.1
> host hardware:x86_64
> host os:  3.4.07-marune
> 
> linux-git-arm-eabi-davinci: WARNINGS
> linux-git-arm-eabi-exynos: WARNINGS
> linux-git-arm-eabi-omap: ERRORS
> linux-git-i686: OK
> linux-git-m32r: OK
> linux-git-mips: WARNINGS
> linux-git-powerpc64: OK
> linux-git-sh: OK
> linux-git-x86_64: OK
> linux-2.6.31.12-i686: WARNINGS
> linux-2.6.32.6-i686: WARNINGS
> linux-2.6.33-i686: WARNINGS
> linux-2.6.34-i686: WARNINGS
> linux-2.6.35.3-i686: WARNINGS
> linux-2.6.36-i686: WARNINGS
> linux-2.6.37-i686: WARNINGS
> linux-2.6.38.2-i686: WARNINGS
> linux-2.6.39.1-i686: WARNINGS
> linux-3.0-i686: WARNINGS
> linux-3.1-i686: WARNINGS
> linux-3.2.1-i686: WARNINGS
> linux-3.3-i686: WARNINGS
> linux-3.4-i686: WARNINGS
> linux-3.5-i686: WARNINGS
> linux-3.6-i686: WARNINGS
> linux-3.7-i686: WARNINGS
> linux-3.8-rc1-i686: WARNINGS
> linux-2.6.31.12-x86_64: WARNINGS
> linux-2.6.32.6-x86_64: WARNINGS
> linux-2.6.33-x86_64: WARNINGS
> linux-2.6.34-x86_64: WARNINGS
> linux-2.6.35.3-x86_64: WARNINGS
> linux-2.6.36-x86_64: WARNINGS
> linux-2.6.37-x86_64: WARNINGS
> linux-2.6.38.2-x86_64: WARNINGS
> linux-2.6.39.1-x86_64: WARNINGS
> linux-3.0-x86_64: WARNINGS
> linux-3.1-x86_64: WARNINGS
> linux-3.2.1-x86_64: WARNINGS
> linux-3.3-x86_64: WARNINGS
> linux-3.4-x86_64: WARNINGS
> linux-3.5-x86_64: WARNINGS
> linux-3.6-x86_64: WARNINGS
> linux-3.7-x86_64: WARNINGS
> linux-3.8-rc1-x86_64: WARNINGS
> apps: WARNINGS
> spec-git: OK
> sparse: ERRORS
> 
> Detailed results are available here:
> 
> http://www.xs4all.nl/~hverkuil/logs/Monday.log

There were a lot of new 'redefined' warnings that I have fixed.

In addition, it turned out that any driver using vb2 wasn't compiled for
kernels <3.2 due to the fact that DMA_SHARED_BUFFER wasn't set. That's fixed
as well, so drivers like em28xx and vivi will now compile on those older
kernels. This also was the reason I never saw that the usb_translate_error
function needed to be added to compat.h: it's used in em28xx but that driver
was never compiled on kernels without usb_translate_error.

Hopefully everything works now.

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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Guennadi Liakhovetski
On Tue, 8 Jan 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001

[snip]

> > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > +struct v4l2_async_notifier *notifier);
> > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > *notifier);
> > > > +/*
> > > > + * If subdevice probing fails any time after v4l2_async_subdev_bind(),
> > > > no
> > > > + * clean up must be called. This function is only a message of
> > > > intention.
> > > > + */
> > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > > 
> > > Could you please explain why you need both a bind notifier and a bound
> > > notifier ? I was expecting a single v4l2_async_subdev_register() call in
> > > subdev drivers (and, thinking about it, I would probably name it
> > > v4l2_subdev_register()).
> > 
> > I think I can, yes. Because between .bind() and .bound() the subdevice
> > driver does the actual hardware probing. So, .bind() is used to make sure
> > the hardware can be accessed, most importantly to provide a clock to the
> > subdevice. You can look at soc_camera_async_bind(). There I'm registering
> > the clock for the subdevice, about to bind. Why I cannot do it before, is
> > because I need subdevice name for clock matching. With I2C subdevices the
> > subdevice name contains the name of the driver, adapter number and i2c
> > address. The latter 2 I've got from host subdevice list. But not the
> > driver name. I thought about also passing the driver name there, but that
> > seemed too limiting to me. I also request regulators there, because before
> > ->bound() the sensor driver, but that could be done on the first call to
> > soc_camera_power_on(), although doing this "first call" thingie is kind of
> > hackish too. I could add one more soc-camera-power helper like
> > soc_camera_prepare() or similar too.
> 
> I think a soc_camera_power_init() function (or similar) would be a good idea, 
> yes.
> 
> > So, the main problem is the clock
> > subdevice name. Also see the comment in soc_camera.c:
> > 
> > /*
> >  * It is ok to keep the clock for the whole soc_camera_device life-time,
> >  * in principle it would be more logical to register the clock on icd
> >  * creation, the only problem is, that at that time we don't know the
> >  * driver name yet.
> >  */
> 
> I think we should fix that problem instead of shaping the async API around a 
> workaround :-)
> 
> >From the subdevice point of view, the probe function should request 
> >resources, 
> perform whatever initialization is needed (including verifying that the 
> hardware is functional when possible), and the register the subdev with the 
> code if everything succeeded. Splitting registration into bind() and bound() 
> appears a bit as a workaround to me.
> 
> If we need a workaround, I'd rather pass the device name in addition to the 
> I2C adapter number and address, instead of embedding the workaround in this 
> new API.

...or we can change the I2C subdevice name format. The actual need to do

snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
 asdl->dev->driver->name,
 i2c_adapter_id(client->adapter), client->addr);

in soc-camera now to exactly match the subdevice name, as created by 
v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if 
the latter changes at some point? Or what if one driver wishes to create 
several subdevices for one I2C device?

Thanks
Guennadi

> > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > +#endif
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] capemgr: Beaglebone DT overlay based cape manager

2013-01-08 Thread Guennadi Liakhovetski
(adding linux-media ML to cc)

Hi Pantelis

On Tue, 8 Jan 2013, Pantelis Antoniou wrote:

> Hi Arnd,
> 
> On Jan 7, 2013, at 11:35 PM, Arnd Bergmann wrote:
> 
> > (Adding Sascha Hauer, Linus Walleij, Lee Jones to Cc)
> > 
> > On Monday 07 January 2013, Tony Lindgren wrote:
> >>> 
> >>> At the end of the line, some kind of hardware glue is going to be needed.
> >>> 
> >>> I just feel that drawing from a sample size of 1 (maybe 2 if I get to 
> >>> throw
> >>> in the beagleboard), it is a bit premature to think about making it overly
> >>> general, besides the part that are obviously part of the infrastructure 
> >>> (like the DT overlay stuff).
> >>> 
> >>> What I'm getting at, is that we need some user experience about this, 
> >>> before
> >>> going away and creating structure out of possible misconception about the 
> >>> uses. 
> >> 
> >> IMHO stuff like this will be needed by many SoCs. Some examples of similar
> >> things for omaps that have eventually become generic frameworks have been
> >> the clock framework, USB OTG support, runtime PM, pinmux framework and
> >> so on.
> >> 
> >> So I suggest a minimal generic API from the start as that will make things
> >> a lot easier in the long run.
> > 
> > I agree. The ux500 platform already has the concept of "user interface 
> > boards",
> > which currently is not well integrated into devicetree. I believe Sascha
> > mentioned that Pengutronix had been shipping some other systems with add-on
> > boards and generating device tree binaries from source for each combination.
> > 
> > Ideally, both of the above should be able to use the same DT overlay logic
> > as BeagleBone, and I'm sure there are more of those.
> > 
> > Arnd
> 
> Hmm, I see. 
> 
> I will need some more information about the interface of the 'user interface 
> boards'.
> I.e. how is the board identified, what is typically present on those boards, 
> etc.
> 
> Can we get some input by the owner of other similar hardware? I know the FPGA
> people have similar requirements for example. There are other people that are 
> hitting
> problems getting DT to work with their systems, like the V4L people with the 
> order
> of initialization; see http://lwn.net/Articles/531068/. I think the V4L 
> problem is
> cleanly solved by the overlay being contained in the V4L device node and 
> applied just before
> the device is probed.

You probably mean these related V4L patches: 
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/58646 
that base upon of asynchronous V4L2 subdevice probing, referenced above. 
Yes, V4L DT nodes, as documented in 
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/58646/focus=58648
 
contain "port" and "endpoint" nodes, that describe the configuration of 
the hardware port and link to devices, connected to it. Not sure how well 
this would work with DT overlays, because endpoint nodes on both sides of 
the video data bus contain references to the other side and I don't know 
whether and how these can be created and / or updated at run-time. 
Otherwise, yes, the approach that we're currently developing on V4L allows 
us to build video data pipelines independent of (sub)device driver probing 
order.

Thanks
Guennadi

> In the meantime it would be better to wait until we have some ack from the 
> maintainers
> of the core subsystems about what they think.
>  
> Regards
> 
> -- Pantelis

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Guennadi,

On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
> > > 
> > > From: Guennadi Liakhovetski 
> > > Date: Fri, 19 Oct 2012 23:40:44 +0200
> > > Subject: [PATCH] media: V4L2: support asynchronous subdevice
> > > registration
> > > 
> > > Currently bridge device drivers register devices for all subdevices
> > > synchronously, tupically, during their probing. E.g. if an I2C CMOS
> > > sensor is attached to a video bridge device, the bridge driver will
> > > create an I2C device and wait for the respective I2C driver to probe.
> > > This makes linking of devices straight forward, but this approach cannot
> > > be used with intrinsically asynchronous and unordered device
> > > registration systems like the Flattened Device Tree. To support such
> > > systems this patch adds an asynchronous subdevice registration framework
> > > to V4L2. To use it respective (e.g. I2C) subdevice drivers must request
> > > deferred probing as long as their bridge driver hasn't probed. The
> > > bridge driver during its probing submits a an arbitrary number of
> > > subdevice descriptor groups to the framework to manage. After that it
> > > can add callbacks to each of those groups to be called at various stages
> > > during subdevice probing, e.g. after completion. Then the bridge driver
> > > can request single groups to be probed, finish its own probing and
> > > continue its video subsystem configuration from its callbacks.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski 
> > > ---
> > > 
> > > v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> > > probe successfully before the bridge, thanks to Prabhakar for reporting
> > > 
> > >  drivers/media/v4l2-core/Makefile |3 +-
> > >  drivers/media/v4l2-core/v4l2-async.c |  284 +++
> > >  include/media/v4l2-async.h   |  113 ++
> > >  3 files changed, 399 insertions(+), 1 deletions(-)
> > >  create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> > >  create mode 100644 include/media/v4l2-async.h
> > 
> > [snip]
> > 
> > > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > > new file mode 100644
> > > index 000..91d436d
> > > --- /dev/null
> > > +++ b/include/media/v4l2-async.h
> > > @@ -0,0 +1,113 @@
> > > +/*
> > > + * V4L2 asynchronous subdevice registration API
> > > + *
> > > + * Copyright (C) 2012, Guennadi Liakhovetski 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#ifndef V4L2_ASYNC_H
> > > +#define V4L2_ASYNC_H
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +
> > > +struct device;
> > > +struct v4l2_device;
> > > +struct v4l2_async_notifier;
> > > +
> > > +enum v4l2_async_bus_type {
> > > + V4L2_ASYNC_BUS_SPECIAL,
> > > + V4L2_ASYNC_BUS_PLATFORM,
> > > + V4L2_ASYNC_BUS_I2C,
> > > +};
> > > +
> > > +struct v4l2_async_hw_device {
> > > + enum v4l2_async_bus_type bus_type;
> > > + union {
> > > + struct {
> > > + const char *name;
> > > + } platform;
> > > + struct {
> > > + int adapter_id;
> > > + unsigned short address;
> > > + } i2c;
> > > + struct {
> > > + bool (*match)(struct device *,
> > > +   struct v4l2_async_hw_device *);
> > > + void *priv;
> > > + } special;
> > > + } match;
> > > +};
> > > +
> > > +/**
> > > + * struct v4l2_async_subdev - sub-device descriptor, as known to a
> > > bridge
> > > + * @hw:  this device descriptor
> > > + * @list:member in a list of subdevices
> > > + */
> > > +struct v4l2_async_subdev {
> > > + struct v4l2_async_hw_device hw;
> > > + struct list_head list;
> > > +};
> > > +
> > > +/**
> > > + * v4l2_async_subdev_list - provided by subdevices
> > > + * @list:member in a list of subdevices
> > > + * @dev: hardware device
> > > + * @subdev:  V4L2 subdevice
> > > + * @asd: pointer to respective struct v4l2_async_subdev
> > > + * @notifier:pointer to managing notifier
> > > + */
> > > +struct v4l2_async_subdev_list {
> > > + struct list_head list;
> > > + struct device *dev;
> > > + struct v4l2_subdev *subdev;
> > > + struct v4l2_async_subdev *asd;
> > > + struct v4l2_async_notifier *notifier;
> > > +};
> > > +
> > > +/**
> > > + * v4l2_async_notifier - provided by bridges
> > > + * @subdev_num:  number of subdevices
> > > + * @subdev:  array of pointers to subdevices
> > > + * @v4l2_dev:pointer to sruct v4l2_device
> > > + * @waiting: list of subdevices, waiting for their drivers
> > > + * 

Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Guennadi Liakhovetski
Hi Laurent

On Tue, 8 Jan 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> Thanks for the patch.
> 
> On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
> > 
> > From: Guennadi Liakhovetski 
> > Date: Fri, 19 Oct 2012 23:40:44 +0200
> > Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
> > 
> > Currently bridge device drivers register devices for all subdevices
> > synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> > is attached to a video bridge device, the bridge driver will create an I2C
> > device and wait for the respective I2C driver to probe. This makes linking
> > of devices straight forward, but this approach cannot be used with
> > intrinsically asynchronous and unordered device registration systems like
> > the Flattened Device Tree. To support such systems this patch adds an
> > asynchronous subdevice registration framework to V4L2. To use it respective
> > (e.g. I2C) subdevice drivers must request deferred probing as long as their
> > bridge driver hasn't probed. The bridge driver during its probing submits a
> > an arbitrary number of subdevice descriptor groups to the framework to
> > manage. After that it can add callbacks to each of those groups to be
> > called at various stages during subdevice probing, e.g. after completion.
> > Then the bridge driver can request single groups to be probed, finish its
> > own probing and continue its video subsystem configuration from its
> > callbacks.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> > 
> > v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> > probe successfully before the bridge, thanks to Prabhakar for reporting
> > 
> >  drivers/media/v4l2-core/Makefile |3 +-
> >  drivers/media/v4l2-core/v4l2-async.c |  284 +++
> >  include/media/v4l2-async.h   |  113 ++
> >  3 files changed, 399 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> >  create mode 100644 include/media/v4l2-async.h
> 
> [snip]
> 
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > new file mode 100644
> > index 000..91d436d
> > --- /dev/null
> > +++ b/include/media/v4l2-async.h
> > @@ -0,0 +1,113 @@
> > +/*
> > + * V4L2 asynchronous subdevice registration API
> > + *
> > + * Copyright (C) 2012, Guennadi Liakhovetski 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef V4L2_ASYNC_H
> > +#define V4L2_ASYNC_H
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +struct device;
> > +struct v4l2_device;
> > +struct v4l2_async_notifier;
> > +
> > +enum v4l2_async_bus_type {
> > +   V4L2_ASYNC_BUS_SPECIAL,
> > +   V4L2_ASYNC_BUS_PLATFORM,
> > +   V4L2_ASYNC_BUS_I2C,
> > +};
> > +
> > +struct v4l2_async_hw_device {
> > +   enum v4l2_async_bus_type bus_type;
> > +   union {
> > +   struct {
> > +   const char *name;
> > +   } platform;
> > +   struct {
> > +   int adapter_id;
> > +   unsigned short address;
> > +   } i2c;
> > +   struct {
> > +   bool (*match)(struct device *,
> > + struct v4l2_async_hw_device *);
> > +   void *priv;
> > +   } special;
> > +   } match;
> > +};
> > +
> > +/**
> > + * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> > + * @hw:this device descriptor
> > + * @list:  member in a list of subdevices
> > + */
> > +struct v4l2_async_subdev {
> > +   struct v4l2_async_hw_device hw;
> > +   struct list_head list;
> > +};
> > +
> > +/**
> > + * v4l2_async_subdev_list - provided by subdevices
> > + * @list:  member in a list of subdevices
> > + * @dev:   hardware device
> > + * @subdev:V4L2 subdevice
> > + * @asd:   pointer to respective struct v4l2_async_subdev
> > + * @notifier:  pointer to managing notifier
> > + */
> > +struct v4l2_async_subdev_list {
> > +   struct list_head list;
> > +   struct device *dev;
> > +   struct v4l2_subdev *subdev;
> > +   struct v4l2_async_subdev *asd;
> > +   struct v4l2_async_notifier *notifier;
> > +};
> > +
> > +/**
> > + * v4l2_async_notifier - provided by bridges
> > + * @subdev_num:number of subdevices
> > + * @subdev:array of pointers to subdevices
> > + * @v4l2_dev:  pointer to sruct v4l2_device
> > + * @waiting:   list of subdevices, waiting for their drivers
> > + * @done:  list of subdevices, already probed
> > + * @list:  member in a global list of notifiers
> > + * @bind:  a subdevice driver is about to probe one of your subdevices
> > + * @bound: a subdevice driver has successfully p

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Laurent Pinchart
Hi Daniel,

On Sunday 06 January 2013 18:46:47 Daniel Vetter wrote:
> On Thu, Dec 27, 2012 at 09:57:25AM -0600, Rob Clark wrote:
> > On Mon, Dec 24, 2012 at 11:09 AM, Laurent Pinchart wrote:
> > > On the topic of discussions, would anyone be interested in a
> > > BoF/brainstorming/whatever session during the FOSDEM ?
> > 
> > I will be at FOSDEM.. and from http://wiki.x.org/wiki/fosdem2013 it
> > looks like at least Daniel will be there.  If enough others are, it
> > could be a good idea.
> 
> Seconded. Jesse should be there, too, and from the Helsinki guys Ville and
> Andy should show up. Doesn't look like Jani will be able to make it. I think
> something on Sunday (to not clash with the X devroom) would be good.
> 
> Should we apply for an offical BOF/Is there a process for tahat? Adding
> Luc in case he knows ...

>From the event website it looks like there are free rooms on Sunday, it would 
be good if we could secure one of them.

Are there other X/display related topics that need to be discussed on Sunday ? 
How much time should we set aside ?

-- 
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 v2 0/5] Common Display Framework

2013-01-08 Thread Laurent Pinchart
On Friday 28 December 2012 01:04:04 Sascha Hauer wrote:
> On Thu, Dec 27, 2012 at 01:57:56PM -0600, Rob Clark wrote:
> > On Thu, Dec 27, 2012 at 1:18 PM, Sascha Hauer wrote:
> > > On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
> > >> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
> > > 
> > > This implies that the master driver knows all potential subdevices,
> > > something which is not true for SoCs which have external i2c encoders
> > > attached to unrelated i2c controllers.
> > 
> > well, it can be brute-forced..  ie. drm driver calls common
> > register_all_panels() fxn, which, well, registers all the
> > panel/display subdev's based on their corresponding CONFIG_FOO_PANEL
> > defines.  If you anyways aren't building the panels as separate
> > modules, that would work.  Maybe not the most *elegant* approach, but
> > simple and functional.
> > 
> > I guess it partly depends on the structure in devicetree.  If you are
> > 
> > assuming that the i2c encoder belongs inside the i2c bus, like:
> >   &i2cN {
> >   
> > foo-i2c-encoder {
> > 
> >   
> > 
> > };
> >   
> >   };
> > 
> > and you are letting devicetree create the devices, then it doesn't
> > quite work.  I'm not entirely convinced you should do it that way.
> > Really any device like that is going to be hooked up to at least a
> > couple busses..  i2c, some sort of bus carrying pixel data, maybe some
> > gpio's, etc.  So maybe makes more sense for a virtual drm/kms bus, and
> > then use phandle stuff to link it to the various other busses it
> > 
> > needs:
> >   mydrmdev {
> > foo-i2c-encoder {
> >i2c = <&i2cN>;
> >gpio = <&gpioM 2 3>
> >...
> > };
> >   };
> 
> This seems to shift initialization order problem to another place. Here we
> have to make sure the controller is initialized before the drm driver. Same
> with suspend/resume.
> 
> It's not only i2c devices, also platform devices. On i.MX for example we
> have a hdmi transmitter which is somewhere on the physical address space.
> 
> I think grouping the different units together in a devicetree blob because
> we think they might form a logical virtual device is not going to work. It
> might make it easier from a drm perspective, but I think doing this will
> make for a lot of special cases. What will happen for example if you have
> two encoder devices in a row to configure? The foo-i2c-encoder would then
> get another child node.
> 
> Right now the devicetree is strictly ordered by (control-, not data-) bus
> topology. Linux has great helper code to support this model. Giving up this
> help to brute force a different topology and then trying to fit the result
> back into the Linux Bus hierarchy doesn't sound like a good idea to me.

I agree. The Linux device model is architectured around a control bus based 
tree, I don't want to change that. With devices hooked up on several busses we 
will have dependency issues anyway, regardless of how we describe them in DT. 
If we hook up the nodes from a data bus perspective we will run into control 
bus dependency issues. It's thus better in my opinion to keep the classic 
control bus based model and solve the data bus dependency issues.

> > ok, admittedly that is a bit different from other proposals about how
> > this all fits in devicetree.. but otoh, I'm not a huge believer in
> > letting something that is supposed to make life easier (DT), actually
> > make things harder or more complicated.  Plus this CDF stuff all needs
> > to also work on platforms not using OF/DT.
> 
> Right, but every other platform I know of is also described by its bus
> topology, be it platform device based or PCI or maybe even USB based.
> 
> CDF has to solve the same problem as ASoC and soc-camera: subdevices for
> a virtual device can come from many different corners of the system. BTW
> one example for a i2c encoder would be the SiI9022 which could not only
> be part of a drm device, but also of an ASoC device.

-- 
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 v2 0/5] Common Display Framework

2013-01-08 Thread Laurent Pinchart
Hi Rob,

On Thursday 27 December 2012 09:54:55 Rob Clark wrote:
> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart wrote:
> > On Tuesday 18 December 2012 00:21:32 Rob Clark wrote:
> >> On Mon, Dec 17, 2012 at 11:04 PM, Dave Airlie  wrote:
> >> >> Many developers showed interest in the first RFC, and I've had the
> >> >> opportunity to discuss it with most of them. I would like to thank (in
> >> >> no particular order) Tomi Valkeinen for all the time he spend helping
> >> >> me to draft v2, Marcus Lorentzon for his useful input during Linaro
> >> >> Connect Q4 2012, and Linaro for inviting me to Connect and providing a
> >> >> venue to discuss this topic.
> >> > 
> >> > So this might be a bit off topic but this whole CDF triggered me
> >> > looking at stuff I generally avoid:
> >> > 
> >> > The biggest problem I'm having currently with the whole ARM graphics
> >> > and output world is the proliferation of platform drivers for every
> >> > little thing. The whole ordering of operations with respect to things
> >> > like suspend/resume or dynamic power management is going to be a real
> >> > nightmare if there are dependencies between the drivers. How do you
> >> > enforce ordering of s/r operations between all the various components?
> >> 
> >> I tend to think that sub-devices are useful just to have a way to probe
> >> hw which may or may not be there, since on ARM we often don't have any
> >> alternative.. but beyond that, suspend/resume, and other life-cycle
> >> aspects, they should really be treated as all one device. Especially to
> >> avoid undefined suspend/resume ordering.
> > 
> > I tend to agree, except that I try to reuse the existing PM infrastructure
> > when possible to avoid reinventing the wheel. So far handling
> > suspend/resume ordering related to data busses in early suspend/late
> > resume operations and allowing the Linux PM core to handle control busses
> > using the Linux device tree worked pretty well.
> > 
> >> CDF or some sort of mechanism to share panel drivers between drivers is
> >> useful.  Keeping it within drm, is probably a good idea, if nothing else
> >> to simplify re-use of helper fxns (like avi-infoframe stuff, for example)
> >> and avoid dealing with merging changes across multiple trees. Treating
> >> them more like shared libraries and less like sub-devices which can be
> >> dynamically loaded/unloaded (ie. they should be not built as separate
> >> modules or suspend/resumed or probed/removed independently of the master
> >> driver) is a really good idea to avoid uncovering nasty synchronization
> >> issues later (remove vs modeset or pageflip) or surprising userspace in
> >> bad ways.
> >
> > We've tried that in V4L2 years ago and realized that the approach led to a
> > dead-end, especially when OF/DT got involved. With DT-based device
> > probing, I2C camera sensors started getting probed asynchronously to the
> > main camera device, as they are children of the I2C bus master. We will
> > have similar issues with I2C HDMI transmitters or panels, so we should be
> > prepared for it.
>
> What I've done to avoid that so far is that the master device registers the
> drivers for it's output sub-devices before registering it's own device.

I'm not sure to follow you here. The master device doesn't register anything, 
do you mean the master device driver ? If so, how does the master device 
driver register its own device ? Devices are not registered by their driver.

> At least this way I can control that they are probed first. Not the
> prettiest thing, but avoids even uglier problems.
>
> > On PC hardware the I2C devices are connected to an I2C master provided by
> > the GPU, but on embedded devices they are usually connected to an
> > independent I2C master. We thus can't have a single self-contained driver
> > that controls everything internally, and need to interface with the rest
> > of the SoC drivers.
> > 
> > I agree that probing/removing devices independently of the master driver
> > can lead to bad surprises, which is why I want to establish clear rules
> > in CDF regarding what can and can't be done with display entities.
> > Reference counting will be one way to make sure that devices don't
> > disappear all of a sudden.
>
> That at least helps cover some issues.. although it doesn't really help
> userspace confusion.
> 
> Anyways, with enough work perhaps all problems could be solved.. otoh, there
> are plenty of other important problems to solve in the world of gpus and
> kms, so my preference is always not to needlessly over-complicate CDF and
> instead leave some time for other things

My customer is interested in CDF at the moment. If they ask me to solve other 
GPU-related problems, sure, I can work on that, but that's not planned.

> >> > The other thing I'd like you guys to do is kill the idea of fbdev and
> >> > v4l drivers that are "shared" with the drm codebase, really just
> >> > implement fbdev and v4l on top of the drm layer, some people might

Re: [RFC v2 0/5] Common Display Framework

2013-01-08 Thread Laurent Pinchart
Hi Tomasz,

On Thursday 27 December 2012 15:43:34 Tomasz Figa wrote:
> On Monday 24 of December 2012 15:12:28 Laurent Pinchart wrote:
> > On Friday 21 December 2012 11:00:52 Tomasz Figa wrote:
> > > On Tuesday 18 of December 2012 08:31:30 Vikas Sajjan wrote:
> > > > On 17 December 2012 20:55, Laurent Pinchart wrote:
> > > > > Hi Vikas,
> > > > > 
> > > > > Sorry for the late reply. I now have more time to work on CDF, so
> > > > > delays should be much shorter.
> > > > > 
> > > > > On Thursday 06 December 2012 10:51:15 Vikas Sajjan wrote:
> > > > > > Hi Laurent,
> > > > > > 
> > > > > > I was thinking of porting CDF to samsung EXYNOS 5250 platform,
> > > > > > what I found is that, the exynos display controller is MIPI DSI
> > > > > > based controller.
> > > > > > 
> > > > > > But if I look at CDF patches, it has only support for MIPI DBI
> > > > > > based Display controller.
> > > > > > 
> > > > > > So my question is, do we have any generic framework for MIPI DSI
> > > > > > based display controller? basically I wanted to know, how to go
> > > > > > about porting CDF for such kind of display controller.
> > > > > 
> > > > > MIPI DSI support is not available yet. The only reason for that is
> > > > > that I don't have any MIPI DSI hardware to write and test the code
> > > > > with :-)
> > > > > 
> > > > > The common display framework should definitely support MIPI DSI. I
> > > > > think the existing MIPI DBI code could be used as a base, so the
> > > > > implementation shouldn't be too high.
> > > > > 
> > > > > Yeah, i was also thinking in similar lines, below is my though for
> > > > > MIPI DSI support in CDF.
> > > > 
> > > > o   MIPI DSI support as part of CDF framework will expose
> > > > §  mipi_dsi_register_device(mpi_device) (will be called mach-xxx-dt.c
> > > > file )
> > > > §  mipi_dsi_register_driver(mipi_driver, bus ops) (will be called
> > > > from platform specific init driver call )
> > > > ·bus ops will be
> > > > o   read data
> > > > o   write data
> > > > o   write command
> > > > §  MIPI DSI will be registered as bus_register()
> > > > 
> > > > When MIPI DSI probe is called, it (e.g., Exynos or OMAP MIPI DSI)
> > > > will initialize the MIPI DSI HW IP.
> > > > 
> > > > This probe will also parse the DT file for MIPI DSI based panel, add
> > > > the panel device (device_add() ) to kernel and register the display
> > > > entity with its control and  video ops with CDF.
> > > > 
> > > > I can give this a try.
> > > 
> > > I am currently in progress of reworking Exynos MIPI DSIM code and
> > > s6e8ax0 LCD driver to use the v2 RFC of Common Display Framework. I
> > > have most of the work done, I have just to solve several remaining
> > > problems.
> > 
> > Do you already have code that you can publish ? I'm particularly
> > interested (and I think Tomi Valkeinen would be as well) in looking at
> > the DSI operations you expose to DSI sinks (panels, transceivers, ...).
> 
> Well, I'm afraid this might be little below your expectations, but here's
> an initial RFC of the part defining just the DSI bus. I need a bit more
> time for patches for Exynos MIPI DSI master and s6e8ax0 LCD.

No worries. I was particularly interested in the DSI operations you needed to 
export, they seem pretty simple. Thank you for sharing the code.

> The implementation is very simple and heavily based on your MIPI DBI
> support and existing Exynos MIPI DSIM framework. Provided operation set is
> based on operation set used by Exynos s6e8ax0 LCD driver. Unfortunately
> this is my only source of information about MIPI DSI.

-- 
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 1/6 v4] media: V4L2: support asynchronous subdevice registration

2013-01-08 Thread Laurent Pinchart
Hi Guennadi,

Thanks for the patch.

On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
> 
> From: Guennadi Liakhovetski 
> Date: Fri, 19 Oct 2012 23:40:44 +0200
> Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
> 
> Currently bridge device drivers register devices for all subdevices
> synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> is attached to a video bridge device, the bridge driver will create an I2C
> device and wait for the respective I2C driver to probe. This makes linking
> of devices straight forward, but this approach cannot be used with
> intrinsically asynchronous and unordered device registration systems like
> the Flattened Device Tree. To support such systems this patch adds an
> asynchronous subdevice registration framework to V4L2. To use it respective
> (e.g. I2C) subdevice drivers must request deferred probing as long as their
> bridge driver hasn't probed. The bridge driver during its probing submits a
> an arbitrary number of subdevice descriptor groups to the framework to
> manage. After that it can add callbacks to each of those groups to be
> called at various stages during subdevice probing, e.g. after completion.
> Then the bridge driver can request single groups to be probed, finish its
> own probing and continue its video subsystem configuration from its
> callbacks.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
> 
> v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> probe successfully before the bridge, thanks to Prabhakar for reporting
> 
>  drivers/media/v4l2-core/Makefile |3 +-
>  drivers/media/v4l2-core/v4l2-async.c |  284 +++
>  include/media/v4l2-async.h   |  113 ++
>  3 files changed, 399 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-async.c
>  create mode 100644 include/media/v4l2-async.h

[snip]

> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> new file mode 100644
> index 000..91d436d
> --- /dev/null
> +++ b/include/media/v4l2-async.h
> @@ -0,0 +1,113 @@
> +/*
> + * V4L2 asynchronous subdevice registration API
> + *
> + * Copyright (C) 2012, Guennadi Liakhovetski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef V4L2_ASYNC_H
> +#define V4L2_ASYNC_H
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +struct device;
> +struct v4l2_device;
> +struct v4l2_async_notifier;
> +
> +enum v4l2_async_bus_type {
> + V4L2_ASYNC_BUS_SPECIAL,
> + V4L2_ASYNC_BUS_PLATFORM,
> + V4L2_ASYNC_BUS_I2C,
> +};
> +
> +struct v4l2_async_hw_device {
> + enum v4l2_async_bus_type bus_type;
> + union {
> + struct {
> + const char *name;
> + } platform;
> + struct {
> + int adapter_id;
> + unsigned short address;
> + } i2c;
> + struct {
> + bool (*match)(struct device *,
> +   struct v4l2_async_hw_device *);
> + void *priv;
> + } special;
> + } match;
> +};
> +
> +/**
> + * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> + * @hw:  this device descriptor
> + * @list:member in a list of subdevices
> + */
> +struct v4l2_async_subdev {
> + struct v4l2_async_hw_device hw;
> + struct list_head list;
> +};
> +
> +/**
> + * v4l2_async_subdev_list - provided by subdevices
> + * @list:member in a list of subdevices
> + * @dev: hardware device
> + * @subdev:  V4L2 subdevice
> + * @asd: pointer to respective struct v4l2_async_subdev
> + * @notifier:pointer to managing notifier
> + */
> +struct v4l2_async_subdev_list {
> + struct list_head list;
> + struct device *dev;
> + struct v4l2_subdev *subdev;
> + struct v4l2_async_subdev *asd;
> + struct v4l2_async_notifier *notifier;
> +};
> +
> +/**
> + * v4l2_async_notifier - provided by bridges
> + * @subdev_num:  number of subdevices
> + * @subdev:  array of pointers to subdevices
> + * @v4l2_dev:pointer to sruct v4l2_device
> + * @waiting: list of subdevices, waiting for their drivers
> + * @done:list of subdevices, already probed
> + * @list:member in a global list of notifiers
> + * @bind:a subdevice driver is about to probe one of your subdevices
> + * @bound:   a subdevice driver has successfully probed one of your
> subdevices + * @complete: all your subdevices have been probed 
> successfully
> + * @unbind:  a subdevice is leaving
> + */
> +struct v4l2_async_notifier {
> + int subdev_num;
> + struct v4l2_async_subdev **subdev;
> + struct v4

Re: [RFC] Initial scan files troubles and brainstorming

2013-01-08 Thread Oliver Schinagl

On 07-01-13 23:44, Jonathan McCrohan wrote:

Hi Christoph,

On Mon, 7 Jan 2013 13:53:25 +0100, Christoph Pfister wrote:

I've been updating the scan data for the past few years, but found
barely no time in the last (many!) months. This won't change in
future, so I've decided to step down from the task; may others do what
is necessary ...

Thanks for your work over the past few years. If nobody else is willing,
I'd be happy to take over this role, and if Oliver's offer still stands,
I'd be happy to share DVB scanfile maintenance with him.
Slightly depends on where this goes. If there is a seperated 
dvb-scanfile tree (or whatever incarnation can be thought of) I'd be 
happy to help out where I can.


The repository is 'done' imo. History is preserved and seperated out. 
Would just need some word from the powers that be. :)

Jon


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk

2013-01-08 Thread Laurent Pinchart
Hi Guennadi,

Thanks for the patch.

On Wednesday 26 December 2012 18:49:07 Guennadi Liakhovetski wrote:
> Instead of centrally enabling and disabling subdevice master clocks in
> soc-camera core, let subdevice drivers do that themselves, using the
> V4L2 clock API and soc-camera convenience wrappers.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/i2c/soc_camera/imx074.c  |   18 ++-
>  drivers/media/i2c/soc_camera/mt9m001.c |   17 ++-
>  drivers/media/i2c/soc_camera/mt9m111.c |   20 ++-
>  drivers/media/i2c/soc_camera/mt9t031.c |   19 ++-
>  drivers/media/i2c/soc_camera/mt9t112.c |   19 ++-
>  drivers/media/i2c/soc_camera/mt9v022.c |   17 ++-
>  drivers/media/i2c/soc_camera/ov2640.c  |   19 ++-
>  drivers/media/i2c/soc_camera/ov5642.c  |   20 ++-
>  drivers/media/i2c/soc_camera/ov6650.c  |   17 ++-
>  drivers/media/i2c/soc_camera/ov772x.c  |   15 ++-
>  drivers/media/i2c/soc_camera/ov9640.c  |   17 ++-
>  drivers/media/i2c/soc_camera/ov9640.h  |1 +
>  drivers/media/i2c/soc_camera/ov9740.c  |   18 ++-
>  drivers/media/i2c/soc_camera/rj54n1cb0c.c  |   17 ++-
>  drivers/media/i2c/soc_camera/tw9910.c  |   18 ++-
>  drivers/media/platform/soc_camera/soc_camera.c |  173  
>  .../platform/soc_camera/soc_camera_platform.c  |2 +-
>  include/media/soc_camera.h |   13 +-
>  18 files changed, 356 insertions(+), 84 deletions(-)

[snip]

> diff --git a/drivers/media/platform/soc_camera/soc_camera.c
> b/drivers/media/platform/soc_camera/soc_camera.c index 0b6ddff..a9e6f01
> 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c

[snip]

> @@ -1068,6 +1091,57 @@ static void scan_add_host(struct soc_camera_host
> *ici) mutex_unlock(&list_lock);
>  }
> 
> +/*
> + * It is invalid to call v4l2_clk_enable() after a successful probing
> + * asynchronously outside of V4L2 operations, i.e. with .host_lock not
> held.
> + */
> +static int soc_camera_clk_enable(struct v4l2_clk *clk)
> +{
> + struct soc_camera_device *icd = clk->priv;
> + struct soc_camera_host *ici;
> +
> + if (!icd || !icd->parent)
> + return -ENODEV;
> +
> + ici = to_soc_camera_host(icd->parent);
> +
> + if (!try_module_get(ici->ops->owner))
> + return -ENODEV;
> +
> + /*
> +  * If a different client is currently being probed, the host will tell
> +  * you to go
> +  */
> + return ici->ops->add(icd);
> +}
> +
> +static void soc_camera_clk_disable(struct v4l2_clk *clk)
> +{
> + struct soc_camera_device *icd = clk->priv;
> + struct soc_camera_host *ici;
> +
> + if (!icd || !icd->parent)
> + return;
> +
> + ici = to_soc_camera_host(icd->parent);
> +
> + ici->ops->remove(icd);
> +
> + module_put(ici->ops->owner);
> +}
> +
> +/*
> + * Eventually, it would be more logical to make the respective host the
> clock
> + * owner, but then we would have to copy this struct for each ici. Besides,
> it
> + * would introduce the circular dependency problem, unless we port all
> client
> + * drivers to release the clock, when not in use.
> + */

Won't we have to solve this problem eventually ? This should probably be put 
on the agenda of the next V4L2 workshop/summit.

> +static const struct v4l2_clk_ops soc_camera_clk_ops = {
> + .owner = THIS_MODULE,
> + .enable = soc_camera_clk_enable,
> + .disable = soc_camera_clk_disable,
> +};
> +
>  #ifdef CONFIG_I2C_BOARDINFO
>  static int soc_camera_init_i2c(struct soc_camera_device *icd,
>  struct soc_camera_desc *sdesc)
> @@ -1077,19 +1151,33 @@ static int soc_camera_init_i2c(struct
> soc_camera_device *icd, struct soc_camera_host_desc *shd =
> &sdesc->host_desc;
>   struct i2c_adapter *adap = i2c_get_adapter(shd->i2c_adapter_id);
>   struct v4l2_subdev *subdev;
> + char clk_name[V4L2_SUBDEV_NAME_SIZE];
> + int ret;
> 
>   if (!adap) {
>   dev_err(icd->pdev, "Cannot get I2C adapter #%d. No driver?\n",
>   shd->i2c_adapter_id);
> - goto ei2cga;
> + return -ENODEV;
>   }
> 
>   shd->board_info->platform_data = &sdesc->subdev_desc;
> 
> + snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> +  shd->board_info->type,
> +  shd->i2c_adapter_id, shd->board_info->addr);
> +
> + icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", 
> icd);

I don't think you should hardcode the clock name to "mclk". The common clock 
framework use a client-specific clock name when requesting a clock, it would 
be better to use a similar mechanism.

> + if (IS_ERR(icd->clk)) {
> + ret = PTR_ERR(icd->clk);
> + goto eclkreg;
> + }
> +
>   subdev = v4l2_i2