Re: [PATCH] media: zoran: move to dma-mapping interface

2018-04-25 Thread Trent Piepho
On Wed, 2018-04-25 at 14:22 -0300, Mauro Carvalho Chehab wrote:
> Em Wed, 25 Apr 2018 17:58:25 +0200
> Arnd Bergmann <a...@arndb.de> escreveu:
> 
> > On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.o
> > rg> wrote:
> > > On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:  
> > > > That thought had occurred to me as well. I removed the oldest ISDN
> > > > drivers already some years ago, and the OSS sound drivers
> > > > got removed as well, and comedi got converted to the dma-mapping
> > > > interfaces, so there isn't much left at all now. This is what we
> > > > have as of v4.17-rc1:  
> > > 
> > > Yes, I've been looking at various grotty old bits to purge.  Usually
> > > I've been looking for some non-tree wide patches and CCed the last
> > > active people to see if they care.  In a few cases people do, but
> > > most often no one does.  
> > 
> > Let's start with this one (zoran) then, as Mauro is keen on having
> > all media drivers compile-testable on x86-64 and arm.
> > 
> > Trent Piepho and Hans Verkuil both worked on this driver in the
> > 2008/2009 timeframe and those were the last commits from anyone
> > who appears to have tested their patches on actual hardware.
> 
> Zoran is a driver for old hardware. I don't doubt that are people
> out there still using it, but who knows?
> 
> I have a few those boards packed somewhere. I haven't work with PCI
> hardware for a while. If needed, I can try to seek for them and
> do some tests. I need first to unpack a machine with PCI slots...
> the NUCs I generally use for development don't have any :-)
> 
> Anyway, except for virt_to_bus() and related stuff, I think that this
> driver is in good shape, as Hans did a lot of work in the past to
> make it to use the current media framework.

I still have a zoran board.  And my recently purchased ryzen system has
PCI slots.  To my surprise they are not uncommon on new socket AM4
boards.  However, I think the zoran board I have is 5V PCI and that is
rather uncommon.  Also becoming uncommon is analog NTSC/PAL video that
this chip is designed for!

If anyone is using these still, they would be in legacy systems for
these legacy video formats.


Re: [ANNOUNCE] git tree repositories

2010-01-20 Thread Trent Piepho
On Thu, 21 Jan 2010, Mauro Carvalho Chehab wrote:
  OTOH, since with git it is common to have multiple branches
  within one repository, I'm not sure how it works. It would
  be cool if git would support per-branch descriptions,
  and git web could display them.

 I don't think git supports it. In kernel.org, people prefer to
 use more than one repository when they have more than one
 need.

stgit lets me set descriptions for each branch.  The descriptions are there
under the branch in the config file.  I don't think git-branch shows any
kind of description for the branch.
--
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: How to use saa7134 gpio via gpio-sysfs?

2010-01-16 Thread Trent Piepho
On Sat, 16 Jan 2010, hermann pitton wrote:
 Am Freitag, den 15.01.2010, 17:27 -0800 schrieb Trent Piepho:
  On Sat, 16 Jan 2010, hermann pitton wrote:
   Am Dienstag, den 12.01.2010, 04:13 +0100 schrieb hermann pitton:
 gpio-sysfs creates
 /sys/class/gpio/export
 /sys/class/gpio/import
 but no gpion entries so far.
 
  The saa713x driver predates the generic gpio layer by years and years, so
  it doesn't use it.  It also doesn't need to use it.  Since the gpios are
  managed by the saa713x driver, and they also used by the saa713x driver,
  there is no need to interface two different drivers together.  There are
  tons of drivers for devices that have gpios like this, but they don't use
  the gpio layer.
 
  But with gpio access via sysfs for generic gpios, there is something useful
  about having the saa713x driver support generic gpios.  IIRC, somehow wrote
  a gpio only bt848 driver that didn't do anything but export gpios.
 
  In order to do this, you'll have to write code for the saa7134 driver to
  have it register with the gpio layer.  I think you could still have the
  saa7134 driver itself use its gpio directly.  That would avoid a
  performance penalty in the driver.

 Thanks for more details, but I'm still wondering what pins ever could be
 interesting in userland, given that they are all treated such different
 per device, and we count up to 200 different boards these days.

There are some cards for intended for survilence or embedded applications
that have headers on them to connect things to the GPIOs.  Like alarms or
camera controllers and stuff like that.

The GPIO only bttv driver was created by someone who just soldered a bunch
of wires on a cheap bt848 card, you can get them for just a few dollars, as
it was a cheap and easy way to get a bunch of gpios in a pc.  See his page
here http://www.bu3sch.de/joomla/index.php/bt8xx-based-gpio-card

There are cards you can get that just have GPIOs, but they end up being
rather expensive.  Here's one:
http://www.acromag.com/parts.cfm?Model_ID=317Product_Function_ID=4Category_ID=18Group_ID=1
Way fancier than a tv card, but it's $600.

I think if I was doing the coding, I'd add a field in the card description
for what GPIOs should be exported.  I.e., which ones have an external
header.  Maybe in addition to, or instead of, I'd have a module option that
would cause GPIOs to be exported.  A bitmask of which to export would be
enough.
--
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: How to use saa7134 gpio via gpio-sysfs?

2010-01-15 Thread Trent Piepho
On Sat, 16 Jan 2010, hermann pitton wrote:
 Am Dienstag, den 12.01.2010, 04:13 +0100 schrieb hermann pitton:
   gpio-sysfs creates
   /sys/class/gpio/export
   /sys/class/gpio/import
   but no gpion entries so far.

You have to explictly export the GPIO lines to get them to appear.  Either
in the kernel with gpio_export() or from sysfs.  You also can't export
GPIOs that something in the kernel is using.  My original design didn't
have this restriction.  I felt there were valid debug and development
reasons for doing so, having used them myself for debug and development of
embedded systems, but David Brownell felt otherwise.  I didn't even have
the concept of export originally, all the gpios would show up.  After all,
all your PCI and USB devices, I2C chips or busses, etc.  show up in sysfs.
Nothing else does this must be exported to show up business.  You can
memory map the saa7133 PCI address space and modify the gpios, and anything
else, with direct register access from userspace, and that's just fine.
But being able to observe and modify the gpios with a gpio-only interface
is just way too dangerous.  Doesn't make sense.  But I'm digressing.

This sysfs interface only works with the gpio using the generic gpio layer.
This was designed for generic gpios on SoCs that would be providing by some
kind of platform driver or I2C gpio extenders.  The gpios get and used by
various other drivers.  Like say write protects on memory cards, or system
LEDs.  I wrote a JTAG driver that used it.  The point of the gpio layer is
to interface drivers the provide gpios with other, different, drivers that
use the gpio.  It was introduced in the mid 2.6.20s IIRC.

The saa713x driver predates the generic gpio layer by years and years, so
it doesn't use it.  It also doesn't need to use it.  Since the gpios are
managed by the saa713x driver, and they also used by the saa713x driver,
there is no need to interface two different drivers together.  There are
tons of drivers for devices that have gpios like this, but they don't use
the gpio layer.

But with gpio access via sysfs for generic gpios, there is something useful
about having the saa713x driver support generic gpios.  IIRC, somehow wrote
a gpio only bt848 driver that didn't do anything but export gpios.

In order to do this, you'll have to write code for the saa7134 driver to
have it register with the gpio layer.  I think you could still have the
saa7134 driver itself use its gpio directly.  That would avoid a
performance penalty in the driver.
--
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] Another approach to IR

2009-12-02 Thread Trent Piepho
On Wed, 2 Dec 2009, Jarod Wilson wrote:
 
  My main point is that each of these devices has device ID that can be 
  determined without having to first do some protocol analysis and table 
  lookups to figure out which device some random IR input is actually 
  coming from.
 
 
  Heh, right back at ya ;) The fact that you need to do some more work
  to separate 2 physical devices does not mean it should not be done.

 No, but it means added complexity inside the kernel. I'm questioning whether 
 the added complexity is worth it, when I doubt the vast majority of users 
 would take advantage of it, and it can already be done in userspace. 
 Although... Damn. The userspace approach would only work if the device were 
 passing raw IR to userspace, so in the in-kernel decoding case, yeah, I guess 
 you'd need separate input devices for each remote to use them independently. 
 Meh. Doubt I'd ever use it, but I guess I'll concede that it makes some sense 
 to do the extra work.

You just need to send a tuple that contrains the keycode plus some kind of
id for the remote it came from.  That's what I did for lirc, it decodes the
sparc/mark into a remote id and key code tuple.  It's certainly a common
thing to want.  Anyone who has existing remotes and components that use
them would want it.  You don't want your computer turning off when you push
the power button on the DVD player's remote, do you?

Each host connected to a network interface doesn't get its own device.
--
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] Another approach to IR

2009-12-02 Thread Trent Piepho
On Wed, 2 Dec 2009, Jon Smirl wrote:
  A bluetooth remote has a specific device ID that the receiver has to pair 
  with. Your usb mouse and keyboard each have specific device IDs. A usb IR 
  *receiver* has a specific device ID, the remotes do not. So there's the 
  major difference from your examples.

 Actually remotes do have an ID. They all transmit vendor/device pairs
 which is exactly how USB works.

*All* remotes?  That's a bold statement.  I'm sure there are some that
only transmit 8 bits of so of scancode.  I think remotes are more like
hosts on a network than usb or bluetooth devices.  Remotes do not join or
leave a receiver, while things like usb devices do explictly join and leave
the hub.

  Now I understand that if 2 remotes send completely identical signals we
  won't be able to separate them, but in cases when we can I think we
  should.
 
  I don't have a problem with that, if its a truly desired feature.  But
  for the most part, I don't see the point.  Generally, you go from
  having multiple remotes, one per device (where device is your TV,
  amplifier, set top box, htpc, etc), to having a single universal remote
  that controls all of those devices.  But for each device (IR receiver),
  *one* IR command set.  The desire to use multiple distinct remotes with
  a single IR receiver doesn't make sense to me.  Perhaps I'm just not
  creative enough in my use of IR.  :)

Most universal remotes I'm familiar with emulate multiple remotes.  I.e.
my tv remote generates one set of scancodes for the numeric keys.  The DVD
remote generates a different set.  The amplifier remote in tv mode
generates the same codes as the tv remote, and in dvd mode the same codes
as the dvd remote.  From the perspective of the IR receiver there is no
difference between having both the DVD and TV remotes, or using the
aplifier remote to control both devices.

Now, my aplifier remote has a number of modes.  Some control devices I
have, like vcr mode, and there is nothing I can do about that.  Some,
like md mode don't control devices I have.  That means they are free to
do things on the computer.  Someone else with the same remote (or any
number of remotes that use the same protocol and scancodes) might have
different devices.

So I want my computer to do stuff when I push JVC MD #xx keys, but ignore
JVC VCR #yyy yets.  Someone with an MD player and not a VCR would want to
opposite.  Rather than force everyone to create custom keymaps, it's much
easier if we can use the standard keymaps from a database of common remotes
and simply tell mythtv to only use remote #xxx or not to use remote #yyy.

It sounds like you're thinking of a receiver that came bundled with a
remote and that's it.  Not someone with a number of remotes that came with
different pieces of AV gear that they want to use with their computer.
--
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: Replace Mercurial with GIT as SCM

2009-12-01 Thread Trent Piepho
On Tue, 1 Dec 2009, Patrick Boettcher wrote:
 To start right away: I'm in favour of using GIT because of difficulties I
 have with my daily work with v4l-dvb. It is in my nature do to mistakes,
 so I need a tool which assists me in fixing those, I have not found a
 simple way to do my stuff with HG.

Try the mq extension.  It's included by default with mercurial, you just
need to add:
[extensions]
hgext.mq=
to your .hgrc file.  It lets you maintain a stack of patches that you can
freely push and pop.  You can make changes and then commit them to one of
the existing patches.  Like git commit -amend, except you can amend any
patch not just the last one.  IMHO, it's better than stock git when you're
trying to make a good patch series.  There is something called stgit which
is very much like mq and a little better I think.
--
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] Should we create a raw input interface for IR's ? - Was: Re: [PATCH 1/3 v2] lirc core device driver infrastructure

2009-11-26 Thread Trent Piepho
On Thu, 26 Nov 2009, Mauro Carvalho Chehab wrote:
  See above. Also, several protocols have a way to check if a keystroke were
  properly received. When handling just one protocol, we can use this to 
  double
  check the key. However, on a multiprotocol mode, we'll need to disable this
  feature.
 
  I don't think so. We can pass the space/mark data to all (configured,
  i.e. with active mapping) protocol handlers at once. Should a check
  fail, we ignore the data. Perhaps another protocol will make some sense
  out of it.

 What happens if it succeeds on two protocol handlers?

Then you use the protocol that fits best.  For instance decoding with one
protocol might produce a scancode that isn't assigned to any key, while
another protocol produces an assigned scancode.  Clearly then the latter is
most likely to be correct.  It also possible to have a space/mark length
that is within the allowable tolerances for one remote, but is even closer
another remote.  You don't want to just find *a* match, you want to find
the *best* match.

The in kernel code in v4l is very simple in that it is only designed to
work with one procotol and one remote.  Once you have multiple remotes of
any type things become much more complicted.  Keep in mind that remotes
that aren't even intended to be used with the computer but are used in the
same room will still be received by the receiver.  It's not enough to
decode the signals you expect to receive, you must also not get confused by
random signals destined for somewhere else.
--
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] Should we create a raw input interface for IR's ? - Was: Re: [PATCH 1/3 v2] lirc core device driver infrastructure

