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

[Takashi Iwai <tiwai@suse.de>]

At Wed, 17 Dec 2008 20:08:30 -0200,
Thadeu Lima de Souza Cascardo wrote:
> 
> On Mon, Jul 14, 2008 at 09:10:39PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Wed, Jul 16, 2008 at 01:47:03AM +0200, Takashi Iwai wrote:
> > > Thanks.
> > > 
> > > Meanwhile, I reconsidered about this problem.  I think the check of
> > > the whole "active" codec slots can be done better in a way like the
> > > following.  Could you give it a try?
> > > 
> > > If this still doesn't work, I suspect it's really the matter of
> > > additional delay.
> > > 
> 
> I've put some more effort on this issue last weekend. Here is the
> result:
> 
> The codec I have is AD1881A. The additional delay makes a pretty
> difference, right. Only the delay is enough to solve it.

It's a good news, at least.

> However, I agree this is not the right solution. The codec_init_bits
> solution does not work.
> 
> I have diff'ed the value of the codec registers and they are different
> from all the other situations I tested, that are:
> 
> * booted my notebook: OK
> * suspended, resumed, reloaded the driver: OK
> * loaded the driver with no cold reset
>   (no AC97_POWER_SAVE on intel8x0): OK
> * same as above, after resuming: OK
> * AC97_POWER_SAVE enabled (cold reset), after resume: NOT ok
> 
> As previously reported, even when probing, after loading the driver, we
> get a different behaviour if we cold reset. When probing, we wait for
> those HZ/4, which may explain why everything works.
> 
> I tested if ac97_resume was writing the same values to the same
> registers if I had the delay or if I didn't. It was. However, the values
> I read from /proc were still different, very similar to what the codec
> specification refered to as the default values.
> 
> The best solution would be to do whatever is needed to do after cold
> resetting that we are not doing, unless it is a hardware issue and the
> driver can't really do anything about it but wait. Reading the Intel
> ICH3 doc and AD1881A doc, I couldn't figure out anything that the driver
> is doing wrong.
> 
> So, right now, the alternatives amount to:
> 
> * Wait for the HZ/4 additional delay. Probing is doing it and my
> previously submitted patch does not add any delay for the probe case
> and, it the resume case, it only adds a delay if we are cold resetting,
> that is, AC97_POWER_SAVE is enabled.
> 
> * Do not do the cold reset at all. It works pretty well for me if ac97
> power save is enabled and we only do a warm reset.
> 
> * Do only one of the above as a quirk for my controller or codec or
> their combination.

I feel it's not only your machine that a similar thing happens.
OTOH, adding the delay selectively would be good, at least, for an old
driver with AC97.  So, checking the machine could be a better option
at this stage.

So, my bet is the third one, adding a (sort of) quirk to disable the
cold reset.  I'm not sure whether yet another module option is the
best way, though...


thanks,

Takashi