[alsa-devel] [PATCH] ALSA: HDA - Check return value to reduce useless delay

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Thursday, October 25, 2012 6:01 AM
> To: Wang, Xingchao
> Cc: alsa-devel at alsa-project.org
> Subject: Re: [PATCH] ALSA: HDA - Check return value to reduce useless delay
> 
> At Wed, 24 Oct 2012 14:53:23 +0800,
> Wang Xingchao wrote:
> >
> > For verb 705h, it's useless to read response, so use *write api would
> > be better. If there's error after sending cmd, just try again without
> > continue after wrong operation.Otherwise there's long time delay.
> 
> Well, this is a bit sensitive part.  Did you do the good test coverage for
> different hardware controllers and codecs, including the non-Intel ones?
> 

Well I only tested the patch on Haswell/Ivybridge platform. For Haswell I meet an issue about codec suspend/resume.
There's no response from codec when there's power state transition from D3 to D0, and the patch could reduced delay time.
For ivybridge the codec works well, so this patch has no active behavior. 
I assume the verb 705h will not work for subnodes if it failed at first for "function group".
Is there such case when set power state for the "Audio Function Group" fail but the sub-nodes could keep on setting power?

Thanks
--xingchao

> 
> Takashi
> 
> > Signed-off-by: Wang Xingchao <xingchao.wang at intel.com>
> > ---
> >  sound/pci/hda/hda_codec.c |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 960800b..0946eca 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -3569,6 +3569,7 @@ static unsigned int hda_set_power_state(struct
> hda_codec *codec,
> >  	hda_nid_t fg = codec->afg ? codec->afg : codec->mfg;
> >  	int count;
> >  	unsigned int state;
> > +	int err;
> >
> >  	/* this delay seems necessary to avoid click noise at power-down */
> >  	if (power_state == AC_PWRST_D3) {
> > @@ -3582,9 +3583,11 @@ static unsigned int hda_set_power_state(struct
> hda_codec *codec,
> >  			codec->patch_ops.set_power_state(codec, fg,
> >  							 power_state);
> >  		else {
> > -			snd_hda_codec_read(codec, fg, 0,
> > +			err = snd_hda_codec_write(codec, fg, 0,
> >  					   AC_VERB_SET_POWER_STATE,
> >  					   power_state);
> > +			if (err < 0)
> > +				continue;
> >  			snd_hda_codec_set_power_to_all(codec, fg, power_state,
> >  						       true);
> >  		}
> > --
> > 1.7.9.5
> >