diff mbox

S3 resume regression [1cf4f629d9d2 ("cpu/hotplug: Move online calls to hotplugged cpu")]

Message ID 20161027172852.GE4617@intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ville Syrjälä Oct. 27, 2016, 5:28 p.m. UTC
On Tue, Aug 09, 2016 at 08:20:57PM +0300, Ville Syrjälä wrote:
> On Thu, Jul 14, 2016 at 04:29:42PM +0800, Feng Tang wrote:
> > if you only want it to work, you can try an old patch
> > https://bugzilla.kernel.org/attachment.cgi?id=76071 from a similar bug
> > https://bugzilla.kernel.org/show_bug.cgi?id=41932
> > 
> > Alistair Buxton confirmed it work for 3.18 at least
> > https://bugzilla.kernel.org/show_bug.cgi?id=107151#c16
> 
> That patch is a bit too ripe by now. Would need a fresh squeezed one.

Since no one else bothered to refresh the patch I did it myself:

Unfortunately it doesn't do anything for me.

The fortunate thing is that acpi-idle has magically been fixed in the
meantime, so I can at least go back to using that one and have working
S3.

Comments

Thomas Gleixner Oct. 27, 2016, 6:48 p.m. UTC | #1
On Thu, 27 Oct 2016, Ville Syrjälä wrote:
> On Tue, Aug 09, 2016 at 08:20:57PM +0300, Ville Syrjälä wrote:
> > On Thu, Jul 14, 2016 at 04:29:42PM +0800, Feng Tang wrote:
> > > if you only want it to work, you can try an old patch
> > > https://bugzilla.kernel.org/attachment.cgi?id=76071 from a similar bug
> > > https://bugzilla.kernel.org/show_bug.cgi?id=41932
> > > 
> > > Alistair Buxton confirmed it work for 3.18 at least
> > > https://bugzilla.kernel.org/show_bug.cgi?id=107151#c16
> > 
> > That patch is a bit too ripe by now. Would need a fresh squeezed one.
> 
> Since no one else bothered to refresh the patch I did it myself:
> 
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index f6aae7977824..d73d094a8972 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -657,8 +657,16 @@ static void tick_handle_oneshot_broadcast(struct clock_event_device *dev)
>  	 * - There are pending events on sleeping CPUs which were not
>  	 * in the event mask
>  	 */
> -	if (next_event.tv64 != KTIME_MAX)
> +	if (next_event.tv64 != KTIME_MAX) {
> +		s64 delta = next_event.tv64 - now.tv64;
> +
> +		if (delta >= 10000000000) {
> +			printk(KERN_CRIT "%s(): The delta is big: %lld\n", __func__, delta);
> +			next_event.tv64 = now.tv64 + 3000000000;
> +		}
> +
>  		tick_broadcast_set_event(dev, next_cpu, next_event);
> +	}
>  
>  	raw_spin_unlock(&tick_broadcast_lock);
> 
> Unfortunately it doesn't do anything for me.

And I'm not surprised, because the original patch forced a 5 seconds event
in the broadcast device on resume, aside of limiting the reprogramming.

What that old patch did, was:

1) Make sure that the broadcast device is actually armed at resume.

   That might cause the HPET to resume proper.

2) Force a max. 3 seconds rearm when the targeted expiry time is > than 10
   seconds

   That might make sure that lower C-States are never entered.
 
> The fortunate thing is that acpi-idle has magically been fixed in the
> meantime, so I can at least go back to using that one and have working
> S3.

What's the lowest C-State with acpi-idle and what's the lowest one with
intel_idle?

Thanks,

	tglx
diff mbox

Patch

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index f6aae7977824..d73d094a8972 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -657,8 +657,16 @@  static void tick_handle_oneshot_broadcast(struct clock_event_device *dev)
 	 * - There are pending events on sleeping CPUs which were not
 	 * in the event mask
 	 */
-	if (next_event.tv64 != KTIME_MAX)
+	if (next_event.tv64 != KTIME_MAX) {
+		s64 delta = next_event.tv64 - now.tv64;
+
+		if (delta >= 10000000000) {
+			printk(KERN_CRIT "%s(): The delta is big: %lld\n", __func__, delta);
+			next_event.tv64 = now.tv64 + 3000000000;
+		}
+
 		tick_broadcast_set_event(dev, next_cpu, next_event);
+	}
 
 	raw_spin_unlock(&tick_broadcast_lock);