On 7/31/10 2:28 PM, Andy Walls wrote:
On Sat, 2010-07-31 at 12:31 -0400, Steven Toth wrote:
Mauro,
Please pull from http://www.kernellabs.com/hg/~stoth/cx23885-mpx
A pretty large patch set which adds a number of important features to
the cx23885 driver.
I have a few cx25840 related comments:
1. Please don't abuse CX25840_AUDIOn and make it mean something other
than its current usage of specifying which input the SIF audio is coming
in on. The cx25840 module has enough confusion in it already. Let's
not overload the current enumerations.
Also the current cx25840 keys off of CX25840_AUDIOn vs
CX25840_AUDIO_SERIAL to set up a number of things: sample rate
convertors and the Baseband Path 1 routing for the CX23885 family at
least.
It would be better to add new enumaerated values for CX23885_AUDIO_LR1,
or whatever, to achieve your desired audio input configuration tasks.
Noted. Thanks for the feedback Andy.
2. The raw VBI startup you added in the cx25840 module is not going to
serve you well. It is true that it is harmless to existing
non-CX2388[578] boards. However, any app action that causes
cx25840_std_setup() to be called will change register 0x474 to probably
something you are not expecting.
It's held up very well in testing. With ntsc-zv-vbi and mencoder I'm not seeing
any issues. It could be that the API sequence is working in the customer favor
but non-the-less I don't have any open bugs against VBI.
It should be made clear that VBI never previous worked on the cx23885, for any
cards.
It would be better for you, in the long run, to fix up
cx25840_std_setup() and cx25840_s_raw_fmt() to suit your needs, rather
than a one off setting in the init function..
(I will admit the setup of the vblank, voffset, etc. in the cx25840 is
not trivial, due to the way the ivtv driver likes to capture raw vbi
lines and sliced vbi as if it were raw vbi data. I puzzled most of it
out and fixed it up in the cx18 driver to be more sensible. It took a
bit of staring at the BT878 data sheet to figure out the timing of the
VBI slicer too, since the data is missing from later data sheets.)
Ack. VBI via the cx25840 is a mess generally.
However, the cx25840 driver is close enough to the avcore that doing a
cx23885-avcore.c and largely closing all of the source code from cx25840 is an
exercise in growing the linuxtv tree for little value.
3. This caught my eye:
8 + if (is_cx2388x(state)) {
9 + /* for cx23885 volume doesn't work,
10 + * the calculation always results in
11 + * e4 regardless.
12 + */
13 + cx25840_write(client, 0x8d4, volume);
14 + } else
15 + cx25840_write(client, 0x8d4, 228 - (vol * 2));
Why is the result always 0xe4? Is that what the register always reads
back?
Yes.
Note that your change won't have an intuitive effect, because you
dropped the '-' sign in front of the volume. The user's slider control
will likely work in reverse: up is soft, down is loud.
This is a mistake on my end, although it represents no known regression given
that 0xe4 note and the lact of audio support in the kernel prior to this
patchset for the cx23885.
I can say the V4L2 control to cx25840 volume scale mapping is a little
funny. Here is documentation on the mapping from V4L2 volume control
values to dB to cx25840 register values:
http://linuxtv.org/hg/v4l-dvb/file/9652f85e688a/linux/drivers/media/video/cx18/cx18-alsa-mixer.c#l38
Thanks.
It is non-linear at the bottom end with a reg value of 228 = 0xe4 for
3/16ths of the entire range. This is due to the cx25840 module's desire
to keep volume levels at the same perceptible level between the MSP400
and the CX2584x chips for users with multiple ivtv cards in their
machine. (Yet another ivtv legacy.)
You ought to look at what values are coming from the V4L2 volume control
slider and see what's wrong. A V4L2 volume control value in the range
0xee00-0xeeff should correspond to 0 dB gain.
It's been a while but form memory I added debug to the function and passed the
volume control and found the register values (and calculations to be way off),
certainly not what I expected. I figured this was an issue outside of the realm
of the cx23885 and thus isolated my change to the cx23885 only, not effecting
any other products.
Regards,
- Steve
--
Steven Toth - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
