[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 7:08 PM
> To: Wang, Xingchao
> Cc: alsa-devel at alsa-project.org
> Subject: Re: [PATCH] ALSA: HDA - Check return value to reduce useless delay
> 
> At Thu, 25 Oct 2012 10:30:35 +0000,
> Wang, Xingchao wrote:
> >
> >
> >
> > > -----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.
> 
> Both are too new :)
> 
> > 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.
> 
> But does it fix the issue itself?

Actually not. :)
> 
> > 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?
> 
> Not sure.  But we need more testing obviously before breaking anything.

Okay, I will try to test it on other platforms. :)

--xingchao
> 
> 
> thanks,
> 
> Takashi
> 
> 
> >
> > 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
> > > >
> >