diff mbox

[Bug,197863] Thinkpad X240 resume dramatically slower on kernels 4.13+

Message ID 4333304.2V78SBapX3@aspire.rjw.lan (mailing list archive)
State RFC, archived
Headers show

Commit Message

Rafael J. Wysocki Feb. 8, 2018, 7:12 p.m. UTC
On Wednesday, February 7, 2018 11:44:15 PM CET Rafael J. Wysocki wrote:
> On Mon, Feb 5, 2018 at 6:06 PM, Rafael J. Wysocki
> <rafael.j.wysocki@intel.com> wrote:
> > On 2/5/2018 3:14 PM, Bjørn Mork wrote:
> >>
> >> "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> writes:
> >>>
> >>> On 2/4/2018 9:28 PM, Bjørn Mork wrote:
> >>>
> >>>> But I do wonder if the attached (completely untested!!) patch makes
> >>>> things any better?
> >>>
> >>> I don't think so, the macro is needed too.
> >>
> >> Doh! Obviously.  Don't know how I managed to miss that.
> >>
> >>> I'll queue up a full revert of 662591461c4b9a1e3b.
> >>
> >> Still with the additional exception for "ec == first_ec"?
> >>
> >
> > No, just a full revert for now.
> 
> That doesn't work, because we made some changes on top of this commit.
> 
> I'll send a patch to try tomorrow.

Below is a patch for the mainline (should be applicable to 4.15.y) to test.
Please let me know if it improves things for you.

The corresponding fix for -stable would still be a full revert of
662591461c4b9a1e3b, but it needs to be fixed in the mainline first.

---
 drivers/acpi/ec.c |    6 ++++++
 1 file changed, 6 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Markus Demleitner Feb. 9, 2018, 1:43 p.m. UTC | #1
On Thu, Feb 08, 2018 at 08:12:06PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, February 7, 2018 11:44:15 PM CET Rafael J. Wysocki wrote:
> > On Mon, Feb 5, 2018 at 6:06 PM, Rafael J. Wysocki
> > <rafael.j.wysocki@intel.com> wrote:
> > > On 2/5/2018 3:14 PM, Bjørn Mork wrote:
> > >>
> > >> "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> writes:
> > >>>
> > >>> On 2/4/2018 9:28 PM, Bjørn Mork wrote:
> > >>>
> > >>>> But I do wonder if the attached (completely untested!!) patch makes
> > >>>> things any better?
> > >>>
> > >>> I don't think so, the macro is needed too.
> > >>
> > >> Doh! Obviously.  Don't know how I managed to miss that.
> > >>
> > >>> I'll queue up a full revert of 662591461c4b9a1e3b.
> > >>
> > >> Still with the additional exception for "ec == first_ec"?
> > >>
> > >
> > > No, just a full revert for now.
> > 
> > That doesn't work, because we made some changes on top of this commit.
> > 
> > I'll send a patch to try tomorrow.
> 
> Below is a patch for the mainline (should be applicable to 4.15.y) to test.
> Please let me know if it improves things for you.

It pretty certainly does (patch applied on top of 4.15.0 from
tarball).  Five out of five wakeup cycles were fast, and the cell
modem is still properly initialised -- the message went out via the
resumed modem.

Thanks!

     -- Markus

[patch retained for reference:]

> 
> The corresponding fix for -stable would still be a full revert of
> 662591461c4b9a1e3b, but it needs to be fixed in the mainline first.
> 
> ---
>  drivers/acpi/ec.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -1927,6 +1927,9 @@ static int acpi_ec_suspend_noirq(struct
>  	    ec->reference_count >= 1)
>  		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
>  
> +	if (acpi_sleep_no_ec_events())
> +		acpi_ec_enter_noirq(ec);
> +
>  	return 0;
>  }
>  
> @@ -1934,6 +1937,9 @@ static int acpi_ec_resume_noirq(struct d
>  {
>  	struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
>  
> +	if (acpi_sleep_no_ec_events())
> +		acpi_ec_leave_noirq(ec);
> +
>  	if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
>  	    ec->reference_count >= 1)
>  		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
>
Rafael J. Wysocki Feb. 9, 2018, 9:26 p.m. UTC | #2
On Fri, Feb 9, 2018 at 2:43 PM, Markus Demleitner <m@tfiu.de> wrote:
> On Thu, Feb 08, 2018 at 08:12:06PM +0100, Rafael J. Wysocki wrote:
>> On Wednesday, February 7, 2018 11:44:15 PM CET Rafael J. Wysocki wrote:
>> > On Mon, Feb 5, 2018 at 6:06 PM, Rafael J. Wysocki
>> > <rafael.j.wysocki@intel.com> wrote:
>> > > On 2/5/2018 3:14 PM, Bjørn Mork wrote:
>> > >>
>> > >> "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> writes:
>> > >>>
>> > >>> On 2/4/2018 9:28 PM, Bjørn Mork wrote:
>> > >>>
>> > >>>> But I do wonder if the attached (completely untested!!) patch makes
>> > >>>> things any better?
>> > >>>
>> > >>> I don't think so, the macro is needed too.
>> > >>
>> > >> Doh! Obviously.  Don't know how I managed to miss that.
>> > >>
>> > >>> I'll queue up a full revert of 662591461c4b9a1e3b.
>> > >>
>> > >> Still with the additional exception for "ec == first_ec"?
>> > >>
>> > >
>> > > No, just a full revert for now.
>> >
>> > That doesn't work, because we made some changes on top of this commit.
>> >
>> > I'll send a patch to try tomorrow.
>>
>> Below is a patch for the mainline (should be applicable to 4.15.y) to test.
>> Please let me know if it improves things for you.
>
> It pretty certainly does (patch applied on top of 4.15.0 from
> tarball).  Five out of five wakeup cycles were fast, and the cell
> modem is still properly initialised -- the message went out via the
> resumed modem.

OK, let's do it, then.

Thanks for testing!
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -1927,6 +1927,9 @@  static int acpi_ec_suspend_noirq(struct
 	    ec->reference_count >= 1)
 		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
 
+	if (acpi_sleep_no_ec_events())
+		acpi_ec_enter_noirq(ec);
+
 	return 0;
 }
 
@@ -1934,6 +1937,9 @@  static int acpi_ec_resume_noirq(struct d
 {
 	struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
 
+	if (acpi_sleep_no_ec_events())
+		acpi_ec_leave_noirq(ec);
+
 	if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
 	    ec->reference_count >= 1)
 		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);