2009-11-26 Thread Trent Piepho
On Thu, 26 Nov 2009, Mauro Carvalho Chehab wrote:
  lircd supports input layer interface. Yet, patch 3/3 exports both devices
  that support only pulse/space raw mode and devices that generate scan
  codes via the raw mode interface. It does it by generating artificial
  pulse codes.
 
  Nonsense! There's no generation of artificial pulse codes in the drivers.
  The LIRC interface includes ways to pass decoded IR codes of arbitrary
  length to userspace.

 I might have got wrong then a comment in the middle of the
 imon_incoming_packet() of the SoundGraph iMON IR patch:

 + /*
 +  * Translate received data to pulse and space lengths.
 +  * Received data is active low, i.e. pulses are 0 and
 +  * spaces are 1.

I'm not sure about this specific code, but what is likely
going on here is the waveform is being RLE encoding.

For example, a cx88 receiver has two ways of being connected (without
using an external decoder chip).  One generates an IRQ on each
edge of the signal.  The time between IRQs gives mark/space lengths
which is what lirc expects.  This is how a simple serial port receiver
works too.

Another connections effectivly samples the waveform one bit deep at IIRC
4kHz.  I think that's what the code you are looking at gets.  The code
extracts the edges from the waveform and returns the time between them.  In
effect one is run length encoding a sequence of bits.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: IR raw input is not sutable for input system

2009-11-24 Thread Trent Piepho
On Wed, 25 Nov 2009, Maxim Levitsky wrote:

 Its not the case.
 There are many protocols, I know that by experimenting with my universal
 remote. There are many receivers, and all have different accuracy.
 Most remotes aren't designed to be used with PC, thus user has to invent
 mapping between buttons and actions.
 Its is not possible to identify remotes accurately, many remotes send
 just a 8 bit integer that specifies the 'model' thus many remotes can
 share it.

The signal recevied by the ir receiver contains glitches.  Depending on the
receiver there can be quite a few.  It is also not trivial to turn the raw
signal sent by the remote into a digital value, even if you know what to
expect.  It takes digital signal processing techniques to turn the messy
sequence of inaccurate mark and space lengths into a best guess at what
digital code the remote sent.

It's like turning raw VBI data into decoded ASCII teletext from a simulated
keyboard device, all in the kernel.

 Kernel job is to take the information from device and present it to
 userspace using uniform format, that is kernel does 1:1 translating, but
 doesn't parse the data.

One thing that could be done, unless it has changed much since I wrote it
10+ years ago, is to take the mark/space protocol the ir device uses and sent
that data to lircd via the input layer.  It would be less efficient, but
would avoid another kernel interface.  Of course the input layer to lircd
interface would be somewhat different than other input devices, so
it's not entirely correct to say another interface is avoided.
--
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] tuner-simple: possible read buffer overflow?

2009-10-03 Thread Trent Piepho
On Sat, 3 Oct 2009, Roel Kluin wrote:
 Prevent read from t_params-ranges[-1].

 Signed-off-by: Roel Kluin roel.kl...@gmail.com
 ---
 This is only required when t_params-count can be 0, can it?

Shouldn't be possible, or the tuner would be useless.

 - if (i == t_params-count) {
 + if (i == t_params-count  i) {
   tuner_dbg(frequency out of range (%d  %d)\n,
 *frequency, t_params-ranges[i - 1].limit);
   *frequency = t_params-ranges[--i].limit;
--
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: V4L2 drivers: potentially dangerous and inefficient msecs_to_jiffies() calculation

2009-09-14 Thread Trent Piepho
On Mon, 14 Sep 2009, Andreas Mohr wrote:
 cam-module_param.frame_timeout *
 1000 * msecs_to_jiffies(1) );
 multiple times each.
 What they should do instead is
 frame_timeout * msecs_to_jiffies(1000), I'd think.
 msecs_to_jiffies(1) is quite a bit too boldly assuming
 that all of the msecs_to_jiffies(x) implementation branches
 always round up.

It might also wait far longer than desired.  On a 100 Hz kernel a jiffie is
10 ms.  1000 * msecs_to_jiffies(1) will wait 10 seconds instead of 1.

But, I believe there is an issue with msecs_to_jiffies(x) overflowing for
very high values of x.  I'm not sure where that point is though.
--
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] Infrared Keycode standardization

2009-08-27 Thread Trent Piepho
On Thu, 27 Aug 2009, Devin Heitmueller wrote:
 The biggest challenge with that approach is that lirc is still
 maintained out-of-kernel, and the inputdev solution does not require
 lirc at all (which is good for inexperienced end users who want their
 product to just work).

If distros started packing lirc as a basic system daemon things would
generally just work too.  After all, there is plenty of other user space
software one needs to do anything.

 The other big issue is that right now remotes get associated
 automaticallywith products as part of the device profile.  While this
 has the disadvantage that there is not a uniform mechanism to specify
 a different remote than the one that ships with the product, it does
 have the advantage of the product working out-of-the-box with
 whatever remote it came with.  It's a usability issue, but what I
 would consider a pretty important one.

lirc isn't limited to one remote at a time.  You can have many different
remotes supported at once.  So it's not always necessary to know which
remote you have before the remote will work.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-dvb] Can ir polling be turned off in cx88 module for Leadtek 1000DTV card?

2009-08-26 Thread Trent Piepho
On Wed, 26 Aug 2009, Andy Walls wrote:
 On Wed, 2009-08-26 at 07:33 -0700, Dalton Harvie wrote:
If there isn't, would it be a good idea?

 Maybe.

  Thanks for any help.


 Try this.  It adds a module option noir that accepts an array of
 int's.  For a 0, that card's IR is set up as normal; for a 1, that
 card's IR is not initialized.

   # modprobe cx88 noir=1,1

I think this is a good idea.  I was going to do someting similar
to stop the excessive irqs from my cx88 cards, which don't
even have remote receivers.

I haven't tried, but maybe it is possible to only turn on polling when the
event device is opened.
--
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 review 6/6] radio-mr800: redesign radio-users counter

2009-08-08 Thread Trent Piepho
On Sat, 8 Aug 2009, Alexey Klimov wrote:
 Redesign radio-users counter. Don't allow more that 5 users on radio in

Why?

--
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: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb

2009-08-06 Thread Trent Piepho
On Thu, 6 Aug 2009, Hans Verkuil wrote:
 On Thursday 06 August 2009 20:10:38 Trent Piepho wrote:
  On Thu, 6 Aug 2009, Hans Verkuil wrote:
Why do you need two routines that will always return zero? Why to 
create a
code
that will never be used? v4l2-compat-ioctl32.c is already complex enough
without adding any bogus code.
  
   When the RDS encoder driver from Eduardo is added, then he will add the
   string controls to ctrl_is_pointer() since his driver is the first to
   actually implement string controls.
 
  Could one of the upper bits of the control id be used as a flag?  It would
  make it a lot easier to check the control's type than by using some inline
  function with a giant case statement that needs to be updated each time a
  new control is added.

 No, you can't. That would require users to OR that flag each time you want
 to use such a control. My original plan was to just check for a non-zero

There is no reason we need to allocated control IDs sequentially, is there?

So just give string controls IDs in the range 0x8000 to 0x and
then one can tell if a control is a string control be ANDing the id with
(131).
--
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: double unlock in bttv_poll() and in saa7134-video.c

2009-07-29 Thread Trent Piepho
On Sat, 25 Jul 2009, Dan Carpenter wrote:
 My source code checker, smatch (http://repo.or.cz/w/smatch.git),
 complains about a double unlock in bttv_poll() from
 drivers/media/video/bt8xx/bttv-driver.c.  It unlocks on line 3190 and
 again on 3201.

How about this:

http://linuxtv.org/hg/~tap/bttv?cmd=changeset;node=35ddb77b68f8

diff -r fd96af63f79b -r 35ddb77b68f8 
linux/drivers/media/video/bt8xx/bttv-driver.c
--- a/linux/drivers/media/video/bt8xx/bttv-driver.c Fri Jun 19 19:56:56 
2009 +
+++ b/linux/drivers/media/video/bt8xx/bttv-driver.c Wed Jul 29 16:33:45 
2009 -0700
@@ -3191,15 +3191,14 @@ static unsigned int bttv_poll(struct fil
return videobuf_poll_stream(file, fh-vbi, wait);
}

+   mutex_lock(fh-cap.vb_lock);
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
-   mutex_lock(fh-cap.vb_lock);
/* streaming capture */
if (list_empty(fh-cap.stream))
goto err;
buf = list_entry(fh-cap.stream.next,struct 
bttv_buffer,vb.stream);
} else {
/* read() capture */
-   mutex_lock(fh-cap.vb_lock);
if (NULL == fh-cap.read_buf) {
/* need to capture a new frame */
if (locked_btres(fh-btv,RESOURCE_VIDEO_STREAM))
@@ -3217,7 +3216,6 @@ static unsigned int bttv_poll(struct fil
fh-cap.ops-buf_queue(fh-cap,fh-cap.read_buf);
fh-cap.read_off = 0;
}
-   mutex_unlock(fh-cap.vb_lock);
buf = (struct bttv_buffer*)fh-cap.read_buf;
}

--
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: lsmod path hardcoded in v4l/Makefile

2009-07-28 Thread Trent Piepho
On Mon, 27 Jul 2009, Mauro Carvalho Chehab wrote:
 Em Tue, 21 Jul 2009 09:14:36 +0200
 Matthias Schwarzott z...@gentoo.org escreveu:
 It is not a good idea to run as root. Most people compile everything
 with a normal user and then use sudo command to install/remove/insert
 modules. Unfortunately, depending on the distribution, sudo inherits PATH from
 the normal user, instead of root. Due to that, if you replace it for just
 lsmod, it will fail for people that don't use gentoo.

 Maybe good solution is to test if lsmod (and other similar tools) are at /sbin
 or /usr/sbin.

 Alternatively, we can try to replace lsmod by something like (untested):

 v4l_modules := $(shell PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin lsmod|cut 
 -d' ' -f1 ) $(patsubst %.ko,%,$(inst-m))

Check my patch again, we can just delete the v4l_modules line as nothing
uses 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: [Bugme-new] [Bug 13709] New: b2c2-flexcop: no frontend driver found for this B2C2/FlexCop adapter w/ kernel-2.6.31-rc2

2009-07-22 Thread Trent Piepho
On Mon, 20 Jul 2009, Andrew Morton wrote:
 On Mon, 20 Jul 2009 13:21:33 -0700 (PDT)
 Trent Piepho xy...@speakeasy.org wrote:
  On Mon, 20 Jul 2009, Andrew Morton wrote:
  I produced a patch that fixed this problem over a month ago,
  http://www.linuxtv.org/hg/~tap/v4l-dvb/rev/748c762fcf3e

 Where is that patch now?  It isn't present in linux-next.

Mauro has how pulled it from me and so it will probably show up in his tree
soon.

 Also, is there any way of avoiding this?

 +#define FE_SUPPORTED(fe) (defined(CONFIG_DVB_##fe) || \
 + (defined(CONFIG_DVB_##fe##_MODULE)  defined(MODULE)))

 That's just way too tricky.  It expects all versions of the
 preprocessor to be correctly implemented (unlikely) and there are other
 tools like unifdef which want to parse kernel #defines.

What's so tricky about it?  A quick grep shows hundreds of uses of
## for concatenation.

 otoh the trick does produce a nice result and doing it any other way
 (which I can think of) would make a mess.

This particular kind of test is something that is used many times in the
dvb code.  This isn't the first time someone has gotten it wrong by a long
shot.  Mauro suggested, and I agree, that for the next kernel we should put
FE_SUPPORTED into a dvb header file and convert the many existing tests to
use it.  Maybe this will reduce the number of mistakes.

 Or just revert whichever patch broke things.  Your changelog describes
 this as simply A recent patch (bad changelog!) so I am unable to judge this.

It was commit d66b94b4aa2f40e134f8c07c58ae74ef3d523ee0, but it was not
committed until five days after I made my patch, so there was no hash to
reference yet.  I suppose I should have used the patch title.
--
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: [Bugme-new] [Bug 13709] New: b2c2-flexcop: no frontend driver found for this B2C2/FlexCop adapter w/ kernel-2.6.31-rc2

2009-07-20 Thread Trent Piepho
On Mon, 20 Jul 2009, Andrew Morton wrote:

 (switched to email.  Please respond via emailed reply-to-all, not via the
 bugzilla web interface).


 Guys, this is reportedly a post-2.6.30 regression - I'll ask Rafael to
 add it to the regression tracking list.

 btw, does the flexcop driver have a regular maintainer?  Or someone who
 wants to volunteer?  MAINTAINERS is silent about it..

I produced a patch that fixed this problem over a month ago,
http://www.linuxtv.org/hg/~tap/v4l-dvb/rev/748c762fcf3e

Maybe it should go into 2.6.31?

 Thanks.

 On Sun, 5 Jul 2009 01:36:31 GMT
 bugzilla-dae...@bugzilla.kernel.org wrote:

  http://bugzilla.kernel.org/show_bug.cgi?id=13709
 
 Summary: b2c2-flexcop: no frontend driver found for this
  B2C2/FlexCop adapter w/ kernel-2.6.31-rc2
 Product: v4l-dvb
 Version: unspecified
  Kernel Version: 2.6.31-rc1
Platform: All
  OS/Version: Linux
Tree: Mainline
  Status: NEW
Severity: normal
Priority: P1
   Component: dvb-frontend
  AssignedTo: v4l-dvb_dvb-front...@kernel-bugs.osdl.org
  ReportedBy: bugzilla.kernel@boris64.net
  Regression: Yes
 
 
  Hi kernel people!
 
  Since kernel-2.6.31-rc1 my Technisat SkyStar2 DVB card isn't
  working anymore, because no frontend driver is found.
  The frontend 'ST STV0299 DVB-S' is compiled into the kernel
  and _did_ work fine in pre-2.6.31 kernels.
 
 
  [lspci]
  ...
  05:02.0 Network controller: Techsan Electronics Co Ltd B2C2 FlexCopII DVB 
  chip
  / Technisat SkyStar2 DVB card (rev 02)
  [/lspci]
 
  [dmesg]
  Working kernel-2.6.30.1:
  
  ...
  b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
  successfully
  b2c2_flexcop_pci :05:02.0: PCI INT A - GSI 18 (level, low) - IRQ 18
  b2c2-flexcop: MAC address = 00:d0:d7:0f:30:58
  b2c2-flexcop: found 'ST STV0299 DVB-S' .
  b2c2-flexcop: initialization of 'Air2PC/AirStar 2 ATSC 3rd generation 
  (HD5000)'
  at the 'PCI' bus controlled by a 'FlexCopIIb' complete
  ...
 
  Non-working kernel-2.6.31-rc:
  
  ...
  b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded
  successfully
  b2c2_flexcop_pci :05:02.0: PCI INT A - GSI 18 (level, low) - IRQ 18
  b2c2-flexcop: MAC address = 00:d0:d7:0f:30:58
  b2c2-flexcop: no frontend driver found for this B2C2/FlexCop adapter
  b2c2_flexcop_pci :05:02.0: PCI INT A disabled
  ...
  [/dmesg]
 
 
  I'll attach full dmesg+lspci.
  Please feel free to contact me if you need more infos.
  Thank you in advance ;)
 
  --
  Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
  --- You are receiving this mail because: ---
  You are on the CC list for the bug.
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

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


Re: [PATCH 1/2] Compatibility layer for hrtimer API

2009-07-05 Thread Trent Piepho
On Fri, 3 Jul 2009, Jean Delvare wrote:
 Kernels 2.6.22 to 2.6.24 (inclusive) need some compatibility quirks
 for the hrtimer API. For older kernels, some required functions were
 not exported so there's nothing we can do. This means that drivers
 using the hrtimer infrastructure will no longer work for kernels older
 than 2.6.22.

 Signed-off-by: Jean Delvare kh...@linux-fr.org
 ---
  v4l/compat.h |   18 ++
  1 file changed, 18 insertions(+)

 --- a/v4l/compat.h
 +++ b/v4l/compat.h
 @@ -480,4 +480,22 @@ static inline unsigned long v4l_compat_f
  }
  #endif

 +/*
 + * Compatibility code for hrtimer API
 + * This will make hrtimer usable for kernels 2.6.22 and later.
 + * For earlier kernels, not all required functions are exported
 + * so there's nothing we can do.
 + */
 +
 +#if LINUX_VERSION_CODE  KERNEL_VERSION(2, 6, 25)  \
 + LINUX_VERSION_CODE = KERNEL_VERSION(2, 6, 22)
 +#include linux/hrtimer.h

Instead of including hrtimer.h from compat.h it's better if you check if it
has already been included and only enable the compat code in that case.
That way hrtimer doesn't get included for files that don't need it and
might define something that conflicts with something from hrtimer.  And it
prevents someone from forgetting to include hrtimer when they needed it,
but having the error masked because compat.h is doing it for them.

 +/* Forward a hrtimer so it expires after the hrtimer's current now */
 +static inline unsigned long hrtimer_forward_now(struct hrtimer *timer,
 + ktime_t interval)
 +{
 + return hrtimer_forward(timer, timer-base-get_time(), interval);
 +}
 +#endif
 +
  #endif /*  _COMPAT_H */
--
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: lsmod path hardcoded in v4l/Makefile

2009-06-23 Thread Trent Piepho
On Tue, 23 Jun 2009, Matthias Schwarzott wrote:
  On Mon, 2009-06-22 at 16:36 +0200, Matthias Schwarzott wrote:
   It seems the path to lsmod tool is hardcoded in the Makefile for
   out-of-tree building of v4l-dvb.
 
 Shouldn't $PATH of root be considered safe? Else the distro or the system

I believe make will set the variable whenever the makefile is used, even
when building as non-root.

It turns out that it was just lsmod with no path originally, but Michael
Krufky changed it back in 2005 (commit b0e7b40744ef) to have a hardcoded
path.  Then later in commit c91e7f84a1d6 the only use of 'v4l_modules' was
deleted, so we can just delete this line and not worry about sbin and
paths.

Mauro,

Please pull from http://linuxtv.org/hg/~tap/fix

for the following changeset:

build: Remove module list cruft
http://linuxtv.org/hg/~tap/fix?cmd=changeset;node=fb228bb1ad9f
--
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: lsmod path hardcoded in v4l/Makefile

2009-06-22 Thread Trent Piepho
On Mon, 22 Jun 2009, Andy Walls wrote:
 On Mon, 2009-06-22 at 16:36 +0200, Matthias Schwarzott wrote:
  Hi list!
 
  It seems the path to lsmod tool is hardcoded in the Makefile for out-of-tree
  building of v4l-dvb.
  Now at least gentoo has moved lsmod from /sbin to /bin.

Won't your patch cause breakage for everyone who hasn't moved lsmod from
/sbin and doesn't have sbin in the path?  Which was, and perhaps still is,
the most common situation?  It would be better to do something that does
not break things that used to work.

  Additionally it is bad style (or at least I am told so), to not rely on 
  $PATH
  but hardcode pathes for tools that should be in $PATH.

 It's a potential security hole to rely on $PATH instead of absolute
 paths when running a command as root.

 Since many of the commnads in the Makefile rely on $PATH, including
 executions of 'install' which usually would be run as root, I suppose
 secuirty concerns don't matter.

 -Andy

  So the attached patch removes the hardcoded /sbin from the lsmod call.
 
  Signed-off-by: Matthias Schwarzott z...@gentoo.org
 
  Regards
  Matthias

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

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


Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-misc

2009-06-20 Thread Trent Piepho
On Sat, 20 Jun 2009, Hans Verkuil wrote:
 - compat: fix __fls check for the arm architecture.

This one isn't quite right.  The __fls defined for arm in 2.6.27 (between
v2.6.26-7260-g0c65f45 and v2.6.28-rc6-187-g94fc733) isn't the same as the
__fls() used everwhere else in the kernel.  This alternate fix should
work.

01/01: compat: Fix __fls for certain ARM kernels
http://linuxtv.org/hg/~tap/fix?cmd=changeset;node=518b7754cd3d
--
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] af9015: avoid magically sized temporal buffer in eeprom_dump

2009-06-18 Thread Trent Piepho
On Thu, 18 Jun 2009, Jan Nikitenko wrote:
 Replace printing to magically sized temporal buffer with use of KERN_CONT

temporary not temporal.

 - sprintf(buf2, %02x , val);
 + deb_info(KERN_CONT,  %02x, val);

No comma after KERN_CONT

   else
 - strcpy(buf2, -- );
 - strcat(buf, buf2);
 + deb_info(KERN_CONT,  --);

No comma after KERN_CONT

Just use printk() instead of deb_info() for the ones that use KERN_CONT.

   if (reg == 0xff)
   break;
   }
 - deb_info(%s\n, buf);
 + deb_info(KERN_CONT \n);
   return 0;
  }

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

--
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] adding support for setting bus parameters in sub device

2009-06-17 Thread Trent Piepho
On Wed, 17 Jun 2009, Guennadi Liakhovetski wrote:
 On Wed, 17 Jun 2009, Hans Verkuil wrote:
  It is my strong opinion that while autonegotiation is easy to use, it is
  not a wise choice to make. Filling in a single struct with the bus
  settings to use for each board-subdev combination (usually there is only
  one) is simple, straight-forward and unambiguous. And I really don't see
  why that should take much time at all. And I consider it a very good point
  that the programmer is forced to think about this for a bit.

 Ok, my opinion is, that we should keep autonegotiation, but if you like,
 we can print a BIG-FAT-WARNING if both polarities are supported and no
 platform preference is set.

 I think, we've heard all opinions, unless someone would like to add
 something? Would it be fair to ask Mauro to make a decision? Or we can
 just count votes (which I would obviously prefer), but I'll accept Mauro's
 decision too.

There is a similar situation in the networking code, where there is a
driver for a PHY and another driver for a MAC, much like a sensor and
bridge.  phylib will find the common subset of the supported modes between
the MAC and the detected PHY and use that to configure aneg advertisement
settings.
--
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: [PULL] generic image bounds setting and alignment function

