Re: [PATCH powerd] Inhibit suspend while audio device is open
On Mon, May 21, 2012 at 01:00:29AM +0200, Sascha Silbe wrote: While there's a number of Activities (and probably Gnome applications as well) that don't close the audio device when they're done using audio, thus inhibiting suspend for longer than necessary, that's a problem on its own as it will also prevent other Activities from using the audio device. The most obvious thing to do here would seem to be to use PulseAudio - not only can it allow applications to not worry their pretty little heads about this it'll also give you mixing support and there's some pretty nifty power management stuff in there. ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
Paul Fox p...@laptop.org writes: i'm perfectly happy to enable this, if we think it fixes the problem well enough, and/or we think the applications can be fixed to do the right thing. (for some reasonable value of we.) It does for me, but as much testing as I may have done can never account for a million devices in the field. It does more good than harm (and even the harm is on the safe side of consuming more power rather than disrupting operation), so I'd recommend to merge it at least for the next release (post-12.1). Whether it should go into 12.1 at this point in time is not my call to make. in searching for a relevant ticket on d.l.o. just now, i found #6670, which is quite relevant, if perhaps a bit dated. Thanks for the reference and for CC'ing me. I've commented on the ticket; repeating here for convenience of others following this thread: As Paul pointed out correctly on the mailing list (or maybe IRC), simply reading the current status of the audio device is just a point-in-time check. inotify doesn't notice changes to the status file either. So in theory we're opening ourselves up for a race condition. However, it would be a problem even with a better check: AFAICT using the audio device doesn't increment wakeup_count, so if an Activity started using the audio device right before we suspend, we'd have the same problem. In practice, it will probably be less of a problem, as most usage of audio devices is going to be the direct result of some external event (user input or network traffic). Sascha -- http://sascha.silbe.org/ http://www.infra-silbe.de/ pgpGcv5nktLnz.pgp Description: PGP signature ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
Paul Fox p...@laptop.org writes: 3) as far as the activity testing you did, i agree with your results -- i didn't try everything, but Scratch is certainly the biggest violator, in that it keeps the device in RUNNING state even when not playing anything, and when not in the foreground. observed with strace, Scratch actually continuously writes (using ioctl()) to the device. perhaps someone could work with the scratch authors on this? Thanks for testing that in detail. I've filed a ticket [1] against the Scratch component on bugs.sl.o. It got assigned to Rafael Ortiz; maybe he has a channel to the Scratch devs. Sascha [1] https://bugs.sugarlabs.org/ticket/3630 -- http://sascha.silbe.org/ http://www.infra-silbe.de/ pgpcFktIEvXlx.pgp Description: PGP signature ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
James Cameron qu...@laptop.org writes: Reviewed-by: James Cameron qu...@laptop.org Thanks for the review! Sascha -- http://sascha.silbe.org/ http://www.infra-silbe.de/ pgpesKExRgPw8.pgp Description: PGP signature ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
sascha wrote: Audio playback and recording can be efficient enough not to trigger the CPU threshold, but we still don't want to suspend because it would interrupt the playback resp. recording. While there's a number of Activities (and probably Gnome applications as well) that don't close the audio device when they're done using audio, thus inhibiting suspend for longer than necessary, that's a problem on its own as it will also prevent other Activities from using the audio device. There are many other ways of Activities misbehaving and consuming a lot of resources. The increased battery drain by inhibiting suspend is easy enough for users to observe by watching the power LED. Signed-off-by: Sascha Silbe si...@activitycentral.com --- powerd | 14 ++ 1 files changed, 6 insertions(+), 8 deletions(-) thanks for doing all that testing! i'm perfectly happy to enable this, if we think it fixes the problem well enough, and/or we think the applications can be fixed to do the right thing. (for some reasonable value of we.) in searching for a relevant ticket on d.l.o. just now, i found #6670, which is quite relevant, if perhaps a bit dated. the issue with Distance, in particular, isn't relevant because i believe Distance actively suppresses idle-suspend when a measurement is in progress. but it does raise the issue that the current check used by that patch is an instant in time check, which really isn't optimal. an activity counter would work (as is used for some other powerd measurements), or better, a timestamp, as was proposed by cjb in the ticket. paul diff --git a/powerd b/powerd index c6d9487..09f9153 100755 --- a/powerd +++ b/powerd @@ -1199,13 +1199,10 @@ cpu_or_network_busy() } -# unused for now, because too many sound apps leave the -# device open: the TamTams, Record (after playing), Browse -# and Firefox (after playing) -#audio_busy() -#{ -#grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy -#} +audio_busy() +{ +grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy +} # see if there's been activity during the recent idle period -- @@ -1330,7 +1327,8 @@ general_inhibit() laptop_busy() { -general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy +general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy \ +|| audio_busy } if [ -e $TPAD_RECAL ] -- 1.7.9 =- paul fox, p...@laptop.org ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
sascha wrote: Audio playback and recording can be efficient enough not to trigger the CPU threshold, but we still don't want to suspend because it would interrupt the playback resp. recording. While there's a number of Activities (and probably Gnome applications as well) that don't close the audio device when they're done using audio, thus inhibiting suspend for longer than necessary, that's a problem on its own as it will also prevent other Activities from using the audio device. There are many other ways of Activities misbehaving and consuming a lot of resources. The increased battery drain by inhibiting suspend is easy enough for users to observe by watching the power LED. Signed-off-by: Sascha Silbe si...@activitycentral.com --- powerd | 14 ++ 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/powerd b/powerd index c6d9487..09f9153 100755 --- a/powerd +++ b/powerd @@ -1199,13 +1199,10 @@ cpu_or_network_busy() } -# unused for now, because too many sound apps leave the -# device open: the TamTams, Record (after playing), Browse -# and Firefox (after playing) -#audio_busy() -#{ -#grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy -#} +audio_busy() +{ +grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy +} i've played with this a bit more, including taking a further look at the driver. 1) i think we should expand the expression to: egrep -q 'RUNNING|DRAINING' /proc/asound/card0/pcm0?/sub0/status 2) i tried the inotifywait trick mentioned in #6670. inotify picks up open and close events on the device, which could aid in knowing when the device was used, but it doesn't pick up on the data writes because they're often done with ioctl(SNDRV_PCM_IOCTL_WRITEI_FRAMES,...) instead of write(). so i don't think that's useful. 3) as far as the activity testing you did, i agree with your results -- i didn't try everything, but Scratch is certainly the biggest violator, in that it keeps the device in RUNNING state even when not playing anything, and when not in the foreground. observed with strace, Scratch actually continuously writes (using ioctl()) to the device. perhaps someone could work with the scratch authors on this? the browsers do seem to let the device out of RUNNING state, though as you noted they do still prevent others from using the audio device (a separate problem). the TamTam family isn't great either. this is all just further data. i'm really not in a position to say what the goal should be for powerd and audio for 12.1 or 12.2. paul # see if there's been activity during the recent idle period -- @@ -1330,7 +1327,8 @@ general_inhibit() laptop_busy() { -general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy +general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy \ +|| audio_busy } if [ -e $TPAD_RECAL ] -- 1.7.9 =- paul fox, p...@laptop.org ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
[PATCH powerd] Inhibit suspend while audio device is open
Audio playback and recording can be efficient enough not to trigger the CPU threshold, but we still don't want to suspend because it would interrupt the playback resp. recording. While there's a number of Activities (and probably Gnome applications as well) that don't close the audio device when they're done using audio, thus inhibiting suspend for longer than necessary, that's a problem on its own as it will also prevent other Activities from using the audio device. There are many other ways of Activities misbehaving and consuming a lot of resources. The increased battery drain by inhibiting suspend is easy enough for users to observe by watching the power LED. Signed-off-by: Sascha Silbe si...@activitycentral.com --- powerd | 14 ++ 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/powerd b/powerd index c6d9487..09f9153 100755 --- a/powerd +++ b/powerd @@ -1199,13 +1199,10 @@ cpu_or_network_busy() } -# unused for now, because too many sound apps leave the -# device open: the TamTams, Record (after playing), Browse -# and Firefox (after playing) -#audio_busy() -#{ -#grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy -#} +audio_busy() +{ +grep -q RUNNING /proc/asound/card0/pcm0?/sub0/status trace audio busy +} # see if there's been activity during the recent idle period -- @@ -1330,7 +1327,8 @@ general_inhibit() laptop_busy() { -general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy +general_inhibit || usb_inhibit || ttyusb_inhibit || cpu_or_network_busy \ +|| audio_busy } if [ -e $TPAD_RECAL ] -- 1.7.9 ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel
Re: [PATCH powerd] Inhibit suspend while audio device is open
Reviewed-by: James Cameron qu...@laptop.org -- James Cameron http://quozl.linux.org.au/ ___ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel