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

[ValdikSS]

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.