* [PATCH fixed bug 4032 1/1] Fixed bug 4032. @ 2009-12-14 18:41 Steve Soule 2009-12-14 19:01 ` Takashi Iwai 0 siblings, 1 reply; 5+ messages in thread From: Steve Soule @ 2009-12-14 18:41 UTC (permalink / raw) To: patch; +Cc: alsa-devel [-- Attachment #1: 0001-Fixed-bug-4032.patch --] [-- Type: text/x-patch, Size: 895 bytes --] >From c3387a767f827f57a6b1f6b772d3d7a1b6b39942 Mon Sep 17 00:00:00 2001 From: Steve Soule <sts11dbxr@gmail.com> 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@gmail.com> --- pci/ac97/ac97_codec.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index 20cb60a..c119206 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -2122,7 +2122,7 @@ 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(120); + end_time = jiffies + msecs_to_jiffies(5000); do { if ((snd_ac97_read(ac97, AC97_POWERDOWN) & 0x0f) == 0x0f) goto __ready_ok; -- 1.6.4.2 [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH fixed bug 4032 1/1] Fixed bug 4032. 2009-12-14 18:41 [PATCH fixed bug 4032 1/1] Fixed bug 4032 Steve Soule @ 2009-12-14 19:01 ` Takashi Iwai [not found] ` <4B26A4DD.8010203@gmail.com> 0 siblings, 1 reply; 5+ messages in thread From: Takashi Iwai @ 2009-12-14 19:01 UTC (permalink / raw) To: Steve Soule; +Cc: alsa-devel 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@gmail.com> > 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@gmail.com> 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. thanks, Takashi > --- > pci/ac97/ac97_codec.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c > index 20cb60a..c119206 100644 > --- a/pci/ac97/ac97_codec.c > +++ b/pci/ac97/ac97_codec.c > @@ -2122,7 +2122,7 @@ 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(120); > + end_time = jiffies + msecs_to_jiffies(5000); > do { > if ((snd_ac97_read(ac97, AC97_POWERDOWN) & 0x0f) == 0x0f) > goto __ready_ok; > -- > 1.6.4.2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <4B26A4DD.8010203@gmail.com>]
* Re: [PATCH fixed bug 4032 1/1] Fixed bug 4032. [not found] ` <4B26A4DD.8010203@gmail.com> @ 2009-12-15 9:21 ` Takashi Iwai 2009-12-15 9:35 ` Jaroslav Kysela 0 siblings, 1 reply; 5+ messages in thread From: Takashi Iwai @ 2009-12-15 9:21 UTC (permalink / raw) To: Steve Soule; +Cc: alsa-devel 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@gmail.com> > >> 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@gmail.com> > > > > 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... thanks, Takashi ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH fixed bug 4032 1/1] Fixed bug 4032. 2009-12-15 9:21 ` Takashi Iwai @ 2009-12-15 9:35 ` Jaroslav Kysela 2009-12-15 9:49 ` Takashi Iwai 0 siblings, 1 reply; 5+ messages in thread From: Jaroslav Kysela @ 2009-12-15 9:35 UTC (permalink / raw) To: Takashi Iwai; +Cc: Steve Soule, alsa-devel 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@gmail.com> >>>> 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@gmail.com> >>> >>> 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? 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@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH fixed bug 4032 1/1] Fixed bug 4032. 2009-12-15 9:35 ` Jaroslav Kysela @ 2009-12-15 9:49 ` Takashi Iwai 0 siblings, 0 replies; 5+ messages in thread From: Takashi Iwai @ 2009-12-15 9:49 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Steve Soule, alsa-devel 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@gmail.com> > >>>> 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@gmail.com> > >>> > >>> 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@perex.cz> > Linux Kernel Sound Maintainer > ALSA Project, Red Hat, Inc. > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-12-15 9:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-14 18:41 [PATCH fixed bug 4032 1/1] Fixed bug 4032 Steve Soule
2009-12-14 19:01 ` Takashi Iwai
[not found] ` <4B26A4DD.8010203@gmail.com>
2009-12-15 9:21 ` Takashi Iwai
2009-12-15 9:35 ` Jaroslav Kysela
2009-12-15 9:49 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.