2009-06-17 Thread Trent Piepho
On Wed, 17 Jun 2009, Guennadi Liakhovetski wrote:
 On Tue, 16 Jun 2009, Trent Piepho wrote:

   /* up the smaller alignment until we have enough */
   do {
 - if (walign = halign  walign  wmaxa) {
 + if (halign = hmaxa ||
 + (walign = halign  walign  wmaxa)) {

 Do I understand it right now, that as soon as your halign now reaches
 hmaxa, you'll stop incrementing halign and then keep incrementing walign
 even beyond wmaxa?...

Yes.

It's clearly documented that the function isn't designed to handle
impossible constraints.  If both walign and halign are at max and there
still needs to be more alignment then there is no possible resolution, no
matter what width  height started at.

This is just one of many ways the constraints could be impossible.  Max
width being less than min width is another obvious example.  I decided it
wasn't worth complicating the code to check for all of these things.  And
if the function did return failure the callers would need to check that.
None of the code v4l_bound_align_image() replaces has a failure path for
impossible image size constraints, because the constraints aren't
impossible.
--
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: bttv problem loading takes about several minutes

2009-06-17 Thread Trent Piepho
On Wed, 17 Jun 2009, Halim Sahin wrote:
 Hi,
 In the past I could use this card by typing
 modprobe bttv card=34 tuner=24 gbuffers=16

What card do you actually have?  What is the PCI subsystem vendor/device
and what tuner does it actually have?

Hans, the problem might be with bttv audio probing.  This card has
needs_tvaudio set to 0, which used to mean that tvaudio would not be probed
or loaded.  But with your changes to bttv audio probing this behavior has
changed.  Now tvaudio is always loaded and probed if another audio chip
hasn't been detected.  The needs_tvaudio field is totally ignored.

 Giving this command with current drivers has some problems:
 1. it takes several minutes to load bttv module.
 2. capturing doesn't work any more (dropped frames etc).
 Tested with current v4l-dvb from hg, ubuntu 9.04,
 debian lenny.

 I have a bt878  based card from leadtek.

 Here is my output after loading the driver:
 [ 3013.735459] bttv: driver version 0.9.17 loaded
 [ 3013.735470] bttv: using 32 buffers with 16k (4 pages) each for capture
 [ 3013.735542] bttv: Bt8xx card found (0).
 [ 3013.735562] bttv0: Bt878 (rev 17) at :00:0b.0, irq: 19, latency: 32, 
 mmio
 : 0xf780
 [ 3013.737762] bttv0: using: Leadtek WinFast 2000/ WinFast 2000 XP 
 [card=34,insm
 od option]
 [ 3013.737825] bttv0: gpio: en=, out= in=003ff502 [init]
 [ 3148.136017] bttv0: tuner type=24
 [ 3148.136029] bttv0: i2c: checking for MSP34xx @ 0x80... not found
 [ 3154.536019] bttv0: i2c: checking for TDA9875 @ 0xb0... not found
 [ 3160.936018] bttv0: i2c: checking for TDA7432 @ 0x8a... not found
 [ 3167.351398] bttv0: registered device video0
 [ 3167.351434] bttv0: registered device vbi0
 [ 3167.351463] bttv0: registered device radio0
 [ 3167.351485] bttv0: PLL: 28636363 = 35468950 . ok
 [ 3167.364182] input: bttv IR (card=34) as /class/input/input6

 Please help!
 Regards
 Halim


 --
 Halim Sahin
 E-Mail:
 halim.sahin (at) t-online.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

--
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: [PULL] generic image bounds setting and alignment function

2009-06-17 Thread Trent Piepho
On Wed, 17 Jun 2009, Guennadi Liakhovetski wrote:
 On Tue, 16 Jun 2009, Trent Piepho wrote:
  On Tue, 16 Jun 2009, Guennadi Liakhovetski wrote:
01/14: compat: handle __fls
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=c4b55ce6c273
   
02/14: v4l2: Create helper function for bounding and aligning images
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=b4d3ec8d363d
  
   I am sorry, I will not bother with saving, reformatting, pasting... Just
   wanted to ask about this place:
 
  I guess you do not use Mercurial like all other v4l-dvb developers?

 I do use hg, though not for development, but for interacting with all
 other v4l-dvb developers

  Because you are making a big deal about a simple operation than can be done
  with a few keystrokes.  If I wanted this patch quoted in my editor, I can
  simply type:  !!cd ~/v4l-dvb ; hg export b4d3ec8d363d | sed 's/^/ /'

 No, typing this is not a big deal, as you say. But I do not understand
 _wny_ everyone, wishing to review / comment on your patches has to do
 that. And another problem of your approach you confirm yourself in this
 post:

Using pull requests is something all v4l developers, besides you, do as
well.  No one, besides you, seems to find it a problem.  It's been this way
for years.  I'm not the one who came up with this system.  Sometimes one
needs to go the mountain instead of expecting the mountain to come to
oneself.

 See? Now hg will have two patches, which Mauro will then have to merge
 into one when exporting to git, and which then will be very difficult to

Oh well.  It's happened plenty of times before.  I try not to make a habit
of it.

One can find many many patches in the linux git tree that have bugs in
them.  Often there are patches that fix these bugs included in the same
upstream merge.  IOW, the bug was found and fixed before the patch was
merged upstream but the fix wasn't folded into the original patch, because
the original patch was already in git and someone didn't want to do a
git-rebase.  One advantage of the hg tree is we get an extra opportunity to
fix these things before sending them to git.

 everyone gets to see and review your patches only when they are already in
 your external repository ready to be pulled by the maintainer.

s/in your/posted in the/; s/repository/mailing list/; s/pulled/applied/;

everyone gets to see and review your patches only when they are already
posted in the external mailing list ready to be applied by the maintainer.

Is that really any different?
--
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: [PATCHv7 2/9] v4l2: video device: Add V4L2_CTRL_CLASS_FM_TX controls

2009-06-16 Thread Trent Piepho
On Tue, 16 Jun 2009, Eduardo Valentin wrote:
 On Sun, Jun 14, 2009 at 06:59:13PM +0200, ext Hans Verkuil wrote:
  On Sunday 14 June 2009 18:23:41 Trent Piepho wrote:
 similar V4L2_CID_MPEG_EMPHASIS control and others might well appear 
 in the
 future, so I think this name should be more specific to the FM_TX API.
  
   The cx88 driver could get support for setting the fm preemphasis via a
   control.  I added support via a module option, but a control would be
   better.  You're saying it shouldn't use this fm preemphasis control?
 
  Correct. This set the pre-emphasis when transmitting. For receiving you want
  a separate control. Although the enum should be made generic. So FM_TX can 
  be
  removed from the enum.
 
  Why should we have one rx and one tx control for this? Because you can have
  both receivers and transmitters in one device and you want independent 
  control
  of the two.

 Yes, agreed here. There is the possibility to have receiver and transmitter
 both in the same device. So, I think it is better to have separated controls.

Is both a receiver and transmitter in the same device different than having
two receivers or two transmitters?  In which case, since controls are not
assigned to a specific input, how does one handle that?
--
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: [PULL] http://kernellabs.com/hg/~mkrufky/k2c2

2009-06-16 Thread Trent Piepho
On Tue, 16 Jun 2009, Mauro Carvalho Chehab wrote:
 Em Tue, 16 Jun 2009 11:19:29 -0400
 Michael Krufky mkru...@linuxtv.org escreveu:
  +static int cx23885_dvb_set_frontend(struct dvb_frontend *fe,
  +   struct dvb_frontend_parameters *param)

You could make this an HVR1275 specific function and then do away with the
first case statement.  With a name like cx23885_dvb_set_frontend() it
appears that all boards will use it, when it is only ever called with an
HVR1275.

  +   switch (param-u.vsb.modulation) {
  +   case VSB_8:
  +   cx23885_gpio_clear(dev, GPIO_5);
  +   break;
  +   case QAM_64:
  +   case QAM_256:
  +   default:
  +   cx23885_gpio_set(dev, GPIO_5);
  +   break;

Using the modulation to switch inputs is of course a hack, needed because
the dvb api lacks a concept of multiple inputs.  I do not know if any still
exist, but there were cable systems which broadcast with both QAM and VS.


 Argh! this looks like a hack! Don't you have a better approach for 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: [PULL] generic image bounds setting and alignment function

2009-06-16 Thread Trent Piepho
On Tue, 16 Jun 2009, Mauro Carvalho Chehab wrote:
 Em Tue, 16 Jun 2009 17:57:20 +0200 (CEST)
 Guennadi Liakhovetski g.liakhovet...@gmx.de escreveu:
  On Sat, 30 May 2009, Trent Piepho wrote:
  +   if (walign + halign  salign) {
  +   /* Max walign where there is still a valid width */
  +   unsigned int wmaxa = __fls(wmax ^ (wmin - 1));
 
  I cannot follow correctness of the above, sorry. Take a simple example:
  wmax=0x7f, wmin=7, wmaxa = __fls(0x7f ^ 6) = __fls(0x79) = 0. And the
  comment says it's the maximum walign where there is still a valid width.
  What am I missing?
 
  +
  +   /* up the smaller alignment until we have enough */
  +   do {
  +   if (walign = halign  walign  wmaxa) {
 

 As I'm still cooking the patches, I prefer to postpone the align ones until we
 are comfortable with them.

 Trent,

 Could you please take a look on the above comments

There is no bug with the wmaxa code, but there is a different bug
elsewhere.

Please pull from http://linuxtv.org/hg/~tap/fix

for the following changeset:

01/01: v4l2: Fix flaw in alignment code
http://linuxtv.org/hg/~tap/fix?cmd=changeset;node=4ef7fb102b6c
--
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] High resolution timer for cx88 remotes

2009-06-16 Thread Trent Piepho
On Tue, 16 Jun 2009, Mauro Carvalho Chehab wrote:
 Em Sat, 23 May 2009 14:06:01 +0200
 AH andrzej.ha...@wp.pl escreveu:
  Patched driver seems to work on my system, with kernel 2.6.28.
  I have removed kernel checks for versions below 2.6.20 - they were
  because of API changes in scheduler.

If you are going to break compatibility below 2.6.20 then you should add
the driver to versions.txt
--
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: [PATCHv7 2/9] v4l2: video device: Add V4L2_CTRL_CLASS_FM_TX controls

2009-06-14 Thread Trent Piepho
On Sun, 14 Jun 2009, Eduardo Valentin wrote:
  +/* FM Modulator class control IDs */
  +#define V4L2_CID_FM_TX_CLASS_BASE  (V4L2_CTRL_CLASS_FM_TX | 0x900)
  +#define V4L2_CID_FM_TX_CLASS ? ? ? ? ? ? ? ? (V4L2_CTRL_CLASS_FM_TX | 1)
  +
  +#define V4L2_CID_RDS_ENABLED ? ? ? ? ? ? ? ? (V4L2_CID_FM_TX_CLASS_BASE + 
  1)
  +#define V4L2_CID_RDS_PI ? ? ? ? ? ? ? ? ? ? ? ? ? ? 
  ?(V4L2_CID_FM_TX_CLASS_BASE + 2)
  +#define V4L2_CID_RDS_PTY ? ? ? ? ? ? ? ? ? ? (V4L2_CID_FM_TX_CLASS_BASE + 
  3)
  +#define V4L2_CID_RDS_PS_NAME ? ? ? ? ? ? ? ? (V4L2_CID_FM_TX_CLASS_BASE + 
  4)
  +#define V4L2_CID_RDS_RADIO_TEXT ? ? ? ? ? ? ? ? ? ? 
  ?(V4L2_CID_FM_TX_CLASS_BASE + 5)
 
  I think these RDS controls should be renamed to V4L2_CID_RDS_TX_. This makes
  it clear that these controls relate to the RDS transmitter instead of a
  receiver. I would not be surprised to see similar controls appear for an RDS
  receiver in the future.

So there should there be different controls to set the same thing, one set
for tx and another for rx?

  +#define V4L2_CID_PREEMPHASIS ? ? ? ? ? ? ? ? (V4L2_CID_FM_TX_CLASS_BASE + 
  17)
  +enum v4l2_fm_tx_preemphasis {
  + ? ? V4L2_FM_TX_PREEMPHASIS_DISABLED ? ? ? ? = 0,
  + ? ? V4L2_FM_TX_PREEMPHASIS_50_uS ? ? ? ? ? ?= 1,
  + ? ? V4L2_FM_TX_PREEMPHASIS_75_uS ? ? ? ? ? ?= 2,
  +};
 
  I suggest renaming this to V4L2_CID_FM_TX_PREEMPHASIS. There is already a
  similar V4L2_CID_MPEG_EMPHASIS control and others might well appear in the
  future, so I think this name should be more specific to the FM_TX API.

The cx88 driver could get support for setting the fm preemphasis via a
control.  I added support via a module option, but a control would be
better.  You're saying it shouldn't use this fm preemphasis control?
--
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 -next] v4l: expose function outside of ifdef/endif block

2009-06-12 Thread Trent Piepho
On Fri, 12 Jun 2009, Randy Dunlap wrote:
 From: Randy Dunlap randy.dun...@oracle.com

 Move v4l_bound_align_image() outside of an #ifdef CONFIG_I2C block
 so that it is always built.  Fixes a build error:

clamp_align() should be moved as well, since it's only used by
v4l_bound_align_image().  I'm attaching an alternate version that fixes
this.  Labeled the endif too.

  drivers/media/video/v4l2-common.c |3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 --- linux-next-20090612.orig/drivers/media/video/v4l2-common.c
 +++ linux-next-20090612/drivers/media/video/v4l2-common.c
 @@ -915,6 +915,7 @@ const unsigned short *v4l2_i2c_tuner_add
   return NULL;
  }
  EXPORT_SYMBOL_GPL(v4l2_i2c_tuner_addrs);
 +#endif

  /* Clamp x to be between min and max, aligned to a multiple of 2^align.  min
   * and max don't have to be aligned, but there must be at least one valid
 @@ -986,5 +987,3 @@ void v4l_bound_align_image(u32 *w, unsig
   }
  }
  EXPORT_SYMBOL_GPL(v4l_bound_align_image);
 -
 -#endif# HG changeset patch
# User Trent Piepho xy...@speakeasy.org
# Date 1244834958 25200
# Node ID 23bd6516eafcc06ffb590073e744c7e17382aef9
# Parent  01fd4e3bd1c0fb52cb15acbd22ca7f4857170e6e
v4l2: Move bounding code outside I2C ifdef block

From: Trent Piepho xy...@speakeasy.org

Move v4l_bound_align_image() and clamp_align() outside of an ifdef block
for I2C related code.  Can cause an undefined reference to
`v4l_bound_align_image' if I2C isn't enabled.

Priority: high

Signed-off-by: Trent Piepho xy...@speakeasy.org
Reported-by: Randy Dunlap randy.dun...@oracle.com

diff -r 01fd4e3bd1c0 -r 23bd6516eafc linux/drivers/media/video/v4l2-common.c
--- a/linux/drivers/media/video/v4l2-common.c   Thu Jun 11 15:31:22 2009 -0700
+++ b/linux/drivers/media/video/v4l2-common.c   Fri Jun 12 12:29:18 2009 -0700
@@ -997,6 +997,8 @@ const unsigned short *v4l2_i2c_tuner_add
 }
 EXPORT_SYMBOL_GPL(v4l2_i2c_tuner_addrs);
 
+#endif /* defined(CONFIG_I2C) */
+
 /* Clamp x to be between min and max, aligned to a multiple of 2^align.  min
  * and max don't have to be aligned, but there must be at least one valid
  * value.  E.g., min=17,max=31,align=4 is not allowed as there are no multiples
@@ -1067,5 +1069,3 @@ void v4l_bound_align_image(u32 *w, unsig
}
 }
 EXPORT_SYMBOL_GPL(v4l_bound_align_image);
-
-#endif


Re: PULL request - http://linuxtv.org/hg/~pb/v4l-dvb/

2009-06-11 Thread Trent Piepho
On Thu, 11 Jun 2009, Patrick Boettcher wrote:
 On Wed, 27 May 2009, Trent Piepho wrote:
  On Tue, 26 May 2009, Patrick Boettcher wrote:
  Does this patch to fix these problems look ok?

 In fact, everything looks correct in my eyes. I'll ask Mauro to pull any
 minute from now.

 I even have an explaination why the failing attach of the itd1000 was
 not errored out: when I did the development I was using a userspace
 proprietary driver to validate, for that I needed the demod to be
 attached...

 Thanks for cleaning up this mess.

Now that that patch is done, here is the rest of the series with dvb-pll
conversions.  There is an additional patch to the flexcop card detecting
code.  The #if defined(CONFIG_...) stuff didn't take into account code
being compiled into the kernel.

Here's the series:

01/08: dvb-pll: Add Samsung TDTC9251DH0 DVB-T NIM
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=3d148fee550b

02/08: dvb-pll: Add support for Samsung TBDU18132 DVB-S NIM
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=5a8e32e94aa7

03/08: dvb-pll: Add support for Samsung TBMU24112 DVB-S NIM
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=c28394056a5a

04/08: dvb-pll: Add support for Alps TDEE4 DVB-C NIM
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=90f54e498f90

05/08: b2c2: fix frontends compiled into kernel
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=748c762fcf3e

06/08: b2c2: Use dvb-pll for AirStar DVB-T's tuner
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=fcacc65753f1

07/08: b2c2: Use dvb-pll for Skystar2 rev 2.3 and rev 2.6
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=8ef37f1432e6

08/08: b2c2: Use dvb-pll for Cablestar2
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=01fd4e3bd1c0


 b2c2/flexcop-fe-tuner.c |  283 +++-
 frontends/dvb-pll.c |   75 
 frontends/dvb-pll.h |4
 3 files changed, 172 insertions(+), 190 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: S_FMT vs. S_CROP

2009-06-10 Thread Trent Piepho
On Wed, 10 Jun 2009, Hans Verkuil wrote:
 On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote:
  This question - how S_FMT and S_CROP affest image geometry - has been
  discussed at least twice before - that's only with my participation,
  don't know if and how often it has come up before. But the fact, that in
  two discussions we came up with different results seems to suggest, that
  this is not something trivially known by all except me.
 
  First time I asked this question in this thread
 
  http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html
 
  and Mauro replied (see above thread for a complete reply):
 
  On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote:
   On Wed, 7 Jan 2009 10:14:31 +0100 (CET)
   Guennadi Liakhovetski g.liakhovet...@gmx.de wrote:
 
  [snip]
 
For example on mt9t031
binning and skipping are used for that. Whereas CROP uses the current
scaling configuration and selects a sub-window, so, once you've done
S_FMT to 320x240, a crop request for 640x480 might well fail.
  
   I also understand this way. You cannot crop with a resolution bigger
   than what you've selected.
 
  (Let's call this statement M1:-))

 If I read the spec correctly, in particular section 1.11.1, then cropping
 comes before scaling, so you can crop to 640x480 (S_CROP) and scale that to
 320x240 (S_FMT). S_FMT scales the cropped rectangle.

This is my understanding of how it's supposed to work as well.
--
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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/

2009-06-09 Thread Trent Piepho
On Tue, 9 Jun 2009, Mauro Carvalho Chehab wrote:
 Em Tue, 9 Jun 2009 18:08:38 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:


Should I submit a patch that implement VIDIOC_S_JPEGCOMP support in the
UVC driver and implement a JPEG compression quality control later, or
would you prefer the driver not to implement VIDIOC_S_JPEGCOMP at all ?
As there are existing applications using that ioctl a few users are
pushing for VIDIOC_S_JPEGCOMP support.
  
   I prefer the later. Adding a new ioctl support just to deprecate it on
   the next kernel doesn't seem nice. Let's add directly the newer controls
   and add a patch marking this as deprecated.
 
  I'm not sure whether we can deprecate JPEGCOMP. It is used in too many
  places. Perhaps we should create a set of JPEG controls that match what is
  in v4l2_jpegcompression and convert all the drivers that use JPEGCOMP to
  these new controls. Then the v4l core can map S/G_JPEGCOMP ioctls to a set
  of control read/writes. I'm working on string control support, so that will
  allow us to handle the APP_data and COM_data fields.

 This seems to be the correct approach. Implement it as controls, and let
 video_ioctl2 to handle the calls to the legacy ioctls, while marking it as
 deprecate.

One problem is that COM and APP segment actually have a string of bytes
associated with them.  Right now we only have boolean and interger
controls.  There is no way to set a control to the 32-bytes you want put
into a APP segment.

Now, if we added NUL terminated string and fixed length byte arrays as
control types this could be done.  I know that's something I've mentioned
before.  It would also be a step to allowing drivers to export metadata
about captured images (exposured info, focus tracking, etc.) via a
control-like interface.
--
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] bt8xx: Add support for the Conexant Fusion 878a / Twinhan VP 1025 DVB-S

2009-06-07 Thread Trent Piepho
On Sun, 7 Jun 2009, Michael Stapelberg wrote:
 Add fefe:0001 to the list of identifiers for the bt8xx driver. The chip is
 named Conexant Fusion 878a, the card is a Twinhan VP 1025 DVB-S PCI.

 Please commit the attached patch.

You can remove Conexant Fusion from the board name.  All the boards for
that driver use that same chip.  Just use Twinhan VP 1025 DVB-S.

Don't you need to add it to the list in bttv-cards.c in order to use the
card?
--
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: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function

2009-06-06 Thread Trent Piepho
On Sat, 6 Jun 2009, Hans Verkuil wrote:
 On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
  I propose to change the API as follows:
 
  #define V4L2_I2C_ADDRS(addr, addrs...) \
  ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
 
  struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
  struct i2c_adapter *adapter,
  const char *module_name, const char *client_type,
  u8 addr, const unsigned short *addrs);
 
  struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
  struct i2c_adapter *adapter,
  const char *module_name, const char *client_type,
  int irq, void *platform_data,
  u8 addr, const unsigned short *addrs);
 
  /* Only available for kernels = 2.6.26 */
  struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
  *v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
  i2c_board_info *info, const unsigned short *addrs);

Maybe addrs could be changed to something like probed_addrs so it's
easier to tell that the two address fields are used differently?

Is v4l2_i2c_new_subdev() in effect just a wrapper around
v4l2_i2c_new_subdev_cfg() that calls it with NO_IRQ and NULL for irq and
platform_data?

And could v4l2_i2c_new_subdev_cfg() be done like this?

struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
 struct i2c_adapter *adapter, const char *module_name,
 const char *client_type, int irq, void *platform_data,
 u8 addr, const unsigned short *addrs)
{
struct i2c_board_info info = {
.addr = addr, .platform_data = platform_data, .irq = irq, };

strlcpy(info.type, client_type, sizeof(info.type));
return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
 info, addrs);
}
--
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: proposal for new i2c.h macro to initialize i2c address lists on the fly

2009-06-06 Thread Trent Piepho
On Sat, 6 Jun 2009, Hans Verkuil wrote:
 For video4linux we sometimes need to probe for a single i2c address.
 Normally you would do it like this:

 static const unsigned short addrs[] = {
   addr, I2C_CLIENT_END
 };

 client = i2c_new_probed_device(adapter, info, addrs);

 This is a bit awkward and I came up with this macro:

 #define V4L2_I2C_ADDRS(addr, addrs...) \
 ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })

 This can construct a list of one or more i2c addresses on the fly. But this
 is something that really belongs in i2c.h, renamed to I2C_ADDRS.

 With this macro we can just do:

 client = i2c_new_probed_device(adapter, info, I2C_ADDRS(addr));

 Comments?

I agree it's better placed with i2c.h.  It's also possible to come up with
a more syntax more variadic function like, which can be called like this:

i2c_new_probed_device(adapter, info, addr1);

i2c_new_probed_device(adapter, info, addr1, addr2);


Using a macro like this...

#define i2c_new_probed_device(adap,info,addr,addrs...) ({ \
const unsigned short _addrs[] = {addr, ## addrs, I2C_CLIENT_END }; \
_i2c_new_probed_device(adap, info, _addrs); })
--
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: [PULL] generic image bounds setting and alignment function

2009-06-03 Thread Trent Piepho
On Mon, 1 Jun 2009, Robert Jarzmik wrote:
 Trent Piepho xy...@speakeasy.org writes:
  Please pull from http://linuxtv.org/hg/~tap/v4l-dvb
 
  This series adds a function for bounding and alignment image sizes and
  modifies a number of drivers to use it.  It came up when the pxa patches to
  deal with the alignment issues for that driver were posted.  I haven't
  tested these patches for pxa.

 Hi Trent,

 I didn't see the review of that serie, I'm curious what others said.
 As for my comments, I'll inline your code, sorry about that.

  02/14: v4l2: Create helper function for bounding and aligning images
  http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=b4d3ec8d363d
 
 +static unsigned int clamp_align(unsigned int x, unsigned int min,
 + unsigned int max, unsigned int align)
 +{
 + /* Bits that must be zero to be aligned */
 + unsigned int mask = ~((1  align) - 1);
 +
 + /* Round to nearest aligned value */
 + if (align)
 + x = (x + (1  (align - 1)))  mask;

 If I'm not mistaken, these lines are an equivalent of :
   balign = 1  align;
 if (align)
   x = ALIGN(x + 1 - balign/2, balign);

 Isn't that simpler to read ?

I don't think so, it's not obvious that it will round to the nearest value.
You have to look up ALIGN and then __ALIGN_MASK and see that it will in
effect add balign - 1 and then reduce x + 1 - balign/2 + balign - 1 into x
+ balign/2.

It also generates worse code.  You'd think the compiled would be able to
optimize it into the same code, but it doesn't.  Unless there is some issue
with how it will work with negative values that causes a subtle difference.
--
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: webcam drivers and V4L2_MEMORY_USERPTR support

2009-06-01 Thread Trent Piepho
On Mon, 1 Jun 2009, Stefan Kost wrote:
 I have implemented support for V4L2_MEMORY_USERPTR buffers in gstreamers
 v4l2src [1]. This allows to request shared memory buffers from xvideo,
 capture into those and therefore save a memcpy. This works great with
 the v4l2 driver on our embedded device.

 When I was testing this on my desktop, I noticed that almost no driver
 seems to support it.
 I tested zc0301 and uvcvideo, but also grepped the kernel driver
 sources. It seems that gspca might support it, but I ave not confirmed
 it. Is there a technical reason for it, or is it simply not implemented?

userptr support is relatively new and so it has less support, especially
with driver that pre-date it.  Maybe USB cams use a compressed format and
so userptr with xvideo would not work anyway since xv won't support the
camera's native format.  It certainly could be done for bt8xx, cx88,
saa7134, etc.
--
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] Leadtek WinFast DTV-1800H support

2009-05-31 Thread Trent Piepho
On Sun, 31 May 2009, Miroslav [UTF-8]  ??ustek wrote:
 Trent Piepho xyzzy at speakeasy.org writes:

  Instead of raising the reset line here, why not change the gpio settings in
  the card definition to have it high?  Change gpio1 for television to 0x7050
  and radio to 0x7010.
 Personally, I don't know when these .gpioX members are used (before
 firmware loads or after...).
 But I assume that adding the high on reset pin shouldn't break anything,
 so we can do this.

They are used whenever the video mux is set.  That will happen when
changing inputs and when the cx8800 driver first initializes the card.
Opening the radio device does an implicit change to the radio input.

It looks like firmware is loaded when needed as part of setting the tuner
frequency.  I think that makes it safe to assume that the gpios will be set
before firmware is loaded.   Though it might be possible if the cx8802
driver is loaded before the cx8800 driver.

Since you have the hardware, it would be easy to check.  Add printk's in
the reset callback code you wrote so you'll know when it's called.  Then
set video_debug to 1 and look for video_mux:   lines, which indicate the
card gpio values are being set.

It seems clear that the xc2028 has an active low reset line.  To reset, the
line must be lowered for some time period (probably very short) and then
raised to enable the chip, at which point there should be a delay (probably
longer) while waiting for the chip to come out of reset.  If you think
about it, it does not matter what state the reset line is in before we
lower it.  It can be high, it can be low, the chip be reset either way.

 And shouldn't we put tuner reset pin to 0 when in composite and s-video mode?
 These inputs don't use tuner or do they?

