At Tue, 15 Dec 2009 10:35:12 +0100 (CET),
Jaroslav Kysela wrote:
>
> On Tue, 15 Dec 2009, Takashi Iwai wrote:
>
> > At Mon, 14 Dec 2009 13:49:33 -0700,
> > Steve Soule wrote:
> >>
> >> On 12/14/2009 12:01 PM, Takashi Iwai wrote:
> >>> At Mon, 14 Dec 2009 11:41:55 -0700,
> >>> Steve Soule wrote:
> >>>>
> >>>>> From c3387a767f827f57a6b1f6b772d3d7a1b6b39942 Mon Sep 17 00:00:00 2001
> >>>> From: Steve Soule <sts11dbxr@xxxxxxxxx>
> >>>> Date: Mon, 14 Dec 2009 11:06:03 -0700
> >>>> Subject: [PATCH fixed bug 4032 1/1] Fixed bug 4032.
> >>>>
> >>>>
> >>>> Signed-off-by: Steve Soule <sts11dbxr@xxxxxxxxx>
> >>>
> >>> Could you give a bit more changelog text?
> >>> It's not sure what bug 4032 indicates, and more importantly, why this
> >>> change is needed for fixing what.
> >>
> >> Okay. Here is a thorough changelog:
> >>
> >> This is a fix of a bug in ac97_codec.c. This bug has existed for many
> >> years in many versions of alsa and versions of the kernel. The bug
> >> appears with a Soundblaster 16PCI sound card. With this card, sometimes
> >> the driver works correctly, and sometimes it doesn't. When it doesn't
> >> work correctly, the driver prints the error message:
> >>
> >> AC'97 0 analog subsections not ready
> >>
> >> and fails to detect the complete set of sound controls. In particular,
> >> the driver fails to detect the "Master Playback Volume" control, and so
> >> the sound card volume level is arbitrary--usually very low.
> >>
> >> As far as I can tell, the problem is in the function snd_ac97_mixer() in
> >> ac97_codec.c, in the lines immediately before the error message is
> >> printed. These lines appear to be waiting for the card to come out of
> >> powerdown mode. If the card does not come up within a timeout, the
> >> error message is printed, and the controls are not detected correctly.
> >>
> >> The timeout in the latest version is 120 ms (though it was 100 ms when I
> >> first reported this bug a year and a half ago). I experimented with
> >> different timeout values, and found that 1 second was not sufficient,
> >> and that 5 seconds is sufficient.
> >
> > Hrm, usually 1 second is really long enough for any operations.
> > And 5 seconds is already a history since genesis. If it takes so
> > long, something other is fishy.
> >
> > Although extending the timeout would be relatively harmless, I still
> > don't think this is the point to fix. More likely the problem is in
> > the codec accessor side...
>
> Maybe, but ens1371 chip is really old and it might be that the initial
> codec link synchronization is not ideal.
>
> What about to add the timeout variable to ac97 structure?
Hmm, it's also harmlful, but I don't think it's worth, too.
My point is that we'd need to look at a different place for the real
problem. Meanwhile, Steve's patch is OK as it shouldn't regress.
(Thus I pulled your tree, too.)
thanks,
Takashi
>
> Something like this:
>
> diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h
> index 4940045..2ff650a 100644
> --- a/include/sound/ac97_codec.h
> +++ b/include/sound/ac97_codec.h
> @@ -381,6 +381,7 @@
> #define AC97_SCAP_NO_SPDIF (1<<9) /* don't build SPDIF controls */
> #define AC97_SCAP_EAPD_LED (1<<10) /* EAPD as mute LED */
> #define AC97_SCAP_POWER_SAVE (1<<11) /* capable for aggresive power-saving */
> +#define AC97_SCAP_LONGANALOGTMO (1<<12) /* long timeout for analog subsect */
>
> /* ac97->flags */
> #define AC97_HAS_PC_BEEP (1<<0) /* force PC Speaker usage */
> diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
> index c119206..ec22a9a 100644
> --- a/sound/pci/ac97/ac97_codec.c
> +++ b/sound/pci/ac97/ac97_codec.c
> @@ -2122,7 +2122,10 @@ int snd_ac97_mixer(struct snd_ac97_bus *bus, struct snd_ac97_template *template,
> }
> /* nothing should be in powerdown mode */
> snd_ac97_write_cache(ac97, AC97_GENERAL_PURPOSE, 0);
> - end_time = jiffies + msecs_to_jiffies(5000);
> + end_time = 120;
> + if (ac97->scaps & AC97_SCAP_LONGANALOGTMO)
> + end_time = 5000;
> + end_time = jiffies + msecs_to_jiffies(end_time);
> do {
> if ((snd_ac97_read(ac97, AC97_POWERDOWN) & 0x0f) == 0x0f)
> goto __ready_ok;
> diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c
> index 2b82c5c..8afe173 100644
> --- a/sound/pci/ens1370.c
> +++ b/sound/pci/ens1370.c
> @@ -1631,7 +1631,7 @@ static int __devinit snd_ensoniq_1371_mixer(struct ensoniq *ensoniq,
> ac97.private_data = ensoniq;
> ac97.private_free = snd_ensoniq_mixer_free_ac97;
> ac97.pci = ensoniq->pci;
> - ac97.scaps = AC97_SCAP_AUDIO;
> + ac97.scaps = AC97_SCAP_AUDIO|AC97_SCAP_LONGANALOGTMO;
> if ((err = snd_ac97_mixer(pbus, &ac97, &ensoniq->u.es1371.ac97)) < 0)
> return err;
> if (has_spdif > 0 ||
>
> Jaroslav
>
> -----
> Jaroslav Kysela <perex@xxxxxxxx>
> Linux Kernel Sound Maintainer
> ALSA Project, Red Hat, Inc.
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
