Re: [PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx
On Fri, 2010-08-06 at 17:28 +0200, Hans Verkuil wrote: On Thursday 05 August 2010 14:11:09 Andy Walls wrote: The cx25840 module expects the volume control to have a range from 0-65535. It divides the input control value by 512 (i.e. volume 9). If you are using a control in the cx23885 driver with a range from 0-100 or 0-255, that would explain why it doesn't work. BTW, the actual relationship the cx25840 module is implementing is explained in the end of this old post: http://www.gossamer-threads.com/lists/ivtv/devel/39868#39868 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 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. I had a discussion with Mauro on irc about this. The whole 0-65535 range for the volume in cx25840 is purely historical: Well, yes. the audio V4L1 'controls' always had that fixed range (you couldn't specify min and max values in V4L1) and that was just blindly propagated to V4L2. Ah, I was unaware of the V4L1 origin. For V4L2 it would be much cleaner to base the min/max values on the hardware. I really would like to specify things in dB or 0.1 dB for volume controls. Don't most of the chips' datasheets have a volume control register value to dB specified anyway? 0 dB is a very meaningful data point to anyone whose ever played with an audio mixer board. ALSA controls have a somewhat elaborate scheme for creating controls with a dB scale: http://git.linuxtv.org/hverkuil/v4l-dvb.git?a=blob;f=drivers/media/video/cx18/cx18-alsa-mixer.c;h=341bddc00b774f5eb5338e224c55d6b8d973416a;hb=refs/heads/ctrlfw2#l135 Maybe I shouldn't have done 100ths of a dB (that's just silly) since the '843 only supports steps of 0.5 dB, but I was copying from some example code at the time. Since the v4l2_ctrl framework handles negative values, it should be easy enough to present a scale in 1 dB units to a user. The only disadvantage is that apps that store the default control volume in some config file will suddenly max out the volume if that driver changes the min/max values for the volume to 0-127 (same for bass, treble, etc). Effectively you are changing the public API for these devices. Is it worthwhile doing this? The engineer in me says 'yes', but from a user-perspective it may not be such a great idea after all. Well, blasting the volume at the user will be a one time, auto correcting change (as the user scrambles for the volume control to save his eardrums). However, until subdev device node controls are exposed to userspace, I see no pressing need right now to change the cx25840 module's volume control. The current expected volume range is mostly a private API between the bridge drivers (ivtv, pvrusb2, cx23885, cx231xx) and the cx25840 module. (I assume Steven was just aware of the expected range, as the code has no comments.) When users can manipulate cascaded volume controls, e.g. a WM8775 volume control followed by a CX25843 volume control, to do things like improve audio noise performance, then using a meaningful scale between subdevs will be more important. Currently users will fiddle with the single V4L2 volume control to suit their taste, no matter what the absolute range of values. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx
On Thursday 05 August 2010 14:11:09 Andy Walls wrote:
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.
The cx25840 module expects the volume control to have a range from
0-65535. It divides the input control value by 512 (i.e. volume 9).
If you are using a control in the cx23885 driver with a range from 0-100
or 0-255, that would explain why it doesn't work.
BTW, the actual relationship the cx25840 module is implementing is
explained in the end of this old post:
http://www.gossamer-threads.com/lists/ivtv/devel/39868#39868
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 guess my real gripe is with the conditional code branch when it is not
needed AFAICT, if one feeds the cx25840 module with a volume control
range of 0-65535 that it is expecting.
IIRC the change set also does not have a complementary conditional code
branch when reading out the volume value from the register. (But I
don't have time to double check that right now - late for work.)
Regards,
Andy
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.
I had a discussion with Mauro on irc about this. The whole 0-65535 range for the
volume in cx25840 is purely historical: the audio V4L1 'controls' always had
that
fixed range (you couldn't specify min and max values in V4L1) and that was just
blindly propagated to V4L2.
For V4L2 it would be much cleaner to base the min/max values on the hardware.
The only disadvantage is that apps that store the default control volume in some
config file will suddenly max out the volume if that driver changes the min/max
values for the volume to 0-127 (same for bass, treble, etc).
Effectively you are changing the public API for these devices. Is it worthwhile
doing this? The engineer in me says 'yes', but from a user-perspective it may
not be such a great idea after all.
I hope this clarifies some of the history behind these ranges.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
To unsubscribe from this list: send the line unsubscribe 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.kernellabs.com/hg/~stoth/cx23885-mpx
Em 06-08-2010 12:28, Hans Verkuil escreveu:
On Thursday 05 August 2010 14:11:09 Andy Walls wrote:
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.
The cx25840 module expects the volume control to have a range from
0-65535. It divides the input control value by 512 (i.e. volume 9).
If you are using a control in the cx23885 driver with a range from 0-100
or 0-255, that would explain why it doesn't work.
BTW, the actual relationship the cx25840 module is implementing is
explained in the end of this old post:
http://www.gossamer-threads.com/lists/ivtv/devel/39868#39868
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 guess my real gripe is with the conditional code branch when it is not
needed AFAICT, if one feeds the cx25840 module with a volume control
range of 0-65535 that it is expecting.
IIRC the change set also does not have a complementary conditional code
branch when reading out the volume value from the register. (But I
don't have time to double check that right now - late for work.)
Regards,
Andy
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.
I had a discussion with Mauro on irc about this. The whole 0-65535 range for
the
volume in cx25840 is purely historical: the audio V4L1 'controls' always had
that
fixed range (you couldn't specify min and max values in V4L1) and that was
just
blindly propagated to V4L2.
For V4L2 it would be much cleaner to base the min/max values on the hardware.
The only disadvantage is that apps that store the default control volume in
some
config file will suddenly max out the volume if that driver changes the
min/max
values for the volume to 0-127 (same for bass, treble, etc).
Effectively you are changing the public API for these devices. Is it
worthwhile
doing this? The engineer in me says 'yes', but from a user-perspective it may
not be such a great idea after all.
Well, the API has not changed. It won't require any change at the userspace
programs.
The only action from users is to store a new default.
While the better would be to not needing to do this, e. g. doing it right since
the
first version of the driver, it is OK to change the range, to better reflect
the hardware
needs.
Btw, we've done things like that before, like changing the default values for
the
color control, at cx88 (since the max value there means 200% of color, instead
of
100%).
We just need to be sure that no driver is expecting the old behavior when doing
that
change.
I hope this clarifies some of the history behind these ranges.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx
Mauro, Please pull from http://www.kernellabs.com/hg/~stoth/cx23885-mpx - cx23885: prepare the cx23885 makefile for alsa support - cx23885: merge mijhail's header changes for alsa - cx23885: ALSA support - cx23885: core changes requireed for ALSA - cx23885: add definitions for HVR1500 to support audio - cx23885: correct the contrast, saturation and hue controls - cx23885: hooks the alsa changes into the video subsystem - cx23885: convert call clients into subdevices - cx23885: replaced spinlock with mutex - cx23885: minor function renaming to ensure uniformity - cx23885: setup the dma mapping for raw audio support - cx23885: mute the audio during channel change - cx23885: add two additional defines to simplify VBI register bitmap handling - cx23885: initial support for VBI with the cx23885 - cx23885: initialize VBI support in the core, add IRQ support, register vbi device - cx23885: minor printk cleanups and device registration - cx25840: enable raw cc processing only for the cx23885 hardware - cx23885: vbi line window adjustments - cx23885: add vbi buffer formatting, window changes and video core changes - cx23885: Ensure the VBI pixel format is established correctly. - cx23885: convert from snd_card_new() to snd_card_create() - cx23885: ensure video is streaming before allowing vbi to stream - cx23885: vbi related codingstyle cleanups - cx23885: removal of VBI and earlier VBI printk debugging - cx23885: removal of redundant code, this is no longer required. - cx23885: remove channel dump diagnostics when a vbi buffer times out. - cx23885: Ensure VBI buffers timeout quickly - bugfix for vbi hangs during streaming. - cx23885: coding style violation cleanups - cx23885: Convert a mutex back to a spinlock - cx23885: Name an internal i2c part and declare a bitfield by name - cx25840: Enable support for non-tuner LR1/LR2 audio inputs - cx23885: Allow the audio mux config to be specified on a per input basis. - cx23885: remove a line of debug - cx23885: Enable audio line in support from the back panel - cx25840: Ensure AUDIO6 and AUDIO7 trigger line-in baseband use. - cx23885: Initial support for the MPX-885 mini-card - cx23885: fixes related to maximum number of inputs and range checking - cx23885: add generic functions for dealing with audio input selection - cx23885: hook the audio selection functions into the main driver - cx23885: v4l2 api compliance, set the audioset field correctly - cx23885: Removed a spurious function cx23885_set_scale(). - cx23885: Avoid stopping the risc engine during buffer timeout. - cx23885: Avoid incorrect error handling and reporting - cx23885: Stop the risc video fifo before reconfiguring it. b/linux/drivers/media/video/cx23885/cx23885-alsa.c | 542 + linux/Documentation/video4linux/CARDLIST.cx23885 |1 linux/drivers/media/video/cx23885/Makefile |2 linux/drivers/media/video/cx23885/cx23885-alsa.c | 28 linux/drivers/media/video/cx23885/cx23885-cards.c | 53 linux/drivers/media/video/cx23885/cx23885-core.c | 127 +- linux/drivers/media/video/cx23885/cx23885-i2c.c|1 linux/drivers/media/video/cx23885/cx23885-reg.h|3 linux/drivers/media/video/cx23885/cx23885-vbi.c| 96 + linux/drivers/media/video/cx23885/cx23885-video.c | 556 ++ linux/drivers/media/video/cx23885/cx23885.h| 65 + linux/drivers/media/video/cx25840/cx25840-audio.c |9 linux/drivers/media/video/cx25840/cx25840-core.c | 21 13 files changed, 1257 insertions(+), 247 deletions(-) A pretty large patch set which adds a number of important features to the cx23885 driver. Some early patches for the HVR1500 with add support for analog audio (very rough, much rework on these). The University of California sponsored work for the HVR1800 and HVR1850 and raw video and raw audio and VBI support. The Belac Group sponsored changes related to the MPX cx23885 8 input design, adding raw video and audio support. Mencoder now works correctly with the raw video and audio portions of the driver. GStreamer now works correctly using the v4l interfaces from the driver, live video and audio viewing are now possible. NTSC-ZZ-VBI now works correctly for RAW VBI decoding (although TVTime still refuses to work correctly - tvtime bug) 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx
On Sat, 2010-07-31 at 12:31 -0400, Steven Toth wrote: Mauro, Please pull from http://www.kernellabs.com/hg/~stoth/cx23885-mpx - cx23885: prepare the cx23885 makefile for alsa support - cx23885: merge mijhail's header changes for alsa - cx23885: ALSA support - cx23885: core changes requireed for ALSA - cx23885: add definitions for HVR1500 to support audio - cx23885: correct the contrast, saturation and hue controls - cx23885: hooks the alsa changes into the video subsystem - cx23885: convert call clients into subdevices - cx23885: replaced spinlock with mutex - cx23885: minor function renaming to ensure uniformity - cx23885: setup the dma mapping for raw audio support - cx23885: mute the audio during channel change - cx23885: add two additional defines to simplify VBI register bitmap handling - cx23885: initial support for VBI with the cx23885 - cx23885: initialize VBI support in the core, add IRQ support, register vbi device - cx23885: minor printk cleanups and device registration - cx25840: enable raw cc processing only for the cx23885 hardware - cx23885: vbi line window adjustments - cx23885: add vbi buffer formatting, window changes and video core changes - cx23885: Ensure the VBI pixel format is established correctly. - cx23885: convert from snd_card_new() to snd_card_create() - cx23885: ensure video is streaming before allowing vbi to stream - cx23885: vbi related codingstyle cleanups - cx23885: removal of VBI and earlier VBI printk debugging - cx23885: removal of redundant code, this is no longer required. - cx23885: remove channel dump diagnostics when a vbi buffer times out. - cx23885: Ensure VBI buffers timeout quickly - bugfix for vbi hangs during streaming. - cx23885: coding style violation cleanups - cx23885: Convert a mutex back to a spinlock - cx23885: Name an internal i2c part and declare a bitfield by name - cx25840: Enable support for non-tuner LR1/LR2 audio inputs - cx23885: Allow the audio mux config to be specified on a per input basis. - cx23885: remove a line of debug - cx23885: Enable audio line in support from the back panel - cx25840: Ensure AUDIO6 and AUDIO7 trigger line-in baseband use. - cx23885: Initial support for the MPX-885 mini-card - cx23885: fixes related to maximum number of inputs and range checking - cx23885: add generic functions for dealing with audio input selection - cx23885: hook the audio selection functions into the main driver - cx23885: v4l2 api compliance, set the audioset field correctly - cx23885: Removed a spurious function cx23885_set_scale(). - cx23885: Avoid stopping the risc engine during buffer timeout. - cx23885: Avoid incorrect error handling and reporting - cx23885: Stop the risc video fifo before reconfiguring it. b/linux/drivers/media/video/cx23885/cx23885-alsa.c | 542 + linux/Documentation/video4linux/CARDLIST.cx23885 |1 linux/drivers/media/video/cx23885/Makefile |2 linux/drivers/media/video/cx23885/cx23885-alsa.c | 28 linux/drivers/media/video/cx23885/cx23885-cards.c | 53 linux/drivers/media/video/cx23885/cx23885-core.c | 127 +- linux/drivers/media/video/cx23885/cx23885-i2c.c|1 linux/drivers/media/video/cx23885/cx23885-reg.h|3 linux/drivers/media/video/cx23885/cx23885-vbi.c| 96 + linux/drivers/media/video/cx23885/cx23885-video.c | 556 ++ linux/drivers/media/video/cx23885/cx23885.h| 65 + linux/drivers/media/video/cx25840/cx25840-audio.c |9 linux/drivers/media/video/cx25840/cx25840-core.c | 21 13 files changed, 1257 insertions(+), 247 deletions(-) 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. 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 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
