Re: [PULL] http://www.kernellabs.com/hg/~stoth/cx23885-mpx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 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.)


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?

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.

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. 


Regards,
Andy


> 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
> 


--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux