[Kde-hardware-devel] Review Request: Prevent button action from enabling disabled DPMS

[Oliver Henshaw]

> On Oct. 12, 2012, 5:47 p.m., Oliver Henshaw wrote:
> > I'm wondering if the best fix is set dpms timeouts to 0 at all times and have
> > the dpms action use powerdevil idle timeouts to directly set dpms state. This
> > would hopefully have less corner cases to reason about and wouldn't wake the
> > screen or (I think) reset the timers on profile/configuration/activity change.
> > 
> > I expect that would be something for 4.10 and master, not for 4.9 branch?
> 
> Susanne Oberhauser wrote:
>     do you mean to replace Xorg dpms by powerdevil when kde is running?
>     
>     so that xset -dpms /+dpms anything settings are just overridden?
>     
>     That is then turning bug#295164 and bug#306490 into a feature :)
>     
>     please don't do that.  just don't.  don't duplicate in-core Xorg functionality,
>     and break xorg dpms in a way that is obscure and unexpected to long time X users.
>     
>     thx.
>     
>
> 
> Oliver Henshaw wrote:
>     I don't think you have anything to worry about. The purpose of these two review requests is to fix the two bugs you mention. Then it's a very natural move to simplify powerdevil's dpms handling in the way I suggested above - this should make it harder for similar bugs to creep back in in the future, and would only change powerdevil's behaviour in very minor ways (this will continue to use the same xorg mechanism that xset does, if that's what you're worried about.)
>     
>     That said, you can't avoid clashes if you have two things (xset in a script and powerdevil) responsible for dpms settings. Even when timed screen blanking is disabled in powerdevil there are situations where powerdevil has to change dpms settings to make sure it stays disabled - see the review description above for an example. There are two scenarios that work and one that doesn't:
>     
>     + If you want no timed screen blanking ever then you can disable it in powerdevil and not worry again (once these fixes are in). Therefore disabling dpms with xset is unnecessary.
>     + If you want timed dpms then powerdevil is there for you. Altering dpms settings with xset at the same time obviously has the potential to cause weird interactions.
>     - Using xset to manage timeouts instead of powerdevil isn't going to be foolproof - as mentioned, powerdevil sometimes needs to force dpms timeouts to zero (effectively disabled).
>     
>     My proposal doesn't change any of this - it may change some of the details of what happens when  xset-by-hand and powerdevil clash but clashes will still tend to mean dpms is disabled when you want it enabled and not vice versa.
>     
>     If you have needs that powerdevil doesn't satisfy then it would be interesting to hear what they are.

Any comments on this? I'd like to push this out together with #106793 - the other review request alone won't fix all problems.


- Oliver


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106794/#review20228
-----------------------------------------------------------


On Oct. 12, 2012, 5:45 p.m., Oliver Henshaw wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106794/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2012, 5:45 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> Description
> -------
> 
> Prevent button action from enabling disabled DPMS
> 
> Button actions call triggerImpl, which leaves DPMS enabled. It can't be
> disabled until return from idle, as that would wake the screen. Solve
> this by explicitly setting DPMS timeouts to zero when DPMS is meant to
> be disabled - i.e. when unloading the action or when screen power
> management is inhibited.
> 
> CCBUG: 295164
> 
> 
> Diffs
> -----
> 
>   powerdevil/daemon/actions/dpms/powerdevildpmsaction.cpp a16bf7ee254b9a40c6f033c93e5ca63226469e6e 
> 
> Diff: http://git.reviewboard.kde.org/r/106794/diff/
> 
> 
> Testing
> -------
> 
> (in combination with review request  #106793 )
> 
> Set power button to "Turn off screen" then press it, wait for the screen to turn off then wake the system from idle. Check 'xset -q' has expected values in various situations, e.g.:
> 
> - when timed screen power saving is enabled
> - when it's disabled
> - when it hasn't been enabled in this login
> - when screen power saving is inhibited
> - when profile or configuration or activity change
> - when profile or configuration or activity change when screen power saving is inhibited
> 
> I think this covers all cases, but please hammer away with it and see if you can find any problems I've missed.
> 
> 
> Thanks,
> 
> Oliver Henshaw
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20121031/dc32e1a9/attachment.html>