Re: [PATCH powerd] Inhibit suspend while audio device is open

2012-05-28 Thread Mark Brown
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

2012-05-22 Thread Sascha Silbe
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

2012-05-22 Thread Sascha Silbe
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

2012-05-21 Thread Sascha Silbe
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

2012-05-21 Thread Paul Fox
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

2012-05-21 Thread Paul Fox
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

2012-05-20 Thread Sascha Silbe
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

2012-05-20 Thread James Cameron
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