It should be unnecessary to do that, but might help with power consumption.
To do it, change the composite and s-video gpio1 values to 0x7060.

 If I look in dmesg I can see that firmware is loaded into tuner even
 when these modes are used (I'm using MPlayer to select the input).
 And due to callbacks issued duting firmware loading, tuner is turned on
 (reset pin = 1) no matter if it was turned off by .gpioX setting.

It seems like firmware loaded should only happen on frequency change.

 And shouldn't we use the mask bits [24:16] of MO_GPX_IO
 in .gpioX members too? I know only few GPIO pins and the other I don't
 know either what direction they should be. That means GPIO pins which
 I don't know are set as Hi-Z = inputs... Now, when I think of that,
 if it works it's safer when the other pins are in Hi-Z mode. Never mind.

Normally all the unused gpio lines are just set as inputs.

  Then the reset can be done with:
 
  case XC2028_TUNER_RESET:
  /* GPIO 12 (xc3028 tuner reset) */
  cx_write(MO_GP1_IO, 0x101000);
  mdelay(50);
  cx_write(MO_GP1_IO, 0x101010);
  mdelay(50);
  return 0;
 
 Earlier I was told to use 'cx_set' and 'cx_clear' because using 'cx_write'
 is risky.
 see here: http://www.spinics.net/lists/linux-dvb/msg29777.html

Steven is wrong there and you are right.  The cx88 gpio lines have the mask
bits in the 3rd byte, which allow changing a gpio without modifying the
others with a single write.  It is better to use this than to do a
read-modify-write.  That is actually less safe, since it's not an atomic
operation.

 And when you are using 'cx_set' and 'cx_clear' you need 3 calls.
 The first to set the direction bit, the second to set 0 on reset pin
 and the third to set 1 on reset pin.

You could use cx_andor() to set the direction bit and lower the reset pin
in one call.  cx_set/cx_clear are just calls to cx_andor().  But using the
mask bit and cx_write is best.

  Though I have to wonder why each card needs its own xc2028 reset function.
  Shouldn't they all be the same other than what gpio they change?
 My English goes lame here. Do you mean that reset function shouldn't use
 GPIO at all?

There is other code for xc2028 reset for different cards, e.g.
cx88_dvico_xc2028_callback, cx88_xc3028_geniatech_tuner_callback,
cx88_xc2028_tuner_callback, cx88_pv_8000gt_callback, and even
cx88_xc5000_tuner_callback.  Shouldn't these functions all do the same
thing other than what gpio line they change?

  @@ -2882,6 +2946,16 @@
  cx_set(MO_GP0_IO, 0x0080); /* 702 out of reset */
  udelay(1000);
  break;
  +
  +   case CX88_BOARD_WINFAST_DTV1800H:
  +   /* GPIO 12 (xc3028 tuner reset) */
  +   cx_set(MO_GP1_IO, 0x1010);
  +   mdelay(50);
  +   cx_clear(MO_GP1_IO, 0x10);
  +   mdelay(50);
  +   cx_set(MO_GP1_IO, 0x10);
  +   mdelay(50);
  +   break;
  }
   }
 
  Couldn't you replace this with:
 
  case CX88_BOARD_WINFAST_DTV1800H:
  cx88_xc3028_winfast1800h_callback(code, XC2028_TUNER_RESET, 0);
  break;
 Yes

Re: [PATCH] bttv-driver.c :poll method lose race condition for capture video

2009-05-31 Thread Trent Piepho
On Sun, 31 May 2009, Figo.zhang wrote:

 bttv-driver.c,cx23885-video.c,cx88-video.c: poll method lose race condition 
 for capture video.

Please use patch titles that are not so long.  It would be nice if you
could describe this race condition.


 Signed-off-by: Figo.zhang figo1...@gmail.com
 ---
  drivers/media/video/bt8xx/bttv-driver.c |7 +--
  drivers/media/video/cx23885/cx23885-video.c |   15 +++
  drivers/media/video/cx88/cx88-video.c   |   13 ++---
  3 files changed, 26 insertions(+), 9 deletions(-)

 diff --git a/drivers/media/video/bt8xx/bttv-driver.c 
 b/drivers/media/video/bt8xx/bttv-driver.c
 index 23b7499..9cadfcc 100644
 --- a/drivers/media/video/bt8xx/bttv-driver.c
 +++ b/drivers/media/video/bt8xx/bttv-driver.c
 @@ -3152,6 +3152,7 @@ static unsigned int bttv_poll(struct file *file, 
 poll_table *wait)
   struct bttv_fh *fh = file-private_data;
   struct bttv_buffer *buf;
   enum v4l2_field field;
 + unsigned int rc = 0;

   if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
   if (!check_alloc_btres(fh-btv,fh,RESOURCE_VBI))
 @@ -3160,6 +3161,7 @@ static unsigned int bttv_poll(struct file *file, 
 poll_table *wait)
   }

   if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
 + mutex_lock(fh-cap.vb_lock);
   /* streaming capture */
   if (list_empty(fh-cap.stream))
   return POLLERR;

Won't you return with the mutex held here?

 @@ -3191,8 +3193,9 @@ static unsigned int bttv_poll(struct file *file, 
 poll_table *wait)
   poll_wait(file, buf-vb.done, wait);
   if (buf-vb.state == VIDEOBUF_DONE ||
   buf-vb.state == VIDEOBUF_ERROR)
 - return POLLIN|POLLRDNORM;
 - return 0;
 + rc = POLLIN|POLLRDNORM;
 + mutex_unlock(fh-cap.vb_lock);
 + return rc;
  err:
   mutex_unlock(fh-cap.vb_lock);
   return POLLERR;

I think the logic for these changes will be cleaner to do something like this:

{
unsigned int rc = POLLERR;

if (some error);
goto done;
if (other error)
goto done;

if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
rc = POLLIN|POLLRDNORM;
else
rc = 0;

done:
mutex_unlock(fh-vidq.vb_lock);
return rc;
}
--
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


[PULL] generic image bounds setting and alignment function

2009-05-30 Thread Trent Piepho
Mauro,

Please pull from http://linuxtv.org/hg/~tap/v4l-dvb

This series adds a function for bounding and alignment image sizes and
modifies a number of drivers to use it.  It came up when the pxa patches to
deal with the alignment issues for that driver were posted.  I haven't
tested these patches for pxa.

for the following 14 changesets:

01/14: compat: handle __fls
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=c4b55ce6c273

02/14: v4l2: Create helper function for bounding and aligning images
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=b4d3ec8d363d

03/14: pxa-camera: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=cb48209c1841

04/14: sh_mobile_ceu_camera: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=08dc3db16e9a

05/14: zoran: Use v4l bounding/alignment functiob
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=d65d274ca9da

06/14: vivi: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=67a3342606c2

07/14: saa7134: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=8e79122888da

08/14: cx88: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=7dc2db9c0b34

09/14: w8968cf: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=1733bbc12c3a

10/14: cx23885: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=605ec680bd75

11/14: mt9: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=d92d47e0d76f

12/14: cx231xx: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=3da0c824a487

13/14: em28xx: Use v4l bounding/alignment function
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=42b042c376ec

14/14: cx231xx: TRY_FMT should not actually set anything
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=81ca6b823ec4


 linux/drivers/media/video/cx231xx/cx231xx-avcore.c |   17 +---
 linux/drivers/media/video/cx231xx/cx231xx-video.c  |   26 +--
 linux/drivers/media/video/cx231xx/cx231xx.h|3
 linux/drivers/media/video/cx23885/cx23885-video.c  |   11 ---
 linux/drivers/media/video/cx88/cx88-video.c|   11 ---
 linux/drivers/media/video/em28xx/em28xx-video.c|   38 +++---
 linux/drivers/media/video/mt9m001.c|   12 ---
 linux/drivers/media/video/mt9t031.c|   14 
 linux/drivers/media/video/mt9v022.c|   12 ---
 linux/drivers/media/video/pxa_camera.c |   36 ++
 linux/drivers/media/video/saa7134/saa7134-video.c  |   11 ---
 linux/drivers/media/video/sh_mobile_ceu_camera.c   |   12 ---
 linux/drivers/media/video/v4l2-common.c|   73 -
 linux/drivers/media/video/vivi.c   |   11 ---
 linux/drivers/media/video/w9968cf.c|   39 ---
 linux/drivers/media/video/zoran/zoran_driver.c |   14 +---
 linux/include/media/v4l2-common.h  |   10 ++
 v4l/compat.h   |   15 
 18 files changed, 165 insertions(+), 200 deletions(-)

Thanks,
Trent

--
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: PULL request - http://linuxtv.org/hg/~pb/v4l-dvb/

2009-05-27 Thread Trent Piepho
On Tue, 26 May 2009, Patrick Boettcher wrote:
  MHz to 161 Mhz as welll as 439-447 MHz.  Probably wrong.  My guess is the
  data sheet says, low band 48 to 154 MHz, mid band 161 MHz to 439 MHz,
  etc., where 154 is the frequency of the last channel in the low band and
  161 is the first channel in the mid band.  Then someone translated this to
  code without really understanding what's going on.  It should probably be:
 
  if  (params-frequency  44300) bs = 0x08;
  else if (params-frequency  15750) bs = 0x0a;
  elsebs = 0x09;
 
  The tuner's limits should already be enforced elsewhere.  Or just convert
  this to use dvb_pll.

 Thanks for pointing this out, can you please provide a patch for that?
 Preferably for the dvb_pll-solution?

I've done this, but found some more mistakes in the flexcop code wrt
frontend attachment.

In patch 7469 you changed a failure path dvb_frontend_detach(fc-fe) into
a fc-fe-ops-release(fc-fe), which isn't correct.  There is more stuff
dvb_frontend_detach() does besides call the main release method.

The various attach functions in flexcop-fe-tuner don't return success/fail,
which is a problem when frontend attachment partially fails.  For instance
if mt352 attachment works but dvb-pll attachment then fails the driver
will think everything is ok because fc-fe is non-NULL, but the tuner will
not work.  It makes more sense to consider this an error and clean up.

There are a couple other places where frontend attachment can partially
fail where I wasn't entirely sure what's right.  In
skystar2_rev27_attach(), after the s5h1420 demod is attached, attachment of
a ISL6421 LNB and ITD1000 tuner is attempted.  If these fail an error
message is printed but the rest of the code will consider the frontend to
be successfully attached.  Can the frontend work if the ISL6421 or ITD1000
didn't attach (which can happen if the driver isn't present even if the
hardware is fine)?  I'm guessing not and called these cases an error.  If
it's not an error, then the err() printout should probably be warning or
info level.

Does this patch to fix these problems look ok?

Maybe change if (!i2c_tuner) return 0; into BUG_ON(!i2c_tuner);?

In the case of a partial frontend attachment failure, should we keep
trying the rest of the frontends in the list or fail out early?  This patch
does the former, but the latter could easily be done by adding a break to
the end of the Clean up partially attached frontend code block.


# HG changeset patch
# User Trent Piepho xy...@speakeasy.org
# Date 1243476805 25200
# Node ID 27aa25fe54ae2fd9a166bfdb3cec4c4ecadc9cba
# Parent  480905f8768ce2e5b01da1c6d723cf95cf134d0b
b2c2: Fix problems with frontend attachment

From: Trent Piepho xy...@speakeasy.org

The frontend attachment code didn't handle cases where the frontend
partially failed to attach.  For instance, when the demod was attached
successfully but the tuner driver wasn't compiled or fails to init for some
reason.  In these cases we try to clean up the partial attachment and fail
instead of proceeding with a broken frontend.

If frontend registration fails, clean up with dvb_frontend_detach() rather
than just calling the frontend's main release method.  The former does some
additional stuff, like release an attached tuner and take care of putting
symbols when dynamic binding is used.

In skystar2_rev23_attach() it's not necessary to set fc-dev_type, that
gets set before skystar2_rev23_attach() is called.

Priority: normal

Signed-off-by: Trent Piepho xy...@speakeasy.org

diff -r 480905f8768c -r 27aa25fe54ae 
linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c
--- a/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c   Wed May 27 17:35:06 
2009 -0700
+++ b/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c   Wed May 27 19:13:25 
2009 -0700
@@ -175,23 +175,23 @@ static int skystar23_samsung_tbdu18132_t
return 0;
 }

