On Fri, 2009-12-04 at 09:46 +0100, Martin Dauskardt wrote:
> > Here's the patch I'm ready to apply to the latest ivtv driver, but could
> > you do more extensive testing - starting and stopping captures - to make
> > sure your machine never hangs with your fix.
> >
> >
> > diff -r e0cd9a337600 linux/drivers/media/video/ivtv/ivtv-streams.c
> > --- a/linux/drivers/media/video/ivtv/ivtv-streams.c Sun Nov 29 12:08:02
> > 2009 -0200 +++ b/linux/drivers/media/video/ivtv/ivtv-streams.c Wed Dec
> > 02
> > 20:31:43 2009 -0500 @@ -576,10 +576,16 @@
> > clear_bit(IVTV_F_I_EOS, &itv->i_flags);
> >
> > /* Initialize Digitizer for Capture */
> > - v4l2_subdev_call(itv->sd_video, video, s_stream, 0);
> > + if (itv->sd_video->grp_id & IVTV_HW_CX25840)
> > + v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> > + else
> > + v4l2_subdev_call(itv->sd_video, video, s_stream, 0);
> > ivtv_msleep_timeout(300, 1);
> > +
> > ivtv_vapi(itv, CX2341X_ENC_INITIALIZE_INPUT, 0);
> > - v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> > +
> > + if (!(itv->sd_video->grp_id & IVTV_HW_CX25840))
> > + v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> > }
> >
> > /* begin_capture */
>
>
> why this:
> > + if (itv->sd_video->grp_id & IVTV_HW_CX25840)
> > + v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> we never turned off the cx25840 digitizer before, so there should be no need
> to turn it on.
Martin,
It is not "turning the digitizer on/off" but really "enable/disable the
clock output pins of the digitizer". The cause of the tinny audio is
likely that the clock pins from the CX2584x are floating.
This call makes sure the clock output enables are turned on and the pins
are not floating. I suspect no where else are they explciity enabled in
the driver before the first capture (I can go back and verify).
> Why do you suggest
>
> (itv->sd_video->grp_id & IVTV_HW_CX25840)
>
> instead of
>
> (itv->hw_flags & IVTV_HW_CX25840)
>
> ? What is the difference?
Nothing. They are equivalent. We could use either. I chose to
dereference from itv->sd_video because it was right there in the code.
> My idea is to do it this way (whole code block):
>
> if (atomic_read(&itv->capturing) == 0) {
> /* Clear all Pending Interrupts */
> ivtv_set_irq_mask(itv, IVTV_IRQ_MASK_CAPTURE);
>
> clear_bit(IVTV_F_I_EOS, &itv->i_flags);
>
> /* Initialize Digitizer for Capture */
> if (!itv->hw_flags & IVTV_HW_CX25840) {
> v4l2_subdev_call(itv->sd_video, video, s_stream, 0);
> ivtv_msleep_timeout(300, 1);
> }
> ivtv_vapi(itv, CX2341X_ENC_INITIALIZE_INPUT, 0);
> if (!itv->hw_flags & IVTV_HW_CX25840) {
> v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> }
> }
>
> In this version I assume that the 300ms sleep is only necessary when the
> digitizer was turned off, which only happens for non-cx25840 cards.
> For saa7115-based cards like PVR350 the code remains unchanged. I have two
> PVR350 cards and never had any audio problems.
>
> I am only a hobbyist, so please forgive me if there is an error ...
No that's fine too; as long as *somewhere* in the ivtv driver we turn on
the CX2584x's output enables before the first capture.
Since Hans had reported that this particular
CX2341X_ENC_INITIALIZE_INPUT call can hang a machine, it may be woth
adding a module option to let users who have a problem with a PCI bus
hang to live with fuzzy audio.
> > From: Argus <[email protected]>
> >
> > The trouble is: I am not running a latest kernel anywhere to
> > test your patch below. I was making my patches against a 2.6.28
> > kernel (older, I know, but the machine that holds the 150s is
> > remote and not easily upgraded). Can I compile just the latest
> > ivtv driver and install it over the 2.6.28 module? I'm guessing
> > not ...
>
> I think you need to replace
> v4l2_subdev_call(itv->sd_video, video, s_stream, 0);
> with
> itv->video_dec_func(itv, VIDIOC_STREAMOFF, NULL);
>
> and
> v4l2_subdev_call(itv->sd_video, video, s_stream, 1);
> with
> itv->video_dec_func(itv, VIDIOC_STREAMON, NULL);
>
> @ Andy:
> Is this only a different cody style, or does a subdev_call effect other
> things
> than the VIDIOC_STREAMOFF/ON ioctls?
Generally, there should be no difference, but it may be slightly
different. The newer subdev version strictly targets the analog video
decoder. (Since the CX2584x is both video and audio decoder, s_stream
affects both audio and video clocks). I haven't looked to see if the
legacy code avoided touching the MSP34xx output.
Regards,
Andy
> Greets,
> Martin
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel
