[alsa-devel] Broken powersave in 3.5.2, codec is always active

[Takashi Iwai]

At Tue, 21 Aug 2012 00:47:48 +0700,
ValdikSS wrote:
> 
> Takashi Iwai <tiwai at suse.de> писал(а) в своём письме Mon, 20 Aug 2012  
> 23:21:31 +0700:
> 
> > [Re-added Cc to ML; please don't drop it]
> >
> > At Mon, 20 Aug 2012 22:23:02 +0700,
> > ValdikSS wrote:
> >>
> >> Takashi Iwai <tiwai at suse.de> писал(а) в своём письме Mon, 20 Aug 2012
> >> 16:17:04 +0700:
> >>
> >> > At Sat, 18 Aug 2012 12:44:29 +0700,
> >> > ValdikSS wrote:
> >> >>
> >> >> Hello!
> >> >> I have a Lenovo ThinkPad X220 laptop with a conexant codec and  
> >> recently
> >> >> (after kernel 3.5.2) it is active all the time after KDE login, as
> >> >> powertop says. This couldn't be fixed by logging of from KDE or  
> >> playing
> >> >> some music in mplayer from console. The only kernel I tried in 3.5.x  
> >> is
> >> >> 3.5.2, so I can't say is this bug reproducible in 3.5.0 or 3.5.1, but
> >> >> everything is fine with 3.4.9. This never happens if you don't start  
> >> KDE
> >> >> after reboot, you can listen to some music in mplayer in console and
> >> >> everything seems to be fine. lsof and fuser /dev/snd/* shows nothing.
> >> >
> >> > Just to be sure, check /sys/module/snd_hda_intel/parameters/power_save
> >> > value after logging into KDE.  This value can be changed dynamically.
> >> >
> >> > If the value is still set properly, try to build the kernel with the
> >> > tracing option with the patch below.  Then try to trace the power
> >> > up/down sequences in the tracing buffer.  Some brief description is
> >> > found in Documentation/sound/alsa/HD-Audio.txt Tracepoints section.
> >> >
> >> >
> >> > thanks,
> >> >
> >> > Takashi
> >> >
> >> > ---
> >> > From: Takashi Iwai <tiwai at suse.de>
> >> > Subject: [PATCH] ALSA: hda - Add tracepoints at snd_hda_power_up/down
> >> > entrances.
> >> >
> >> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> >> > ---
> >> >  sound/pci/hda/hda_codec.c |  2 ++
> >> >  sound/pci/hda/hda_trace.h | 24 ++++++++++++++++++++++++
> >> >  2 files changed, 26 insertions(+)
> >> >
> >> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> >> > index 408babc..ad0101e 100644
> >> > --- a/sound/pci/hda/hda_codec.c
> >> > +++ b/sound/pci/hda/hda_codec.c
> >> > @@ -4417,6 +4417,7 @@ static void __snd_hda_power_up(struct hda_codec
> >> > *codec, bool wait_power_down)
> >> > 	spin_lock(&codec->power_lock);
> >> >  	codec->power_count++;
> >> > +	trace_hda_power_count(codec);
> >> >  	/* Return if power_on or transitioning to power_on, unless currently
> >> >  	 * powering down. */
> >> >  	if ((codec->power_on || codec->power_transition > 0) &&
> >> > @@ -4496,6 +4497,7 @@ void snd_hda_power_down(struct hda_codec *codec)
> >> >  {
> >> >  	spin_lock(&codec->power_lock);
> >> >  	--codec->power_count;
> >> > +	trace_hda_power_count(codec);
> >> >  	if (!codec->power_on || codec->power_count ||  
> >> codec->power_transition)
> >> > {
> >> >  		spin_unlock(&codec->power_lock);
> >> >  		return;
> >> > diff --git a/sound/pci/hda/hda_trace.h b/sound/pci/hda/hda_trace.h
> >> > index 9884871..9a34b86 100644
> >> > --- a/sound/pci/hda/hda_trace.h
> >> > +++ b/sound/pci/hda/hda_trace.h
> >> > @@ -87,6 +87,30 @@ DEFINE_EVENT(hda_power, hda_power_up,
> >> >  	TP_ARGS(codec)
> >> >  );
> >> > +TRACE_EVENT(hda_power_count,
> >> > +	TP_PROTO(struct hda_codec *codec),
> >> > +	TP_ARGS(codec),
> >> > +	TP_STRUCT__entry(
> >> > +		__field( unsigned int, card )
> >> > +		__field( unsigned int, addr )
> >> > +		__field( int, power_count )
> >> > +		__field( int, power_on )
> >> > +		__field( int, power_transition )
> >> > +	),
> >> > +
> >> > +	TP_fast_assign(
> >> > +		__entry->card = (codec)->bus->card->number;
> >> > +		__entry->addr = (codec)->addr;
> >> > +		__entry->power_count = (codec)->power_count;
> >> > +		__entry->power_on = (codec)->power_on;
> >> > +		__entry->power_transition = (codec)->power_transition;
> >> > +	),
> >> > +
> >> > +	TP_printk("[%d:%d] power_count=%d, power_on=%d,  
> >> power_transition=%d",
> >> > +		  __entry->card, __entry->addr, __entry->power_count,
> >> > +		  __entry->power_on, __entry->power_transition)
> >> > +);
> >> > +
> >> >  TRACE_EVENT(hda_unsol_event,
> >> > 	TP_PROTO(struct hda_bus *bus, u32 res, u32 res_ex),
> >>
> >> Thanks for the quick reply! I compiled 3.5.2 kernel with this patch,
> >> enabled hda tracing, ran mplayer once in the console and started KDE.  
> >> Once
> >> KDE is started, card is used 100% of time, which can be seen in  
> >> powertop.
> >> When I logout from KDE and kill kdm, no events are generated in trace
> >> since KDE start (no sounds are played from KDE start till KDM kill) and
> >> card is still used 100% of time. If you play any sounds in mplayer in
> >> console after kdm kill, the card is still used 100% time. This can be
> >> fixed by laptop suspend.
> >>
> >> The traces are quite huge so I pasted it to pastebin:
> >> 01-trace-just-enabled http://pastebin.com/JNRWXUPy
> >> 02-mplayer-played-and-stopped-card-is-normal  
> >> http://pastebin.com/r0jZLDey
> >> 03-kde-started-and-stopped-100-percent-card http://pastebin.com/Z7mzbY4c
> >> The last log begins with latest rows from 02
> >
> > It's still not clear from the log.  You can reduce the trace pattern
> > to hda_power_* for reducing the trace log size.
> >
> > In anyway, could you try the patch below?
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 88a9c20..b94e04d 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -4433,6 +4433,8 @@ static void __snd_hda_power_up(struct hda_codec  
> > *codec, bool wait_power_down)
> >  	 * then there is no need to go through power up here.
> >  	 */
> >  	if (codec->power_on) {
> > +		if (codec->power_transition < 0)
> > +			codec->power_transition = 0;
> >  		spin_unlock(&codec->power_lock);
> >  		return;
> >  	}
> 
> Yes, thanks, this patch fixes this bug.

OK, the fix is queued now.  Thanks for a quick check.


Takashi