Re: snd_hda_intel 2.6.24-rc2 bug: interrupts don't always work on Lenovo X60s

[Takashi Iwai <tiwai@suse.de>]

At Thu, 22 Nov 2007 12:42:42 -0500,
Theodore Tso wrote:
> 
> On Tue, Nov 13, 2007 at 04:43:05AM +0100, Takashi Iwai wrote:
> > > By the way, the "polling mode" seems to work OK: I still get normal
> > > playback of music etc.
> > 
> > Yes, the polling mode should work in most cases, too.
> 
> Out of curiosity, how many wakeups/interrupts are involved with the
> sound going into polling mode?  Is it going to make a difference as
> far as battery life is concerned?

Switch to polling mode is judged whether the response comes within one
second.  It's irrelevant of number of interrupts.
This mechanism was introduced at the time many devices have broken
BIOS and indeed irq was screwed up.

But, the polling mode is no dramatic change.  This means that the
driver polls the response value not only waiting for an ACK via 
irq.  Thus no big change over battery life.  The new option,
CONFIG_SND_HDA_POWER_SAVE, is a far bigger behavior change and would
influence a lot more on battery life.


> I'm seeing the message:
> 
> hda_intel: azx_get_response timeout, switching to polling mode: last cmd=0x005f000c
> 
> on my X61s laptop as well, where the last_cmd varies quite a bit.
> Over the past two weeks, I've seen last cmd be:
> 
> 0x003f000c, 0x004f000c, 0x005f000c, 0x006f000c, 0x00db8000,
> 0x011b8000, 0x011ba000, 0x012ba000, 0x012f000c, 0x013f000c,
> 0x014f000d, 0x019f000c, 0x020b0001, 0x020b0003, 0x020b2000,
> 0x020b2001, 0x020b2002, 0x025f0012

A half of them should go away with my patch.  They are wrong PINCAP
verbs.  But others seem not.

> Interestingly, when I was using a post 2.6.24-rc1 and -rc2 kernel, I
> was getting a lot of these "switching polling to mode messages",
> usually within a minute of the machine booting.  Now that I have
> switched to a recent rc3 kernel, they seem to have largely gone away.

Interesting, indeed.

> Looking at my kernel, it looks like the patch you suggested to Roland
> was *not* applied, and "git log sound/pci/hda" shows that the only
> change to that directory was a patch from Ingo Molnar that I had
> cherry picked from LKML.  Given that we were doing a
> schedule_timeout_uninterruptible for a full second, that certainly
> seems to be a likely candidate for why we were getting the response
> timeout message!  Does this analysis make sense to you?

It might be true.  As mentioned, the threshold to polling mode is one
second.  If schedule_timeout_uninterruptible(1) takes one second, of
course, this fails.  I didn't expect that schedule_timeout() may take
that long.

If Ingo's patch really helps in such a situation, we should get it
into 2.6.24.  It's already in ALSA tree (perex/alsa.git mm branch),
but I didn't ask Jaroslav to included in the push chunk.

Jaroslav, care to push again?


thanks,

Takashi

> 
> Regards,
> 
> 						- Ted
> 
> commit 2f7e58208e0d59ca6e4ad1561f47391d4efa19fa
> Author: Ingo Molnar <mingo@elte.hu>
> Date:   Fri Nov 16 11:35:05 2007 -0500
> 
>     snd hda suspend latency: shorten codec read
>     
>     not sleeping for every codec read/write but doing a short udelay and
>     a conditional reschedule has cut suspend+resume latency by about 1
>     second on my T60.
>     
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 3fa0f97..62b9fb3 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -555,7 +555,8 @@ static unsigned int azx_rirb_get_response(struct hda_codec *codec)
>  		}
>  		if (!chip->rirb.cmds)
>  			return chip->rirb.res; /* the last value */
> -		schedule_timeout_uninterruptible(1);
> +		udelay(10);
> +		cond_resched();
>  	} while (time_after_eq(timeout, jiffies));
>  
>  	if (chip->msi) {
>