-static void skystar2_rev23_attach(struct flexcop_device *fc,
-   struct i2c_adapter *i2c)
-{
-   fc-fe = dvb_attach(mt312_attach,
-   skystar23_samsung_tbdu18132_config, i2c);
+static int skystar2_rev23_attach(struct flexcop_device *fc,
+   struct i2c_adapter *i2c)
+{
+   fc-fe = dvb_attach(mt312_attach, skystar23_samsung_tbdu18132_config, 
i2c);
if (fc-fe != NULL) {
struct dvb_frontend_ops *ops = fc-fe-ops;
-   ops-tuner_ops.set_params \
-   = skystar23_samsung_tbdu18132_tuner_set_params;
+   ops-tuner_ops.set_params   =
+   skystar23_samsung_tbdu18132_tuner_set_params;
ops-diseqc_send_master_cmd = flexcop_diseqc_send_master_cmd;
ops-diseqc_send_burst  = flexcop_diseqc_send_burst;
ops-set_tone   = flexcop_set_tone;
ops-set_voltage= flexcop_set_voltage;
fc-fe_sleep= ops-sleep;
ops-sleep

Re: PULL request - http://linuxtv.org/hg/~pb/v4l-dvb/

2009-05-25 Thread Trent Piepho
On Mon, 25 May 2009, Mauro Carvalho Chehab wrote:

 +   if (params-frequency = 4800  params-frequency = 15400) \
 +   bs = 0x09;
 +   if (params-frequency = 16100  params-frequency = 43900) 
 \
 +   bs = 0x0a;
 +   if (params-frequency = 44700  params-frequency = 86300) 
 \
 +   bs = 0x08;

 Just remove the backslash. You don't need they.

The original code has this, but bs will be zero for a frequency between 154
MHz to 161 Mhz as welll as 439-447 MHz.  Probably wrong.  My guess is the
data sheet says, low band 48 to 154 MHz, mid band 161 MHz to 439 MHz,
etc., where 154 is the frequency of the last channel in the low band and
161 is the first channel in the mid band.  Then someone translated this to
code without really understanding what's going on.  It should probably be:

if  (params-frequency  44300) bs = 0x08;
else if (params-frequency  15750) bs = 0x0a;
elsebs = 0x09;

The tuner's limits should already be enforced elsewhere.  Or just convert
this to use dvb_pll.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC,PATCH] VIDIOC_G_EXT_CTRLS does not handle NULL pointer correctly

2009-05-25 Thread Trent Piepho
On Mon, 25 May 2009, Laurent Pinchart wrote:
 diff -r e0d881b21bc9 linux/drivers/media/video/v4l2-ioctl.c
 --- a/linux/drivers/media/video/v4l2-ioctl.c  Tue May 19 15:12:17 2009 +0200
 +++ b/linux/drivers/media/video/v4l2-ioctl.c  Sun May 24 18:26:29 2009 +0200
 @@ -402,6 +402,10 @@
  a specific control that caused it. */
   p-error_idx = p-count;
   user_ptr = (void __user *)p-controls;
 + if (p-count  KMALLOC_MAX_SIZE / sizeof(p-controls[0])) {
 + err = -ENOMEM;
 + goto out_ext_ctrl;
 + }
   if (p-count) {
   ctrls_size = sizeof(struct v4l2_ext_control) * p-count;
   /* Note: v4l2_ext_controls fits in sbuf[] so mbuf is 
 still NULL. */
 @@ -1859,6 +1863,10 @@
  a specific control that caused it. */
   p-error_idx = p-count;
   user_ptr = (void __user *)p-controls;
 + if (p-count  KMALLOC_MAX_SIZE / sizeof(p-controls[0])) {
 + err = -ENOMEM;
 + goto out_ext_ctrl;
 + }
   if (p-count) {
   ctrls_size = sizeof(struct v4l2_ext_control) * p-count;
   /* Note: v4l2_ext_controls fits in sbuf[] so mbuf is 
 still NULL. */

 Restricting v4l2_ext_controls::count to values smaller than KMALLOC_MAX_SIZE /
 sizeof(struct v4l2_ext_control) should be enough, but we might want to
 restrict the value even further. I'd like opinions on this.

One thing that could be done is to call access_ok() on the range before
kmalloc'ing a buffer.  If p-count is too high, then it's possible that the
copy_from_user will fail because the process does not have the address
space to copy.
--
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 - part 2] V4L2 patches for Intel Moorestown Camera Imaging Drivers

2009-05-23 Thread Trent Piepho
On Sat, 23 May 2009, Mauro Carvalho Chehab wrote:
  + + if (intel-open) { + ++intel-open; + DBG_DD((device has opened
  already - %d\n, intel-open)); + return 0; + } + + file-private_data
  = dev; + /* increment our usage count for the driver */ +
  ++intel-open; + DBG_DD((intel_open is %d\n, intel-open)); +

 Open is not the proper place to clean the configuration, since a V4L2
 device should support more than one open.  You can use a different
 userspace app to control your device, while other application is
 streaming it.

It looks like only the first open will set the configuration, subsequent
ones don't do anything.  So maybe this driver is ok for multiple opens?
Doesn't the kernel make open and close atomic, so some kind of barrier or
atomic variable isn't necessary?

  +static int intel_g_fmt_cap(struct file *file, void *priv,
  +   struct v4l2_format *f)
  +{
  +   struct video_device *dev = video_devdata(file);
  +   struct intel_isp_device *intel = video_get_drvdata(dev);
  +   int ret;
  +
  +   DBG_DD((intel_g_fmt_cap\n));
  +   if (f-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {

Once switched to video-ioctl2, it don't be necessary to check the type of
buffer.  vidioc_g_fmt_cap will only be called with video capture buffers.
It's the same with all the other vidioc_*_cap methods.

  +static int intel_s_input(struct file *file, void *priv, unsigned int i)
  +{
  +   return 0;
  +}

Wouldn't it be better to return an error if the input is something other
than zero?

  +   snrcfg = intel-sys_conf.isi_config;
  +   index = f-index;
  +
  +   if (f-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
  +   ret = -EINVAL;
  +   else {
  +   if (snrcfg-type == SENSOR_TYPE_SOC)
  +   if (index = 8)
  +   return -EINVAL;
  +   if (index = sizeof(fmts) / sizeof(*fmts))
  +   return -EINVAL;
  +   memset(f, 0, sizeof(*f));
  +   f-index = index;

Saving index, clearing f, and restoring index isn't necessary.
video-ioctl2 will take care of clearing everything that needs to be
cleared.

  +   f-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

Not necessary either, you already know type is set correctly.

  +   if (buf-memory != V4L2_MEMORY_MMAP ||
  +   buf-type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
  +   buf-index = intel-num_frames || buf-index  0)
  +   return  -EINVAL;

You don't need to check buf-type, it will be a type your driver supports.
buf-index is unsigned, obviously it can't be less than zero.  The same
applies to all the other buffer functions.  Looks like you copied from old
code.  Some drivers had these unnecessary checks but they should have all
been cleaned up by now.

  +   pci_read_config_word(dev, PCI_VENDOR_ID, intel-vendorID);
  +   pci_read_config_word(dev, PCI_DEVICE_ID, intel-deviceID);

Do you ever use these after saving them?
--
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: Sound capture with Osprey 230

2009-05-14 Thread Trent Piepho
On Mon, 11 May 2009, Sverker Abrahamsson wrote:
 Hi all,
 I've been using Osprey 230 cards for AV capture for several years, earlier
 with a modified version of Viewcast's driver but it was never very stable.
 When doing a new setup I therefore wanted to get the Alsa driver to work. I
 found that there were two trees in the repository in regards to these cards,
 http://linuxtv.org/hg/~mchehab/osprey and http://linuxtv.org/hg/~tap/osprey.
 It seems that mchehab tree is the patches that Viewcast submitted which does
 not address the necessary changes for ALSA driver while tap tree does but
 for Osprey 440 and older kernels.

Mauro's tree with viewcast's patches is even older than mine, wrt kernel
support.

 I've therefore ported the changes from tap to the main tree and added
 support for detecting Osprey 210/220/230 plus a minor fix to support
 specifying digital_rate as module parameter. It might also work for Osprey
 240 (which is PCI-e variant of 230) but I don't have any such card so I
 haven't been able to test.

Instead of modifying my patch, it would be better if you could provide a
patch on top of it that adds support for your new card.

 The only question mark I have is that the current implementation use the
 depreciated interfaces from bttv-if.c to find which bttv driver corresponds
 to this audio driver and adds a function to get the bttv core. It is
 suggested to use the routines in bttv-gpio.c instead but I don't find an
 obvious replacement for bttv_get_pcidev nor how to get bttv_core.

The interface in bttv-if.c has been deprecated for years now, yet no one
has come up with something to replace it with.  I think Gerd was getting a
bit ahead of himself when he declared it obsolete.

 I see two alternatives:
 1. Implement snd-87x module as a subdevice to bttv. Is this correct as the
 video and audio devices are two separate pci devices?

The audio and video devices aren't just separate pci devices, they are also
two unrelated devices to the linux device model.  The driver model doesn't
have any means to call one a subdevice of the other.

Somehow, there needs to be a means for the audio driver to find the video
driver so that it can get access to the gpio lines and the i2c bus.  But,
this is only necessary for the osprey cards.  The audio driver for other
cards doesn't need gpios or i2c.  So, it would be nice to allow just the
audio driver with no video to be loaded.

The problem with my implementation is that after the audio bttv driver gets
a pointer to the video driver's core, the video driver could go away and
leave the audio driver with a dangling pointer.  That's one of the reasons
I haven't merged my osprey code.  The other is that I have cards with bttv
audio to test with.
--
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: [PULL] http://endr...@linuxtv.org/hg/~endriss/v4l-dvb

2009-05-13 Thread Trent Piepho
On Wed, 13 May 2009, Oliver Endriss wrote:
 02/05: dvb-ttpci: Check transport error indicator flag
 http://endr...@linuxtv.org/hg/~endriss/v4l-dvb?cmd=changeset;node=8a742338523d

Are you sure this is a good idea?  The cx88 driver doesn't do this.
--
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 4/4] zoran: fix /|| error

2009-05-12 Thread Trent Piepho
On Tue, 12 May 2009, Mauro Carvalho Chehab wrote:
 Em Tue, 12 May 2009 17:18:20 -0400
 Devin Heitmueller dheitmuel...@kernellabs.com escreveu:

  On Tue, May 12, 2009 at 4:39 PM,  a...@linux-foundation.org wrote:
   From: Roel Kluin roel.kl...@gmail.com
  
   Fix /|| typo. `default_norm' can be 0 (PAL), 1 (NTSC) or 2 (SECAM),
   the condition tested was impossible.
  
   Signed-off-by: Roel Kluin roel.kl...@gmail.com
   Cc: Mauro Carvalho Chehab mche...@redhat.com
   Cc: Hans Verkuil hverk...@xs4all.nl
   Signed-off-by: Andrew Morton a...@linux-foundation.org
   ---
 
  Hello,
 
  Was the patch actually tested against the hardware in question?  While
  I agree that it looks ok, it can result in the default logic being
  inverted in some cases, which could expose other bugs and result in a
  regression.
 
  I just want to be confident that this patch was tested by somebody
  with the hardware and it isn't going into the codebase because it
  obviously cannot be right.

 Hans and Jean worked on it. Both are at PAL area, so they won't notice such
 error without a standards generator, since the default is to assume that the
 signal is PAL.

 With this patch, PAL should keep working, but I can't see how NTSC or SECAM
 would work without it.

NTSC works fine without it.  The code with the bug was supposed to check
for an out of range module parameter and fix it, but it was broken and did
nothing.  There is no problem if default_norm was set to an ok value, but
if someone specified default_norm=42 then the driver wouldn't fix it and
something bad might happen.  Maybe it would read off the end of the norms
array and crash?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] v4l2: fill the reserved fields of VIDIOC_REQBUFS ioctl

2009-04-30 Thread Trent Piepho
On Wed, 29 Apr 2009, [UTF-8] N??meth M??rton wrote:
 The parameter of VIDIOC_REQBUFS is a pointer to struct v4l2_requestbuffers.
 This structure has reserved fields which has to be filled with zeros
 according to the V4L2 API specification, revision 0.24 [1].

As I read the spec, the reserved fields can be used for input from user
space if the buffer is of type V4L2_BUF_TYPE_PRIVATE or higher.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] v4l2: fill the reserved fields of VIDIOC_REQBUFS ioctl

2009-04-30 Thread Trent Piepho
On Thu, 30 Apr 2009, Trent Piepho wrote:
 On Wed, 29 Apr 2009, [UTF-8] N??meth M??rton wrote:
  The parameter of VIDIOC_REQBUFS is a pointer to struct v4l2_requestbuffers.
  This structure has reserved fields which has to be filled with zeros
  according to the V4L2 API specification, revision 0.24 [1].

 As I read the spec, the reserved fields can be used for input from user
 space if the buffer is of type V4L2_BUF_TYPE_PRIVATE or higher.

Here is a patch that fixes this problem, along with the S_FMT problem you
patched earlier.  TRY_FMT had the same problem as S_FMT, so I fixed that
one as well too.

v4l2-ioctl: Clear buffer type specific trailing fields/padding

From: Trent Piepho xy...@speakeasy.org

Some ioctls have structs that are a different size depending on what type
of buffer is being used.  If the buffer type leaves a field unused or has
padding space at the end, this space should be zeroed out.

The problems with S_FMT and REQBUFS were original identified and patched by
M??rton N??meth nm...@freemail.hu.

Priority: normal

Signed-off-by: Trent Piepho xy...@speakeasy.org

diff -r 7b786cb576e5 -r 82ef5d6e29e3 linux/drivers/media/video/v4l2-ioctl.c
--- a/linux/drivers/media/video/v4l2-ioctl.cThu Apr 30 09:14:13 2009 -0700
+++ b/linux/drivers/media/video/v4l2-ioctl.cThu Apr 30 09:15:56 2009 -0700
@@ -42,6 +42,12 @@
if (vfd-debug  V4L2_DEBUG_IOCTL_ARG)  \
printk(KERN_DEBUG %s:  fmt, vfd-name, ## arg);\
} while (0)
+
+/* Zero out the end of the struct pointed to by p.  Everthing after, but
+ * not including, the specified field is cleared. */
+#define CLEAR_AFTER_FIELD(p, field) \
+   memset((u8 *)(p) + offsetof(typeof(*(p)), field) + sizeof((p)-field), \
+   0, sizeof(*(p)) - offsetof(typeof(*(p)), field) - sizeof((p)-field))

 struct std_descr {
v4l2_std_id std;
@@ -783,44 +789,53 @@ static long __video_do_ioctl(struct file

switch (f-type) {
case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+   CLEAR_AFTER_FIELD(f, fmt.pix);
v4l_print_pix_fmt(vfd, f-fmt.pix);
if (ops-vidioc_s_fmt_vid_cap)
ret = ops-vidioc_s_fmt_vid_cap(file, fh, f);
break;
case V4L2_BUF_TYPE_VIDEO_OVERLAY:
+   CLEAR_AFTER_FIELD(f, fmt.win);
if (ops-vidioc_s_fmt_vid_overlay)
ret = ops-vidioc_s_fmt_vid_overlay(file,
fh, f);
break;
case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+   CLEAR_AFTER_FIELD(f, fmt.pix);
v4l_print_pix_fmt(vfd, f-fmt.pix);
if (ops-vidioc_s_fmt_vid_out)
ret = ops-vidioc_s_fmt_vid_out(file, fh, f);
break;
case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
+   CLEAR_AFTER_FIELD(f, fmt.win);
if (ops-vidioc_s_fmt_vid_out_overlay)
ret = ops-vidioc_s_fmt_vid_out_overlay(file,
fh, f);
break;
case V4L2_BUF_TYPE_VBI_CAPTURE:
+   CLEAR_AFTER_FIELD(f, fmt.vbi);
if (ops-vidioc_s_fmt_vbi_cap)
ret = ops-vidioc_s_fmt_vbi_cap(file, fh, f);
break;
case V4L2_BUF_TYPE_VBI_OUTPUT:
+   CLEAR_AFTER_FIELD(f, fmt.vbi);
if (ops-vidioc_s_fmt_vbi_out)
ret = ops-vidioc_s_fmt_vbi_out(file, fh, f);
break;
case V4L2_BUF_TYPE_SLICED_VBI_CAPTURE:
+   CLEAR_AFTER_FIELD(f, fmt.sliced);
if (ops-vidioc_s_fmt_sliced_vbi_cap)
ret = ops-vidioc_s_fmt_sliced_vbi_cap(file,
fh, f);
break;
case V4L2_BUF_TYPE_SLICED_VBI_OUTPUT:
+   CLEAR_AFTER_FIELD(f, fmt.sliced);
if (ops-vidioc_s_fmt_sliced_vbi_out)
ret = ops-vidioc_s_fmt_sliced_vbi_out(file,
fh, f);
break;
case V4L2_BUF_TYPE_PRIVATE:
+   /* CLEAR_AFTER_FIELD(f, fmt.raw_data); - does nothing 
*/
if (ops-vidioc_s_fmt_type_private)
ret = ops-vidioc_s_fmt_type_private(file,
fh, f);
@@ -837,46 +852,55 @@ static long __video_do_ioctl(struct file

[pull] http://linuxtv.org/hg/~tap/v4l-dvb

2009-04-30 Thread Trent Piepho
Mauro,

Please pull from http://linuxtv.org/hg/~tap/v4l-dvb

for the following 4 changesets:

01/04: compat: Add DMA_BIT_MASK() macro
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=486add0e3f1f

02/04: zoran: fix bug when enumerating format -1
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=f144948f22c8

03/04: v4l2-ioctl: Check buffer types using g_fmt instead of try_fmt
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=b54857195e2c

04/04: v4l2-ioctl: Clear buffer type specific trailing fields/padding
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=7cbf08aeb840


 linux/drivers/media/video/v4l2-ioctl.c |   45 -
 linux/drivers/media/video/zoran/zoran_driver.c |   30 +++-
 v4l/compat.h   |7 ++-
 3 files changed, 55 insertions(+), 27 deletions(-)

Thanks,
Trent
--
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] [0904_14] Siano: assemble all components to one kernel module

2009-04-28 Thread Trent Piepho
On Mon, 27 Apr 2009, Uri Shkolnik wrote:
 --- On Tue, 4/21/09, Mauro Carvalho Chehab mche...@infradead.org wrote:
If the system includes SDIO and OMAP SPI/SPP, the
  module build will discard the USB interface driver, but the
  SDIO and the OMAP SPI will be built.
 
  The patch you've provided just merge everything. If you're
  proposing a newer
  model, you should send a patchset with the complete Kbuild
  refactor. For now,
  it is better to postpone this patch until we merge
  non-kbuild changes.
 
   Can you name another driver that works this way??
  It is considered better
   to build a new module for a different interface.?
  That way one can decide
   at run time if the interface is needed or not and only
  load the module if
   needed.? If everything is built into one module
  then one must decide at
   compile time what interfaces to support.? But it
  is often the case that
   kernels are compiled on different systems than run
  them.
 
  This model also sounds different to me from what I've seen
  so far.
 

 1) PLUTO Kconfig

 config DVB_PLUTO2
   tristate Pluto2 cards
   depends on DVB_CORE  PCI  I2C
 As we can see, it depends on PCI and I2C (and DVB_CORE, but that is outside 
 the current discussion), which means, that if the target system lacks either 
 PCI or I2C (and many Linux target do not have those) the entire Pluto module 
 will not be built, even if selected.

I do not think you understand.  This driver is for a device that is access
via a PCI bus.  It creates an i2c adapter that is used to control the tuner
chip on the device.  The device is not accessed via i2c.  What's more, and
this is a key point you are missing, it can not be accessed by PCI *or*
I2C.  This is the same with all the other drivers you may have found that
use both PCI and I2C.  It's a totally different situation.

Look at the how the B2C2 flexcop driver works.  This hardware is availble
in both PCI and USB versions.


 config DVB_TTUSB_BUDGET
   tristate Technotrend/Hauppauge Nova-USB devices
   depends on DVB_CORE  USB  I2C  PCI
 Support for external USB adapters designed by Technotrend and

 This module while not be build (even if chosen) on target system that lacks 
 USB stack (additional to PCI and I2C).

This looks like a mistake.  The driver is for a USB device and should not
depend on PCI, but the driver uses the pci dma api.

 First, SMS (Siano based device) requires at least one available bus (either 
 USB, SDIO, SPI/SPP, HIF/Parallel, TS_I2C, ), not a combination like the 
 TTUSB. (Note, if multiple buses available, the module can use them 
 simultaneously, for example - two different instances (SDIO and USB) dongles 
 may co-exist on the same system).

 There is no reason to prevent some systems which have only SDIO interface to 
 work if there is no USB and vice versa.

 Only if there are no supported (by SMS) buses at all, the drive will not be 
 built.

 Second, in order to reduce module size, there is no need to compile and link 
 to the binary module a bus interface component that will never be used, more 
 than that, if there are no suitable (external) headers, the build process 
 will fail.
 So, the Kconfig and Makefile need to ensure that the module will be built and 
 function OK on target systems which have full set AND subset of the total 
 supported buses (by SMS).

The point you are missing is that by compiling all support into one module
you are forcing the supported busses to be chosen at compile time.  If
they were separate modules then the supported busses could be loaded
at run time.
--
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: [Mjpeg-users] [PATCH] zoran: invalid test on unsigned

2009-04-27 Thread Trent Piepho
On Mon, 27 Apr 2009, Alan Cox wrote:
 On Sun, 26 Apr 2009 17:45:07 +0200
 Roel Kluin roel.kl...@gmail.com wrote:
  fmt-index is unsigned. test doesn't work
 
  Signed-off-by: Roel Kluin roel.kl...@gmail.com
  ---
  Is there another test required?
 
  +++ b/drivers/media/video/zoran/zoran_driver.c
  @@ -1871,7 +1871,7 @@ static int zoran_enum_fmt(struct zoran *zr, struct 
  v4l2_fmtdesc *fmt, int flag)
  if (num == fmt-index)
  break;
  }
  -   if (fmt-index  0 /* late, but not too late */  || i == NUM_FORMATS)
  +   if (i == NUM_FORMATS)
  return -EINVAL;

 If it's unsigned then don't we need i = NUM_FORMATS ?

That part of the patch is fine.  It turns out there is another problem that
already existed in this function.  It's clearer with a few more lines of
context.

int num = -1, i;
for (i = 0; i  NUM_FORMATS; i++) {
if (zoran_formats[i].flags  flag)
num++;
if (num == fmt-index)
break;
}
if (i == NUM_FORMATS)
return -EINVAL;
/* stuff to return format i */

The v4l2 api enumerates formats separately for each buffer type, e.g. there
is one list of formats for video capture and a different list for video
output.  The driver just has one list of formats and each format is flagged
with the type(s) it can be used with.  So when someone requests the capture
format at index 2 we search the list and return the 3rd capture format we
find.  Since we don't know how many capture formats there are (NUM_FORMATS
is the number of formats in the driver's single list, i.e. the union of all
format types) we can't reject an index that is too large until after
searching the whole list.

The problem with this code is if someone requests a format at fmt-index ==
(unsigned)-1.  If the first format in the array doesn't have the requested
type then num will still be -1 when it's compared to fmt-index and there
will appear to be a match.

Here's a patch to redo the function that should fix everything:

zoran: fix bug when enumerating format -1

If someone requests a format at fmt-index == (unsigned)-1 and the first
format in the array doesn't have the requested type then num will still be
-1 when it's compared to fmt-index and there will appear to be a match.

Restructure the loop so this can't happen.  It's simpler this way too.  The
unnecessary check for (unsigned)fmt-index  0 found by Roel Kluin
roel.kl...@gmail.com is removed this way too.

Signed-off-by: Trent Piepho xy...@speakeasy.org

diff -r 63eba6df4b8a -r c247021eb11c 
linux/drivers/media/video/zoran/zoran_driver.c
--- a/linux/drivers/media/video/zoran/zoran_driver.cMon Apr 27 12:13:04 
2009 -0700
+++ b/linux/drivers/media/video/zoran/zoran_driver.cMon Apr 27 12:25:51 
2009 -0700
@@ -1871,22 +1871,20 @@ static int zoran_querycap(struct file *f

 static int zoran_enum_fmt(struct zoran *zr, struct v4l2_fmtdesc *fmt, int flag)
 {
-   int num = -1, i;
-
-   for (i = 0; i  NUM_FORMATS; i++) {
-   if (zoran_formats[i].flags  flag)
-   num++;
-   if (num == fmt-index)
-   break;
-   }
-   if (fmt-index  0 /* late, but not too late */  || i == NUM_FORMATS)
-   return -EINVAL;
-
-   strncpy(fmt-description, zoran_formats[i].name, 
sizeof(fmt-description)-1);
-   fmt-pixelformat = zoran_formats[i].fourcc;
-   if (zoran_formats[i].flags  ZORAN_FORMAT_COMPRESSED)
-   fmt-flags |= V4L2_FMT_FLAG_COMPRESSED;
-   return 0;
+   unsigned int num, i;
+
+   for (num = i = 0; i  NUM_FORMATS; i++) {
+   if (zoran_formats[i].flags  flag  num++ == fmt-index) {
+   strncpy(fmt-description, zoran_formats[i].name,
+   sizeof(fmt-description) - 1);
+   /* fmt struct pre-zeroed, so adding '\0' not neeed */
+   fmt-pixelformat = zoran_formats[i].fourcc;
+   if (zoran_formats[i].flags  ZORAN_FORMAT_COMPRESSED)
+   fmt-flags |= V4L2_FMT_FLAG_COMPRESSED;
+   return 0;
+   }
+   }
+   return -EINVAL;
 }

 static int zoran_enum_fmt_vid_cap(struct file *file, void *__fh,

--
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] zoran: invalid test on unsigned

2009-04-26 Thread Trent Piepho
On Sun, 26 Apr 2009, Roel Kluin wrote:
 fmt-index is unsigned. test doesn't work

 Signed-off-by: Roel Kluin roel.kl...@gmail.com
 ---
 Is there another test required?

This is an old driver and I think back in v4l1 the indexes weren't all
unsigned.  There were a number of tests like this in it.  Patch is fine.

Acked-by: Trent Piepho xy...@speakeasy.org


 diff --git a/drivers/media/video/zoran/zoran_driver.c 
 b/drivers/media/video/zoran/zoran_driver.c
 index 092333b..0db5d0f 100644
 --- a/drivers/media/video/zoran/zoran_driver.c
 +++ b/drivers/media/video/zoran/zoran_driver.c
 @@ -1871,7 +1871,7 @@ static int zoran_enum_fmt(struct zoran *zr, struct 
 v4l2_fmtdesc *fmt, int flag)
   if (num == fmt-index)
   break;
   }
 - if (fmt-index  0 /* late, but not too late */  || i == NUM_FORMATS)
 + if (i == NUM_FORMATS)
   return -EINVAL;

   strncpy(fmt-description, zoran_formats[i].name, 
 sizeof(fmt-description)-1);
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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] [0904_14] Siano: assemble all components to one kernel module

2009-04-20 Thread Trent Piepho
On Mon, 20 Apr 2009, Uri Shkolnik wrote:

 better to have the BUS configurable, e. g. just because you have USB 
 interface, it doesn't mean that you want siano for USB, instead of using 
 SDIO.

 Since the module is using dynamic registration, I don't find it a problem.
 When the system has both USB and SDIO buses, both USB and SDIO interface 
 driver will be compiled and linked to the module. When a Siano based device 
 (or multiple Siano devices) will be connected, they will be register 
 internally in the core and activated. Any combination is allow (multiple 
 SDIO, multiple USB and any mix).

This is not the way linux drivers normally work.  Usually there are
multiple modules so that only the ones that need to be loaded are loaded.
It sounds like you are designing this to be custom compiled for each
system, but that's not usually they way things work.
--
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: s2255drv high quality mode and video status querying

2009-04-07 Thread Trent Piepho
On Tue, 7 Apr 2009, Dean A. wrote:
 +static int vidioc_g_parm(struct file *file, void *priv,
 +  struct v4l2_streamparm *sp)
 +{
 + struct s2255_fh *fh = priv;
 + struct s2255_dev *dev = fh-dev;
 + if (sp-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 + return -EINVAL;

You do not need to check the buffer type, video_ioctl2 does it already.
--
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] [0904_16] Siano: smsdvb - additional case of endian handling.

2009-04-05 Thread Trent Piepho
On Sun, 5 Apr 2009, Uri Shkolnik wrote:
   PidMsg.xMsgHeader.msgLength = sizeof(PidMsg);
   PidMsg.msgData[0] = feed-pid;

 - /* smsendian_handle_tx_message((struct SmsMsgHdr_ST *)PidMsg); */
 + smsendian_handle_tx_message((struct SmsMsgHdr_ST *)PidMsg);
   return smsclient_sendrequest(client-smsclient, PidMsg,
   sizeof(PidMsg));

I don't get it, you wrote the code commented out in one patch, and now
you're going to sumbit patches to uncomment it one line at a time?
--
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 3/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-05 Thread Trent Piepho
On Sun, 5 Apr 2009, Andy Walls wrote:

 Here is where LIRC may be its own worst enemy.  LIRC has filled some
 shortcomings in the kernel for support of IR device functions for so
 long (LWN says LIRC is 10 years old), that large numbers of users have
 come to depend on its operation, while at the same time apparently
 removing impetus for doing much to update the in kernel IR device
 support.

More than that.  In 1997 I bought a serial port remote off ebay and tried
to get it to work with linux.  I found an abandoned project from the
Metzler brothers called LIRC, though it didn't work.  I wrote a new
protocol for the serial port driver, which was the only one at the time,
rewrote the remote pulse decoding code and came up a new socket based the
client/server protocol and wrote the x-event client.  At that point remotes
were defined in header files so make was still needed to add a new one.
--
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 3/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-05 Thread Trent Piepho
On Sun, 5 Apr 2009, Mike Isely wrote:
 1. The switch statement in ir-kbd-i2c.c:ir_attach() is apparently
 implicitly trying to assume a particular type of remote based on the I2C
 address of the IR receiver it's talking to.  Yuck.  That's really not
 right at all.  The IR receiver used does not automatically mean which
 remote is used.  What if the vendor switches remotes?  That's happened
 with the PVR-USB2 hardware in the past (based on photos I've seen).
 Who's to say the next remote to be supplied is compatible?

IMHO, the way the remote supported is compiled into the kernel is absurd.
The system I setup 12 years ago was better than this.  At least the remote
was compiled into a userspace daemon and multiple remotes were supported at
the same time.  The keycode system I used had a remote id/key id split, so
you could have volume up key on any remote control the mixer but make the
power buttons on different remotes turn on different apps.

 3. A given IR remote may be described by much more than what 'scan
 codes' it produces.  I don't know a lot about IR, but looking at the
 typical lirc definition for a remote, there's obvious timing and
 protocol parameters as well.  Just being able to swap scan codes around
 is not always going to be enough.

A remote typically sends a header sequence of a long pulse and space before
the code.  The length of the pulse on the space varies greatly by remote
and makes a good way to identify the remote when multiple ones are
supported.

Then a pulse coded remote sends a sequence bits, usually 8 to 32.  The
length of the pulse identifies 1s or 0s.  Different remotes have different
pulse lengths and different spaces between them.  RC5 remotes use
Manchester encoding for this part.

When you hold a key down some remotes just repeat the same sequence over
and over again.  Some repeat the scan code but omit the header part.  Some
send out a special pulse sequence to indicate the last key is being held
down.  With the latter two methods you can tell the difference between a
key being held down and a key being pressed repeatedly.  With the first you
have guess based on how fast the repeats are coming in.  This is very
different than a keyboard, which sends a code when you press a key and
another when you release it.

The rate at which remotes repeat varies greatly.  You might find that one
remote makes your volume change annoying slowly while another is much too
fast to be usable.  Remote keys usually start repeating without delay, so
if you let a toggle like 'mute' repeat it becomes almost impossible to hit
it just once.  Entering numbers becomes impossible as well.
--
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: When is a wake_up() not a wake up?

2009-04-04 Thread Trent Piepho
On Fri, 3 Apr 2009, Andy Walls wrote:
 1. A work queue thread or read() call needs to send a command to the
 CX23418 using the cx18_api_call() function
 2. It fills out a mailbox with a command for the CX23418
 3. It prepares to wait, just in case a wait is needed
 4. A SW1 interrupt is sent to the CX23418 telling it a mailbox is ready
 5. The ack filed in the mailbox, a PCI MMIO location, is checked to see
 if the mailbox was ack'ed already
 6. If not, schedule_timeout() for up to 10 msecs (a near eternity...)
 7. Clean up the wait and move on

10ms isn't that long.  With a 100 HZ kernel it's only one jiffie.  The
shortest time msleep() can sleep on a 100 HZ kernel is 20 ms.

Is it possible that it's simply taking more than 10 ms for your process to
get run?

 Mar 31 23:36:56 palomino kernel: cx18-0:  irq: sending interrupt SW1: 8 to 
 send CX18_CPU_DE_SET_MDL
 Mar 31 23:36:56 palomino kernel: cx18-0:  api: scheduling to wait for ack of 
 CX18_CPU_DE_SET_MDL: req 51267 ack 51266, pid 21092, state 2
 Mar 31 23:36:56 palomino kernel: cx18-0:  irq: received interrupts SW1: 0  
 SW2: 8  HW2: 0
 Mar 31 23:36:56 palomino kernel: cx18-0:  irq: Wake up initiated on pid 21092 
 in state 2
 Mar 31 23:36:56 palomino kernel: cx18-0:  irq: Wake up succeeded on pid 
 21092, state 2 - 0
 Mar 31 23:36:56 palomino kernel: cx18-0:  api: done wait for ack of 
 CX18_CPU_DE_SET_MDL: req 51267 ack 51267, current pid 21092, current state 0, 
 state 0
 Mar 31 23:36:56 palomino kernel: cx18-0:  warning: failed to be awakened upon 
 RPU acknowledgment sending CX18_CPU_DE_SET_MDL; timed out waiting 10 msecs

Try some higher resolution time stamps, you can't really tell much about
when things are happening.

Here's some code I wrote to do it on x86.

#include linux/calc64.h
#define MHZ 1666.787
#define INT (unsigned int)(MHZ * (112) + 0.5)
#define FRAC(unsigned int)(MHZ * (121) / 1000.0 + 0.5)
#define timestamp(str, args...) { u64 _time; u32 _rem; rdtscll(_time); \
_time = (_time - starttime)12; \
_rem = do_div(_time, INT); \
printk(%s - %u.%03u us  str \n, __func__, \
(unsigned int)_time, (_rem  9) / FRAC , ## args); }
static u64 starttime;

Change MHZ to what /proc/cpuinfo tells you.  Call 'rdtscll(starttime)' some
time when your driver inits or a device gets opened or something like that.
Makes the time stamps easier to read when they start near zero.
--
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: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM

2009-03-30 Thread Trent Piepho
On Sun, 29 Mar 2009, Hans Verkuil wrote:
 On Sunday 29 March 2009 13:03:13 Trent Piepho wrote:
  How does overlay depend on video capture in any way?  It's perfectly
  reasonable for a driver to support _only_ overlay and not video capture.
  The zr36067 chip is only designed to support uncompressed data for video
  overlay for example.  Allowing uncompressed video capture is a hack that
  the driver didn't have at one point.

 Ah. Live and learn. In that case the spec is out of date and needs to be
 updated.

Do you mean there is something in the spec that makes overlay depend on
video capture?  Or that s/g_parm don't mention buffer types other than
video capture and video output?

   No, I agree with the spec in that I see no use case for it. Should
   there be one, then I'd like to see that in an actual driver
   implementation and in that case the spec should be adjusted as well.
 
  How about getting the frame rate of video overlay?  Works with bttv.

 Hmm, I grepped only on s_parm, not on g_parm.

It would be nice to use s_parm with drivers like bttv to reduce the frame
rate.  With multi-chip capture cards running out of bus bandwidth is a big
problem.  Reducing the frame rate is one way to deal with it.  The bt8x8
and cx2388x chips do have a temporal decimation feature and I've tried to
add support for it but I never got it to work correctly.

   In addition, g/s_parm is only used in combination with webcams/sensors
   for which overlays and vbi are irrelevant.
 
  There are several drivers for non-webcams, like bttv, saa7134, and
  saa7146, that support g_parm.  Why is returning the frame rate for video
  capture not valid?  Why does the number of buffers used for read() mode
  only make sense for webcams?

 OK, I'd forgot to check for the usage of g_parm. My bad.

There is also a default g_parm handler in video_ioctl2 that might be used
if the driver doesn't provide one.  I don't know what drivers actually use
it.

Thinking about it now, I think what makes the most sense is to use
capture for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE.
And use output for VBI_OUTPUT, SLICED_VBI_OUTPUT and
VIDEO_OUTPUT_OVERLAY.

 You're absolutely correct. I was too hasty.

 Can you update the spec to reflect this?

I'll try, but I hear the doc build system is quite a challenge.

 I also wonder whether check_fmt shouldn't check for the presence of
 the s_fmt callbacks instead of try_fmt since try_fmt is an optional
 ioctl.
   
I noticed that too.  saa7146 doesn't have a try_fmt call for
vbi_capture but is apparently supposed to support it.  I sent a
message about that earlier.
  
   I saw that. So why not check for s_fmt instead of try_fmt? That would
   solve this potential problem.
 
  Because that's clearly a change that belongs in another patch.

 OK, great.

My patch just called check_fmt() for s/g_parm.  I haven't touched
check_fmt().  But you're right that try_fmt is optional so it makes a bad
test.

Maybe it should use g_fmt?  saa7146 doesn't provide a s_fmt call either for
vbi capture, only g_fmt.  Though maybe this is a bug in saa7146?  It seems
like any driver that provides g_fmt can always just use that same method
for s_fmt as well and be correct.
--
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: Problems with Hauppauge HVR 1600 and cx18 driver

2009-03-30 Thread Trent Piepho
On Sun, 29 Mar 2009, Andy Walls wrote:
 On Sun, 2009-03-29 at 01:24 -0700, Trent Piepho wrote:
  wait_event() should take care of this.  wait_event(q, test) basically does:
 
  for(;;) {
  // point A
  add_me_to_waitqueue(q);
  set_current_state(TASK_UNINTERRUPTIBLE);
  if (test)
  break;
  // point B
  schedule();
  }
  clean_up_wait_stuff();

 As you know, the condition is checked even before this loop is entered,
 to avoid even being even added to a waitqueue.  (Thank God for ctags...)

I think the initial check of the condition is just an optimization and
everything will still work without it.  Seeing as all this is inlined, I
wonder if it's a good optimization...

 As you may have noticed, the original code was using
 wait_event_timeout() before like this:

 CX18_DEBUG_HI_IRQ(sending interrupt SW1: %x to send %s\n,
   irq, info-name);
 cx18_write_reg_expect(cx, irq, SW1_INT_SET, irq, irq);

 ret = wait_event_timeout(
*waitq,
cx18_readl(cx, mb-ack) == cx18_readl(cx, 
 mb-request),
timeout);

 Because waiting for the ack back is the right thing to do, but certainly
 waiting too long is not warranted.

 This gave me the occasional log message like this:

 1: cx18-0:  irq: sending interrupt SW1: 8 to send CX18_CPU_DE_SET_MDL
 2: cx18-0:  irq: received interrupts SW1: 0  SW2: 8  HW2: 0
 3: cx18-0:  irq: received interrupts SW1: 1  SW2: 0  HW2: 0
 4: cx18-0:  warning: sending CX18_CPU_DE_SET_MDL timed out waiting 10 msecs 
 for RPU acknowledgement

 Where line 1 is the driver notifiying the firmware with a SW1 interrupt.
 Line 2 is the firmware responding back to the cx18_irq_handler() with
 the Ack interrupt in SW2 (the flags match, 8  8, by design).
 Line 3 is an unrelated incoming video buffer notification for the cx18
 driver.
 Line 4 is the wait_event_timeout() timing out.

Could it be that the wait_event doesn't actually run and check its
condition until _after_ line 3?  In that case SW2 != 8 and so it goes back
to sleep?  Calling wake_up() just makes the processes on the waitq
runnable, they don't actually run until later, possibly much later.

  If your event occurs and wake_up() is called at point A, then the test
  should be true when it's checked and schedule() is never called.  If the
  event happens at point B, then the process' state will have been changed to
  TASK_RUNNING by wake_up(), remember it's already on the waitqueue at this
  point, and schedule() won't sleep.

 OK, for some reason, I thought schedule() and schedule_timeout() would
 go to sleep anyway.

AFAIK, they'll still cause the kernel schedule a process.  Maybe a
different process.  But the original process is still in TASK_RUNNING state
and so still in the run queue and will get run again.  If it was in
TASK_(UN)INTERRUPTIBLE state then it wouldn't be in the run queue and
wouldn't run again until something woke it up.

  I think what's probably happening is the test, cx18_readl(cx, mb-ack) ==
  cx18_readl(cx, mb-request), is somehow not true even though the ack has
  been received.

 A PCI bus read error could be the culprit here.  That's the only thing I
 can think of.  We only get one notification via IRQ from the firmware.


Maybe a new request was added?

 No, I lock the respective epu2apu or epu2cpu mailboxes respectively with
 a mutex.

But in your log:
 1: cx18-0:  irq: sending interrupt SW1: 8 to send CX18_CPU_DE_SET_MDL
 2: cx18-0:  irq: received interrupts SW1: 0  SW2: 8  HW2: 0
 3: cx18-0:  irq: received interrupts SW1: 1  SW2: 0  HW2: 0
 4: cx18-0:  warning: sending CX18_CPU_DE_SET_MDL timed out waiting 10 msecs 
 for RPU acknowledgement

Isn't the wait_event_timeout() waiting until line 4?  And doesn't line 3
mean something has changed the registers?  Changed them before the
wait_event finished?

  I think calling wait_event()'s with something that tests a hardware
  register is a little iffy.  It's better if the irq handler sets some driver
  state flag (atomically!) that indicates the event you were waiting for has
  happened and then you check that flag.

 I was toying with setting an atomic while in the IRQ handler.  But then
 I realized when we get the ack interrupt, the firmware should actually
 be done. So really the wakeup() is the only indicator I really need.
 Checking for ack == req is just a formality I guess.

If you use an interruptible timeout, then you could get interrupted with a
signal before the irq handler has woken you.

 There wasn't a wait_timeout(), so I had tried something like this in my
 first iteration:

It's called sleep_on_timeout(q, timeout).
--
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 3/4] ARM: DaVinci: DM646x Video: Add VPIF display driver

2009-03-30 Thread Trent Piepho
On Mon, 30 Mar 2009, Hans Verkuil wrote:
 On Thursday 26 March 2009 14:22:32 Chaithrika U S wrote:
  +   /* one field is displayed configure the next
  +  frame if it is available else hold on current
  +  frame */

Comment isn't in standard format with *'s on the side.  If you split this
into multiple functions like Hans suggested it wouldn't be indented so
much.

  +static int vpif_enum_fmt_vid_out(struct file *file, void  *priv,
  +   struct v4l2_fmtdesc *fmt)
  +{
  +   unsigned int index = 0;
  +
  +   if (fmt-index != 0) {
  +   v4l2_err(vpif_obj.v4l2_dev, Invalid format index\n);
  +   return -EINVAL;
  +   }
  +
  +   /* Fill in the information about format */
  +   index = fmt-index;
  +   memset(fmt, 0, sizeof(*fmt));

 For most if not all of these functions v4l2_ioctl2 will take care of zeroing
 the structs.

It is supposed to be all of them.  If there are any I missed let me know so
I can fix it.

  +static int vpif_g_fmt_vid_out(struct file *file, void *priv,
  +   struct v4l2_format *fmt)
  +{
[...]
  +   /* Check the validity of the buffer type */
  +   if (common-fmt.type != fmt-type)
  +   return -EINVAL;
  +
  +   if (V4L2_BUF_TYPE_VIDEO_OUTPUT != fmt-type) {
  +   if (vid_ch-std_info.vbi_supported == 0)
  +   return -EINVAL;
  +   }

All XXX_fmt_vid_out methods will only be called with fmt-type equal to
VIDEO_OUTPUT.  You don't need to check.

  +static int vpif_s_fmt_vid_out(struct file *file, void *priv,
  +   struct v4l2_format *fmt)
  +{
  +   if (V4L2_BUF_TYPE_VIDEO_OUTPUT == fmt-type) {

Don't need this check.

  +   struct v4l2_pix_format *pixfmt = fmt-fmt.pix;
  +   /* Check for valid field format */
  +   ret = vpif_check_format(channel, pixfmt);
  +   if (ret)
  +   return ret;

Rather than fail if the format requested isn't acceptable to the driver,
you should modify the requested format to something that is.  For example,
if you need an even number of lines and they ask for an odd number, reduce
the number of lines by 1 instead of failing.

  +static int vpif_enum_output(struct file *file, void *fh,
  +   struct v4l2_output *output)
  +{
  +
  +   struct vpif_config *config = vpif_dev-platform_data;
  +   int index = output-index;
  +
  +   memset(output, 0, sizeof(*output));
  +   if (index  config-output_count) {
  +   v4l2_dbg(1, debug, vpif_obj.v4l2_dev,
  +   Invalid output index\n);
  +   return -EINVAL;
  +   }
  +
  +   output-index = index;

Another save index, memset, restore index sequence that isn't needed.

  +#define ISALIGNED(a)(0 == (a%8))

 Here you need parenthesis: (0 == ((a) % 8))

If 'a' isn't unsigned, then (0 == ((a)  7)) will be more efficient.  For
unsigned values the compiler should be able to make the transformation to
using  instead of %, but not for signed.

  +struct vpif_config_params {
  +   u8 min_numbuffers;
  +   u8 numbuffers[VPIF_DISPLAY_NUM_CHANNELS];
  +   u32 min_bufsize[VPIF_DISPLAY_NUM_CHANNELS];
  +   u32 channel_bufsize[VPIF_DISPLAY_NUM_CHANNELS];
  +};

If you put the larger fields first the structure will be padded less.
--
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: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM

2009-03-29 Thread Trent Piepho
On Sun, 29 Mar 2009, Hans Verkuil wrote:
 On Sunday 29 March 2009 12:21:58 Trent Piepho wrote:
  On Sun, 29 Mar 2009, Hans Verkuil wrote:
   On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote:
From: Trent Piepho  xy...@speakeasy.org
v4l2-ioctl:  Check format for S_PARM and G_PARM
   
   
Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the
driver doesn't define a -vidioc_try_fmt_XXX() method for.  Several
other ioctls, like QUERYBUF, QBUF, and DQBUF, etc.  do this too.  It
saves each driver from having to check if the buffer type is one that
it supports.
  
   Hi Trent,
  
   I wonder whether this change is correct. Looking at the spec I see that
   g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up.
  
   So what should happen if the type is VIDEO_OVERLAY? I think the
   g/s_parm implementation in v4l2-ioctl.c should first exclude the
   unsupported types before calling check_fmt.
 
  This change doesn't actually enable g/s_parm for VIDEO_OVERLAY (or
  VBI_CAPTURE).  It's the later bttv and saa7146 changes that do that.  I
  considered this when I made those changes, as mentioned in those patch
  descriptions, but decided it was better to allow it.
 
  In those drivers g_parm only returns the frame rate, which seems just as
  valid for VIDEO_OVERLAY as it does for VIDEO_CAPTURE.  Why should the
  driver not be allowed to return the frame rate for video overlay?  Why
  should setting the overlay frame rate not be allowed?  Those seems like
  perfectly reasonable operations to me.

 Not to me. VIDEO_OVERLAY just defines where the overlay is. But the actual
 framerate is entirely dependent on the VIDEO_CAPTURE framerate. Just keep
 to the spec for now. If a new driver appears that needs it then we can
 always change it.

How does overlay depend on video capture in any way?  It's perfectly
reasonable for a driver to support _only_ overlay and not video capture.
The zr36067 chip is only designed to support uncompressed data for video
overlay for example.  Allowing uncompressed video capture is a hack that
the driver didn't have at one point.

  The spec doesn't explicitly say that only VIDEO_CAPTURE, VIDEO_OUTPUT and
  PRIVATE are supported.  It says the capture field of the parm union is
  used when type is VIDEO_CAPTURE, the output field is used for
  VIDEO_OUTPUT, and raw_data is used for PRIVATE or higher.  You're right
  in that it doesn't say what you're supposed to use for VIDEO_OVERLAY,
  VBI_CAPTURE or any other buffer types.  But it doesn't say they're not
  allowed either.  IMHO, it's likely the spec authors' intent wasn't to not
  allow g_parm with VIDEO_OVERLAY, but rather that they just didn't think
  of that case.

 No, I agree with the spec in that I see no use case for it. Should there be
 one, then I'd like to see that in an actual driver implementation and in
 that case the spec should be adjusted as well.

How about getting the frame rate of video overlay?  Works with bttv.

 In addition, g/s_parm is only used in combination with webcams/sensors for
 which overlays and vbi are irrelevant.

There are several drivers for non-webcams, like bttv, saa7134, and saa7146,
that support g_parm.  Why is returning the frame rate for video capture not
valid?  Why does the number of buffers used for read() mode only make
sense for webcams?

  Thinking about it now, I think what makes the most sense is to use
  capture for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE.  And
  use output for VBI_OUTPUT, SLICED_VBI_OUTPUT and VIDEO_OUTPUT_OVERLAY.
 
   I also wonder whether check_fmt shouldn't check for the presence of the
   s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl.
 
  I noticed that too.  saa7146 doesn't have a try_fmt call for vbi_capture
  but is apparently supposed to support it.  I sent a message about that
  earlier.

 I saw that. So why not check for s_fmt instead of try_fmt? That would solve
 this potential problem.

Because that's clearly a change that belongs in another patch.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[pull] http://linuxtv.org/hg/~tap/v4l-dvb

2009-03-28 Thread Trent Piepho
Mauro,

Please pull from http://linuxtv.org/hg/~tap/v4l-dvb

for the following 14 changesets:

01/14: build: Fix kernel output directory support
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=346bab8698ea

02/14: v4l2-ioctl:  Check format for S_PARM and G_PARM
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=44632c5fe5b2

03/14: saa7146: Remove buffer type check from vidioc_g_parm
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=c879e8f8c32b

04/14: bttv: Remove buffer type check from vidioc_g_parm
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=935d095cbc31

05/14: gspca: Stop setting buffer type, and avoid memset in querycap
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=089aaa41d473

06/14: omap24xxcam: Remove buffer type check from vidioc_s/g_parm
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=c545d5d1afcc

07/14: stkwebcam: Remove buffer type check from g_parm and q/dq/reqbufs
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=fb30bf07ca40

08/14: vino: Remove code for things already done by video_ioctl2
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=900a6e99e7b0

09/14: cafe_ccic: Remove buffer type check from XXXbuf
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=e698f5e2e1db

10/14: cx23885-417: Don't need to zero ioctl parameter fields
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=76e7154479e4

11/14: cx88-blackbird: Stop setting buffer type in XXX_fmt_vid_cap
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=ea402dd306a6

12/14: meye: Remove buffer type checks from XXX_fmt_vid_cap, XXXbuf
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=1d2ae100255d

13/14: usbvision: Remove buffer type checks from enum_fmt_vid_cap, XXXbuf
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=5dd6945acfc3

14/14: zr364xx: Remove code for things already done by video_ioctl2
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=406c54139ea6


 linux/drivers/media/common/saa7146_video.c|4
 linux/drivers/media/video/bt8xx/bttv-driver.c |3
 linux/drivers/media/video/cafe_ccic.c |   14
 linux/drivers/media/video/cx23885/cx23885-417.c   |   29 -
 linux/drivers/media/video/cx88/cx88-blackbird.c   |5
 linux/drivers/media/video/gspca/gspca.c   |5
 linux/drivers/media/video/meye.c  |   32 -
 linux/drivers/media/video/omap24xxcam.c   |7
 linux/drivers/media/video/stk-webcam.c|   11
 linux/drivers/media/video/usbvision/usbvision-video.c |   10
 linux/drivers/media/video/v4l2-ioctl.c|7
 linux/drivers/media/video/vino.c  |  335 +++---
 linux/drivers/media/video/zr364xx.c   |   16
 v4l/Makefile  |3
 14 files changed, 158 insertions(+), 323 deletions(-)

Thanks,
Trent

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


Re: [PATCH] v4l2: fill reserved fields of VIDIOC_ENUMAUDIO also

2009-03-27 Thread Trent Piepho
On Fri, 27 Mar 2009, Mauro Carvalho Chehab wrote:
 On Wed, 25 Mar 2009 17:51:39 +0100
 N?meth M?rton nm...@freemail.hu wrote:

  From: M?rton N?meth nm...@freemail.hu
 
  When enumerating audio inputs with VIDIOC_ENUMAUDIO the gspca_sunplus driver
  does not fill the reserved fields of the struct v4l2_audio with zeros as
  required by V4L2 API revision 0.24 [1]. Add the missing initializations to
  the V4L2 framework.
 
  The patch was tested with v4l-test 0.10 [2] with gspca_sunplus driver and
  with Trust 610 LCD pow...@m ZOOM webcam.

 It didn't apply against the development tree. Anyway, a recent patch removed
 the need of memset there. the memory fill with zero now happens at the same
 code we copy the structure values.

That code is in video_ioctl2, which gspca doesn't use.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] v4l2: fill reserved fields of VIDIOC_ENUMAUDIO also

2009-03-27 Thread Trent Piepho
On Fri, 27 Mar 2009, Hans Verkuil wrote:
 On Friday 27 March 2009 20:45:40 Trent Piepho wrote:
  On Fri, 27 Mar 2009, Mauro Carvalho Chehab wrote:
   On Wed, 25 Mar 2009 17:51:39 +0100
  
   N?meth M?rton nm...@freemail.hu wrote:
From: M?rton N?meth nm...@freemail.hu
   
When enumerating audio inputs with VIDIOC_ENUMAUDIO the gspca_sunplus
driver does not fill the reserved fields of the struct v4l2_audio
with zeros as required by V4L2 API revision 0.24 [1]. Add the missing
initializations to the V4L2 framework.
   
The patch was tested with v4l-test 0.10 [2] with gspca_sunplus driver
and with Trust 610 LCD pow...@m ZOOM webcam.
  
   It didn't apply against the development tree. Anyway, a recent patch
   removed the need of memset there. the memory fill with zero now happens
   at the same code we copy the structure values.
 
  That code is in video_ioctl2, which gspca doesn't use.

 Yes, gspca does use video_ioctl2. You're probably confused with uvcvideo,
 which doesn't use it.

You're right, I was thinking about N?meth's earlier patches for the same
things in uvcvideo.  This patch wasn't for gspca anyway, it was for the
v4l2 core, and Mauro's right it's not necessary as my patch series fixed
all these problems.
--
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: Fw: [PATCH] cx88: Missing failure checks

2009-03-26 Thread Trent Piepho
On Thu, 26 Mar 2009, Mauro Carvalho Chehab wrote:
 From: Alan Cox a...@lxorguk.ukuu.org.uk
 To: linux-ker...@vger.kernel.org, mche...@infradead.org
 Subject: [PATCH] cx88: Missing failure checks

 The ioremap one was reported in October 2007 (Bug 9146), the kmalloc one
 was blindingly obvious while looking at the ioremap one

 The bug suggests some other configuration for lots of I/O memory (32MB per
 device is ioremapped) but I'll leave that to the real maintainers

Each function has a 16 MB window and Linux can use three of the functions.

IIRC, all the function's register windows are the same so only one needs to
be mapped.
--
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: The right way to interpret the content of SNR, signal strength and BER from HVR 4000 Lite

2009-03-20 Thread Trent Piepho
On Fri, 20 Mar 2009, Devin Heitmueller wrote:
 On Thu, Mar 19, 2009 at 7:06 PM, Trent Piepho xy...@speakeasy.org wrote:
  The argument being put forth is based on the relative efficiency of
  the multiply versus divide opcodes on modern CPU architectures?? ?And
 
  Maybe I just like writing efficient code to do interesting things?

 Wow, um, ok.  You realize that getting the SNR on most devices is
 probably going to require an i2c call that is going to take a couple
 hundred CPU instructions, not to mention I/O, right?  And that you're
 doing this in an expensive ioctl call?  So perhaps a
 micro-optimization with no visible gain, at the cost of readability
 and complexity shouldn't be a overriding consideration?

Did I submit a patch to modify dvb-apps?  I just wanted to show how to
convert fixed point numbers to IEEE 754 floating using only integer math.
I think such things are interesting.

  that you're going to be able to get an SNR with a higher level of
  precision than 0.1 dB?? (if the hardware suggests that it can then
  it's LYING to you)
 
  Not really. ?Absolute accuracy is not going to be that good of course. ?But
  the error measurements from which SNR is calculated do support precision of
  better than 0.1 dB. ?That precision does give you more power when fine
  tuning antenna position.
 
  Put another way, what advantage is there of less precision?

 Well, here is one disadvantage:  The driver decides the SNR is 23.1.
 So I convert that to your format:  0x1719.  Then userland gets it back

You've assumed the driver will find SNR in decimal fixed point.  The lgdt
demods and the oren demods all *start* with binary fixed point.  So you
have the opposite problem were converting to decimal fixed point changes
the result.

 and goes to display it.  I'm going to show the user an SNR of
 23.09765625, and I have no way to know what the expected precision
 (and thus I don't know where to round).  So the end result is the user
 sees a really stupid number in the GUI (and might actually think it is
 more accurate than it really is).  Or when I push patches to
 applications I just round to 0.1dB anyway.  It also means apps like
 femon and zap are going to have to change to support a non-fixed width
 result with no appreciable gain in value.

Since 8 binary digits is 2.4 decimal digits, it's perfectly ok to display
SNR with 3 fixed decimal digits.  You could just as well round to 1 fixed
decimal digit too, in which case you've got exactly what you'd get if you
started with the decimal fixed point, except the extra precision is there
for a situation where it is useful.

 By saying explicitly there is one digit of precision - it allows for
 applications to know how to round, and I continue to disagree with

Binary fixed point says explicitly that there are 8 binary digits of
precision.  Decimal fixed point says there are 3.3219281 binary digits of
precision.  We stopped using BCD 30 years ago, computers do everything in
binary now.
--
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: Results of the 'dropping support for kernels 2.6.22' poll

2009-03-20 Thread Trent Piepho
On Fri, 20 Mar 2009, Devin Heitmueller wrote:
 On Fri, Mar 20, 2009 at 6:20 PM, Mauro Carvalho Chehab
 mche...@infradead.org wrote:
  My suggestion is to keep a backporting system, but more targeted at the
  end-users. The reasons are the ones explained above. Basically:

 Ok, so just so we're all on the same page - we're telling all the
 developers not willing to run a bleeding edge rc kernel to screw off?

 Got an Nvidia video card?  Go away!
 The wireless broken in this week's -rc candidate?  Go away!
 Your distro doesn't yet support the bleeding edge kernel?   Go away!
 Want to have a stable base on which to work so you can focus on
 v4l-dvb development?  Go away!

Maybe you're supposed to have ten computers running different kernels?
--
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] gspca: add missing .type field check in VIDIOC_G_PARM

2009-03-19 Thread Trent Piepho
On Thu, 19 Mar 2009, [ISO-8859-1] N?meth M?rton wrote:
 David Ellingsworth wrote:
  2009/3/18 N?meth M?rton nm...@freemail.hu:
  From: M?rton N?meth nm...@freemail.hu
 
  The gspca webcam driver does not check the .type field of struct 
  v4l2_streamparm.
  This field is an input parameter for the driver according to V4L2 API 
  specification,
  revision 0.24 [1]. Add the missing check.
 
  The missing check was recognised by v4l-test 0.10 [2] together with 
  gspca_sunplus driver
  and with Trust 610 LCD pow...@m ZOOM webcam. This patch was verified 
  also with
  v4l-test 0.10.
 
  References:
  [1] V4L2 API specification, revision 0.24
 http://v4l2spec.bytesex.org/spec/r11680.htm
 
  [2] v4l-test: Test environment for Video For Linux Two API
 http://v4l-test.sourceforge.net/
 
  Signed-off-by: M?rton N?meth nm...@freemail.hu
  ---
  --- linux-2.6.29-rc8/drivers/media/video/gspca/gspca.c.orig 2009-03-14 
  12:29:38.0 +0100
  +++ linux-2.6.29-rc8/drivers/media/video/gspca/gspca.c  2009-03-18 
  16:51:03.0 +0100
  @@ -1320,6 +1320,9 @@ static int vidioc_g_parm(struct file *fi
   {
 struct gspca_dev *gspca_dev = priv;
 
  +   if (parm-type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
  +   return -EINVAL;
  +
 memset(parm, 0, sizeof *parm);
 parm-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
  ^^^
  This line should be deleted as it's no longer needed.

 Because memset() clears the whole parm structure this line is necessary. In 
 other
 drivers the following code is there:

 tmp = parm-type;
 memset(parm, 0, sizeof(*parm));
 parm-type = parm;

The memset isn't needed anymore either, I put it into v4l2_ioctl.  I
removed most of the code like that but I may have missed some drivers.
--
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: Results of the 'dropping support for kernels 2.6.22' poll

2009-03-19 Thread Trent Piepho
On Sun, 15 Mar 2009, Hans Verkuil wrote:
 On Sunday 15 March 2009 17:39:11 Trent Piepho wrote:
  Because there are patches that touch both the media tree and outside it?
  I don't buy it.  Even for sub-systems that only use full git trees, you
  almost never see a patch that touches multiple areas of maintainership.
  It's just too hard to deal with getting acks from everyone involved,
  dealing with the out-of-sync development git trees of the multiple areas
  you want to touch, figuring out who will take your patch, etc.

 Think embedded devices like omap, davinci and other SoC devices. These all
 require changes in both v4l-dvb and arch at the same time. Easy to do if
 you have a full git tree, much harder to do in the current situation.

 These devices will become much more important in the coming months and
 years, so having a proper git tree will definitely help.

 This is a relatively new development and before that it was indeed rare to
 see patches touching on areas outside the media tree. Not anymore, though.

ALSA has a full git tree now, so there should be all these patches that
touch sound/ and something out side of sound too?  I'm not seeing them.
The only patches that touch inside and outside of ALSA are the ones that
fix some misspelled word in 100 random files or rename a linux header file.
--
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] gspca: add missing .type field check in VIDIOC_G_PARM

2009-03-19 Thread Trent Piepho
On Thu, 19 Mar 2009, [UTF-8] N??meth M??rton wrote:
 The gspca webcam driver does not check the .type field of struct 
 v4l2_streamparm.
 This field is an input parameter for the driver according to V4L2 API 
 specification,
 revision 0.24 [1]. Add the missing check.

I think this check could go in the v4l2 core too.  It already does a
similar check for QUERYBUF, QBUF, DQBUF, etc.  If the driver doesn't
provide a method for -vidioc_try_fmt_foo() then the v4l2 core will reject
a call with .type == V4L2_BUF_TYPE_foo.

It seems like it should be ok to do this for S_PARM and G_PARM too.
--
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: Improve DKMS build of v4l-dvb?

2009-03-17 Thread Trent Piepho
On Tue, 17 Mar 2009, Alain Kalker wrote:
 Op vrijdag 13-03-2009 om 02:12 uur [tijdzone -0700], schreef Trent
 Piepho:
  On Mon, 9 Mar 2009, Alain Kalker wrote:
   Firstly: generating a .config with just one config variable for the
   requested driver set to 'm' merged with the config for the kernel being
   built for, and then doing a make silentoldconfig. Big disatvantage is
   that full kernel source is required for the 'silentoldconfig' target to
   be available.
 
  Does that actually work?  Figuring out that needs to be turned on to enable
  some config options is a hard problem.  It's not just simple dependencies
  between modules, but complex expressions that need to be satisfied.  E.g.,
  something depends on A || B, which do you turn on, A or B?  There are
  multiple solutions so how does the code decide which is best?

 Well, make_kconfig.pl does quite a nice job trying to select as many
 drivers without causing conflicts.

What I did in make_kconfig.pl was just turn everything on, then recursively
disable anything that has a failed dependency.  There isn't any
intelligence when it comes to choices where you can have driver set A or
driver set B, but not both.  Options that we want disabled, like some
drivers' advanced debug controls, must be explicitly disabled in
make_kconfig.  Still, it ends up doing what we want in the end, which is to
compile all the drivers that we can compile.

 Anyway, you're quite right about this being a hard problem, and the
 fact that the Kconfig system wasn't designed to be very helpful in
 auto-selecting dependencies and resolving conflicts the same way modern
 package managers are, doesn't make it any easier.

From what I can tell, solving the dependency problem is easily shown to be
the same as the classical satisfiability problem, which is proven to be NP
complete.  Now, there are heuristics that can usually solve SAT problems
quicker but finding the best solution quickly is quite a bit harder.

 For the moment, I would suggest either to choose a default which works
 for most people, or ask the user (using any Kconfig menu tool, if only
 they didn't need write access to the kernel source, grrr!) to choose
 among alternatives if no combination of options can be selected
 automatically.

You don't need write access to the kernel source.  The kernel's config
programs have to be built, but that can be done ahead of time.  Once they
are, then you can use that menu tool from v4l-dvb without write access to
the kernel source.

There is support for an alternate output directory for the kernel that can
work too.  In the kernel dir, run make O=~/kernel-output-dir menuconfig.
That should not require write access to the kernel source dir and will put
the necessary config programs in ~/kernel-output-dir.  Then point v4l-dvb
at the kernel output dir, with make release DIR=~/kernel-output-dir.

See the explanation from my changeset that added this,
http://linuxtv.org/hg/v4l-dvb/log/6331 Good thing I wrote this 17 months
ago when I did the work, instead of just using some two word patch
description, since I sure wouldn't remember how all that works today.
--
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: bttv, tvaudio and ir-kbd-i2c probing conflict

2009-03-17 Thread Trent Piepho
On Tue, 17 Mar 2009, Jean Delvare wrote:
 On Mon, 16 Mar 2009 15:47:17 -0700 (PDT), Trent Piepho wrote:
  On Mon, 16 Mar 2009, Jean Delvare wrote:
   You are unfair. The pull request came with a short log of all the
   changes.
 
  short log.  His entire series was decribed with fewer words than I would
  use on a single patch that changes ten lines.

 In general I tend to like detailed patch logs as much as you do. But in
 this case Hans is doing almost all the work by himself and it is very
 needed, and the faster completed, the better. So I am really to trade
 log details for a faster conversion.

I guess that I don't consider documentation to be optional.

   (...)
   I am not familiar enough with this part of the code to say. But I guess
   it doesn't really matter, as it wasn't my point anyway.
 
  It seems like your point was that conversions to v4l2_subdev allow drivers
  to be more efficient remove lots of code.  The numbers I see just don't
  support that claim.

 No, sorry if I didn't make it clear, but that wasn't my point. My point
 was only about the change in i2c binding model. This change clearly
 results in a net shrink as far as lines of code are concerned.

Does it?  When we can use the model as it's designed, then I think it's
clearly much better.  But when one is emulating the detection behaviour,
like it appears the bttv patches do, I don't see what's better.
--
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: [PULL] http://linuxtv.org/hg/~dheitmueller/hvr950q-analog2/

2009-03-16 Thread Trent Piepho
On Sun, 15 Mar 2009, Devin Heitmueller wrote:
 au0828: remove memset calls in v4l2 routines.

The userland callers are responsible for clearing the output buffers, so
remove the unneeded memset calls.

A driver should not assume that _userspace_ has cleared the buffers.  In
some cases userspace is supposed to clear certain fields, but you shouldn't
assume it.  AFAIK, for read-only ioctls there is no expectation at all that
userspace will clear the buffer.

Your patch is still right though, as now the videodev core will take care
of this so individual drivers don't have to.
--
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: REVIEW: bttv conversion to v4l2_subdev

2009-03-16 Thread Trent Piepho
On Mon, 16 Mar 2009, Hans Verkuil wrote:
  +++ b/linux/drivers/media/video/bt8xx/bttvp.h  Sun Mar 15 13:07:15 2009
  +0100
  @@ -331,6 +331,7 @@ struct bttv {
 unsigned int tuner_type;  /* tuner chip type */
 unsigned int tda9887_conf;
 unsigned int svhs, dig;
  +  int has_saa6588;
 
  Does it need to be a 32 or 64 bit integer?

 I'll replace it with a u8.

 
 struct bttv_pll_info pll;
 int triton1;
 int gpioirq;

Due to field alignment in the structure it won't make any difference.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/4] pxa_camera: Redesign DMA handling

2009-03-16 Thread Trent Piepho
On Mon, 16 Mar 2009, Robert Jarzmik wrote:
 Guennadi Liakhovetski g.liakhovet...@gmx.de writes:
  What is QIF? Do you mean Quick Capture Interface - QCI? I also see CIF
  used in the datasheet, probably, for Capture InterFace, but I don't see
  QIF anywhere. Also, please explain the first time you use the
  abbreviation. Also fix it in the commit message to patch 1/4.
 OK, will replace with QCI, my bad.

In video capture, QIF and CIF usually refer to image size.  CIF would be
320x240 for NTSC and QIF or QCIF would be 160x120.
--
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: bttv, tvaudio and ir-kbd-i2c probing conflict

2009-03-16 Thread Trent Piepho
On Mon, 16 Mar 2009, Jean Delvare wrote:
 On Mon, 16 Mar 2009 12:43:33 -0700 (PDT), Trent Piepho wrote:
  On Mon, 16 Mar 2009, Jean Delvare wrote:
   Come on, just look at ir-kbd-i2c and tvaudio again, see how great are
   these drivers which have been designed on top of the legacy i2c
   binding model. Look at the bttv mess. Look at the zoran driver
   conversion done by Hans a few weeks ago, which killed what, 3000 lines
   of code? The old binding model was so bad that DVB doesn't even use it.
 
  IIRC, the zoran patch removed more like 1000 lines.

 You don't remember correctly:
  37 files changed, 4570 insertions(+), 7404 deletions(-)
 That's 2834 lines. 1570 of which is for the removal of the saa7111 and
 saa7114 and I agree this shouldn't count, so we're down to 1264 lines
 removed. The removal of some features also shouldn't count but I
 doubt it amounts to more than a few hundred lines.

1264 lines is more like 1000 lines than it is like 3000 lines!

 In fact, to get a more accurate figure you can just look at the result
 of my own conversion. There were two patches:
  13 files changed, 510 insertions(+), 625 deletions(-)
  11 files changed, 549 deletions(-)
 This is -664 lines total. The exact number doesn't really matter. The
 bottom line is that the new binding model requires significantly less
 glue code than the legacy model. If you think some more about it, it
 means a lot.

Then how come the bttv patch added lines of code and a couple kB to the
compiled driver size?

  But it also deleted
  v4l1 support, highmem support, and bigphys_area support.  Maybe other
  things, Hans doesn't decribe his patches, so there's really no way to know
  what the zoran patch really did other than to weed through 10,000+ lines of
  diff which is mostly but not entirely moving blocks of code from one space
  to another and reindenting them.

 You are unfair. The pull request came with a short log of all the
 changes.

short log.  His entire series was decribed with fewer words than I would
use on a single patch that changes ten lines.

  If one includes the v4l1-compat module that is now providing v4l1 support
  (though it doesn't work correctly for zoran), the driver and the compat
  module are larger than the old driver was.  Of course one can now turn off
  v4l1 support and get a smaller driver than before.  And the v4l1 compat
  already existed and can be shared.  But I think it's more correct to say
  the size reduction of the zoran driver was from removing features and not
  from v4l2_subdev.  It seems like more of the the other subdev conversions
  have overall added more code than they removed.

 I am not familiar enough with this part of the code to say. But I guess
 it doesn't really matter, as it wasn't my point anyway.

It seems like your point was that conversions to v4l2_subdev allow drivers
to be more efficient remove lots of code.  The numbers I see just don't
support that claim.
--
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] LED control

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Jean-Francois Moine wrote:
 On Sat, 14 Mar 2009 13:16:11 -0700 (PDT)
 Trent Piepho xy...@speakeasy.org wrote:

  There is already a sysfs led interface, you could just have the driver
  export the leds to the led subsystem and use that.

 Yes, but:
 - this asks to have a kernel generated with CONFIG_NEW_LEDS,

So?

 - the user must use some new program to access /sys/class/leds/device,

echo, cat?

 - he must know how the LEDs of his webcam are named in the /sys tree.

Just give them a name like video0:power and it will be easy enough to
associate them with the device.  I think links in sysfs would do it to,
/sys/class/video4linux/video0/device/ledname or something like that.

The advantage of using the led class is that you get support for triggers
and automatic blink functions, etc.
--
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: REVIEW: bttv conversion to v4l2_subdev

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Hans Verkuil wrote:
 Hi Mauro,

 Can you review my ~hverkuil/v4l-dvb-bttv2 tree?

It would be a lot easier if you would provide patch descriptions.
--
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: REVIEW: bttv conversion to v4l2_subdev

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Hans Verkuil wrote:
 On Sunday 15 March 2009 17:04:43 Trent Piepho wrote:
  On Sun, 15 Mar 2009, Hans Verkuil wrote:
   Hi Mauro,
  
   Can you review my ~hverkuil/v4l-dvb-bttv2 tree?
 
  It would be a lot easier if you would provide patch descriptions.

 Here it is:

 - bttv: convert to v4l2_subdev.

You aren't even trying.  I could easily write two pages on this patch.

What new module parameters did you add?  Why?  What module parameters did
you delete?  Why?  How does one translate a existing modprobe.conf file?

Why are the i2c addresses from various i2c chips moved into the bttv
driver?  Doesn't it make more sense that the addresses for chip X should be
in the driver for chip X?

How has module loading changed?  Can one no longer *not* autoload modules if
you are trying to test drivers that are not installed in /lib/modules?

What fields did you add to the card database?  Why?  How much did the size
increase?  What is the never set has_saa6588 field in tvcards needed for?

What are the parameters to bttv_call_all?

How did you change the probing sequence?  What was it before?  What is it
now?

Where do the subdevs you created get deleted?
--
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: bttv, tvaudio and ir-kbd-i2c probing conflict

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Jean Delvare wrote:
 On Sun, 15 Mar 2009 13:44:01 +0100, Hans Verkuil wrote:
 This is the typical multifunction device problem. It isn't specifically
 related to I2C, the exact same problem happens for other devices, for
 example a PCI south bridge including hardware monitoring and SMBus, or
 a Super-I/O chip including hardware monitoring, parallel port,
 infrared, watchdog, etc. Linux currently only allows one driver to bind
 to a given device, so it becomes very difficult to make per-function
 drivers for such devices.

 For very specific devices, it isn't necessarily a big problem. You can
 simply make an all-in-one driver for that specific device. The real
 problem is when the device in question is fully compatible with other
 devices which only implement functionality A _and_ fully compatible with
 other devices which only implement functionality B. You don't really
 want to support functions A and B in the same driver if most devices
 out there have either function but not both.

You can also split the device into multiple devices.  Most SoCs have one
register block where all kinds of devices, from i2c controllers to network
adapters, exist.  This is shown to linux as many devices, rather than one
massive multifunction device.
--
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: REVIEW: bttv conversion to v4l2_subdev

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Andy Walls wrote:
 On Sun, 2009-03-15 at 10:28 -0700, Trent Piepho wrote:

  Why are the i2c addresses from various i2c chips moved into the bttv
  driver?  Doesn't it make more sense that the addresses for chip X should be
  in the driver for chip X?

 One reason that this may be undesirable is that the devices can be set
 to slightly different addresses via external straps (probably a corner
 case, I know).  The bridge driver has the best chance of knowing what
 chips are where with certainty.

If one knows that some card has a certain chip at a certain address, then
it makes the most sense to store that with the rest of the card's data,
i.e. in the bridge driver.

But the bttv code looks like (I don't know what Hans intended, since he
didn't feel anyone else needed to know what, why, or how his code does
whatever it is that it does) it is probing all known address that a given
chip could be at to see if the chip is there.  In that case what's really
being used is an address list from the I2C chip's datasheet and any bridge
driver that wants to probe like that will use the same list, which IMHO,
makes more sense to put in the driver of the I2C chip rather than
every driver that uses the I2C chip.
--
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: bttv, tvaudio and ir-kbd-i2c probing conflict

2009-03-15 Thread Trent Piepho
On Sun, 15 Mar 2009, Andy Walls wrote:
 On Sun, 2009-03-15 at 18:12 +0100, Jean Delvare wrote:

  This is the typical multifunction device problem. It isn't specifically
  related to I2C,

 But the specific problem that Hans' brings up is precisely a Linux
 kernel I2C subsystem *software* prohibition on two i2c_clients binding
 to the same address on the same adapter.

For a lot of i2c devices, it would be difficult for two drivers to access
the device at the same time without some kind of locking.

If you take the reads and writes of one driver and then intersperse the
reads and writes of another driver, the resulting sequence from the i2c
device's point of view is completely broken.

But, I suppose there are some devices where if the drivers all use
i2c_smbus_read/write_byte/word_data or equivalent atomic transactions
with i2c_transfer(), then you could get away with two drivers talking to
the same chip.
--
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] LED control

2009-03-14 Thread Trent Piepho
On Sat, 14 Mar 2009, Mauro Carvalho Chehab wrote:
 On Sat, 14 Mar 2009 12:59:23 +0100
 Jean-Francois Moine moin...@free.fr wrote:

  +   entryconstantV4L2_CID_LEDS/constant/entry
  +   entryinteger/entry
  +   entrySwitch on or off the LEDs or illuminators of the device.
  +In the control value, each LED may be coded in one bit (0: off, 1: on) or 
  in
  +many bits (light intensity)./entry
  + /row
  + row

 The idea of having some sort of control over the LEDs is interesting, but we
 should have a better way of controlling it. If the LED may have more than one
 bit, maybe the better would be to create more than one CID entry. Something 
 like:

 V4L2_CID_LED_POWER- for showing that the camera is being used
 V4L2_CID_LED_LIGHT- for normal white light
 V4L2_CID_LED_INFRARED - for dark light, using infrared
 ...

 This way a driver can enumberate what kind of leds are available, and get the
 power intensity range for each individual one.

There is already a sysfs led interface, you could just have the driver
export the leds to the led subsystem and use that.
--
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: identifying camera sensor

2009-03-13 Thread Trent Piepho
On Thu, 5 Mar 2009, Hans Verkuil wrote:
 Reserved bits are there for a reason. If a particular bit of information
 it a perfect match with for that API, then it seems utterly pointless to
 me to decide not to use them 'just because we might run out in the
 future'.

It would be one thing if there was just a possibility of them running out
in the distant future.  Rather, they'll run out before you even get started
on adding what's already available for digital cameras.  Focus distance,
exposure compensation, sensor manufacture and model, gravity sensors,
detailed gamma compensation curves, and so on.


  Though if one had considered allowing the control api to be used to
  provide
  sensor properties, then the solution to this problem would now be quite
  simple and obvious.

 In this case you want to have device names. While not impossible, it is
 very hard to pass strings over the control api. Lots of issues with 32-64
 bit compatibility and copying to/from user space. Also, in this case the

That's true.  While I think the control api is the best one for providing
ancillary data for images and sensor attributes, it's not perfect.  I'd add
a means to define the data type of a control/attribute and allow things
besides an s32.  Probably ASCIIZ strings and fix length byte arrays.

Also flags for:

constant vs volatile
  Constant attributes don't change, e.g. sensor manufacturer name.  Volatile
  ones can change, e.g. focus distance.

global vs per-input
  Global attributes are the same for the entire device, while per-input
  attributes are different for each input.  An API for querying the
  attribute for an input different the current might be nice, but I think
  it might be one of things that seem more important than they really are.

Frame syncable
  Frame syncable attributes (which only make sense for volatile attributes)
  can be synced to the exact frame they were measured at.  For instance,
  the the camera provides a focus distance value for software face
  tracking, it's important that the right focus distance be associated with
  the correct frame.  There should be an API by which one can request that
  the attribute be provided with each frame, maybe tacked onto the end of
  the v4l2_buffer structure.

 control API is NOT a good match, since this isn't a single piece of data,
 instead you can have multiple sensor devices or other video enhancement
 devices that an application might need to know about. Which is why my last
 brainstorm suggestion was an ENUM_CHIPS ioctl.

How would enum chips be different than enum controls?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] cx88: Add IR support to pcHDTV HD3000 HD5500

2009-03-13 Thread Trent Piepho
On Thu, 5 Mar 2009, Erik S. Beiser wrote:
 Thanks for your comments, Trent.  My responses below:

 Trent Piepho wrote:
  On Sat, 28 Feb 2009, Erik S. Beiser wrote:
 
  cx88: Add IR support to pcHDTV HD3000  HD5500
 
  Signed-off-by: Erik S. Beiser er...@bu.edu
 
  ---
 
  Idea originally from http://www.pchdtv.com/forum/viewtopic.php?t=1529
  I made it into this small patch and added the HD3000 support also, which I 
  have  I've actually
  been using this for over a year, updating for new kernel versions.  I'm 
  tired of doing so,
  and would like to try and have it merged upstream -- I hope I got all the 
  patch-mechanics
  correct.  I just updated and tested it today on 2.6.28.7 vanilla.  Thanks.
 
  You forgot a patch description.
 Ok, I had looked around and saw many patches that had the one-liner
 descriptions, which I thought would be sufficient for a four line
 patch.  At your request, I can add a couple lines description when I
 fix it (see below).

You won't see such patch descriptions from me.  Clearly your patch made
some very significant changes and assumptions that really should have in
the description.

  Since neither the HD-3000 or HD-5500 came with a remote, and at least my
  HD-3000 didn't even come with an IR receiver.  So I have to ask why
  configuring the driver to work a remote you happened to have is any more
  correct than configuring it to work a remote someone else happens to have?
 True, the vendor doesn't seem to sell a remote or IR receiver with
 these cards.  I was actually surprised when I got mine to see the jack
 for an IR receiver, which can be made to work if one has those parts
 from another source.  I don't think that because these cards were sold
 without a remote and receiver should mean that we don't expose the
 receiver functionality.  I didn't see that happening elsewhere, so I
 adopted this 'worksforme' solution.  You have a valid point about the
 key mapping, which I did not fully consider.  I don't have a good
 justification.  OTOH how does someone with one of those cards use a
 remote different from what came with their card?  Is that possible?

The problem with exposing the receiver this way is that it's unlikely to be
useful to anyone but yourself.  Given the significant performance cost of
enabling ir sampling, and the very limited usefulness, I don't think this
belongs in the kernel.

 All I'm really trying to accomplish is to somehow get inputs from a
 remote exposed through some device, with which I could parse stuff
 with lirc.  This method exposed it via a /dev/input/eventN node.  I
 admit I hadn't paid too much attention to the keymapping before.
 (Just trying to get the thing to work.)  But you prompted me to dig
 deeper, and I see that in drivers/media/common/ir-keymaps.c there is a
 ir_codes_empty mapping.  I tried it tonight with that mapping instead,
 and a /dev/input/eventN device was created, but I don't seem to get
 anything from it.  Which I guess isn't too surprising, but if so, then
 how can I go about generically exposing signals from the IR port to
 userspace?

You might look at the patches from Jon Simrl that try to add IR support to
the input system.  They use configfs to define remote keycodes.

You could also create a device the exports the raw timings to lircd to
decode.  I think lircd might still use the mode2 interface I created for
the ir serial driver over a decade ago.  That would be easy to copy.

  This patch also causes these cards to generate 101 interrupts per second
  per card, even when not in use.  That seems pretty costly for a card that
  doesn't even come with an ir sensor.
 Whoa, I didn't realize that.  Can you point me to how I can verify
 that?  Is that simply an effect of the ir-sampling type?  Might a
 different type be preferred?  I could do some experimenting.

Just look at /proc/interrupts.  The ir sampling mode isn't documented in
the cx23880 datasheet, but it looks like it samples gpio16 at about 250 us
intervals and generate an interrupt each time 32 samples are ready.  Hugely
inefficient.  A serial port ir receiver would be much better.  The cx23880
can operate that way with gpio22 and gpio23, but I think the mpeg ts
interface uses those gpios.

Maybe there is some way to turn on the remote sampling when something is
listening for remote events?
--
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: Kconfig changes in /hg/v4l-dvb caused dvb_usb_cxusb to stop building (fwd)

2009-03-13 Thread Trent Piepho
On Mon, 9 Mar 2009, Mauro Carvalho Chehab wrote:
 Btw, if you look at DVB_FE_CUSTOMISE help, it is recommended tho unselect it,
 if you're not sure what to do.

   
   Anyways, here's what I get:
 
   $ grep ^CONFIG .config
   [everything is 'm']
   CONFIG_DVB_VES1820=m
   CONFIG_DVB_STV0297=m
   CONFIG_DVB_LNBP21=m

 Seems perfect to my eyes.

I think it might be nicer if the default value for a frontend when
customize was turned on was whatever it was selected to by the drivers that
use it.

When you don't use customize, all the frontends default to 'n'.  If you
set some driver to 'm', it will set all the frontends it uses to 'm'.  Set
the driver to 'y' and then those frontends get set to 'y'.

If you turn on customize, the default for the frontends should be same as
what they are when customize is off.  The difference is now you can see
them and change their value.
--
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: Improve DKMS build of v4l-dvb?

2009-03-13 Thread Trent Piepho
On Mon, 9 Mar 2009, Alain Kalker wrote:
 Martin has an older version of the drivers packaged for building with
 DKMS on Ubuntu in his PPA[5], but it currently has some disadvantages:

 A. It builds all available drivers, no matter which hardware is actually
 installed in the system. This takes a lot of time, and may not be
 practical at all on systems with limited resources (e.g. embedded, MIDs,
 netbooks)
 B. It currently has no support for Jockey to detect installed hardware,
 so individual drivers can be selected.

 To address these issues, I would like to propose the following:

 A. Building individual drivers (i.e. sets of modules which constitute a
 fully-functional driver), without having to manually configure them
 using make menuconfig

 I see two possibilities for realizing this:
 Firstly: generating a .config with just one config variable for the
 requested driver set to 'm' merged with the config for the kernel being
 built for, and then doing a make silentoldconfig. Big disatvantage is
 that full kernel source is required for the 'silentoldconfig' target to
 be available.

Does that actually work?  Figuring out that needs to be turned on to enable
some config options is a hard problem.  It's not just simple dependencies
between modules, but complex expressions that need to be satisfied.  E.g.,
something depends on A || B, which do you turn on, A or B?  There are
multiple solutions so how does the code decide which is best?

 Secondly, the script v4l/scripts/analyze_build.pl generates a list of
 modules that will get built for each Kconfig variable selected, but it
 currently has no way of determing all the module dependencies that make
 up a fully functional driver.

I just wrote analyze_build.pl to make it easier for developers to figure
out that source files make up a module and how to enable it.  It's not
actually used by the build system.  It's also not perfect when it comes to
parsing makefiles, i.e. it no where near a re-implementation of make's
parser in perl.  It understands the typical syntax used by the kernel
makefiles but sometimes there is some unusual bit of make code that it
won't parse.

 The script v4l/scripts/check_deps.pl tries to discover dependencies
 between Kconfig variables, but it currently is somewhat slow, and hase a
 few other problems.

That it is!  It's not totally perfect either.  Sometimes a driver will only
depend on another if something is turned on.  But the way check_deps.pl
works won't know that.  There are also lots of Kconfig variables that don't
turn on a module but instead modify what a module does or are used for
menus.  I think a better system would be use the dependencies in the
Kconfig files instead of trying to figure them out from the source.

 B. To enable hardware autodetection before installing drivers, we need
 to have a list of modaliases of all supported hardware. This may be the
 hardest part, because many VendorIDs and ProductIDs are scattered
 throughout the code. Also coldbooting/warmbooting hardware is a problem.

Extract that from the compiled modules should be easy.
--
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


[pull] http://linuxtv.org/hg/~tap/v4l-dvb

2009-03-13 Thread Trent Piepho
Mauro,

Please pull from http://linuxtv.org/hg/~tap/v4l-dvb

for the following changeset:

01/01: build: have make_kconfig.pl ignore comments
http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=9debb0a2ec70


 make_kconfig.pl |   13 +
 1 files changed, 9 insertions(+), 4 deletions(-)

Thanks,
Trent

--
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: The right way to interpret the content of SNR, signal strength and BER from HVR 4000 Lite

2009-03-13 Thread Trent Piepho
On Fri, 13 Mar 2009, Devin Heitmueller wrote:
 On Fri, Mar 13, 2009 at 5:11 PM, Trent Piepho xy...@speakeasy.org wrote:
  I like 8.8 fixed point a lot better. ?It gives more precision. ?The range
  is more in line with that the range of real SNRs are. ?Computers are
  binary, so the math can end up faster. ?It's easier to read when printed
  out in hex, since you can get the integer part of SNR just by looking at
  the first byte. ?E.g., 25.3 would be 0x194C, 0x19 = 25 db, 0x4c = a little
  more than quarter. ?Several drivers already use it.

 Wow, I know you said you like that idea alot better, but I read it and
 it made me feel sick to my stomach.  Once we have a uniform format, we
 won't need to show it in hex at all.  Tools like femon and scan can

But if you do see it in hex, it's easier to understand.  If it's not shown
in hex, you still have better precision and better math.  What advantage is
there to using something that's 4.1 decimal fixed point on a binary
computer?

 On a separate note, do you know specifically which drivers use that
 format?  I was putting together a table of all the various

or51211, or51132, and lgdt330x at least.  Don't know about the other lg
demods.  The dvb match code they all use makes it's easy to get the snr in
8.8 fixed point with the typical log caclulations required.
--
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: The right way to interpret the content of SNR, signal strength and BER from HVR 4000 Lite

2009-03-13 Thread Trent Piepho
On Fri, 13 Mar 2009, Andy Walls wrote:
 On Fri, 2009-03-13 at 10:27 -0400, Devin Heitmueller wrote:
  On Fri, Mar 13, 2009 at 12:19 AM, Ang Way Chuang wc...@nav6.org wrote:
  
   Yes, please :)
 
  Yeah, Michael Krufky and I were discussing it in more detail yesterday
  on the #linuxtv ML.  Essentially there are a few issues:
 
  1.  Getting everyone to agree on the definition of SNR, and what
  units to represent it in.  It seems like everybody I have talked to
  has no issue with doing in 0.1 dB increments, so for example an SNR of
  25.3 would be presented as 0x00FD.

 +/- 0.1 dB indicates increases or decreases of about 2.3% which should
 be just fine as a step size.

I've found that the extra precision helps when trying to align an antenna.
I turn the antenna a few degrees and then measure snr for a while.  Then
make a plot of snr vs antenna rotation.  With the extra precision you can
see the average snr change as you fine tune to rotation.  Rounding off the
extra digit makes it harder to see.

What is the advantage to using base 10 fixed point on binary computer?
--
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] Add support for ProVideo PV-183 to bttv

2009-03-12 Thread Trent Piepho
On Fri, 13 Mar 2009, Alan McIvor wrote:
 +
 +{ 0x15401830, BTTV_BOARD_PV183, Provideo PV183-1 },
 +{ 0x15401831, BTTV_BOARD_PV183, Provideo PV183-2 },
 +{ 0x15401832, BTTV_BOARD_PV183, Provideo PV183-3 },
 +{ 0x15401833, BTTV_BOARD_PV183, Provideo PV183-4 },
 +{ 0x15401834, BTTV_BOARD_PV183, Provideo PV183-5 },
 +{ 0x15401835, BTTV_BOARD_PV183, Provideo PV183-6 },
 +{ 0x15401836, BTTV_BOARD_PV183, Provideo PV183-7 },
 +{ 0x15401837, BTTV_BOARD_PV183, Provideo PV183-8 },
 +
   { 0, -1, NULL }
  };

Looks like you used spaces here instead of tabs.  If you run make commit
from the v4l-dvb tree it will fix these things.
--
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/4] pxa_camera: Remove YUV planar formats hole

2009-03-10 Thread Trent Piepho
On Mon, 9 Mar 2009, Robert Jarzmik wrote:
  Ok, this one will change I presume - new alignment calculations and
  line-breaking. In fact, if you adjust width and height earlier in set_fmt,
  maybe you'll just remove any rounding here completely.
 Helas, not fully.
 The problem is with passthrough and rgb formats, where I don't enforce
 width/height. In the newest form of the patch I have this :
   if (pcdev-channels == 3)
   *size = icd-width * icd-height * 2;
   else
   *size = roundup(icd-width * icd-height *
   ((icd-current_fmt-depth + 7)  3), 8);

If icd-current_fmt-depth could be set to 16 for planar formats, then you
could get rid of the special case here.
--
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


  1   2   >