<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5320/">http://svn.reviewboard.kde.org/r/5320/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 13th, 2010, 4:16 p.m., <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">To be honest, I don't like this patch.
The correct solution is to get a SIGNAL "resumed()", which is caught by the dataengines and which then just updates. The polling and retrying in timeengine.cpp is really ugly, IMO. (No offense ;)) I want this fixed in Solid to get a reliable signal that we're resumed now.
Your patch doesn't take hibernation into account, btw, which has the exact same problem. Also starting two second polling for two minutes when the user hits suspend looks like draining the battery unnecessarily (CPU wakeups), which is exactly not what you want on machines where suspend/resume is critical.
Thanks a lot for looking into this (admittedly very annoying) problem, but I think that this is just a workaround for something that is important enough to be fixed in the right way.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I want to have this fixed in solid too, but in the meantime this is all what can be done. All you said is true. But if you look in powertop you have at least 40 wakeups per second (if you don't do anything and have much luck). Under nurmal workload in can climb up into the thousands. This patch adds a single wakeup all two seconds for a short period of time only if the user requested a sleeping mode. After waking up there is much cpu activity anyway, there is not much time for deep sleeps anyway. I cannot imagine that this patch really reduces battery time in any noticeable amount.
This patch does respect hibernate.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 13th, 2010, 4:16 p.m., <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/5320/diff/2/?file=35751#file35751line814" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void PowerDevilDaemon::suspendToDisk(bool automated)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">814</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">emit</span> <span class="n">standbyRequested</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is problematic. You cannot assume that we resumed already, since the job in 808 is async. Suspension can still happen at this point.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Don't understand. I don't assume that.</pre>
<br />
<p>- Björn</p>
<br />
<p>On September 13th, 2010, 11:20 a.m., Björn Ruberg wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Solid.</div>
<div>By Björn Ruberg.</div>
<p style="color: grey;"><i>Updated 2010-09-13 11:20:38</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">NOTE: This a little bit ugly as I have to workaround the missing "going to suspend"-signal in the linux userland
This patch adds a signal to powerdevil that is emitted when a suspend/hibernate/standby is requested there. The signal is caught by the time engine what starts to look for an unexpected change in system time for two minutes by polling. If a clock skrew is detected, all sources are updated immediatly.
This fixes the bug that the plasma clocks are showing old time after suspending your machine up until 59 seconds (whenever the normal update is scheduled). This is not exactly a patch for bug 181380. But the clock skrew detection code can be used quite similar for detecting normal time changes. I'll look into that as soon this is accepted.
Please tell me whether I can commit this to 4.5 trunk as it is a bugfix for a nasty glitch.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Suspended machine, waited three minutes, woke up again - and noticed that all clocks on screen showed the correct time almost immediatly :)</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=181380">181380</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdebase/workspace/plasma/generic/dataengines/time/timeengine.h <span style="color: grey">(1166313)</span></li>
<li>/trunk/KDE/kdebase/workspace/plasma/generic/dataengines/time/timeengine.cpp <span style="color: grey">(1166313)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.h <span style="color: grey">(1166313)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.cpp <span style="color: grey">(1166313)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/org.kde.PowerDevil.xml <span style="color: grey">(1166313)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5320/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>