Re: [PATCH] Wait for all codecs to be ready if doing a cold reset

[Thadeu Lima de Souza Cascardo]

On Mon, Jul 07, 2008 at 06:39:09PM +0200, Takashi Iwai wrote:
> At Sun, 6 Jul 2008 14:15:56 -0300,
> Thadeu Lima de Souza Cascardo wrote:
> > 
> > If AC97_POWER_SAVE is enabled, intel8x0 does a cold reset when
> > initializing the codecs. While resuming, it does not wait for all codecs
> > to be active. Sound card does not work after a resume without it,
> > however. This patch fixes it.
> 
> Thanks for the patch.
> 
> But, I still don't figure out why this is needed.
> In the else block (with the comment "resume phase"), you can find the
> loop to wait for the all *probed* codecs.  The difference with the
> code you moved is that it checks only the bits corresponding to the
> already probed codecs.  In other words, the other bits should be
> irrelevant with the hardware.
> 
> I guess it's not about the loop but the additional 1/4 sec delay that
> did fix the resume on your device.  Please check how is the status
> bits and whether it passed the loop in the middle.
> 
> 
> Takashi
> 
> 

The 1/4 sec delay came in my mind as one of the possible reasons, and
that's why I've made some tests. status and nstatus are 0x200, while
codec_isr_bits is 0x300. The loop waits for the status register give us
0x300 as the active codecs, instead of the only one probed. Since the
cold reset in the case of a power saving cleans up all codec registers,
it is needed that we wait for all codecs again (like in the probe case).

If you need more information or that I ran any more tests, I'd be glad
to do it. By the way, I've tested the patch against your audio and modem
merged version and it works OK.

Best Regards,
Thadeu Cascardo.

> > ---
> >  sound/pci/intel8x0.c |   24 ++++++++++++++----------
> >  1 files changed, 14 insertions(+), 10 deletions(-)
> > 
> > diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> > index 048d99e..7228a0a 100644
> > --- a/sound/pci/intel8x0.c
> > +++ b/sound/pci/intel8x0.c
> > @@ -2335,16 +2335,6 @@ static int snd_intel8x0_ich_chip_init(struct intel8x0 *chip, int probing)
> >  				   igetdword(chip, ICHREG(GLOB_STA)));
> >  			return -EIO;
> >  		}
> > -
> > -		/* wait for other codecs ready status. */
> > -		end_time = jiffies + HZ / 4;
> > -		while (status != chip->codec_isr_bits &&
> > -		       time_after_eq(end_time, jiffies)) {
> > -			schedule_timeout_uninterruptible(1);
> > -			status |= igetdword(chip, ICHREG(GLOB_STA)) &
> > -				chip->codec_isr_bits;
> > -		}
> > -
> >  	} else {
> >  		/* resume phase */
> >  		int i;
> > @@ -2363,6 +2353,20 @@ static int snd_intel8x0_ich_chip_init(struct intel8x0 *chip, int probing)
> >  		} while (time_after_eq(end_time, jiffies));
> >  	}
> >  
> > +#ifndef CONFIG_SND_AC97_POWER_SAVE
> > +	if (probing)
> > +#endif
> > +	{
> > +		/* wait for other codecs ready status. */
> > +		end_time = jiffies + HZ / 4;
> > +		while (status != chip->codec_isr_bits &&
> > +		       time_after_eq(end_time, jiffies)) {
> > +			schedule_timeout_uninterruptible(1);
> > +			status |= igetdword(chip, ICHREG(GLOB_STA)) &
> > +				chip->codec_isr_bits;
> > +		}
> > +	}
> > +
> >  	if (chip->device_type == DEVICE_SIS) {
> >  		/* unmute the output on SIS7012 */
> >  		iputword(chip, 0x4c, igetword(chip, 0x4c) | 1);
> > -- 
> > 1.5.6
> >

Attachment: signature.asc
Description: Digital signature