[PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Hans de Goede]

The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:

            Method (_LID, 0, NotSerialized)  // _LID: Lid Status
            {
                If ((STAS & One))
                {
                    Local0 = One
                    PBCG |= 0x05000000
                    HMCG |= 0x05000000
                }
                Else
                {
                    Local0 = Zero
                    PBCG &= 0xF0FFFFFF
                    HMCG &= 0xF0FFFFFF
                }

                ^^PCI0.GFX0.CLID = Local0
                Return (Local0)
            }

The problem here is the accesses to the PBCG and HMCG, these are the
pinconf0 registers for the power, resp. the home button GPIO,
e.g. PBCG is declared as:

            OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
            Field (PWBT, DWordAcc, NoLock, Preserve)
            {
                PBCG,   32,
                PBV1,   32,
                PBSA,   32,
                PBV2,   32
            }

Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
the offset for the pin used for the powerbutton.

The problem here is this line in _LID:
                    PBCG |= 0x05000000

This changes the trigger flags of the GPIO, changing when it generates
interrupts. Note it does not clear the original flags. Linux uses an
edge triggered interrupt on both positive and negative edges. This |=
adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
interrupt which fires both when low and high, iow it simply always
fires leading to an interrupt storm, the tablet immediately waking up
from suspend again, etc.

There is nothing we can do to fix this, except for a DSDT override,
which the user needs to do manually. The only thing we can do is
never call _LID, which requires disabling the lid-switch functionality
altogether.

This commit adds a quirk for this, as no lid-switch function is better
then the interrupt storm. A user manually applying a DSDT override can
also override the quirk on the kernel cmdline.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/button.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index d27b01c0323d..b758b45737f5 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -77,6 +77,19 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids);
 
 /* Please keep this list sorted alphabetically by vendor and model */
 static const struct dmi_system_id dmi_lid_quirks[] = {
+	{
+		/*
+		 * Acer Switch 10 SW5-012. _LID method messes with home and
+		 * power button GPIO IRQ settings causing an interrupt storm on
+		 * both GPIOs. This is unfixable without a DSDT override, so we
+		 * have to disable the lid-switch functionality altogether :|
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
+		},
+		.driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED,
+	},
 	{
 		/*
 		 * Asus T200TA, _LID keeps reporting closed after every second
-- 
2.23.0

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Mika Westerberg]

On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
> 
>             Method (_LID, 0, NotSerialized)  // _LID: Lid Status
>             {
>                 If ((STAS & One))
>                 {
>                     Local0 = One
>                     PBCG |= 0x05000000
>                     HMCG |= 0x05000000
>                 }
>                 Else
>                 {
>                     Local0 = Zero
>                     PBCG &= 0xF0FFFFFF
>                     HMCG &= 0xF0FFFFFF
>                 }
> 
>                 ^^PCI0.GFX0.CLID = Local0
>                 Return (Local0)
>             }
> 
> The problem here is the accesses to the PBCG and HMCG, these are the
> pinconf0 registers for the power, resp. the home button GPIO,
> e.g. PBCG is declared as:
> 
>             OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
>             Field (PWBT, DWordAcc, NoLock, Preserve)
>             {
>                 PBCG,   32,
>                 PBV1,   32,
>                 PBSA,   32,
>                 PBV2,   32
>             }
> 
> Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
> the offset for the pin used for the powerbutton.
> 
> The problem here is this line in _LID:
>                     PBCG |= 0x05000000
> 
> This changes the trigger flags of the GPIO, changing when it generates
> interrupts. Note it does not clear the original flags. Linux uses an
> edge triggered interrupt on both positive and negative edges. This |=
> adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
> interrupt which fires both when low and high, iow it simply always
> fires leading to an interrupt storm, the tablet immediately waking up
> from suspend again, etc.

Hmm, does it work in Windows?

> There is nothing we can do to fix this, except for a DSDT override,
> which the user needs to do manually. The only thing we can do is
> never call _LID, which requires disabling the lid-switch functionality
> altogether.
> 
> This commit adds a quirk for this, as no lid-switch function is better
> then the interrupt storm. A user manually applying a DSDT override can
> also override the quirk on the kernel cmdline.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Hans de Goede]

Hi,

On 19-11-2019 09:26, Mika Westerberg wrote:
> On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
>> The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
>>
>>              Method (_LID, 0, NotSerialized)  // _LID: Lid Status
>>              {
>>                  If ((STAS & One))
>>                  {
>>                      Local0 = One
>>                      PBCG |= 0x05000000
>>                      HMCG |= 0x05000000
>>                  }
>>                  Else
>>                  {
>>                      Local0 = Zero
>>                      PBCG &= 0xF0FFFFFF
>>                      HMCG &= 0xF0FFFFFF
>>                  }
>>
>>                  ^^PCI0.GFX0.CLID = Local0
>>                  Return (Local0)
>>              }
>>
>> The problem here is the accesses to the PBCG and HMCG, these are the
>> pinconf0 registers for the power, resp. the home button GPIO,
>> e.g. PBCG is declared as:
>>
>>              OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
>>              Field (PWBT, DWordAcc, NoLock, Preserve)
>>              {
>>                  PBCG,   32,
>>                  PBV1,   32,
>>                  PBSA,   32,
>>                  PBV2,   32
>>              }
>>
>> Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
>> the offset for the pin used for the powerbutton.
>>
>> The problem here is this line in _LID:
>>                      PBCG |= 0x05000000
>>
>> This changes the trigger flags of the GPIO, changing when it generates
>> interrupts. Note it does not clear the original flags. Linux uses an
>> edge triggered interrupt on both positive and negative edges. This |=
>> adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
>> interrupt which fires both when low and high, iow it simply always
>> fires leading to an interrupt storm, the tablet immediately waking up
>> from suspend again, etc.
> 
> Hmm, does it work in Windows?

I bought this machine 2nd hand and the Windows install is broken
(the eMMC is dead) so I do not know with 100% certainty.

I guess it does work in Windows, I would assume so at least. I suspect
that the Windows driver for "PNP0C40" GPIO buttons devices uses level
interrupts only listening for presses which would match the "5" in the
mask.  Note that that would very much go against the ACPI description,
which describes the 4 GPIOs for pwrbutton/home/vol+/vol- as follows:

     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
     {
         Name (RBUF, ResourceTemplate ()
         {
             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
                 "\\_SB.GPO2", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0010
                 }
             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
                 "\\_SB.GPO2", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0015
                 }
             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
                 "\\_SB.GPO0", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0001
                 }
             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
                 "\\_SB.GPO0", 0x00, ResourceConsumer, ,
                 )
                 {   // Pin list
                     0x0000
                 }
         })
         Return (RBUF) /* \_SB_.TBAD._CRS.RBUF */
     }

Notice how all GPIOs are specified as GpioInt's which are active on
both edges and this is what the linux gpio_keys driver uses.

Working around this is not impossible, but it will be quite ugly and given
the age of the machine IMHO not worth it. I've also found out that I need a
DSDT override to be able to control the LCD backlight, this is controlled by
the 1st PWM controller in the SoC LPSS block, which is normally enumerated
through ACPI but the entire Device (PWM1) {} block is missing from the
DSDT :|  Adding it from similar hardware fixes things and makes the backlight
controllable. TL;DR: it seems that this is one of the rare cased where
people who want to run Linux will need to do a manual DSDT override :|

When they do that override they can also fix the _LID method and
then re-enable LID functionality on the kernel commandline overriding
this DMI quirk.

I will probably do a blog post on this (some people have asked me
to do some blogposts about how to analyze DSDT-s, this will be a nice
example) and add a link to the DSDT override to the blogpost, I believe
that this is the best we can do for users of this device.

In the meantime this quirk at least avoids the interrupt storm making
the device mostly usable even without the DSDT override.

>> There is nothing we can do to fix this, except for a DSDT override,
>> which the user needs to do manually. The only thing we can do is
>> never call _LID, which requires disabling the lid-switch functionality
>> altogether.
>>
>> This commit adds a quirk for this, as no lid-switch function is better
>> then the interrupt storm. A user manually applying a DSDT override can
>> also override the quirk on the kernel cmdline.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks.

Regards,

Hans

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Mika Westerberg]

On Tue, Nov 19, 2019 at 12:12:35PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 19-11-2019 09:26, Mika Westerberg wrote:
> > On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> > > The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
> > > 
> > >              Method (_LID, 0, NotSerialized)  // _LID: Lid Status
> > >              {
> > >                  If ((STAS & One))
> > >                  {
> > >                      Local0 = One
> > >                      PBCG |= 0x05000000
> > >                      HMCG |= 0x05000000
> > >                  }
> > >                  Else
> > >                  {
> > >                      Local0 = Zero
> > >                      PBCG &= 0xF0FFFFFF
> > >                      HMCG &= 0xF0FFFFFF
> > >                  }
> > > 
> > >                  ^^PCI0.GFX0.CLID = Local0
> > >                  Return (Local0)
> > >              }
> > > 
> > > The problem here is the accesses to the PBCG and HMCG, these are the
> > > pinconf0 registers for the power, resp. the home button GPIO,
> > > e.g. PBCG is declared as:
> > > 
> > >              OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
> > >              Field (PWBT, DWordAcc, NoLock, Preserve)
> > >              {
> > >                  PBCG,   32,
> > >                  PBV1,   32,
> > >                  PBSA,   32,
> > >                  PBV2,   32
> > >              }
> > > 
> > > Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
> > > the offset for the pin used for the powerbutton.
> > > 
> > > The problem here is this line in _LID:
> > >                      PBCG |= 0x05000000
> > > 
> > > This changes the trigger flags of the GPIO, changing when it generates
> > > interrupts. Note it does not clear the original flags. Linux uses an
> > > edge triggered interrupt on both positive and negative edges. This |=
> > > adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
> > > interrupt which fires both when low and high, iow it simply always
> > > fires leading to an interrupt storm, the tablet immediately waking up
> > > from suspend again, etc.
> > 
> > Hmm, does it work in Windows?
> 
> I bought this machine 2nd hand and the Windows install is broken
> (the eMMC is dead) so I do not know with 100% certainty.
> 
> I guess it does work in Windows, I would assume so at least. I suspect
> that the Windows driver for "PNP0C40" GPIO buttons devices uses level
> interrupts only listening for presses which would match the "5" in the
> mask.  Note that that would very much go against the ACPI description,
> which describes the 4 GPIOs for pwrbutton/home/vol+/vol- as follows:
> 
>     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>     {
>         Name (RBUF, ResourceTemplate ()
>         {
>             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
>                 "\\_SB.GPO2", 0x00, ResourceConsumer, ,
>                 )
>                 {   // Pin list
>                     0x0010
>                 }
>             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
>                 "\\_SB.GPO2", 0x00, ResourceConsumer, ,
>                 )
>                 {   // Pin list
>                     0x0015
>                 }
>             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
>                 "\\_SB.GPO0", 0x00, ResourceConsumer, ,
>                 )
>                 {   // Pin list
>                     0x0001
>                 }
>             GpioInt (Edge, ActiveBoth, ExclusiveAndWake, PullDefault, 0x0000,
>                 "\\_SB.GPO0", 0x00, ResourceConsumer, ,
>                 )
>                 {   // Pin list
>                     0x0000
>                 }
>         })
>         Return (RBUF) /* \_SB_.TBAD._CRS.RBUF */
>     }
> 
> Notice how all GPIOs are specified as GpioInt's which are active on
> both edges and this is what the linux gpio_keys driver uses.

OK thanks for the details.

> Working around this is not impossible, but it will be quite ugly and given
> the age of the machine IMHO not worth it.

I agree.

> I've also found out that I need a DSDT override to be able to control
> the LCD backlight, this is controlled by the 1st PWM controller in the
> SoC LPSS block, which is normally enumerated through ACPI but the
> entire Device (PWM1) {} block is missing from the DSDT :|  Adding it
> from similar hardware fixes things and makes the backlight
> controllable. TL;DR: it seems that this is one of the rare cased where
> people who want to run Linux will need to do a manual DSDT override :|
> 
> When they do that override they can also fix the _LID method and
> then re-enable LID functionality on the kernel commandline overriding
> this DMI quirk.
> 
> I will probably do a blog post on this (some people have asked me
> to do some blogposts about how to analyze DSDT-s, this will be a nice
> example) and add a link to the DSDT override to the blogpost, I believe
> that this is the best we can do for users of this device.
> 
> In the meantime this quirk at least avoids the interrupt storm making
> the device mostly usable even without the DSDT override.

Yup, makes sense.

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Andy Shevchenko]

On Tue, Nov 19, 2019 at 12:12:35PM +0100, Hans de Goede wrote:
> On 19-11-2019 09:26, Mika Westerberg wrote:
> > On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:

> Working around this is not impossible, but it will be quite ugly and given
> the age of the machine IMHO not worth it. I've also found out that I need a
> DSDT override to be able to control the LCD backlight, this is controlled by
> the 1st PWM controller in the SoC LPSS block, which is normally enumerated
> through ACPI but the entire Device (PWM1) {} block is missing from the
> DSDT :|  Adding it from similar hardware fixes things and makes the backlight
> controllable. TL;DR: it seems that this is one of the rare cased where
> people who want to run Linux will need to do a manual DSDT override :|

If it's missing it's easy to inject entire block from EFI variable or using
ConfigFS (see meta-acpi project [1] for details).

> When they do that override they can also fix the _LID method and
> then re-enable LID functionality on the kernel commandline overriding
> this DMI quirk.

Yes, if you override entire DSDT it can be fixed for many bugs at once.

> I will probably do a blog post on this (some people have asked me
> to do some blogposts about how to analyze DSDT-s, this will be a nice
> example) and add a link to the DSDT override to the blogpost, I believe
> that this is the best we can do for users of this device.

Perhaps above mentioned project somehow can be extended to keep DSDT ASL code
for overriding? Mika?

[1]: https://github.com/westeri/meta-acpi/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Andy Shevchenko]

On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
> 
>             Method (_LID, 0, NotSerialized)  // _LID: Lid Status
>             {
>                 If ((STAS & One))
>                 {
>                     Local0 = One
>                     PBCG |= 0x05000000
>                     HMCG |= 0x05000000
>                 }
>                 Else
>                 {
>                     Local0 = Zero
>                     PBCG &= 0xF0FFFFFF
>                     HMCG &= 0xF0FFFFFF
>                 }
> 
>                 ^^PCI0.GFX0.CLID = Local0
>                 Return (Local0)
>             }
> 
> The problem here is the accesses to the PBCG and HMCG, these are the
> pinconf0 registers for the power, resp. the home button GPIO,
> e.g. PBCG is declared as:
> 
>             OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
>             Field (PWBT, DWordAcc, NoLock, Preserve)
>             {
>                 PBCG,   32,
>                 PBV1,   32,
>                 PBSA,   32,
>                 PBV2,   32
>             }
> 
> Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
> the offset for the pin used for the powerbutton.
> 
> The problem here is this line in _LID:
>                     PBCG |= 0x05000000
> 
> This changes the trigger flags of the GPIO, changing when it generates
> interrupts. Note it does not clear the original flags. Linux uses an
> edge triggered interrupt on both positive and negative edges. This |=
> adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
> interrupt which fires both when low and high, iow it simply always
> fires leading to an interrupt storm, the tablet immediately waking up
> from suspend again, etc.
> 
> There is nothing we can do to fix this, except for a DSDT override,
> which the user needs to do manually. The only thing we can do is
> never call _LID, which requires disabling the lid-switch functionality
> altogether.
> 
> This commit adds a quirk for this, as no lid-switch function is better
> then the interrupt storm. A user manually applying a DSDT override can
> also override the quirk on the kernel cmdline.
> 

Fair enough, thanks for the quirk.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/button.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index d27b01c0323d..b758b45737f5 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -77,6 +77,19 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids);
>  
>  /* Please keep this list sorted alphabetically by vendor and model */
>  static const struct dmi_system_id dmi_lid_quirks[] = {
> +	{
> +		/*
> +		 * Acer Switch 10 SW5-012. _LID method messes with home and
> +		 * power button GPIO IRQ settings causing an interrupt storm on
> +		 * both GPIOs. This is unfixable without a DSDT override, so we
> +		 * have to disable the lid-switch functionality altogether :|
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
> +		},
> +		.driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED,
> +	},
>  	{
>  		/*
>  		 * Asus T200TA, _LID keeps reporting closed after every second
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Mika Westerberg]

On Tue, Nov 19, 2019 at 02:44:11PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 19, 2019 at 12:12:35PM +0100, Hans de Goede wrote:
> > On 19-11-2019 09:26, Mika Westerberg wrote:
> > > On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> 
> > Working around this is not impossible, but it will be quite ugly and given
> > the age of the machine IMHO not worth it. I've also found out that I need a
> > DSDT override to be able to control the LCD backlight, this is controlled by
> > the 1st PWM controller in the SoC LPSS block, which is normally enumerated
> > through ACPI but the entire Device (PWM1) {} block is missing from the
> > DSDT :|  Adding it from similar hardware fixes things and makes the backlight
> > controllable. TL;DR: it seems that this is one of the rare cased where
> > people who want to run Linux will need to do a manual DSDT override :|
> 
> If it's missing it's easy to inject entire block from EFI variable or using
> ConfigFS (see meta-acpi project [1] for details).
> 
> > When they do that override they can also fix the _LID method and
> > then re-enable LID functionality on the kernel commandline overriding
> > this DMI quirk.
> 
> Yes, if you override entire DSDT it can be fixed for many bugs at once.
> 
> > I will probably do a blog post on this (some people have asked me
> > to do some blogposts about how to analyze DSDT-s, this will be a nice
> > example) and add a link to the DSDT override to the blogpost, I believe
> > that this is the best we can do for users of this device.
> 
> Perhaps above mentioned project somehow can be extended to keep DSDT ASL code
> for overriding? Mika?
> 
> [1]: https://github.com/westeri/meta-acpi/

No objections.

Maybe we should have a mechanism in the kernel that allows you to have
ACPI table quirks like this for multiple different systems (based on DMI
indentifiers perhaps) inside a single initrd and the kernel then loads
tables only matching the running system. That would allow distros to
ship these for broken systems.

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Hans de Goede]

Hi,

On 19-11-2019 13:57, Mika Westerberg wrote:
> On Tue, Nov 19, 2019 at 02:44:11PM +0200, Andy Shevchenko wrote:
>> On Tue, Nov 19, 2019 at 12:12:35PM +0100, Hans de Goede wrote:
>>> On 19-11-2019 09:26, Mika Westerberg wrote:
>>>> On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
>>
>>> Working around this is not impossible, but it will be quite ugly and given
>>> the age of the machine IMHO not worth it. I've also found out that I need a
>>> DSDT override to be able to control the LCD backlight, this is controlled by
>>> the 1st PWM controller in the SoC LPSS block, which is normally enumerated
>>> through ACPI but the entire Device (PWM1) {} block is missing from the
>>> DSDT :|  Adding it from similar hardware fixes things and makes the backlight
>>> controllable. TL;DR: it seems that this is one of the rare cased where
>>> people who want to run Linux will need to do a manual DSDT override :|
>>
>> If it's missing it's easy to inject entire block from EFI variable or using
>> ConfigFS (see meta-acpi project [1] for details).
>>
>>> When they do that override they can also fix the _LID method and
>>> then re-enable LID functionality on the kernel commandline overriding
>>> this DMI quirk.
>>
>> Yes, if you override entire DSDT it can be fixed for many bugs at once.
>>
>>> I will probably do a blog post on this (some people have asked me
>>> to do some blogposts about how to analyze DSDT-s, this will be a nice
>>> example) and add a link to the DSDT override to the blogpost, I believe
>>> that this is the best we can do for users of this device.
>>
>> Perhaps above mentioned project somehow can be extended to keep DSDT ASL code
>> for overriding? Mika?
>>
>> [1]: https://github.com/westeri/meta-acpi/
> 
> No objections.
> 
> Maybe we should have a mechanism in the kernel that allows you to have
> ACPI table quirks like this for multiple different systems (based on DMI
> indentifiers perhaps) inside a single initrd and the kernel then loads
> tables only matching the running system. That would allow distros to
> ship these for broken systems.

I would love to have something like this, but I'm afraid that the distros
cannot just distribute modified DSDT's. I know we ask people to upload
acpidump's to bugzilla, etc. all the time. But one can reasonably argue
that that is fair-use (IANAL, TINLA). OTOH for something to be distributed
by distros we are going to need something a lot less handwavy wrt
re-dsitribution of these files, which AFAIK is impossible to get.

I had a discussion about this a while ago at my local hackerspace (*),
and someone there suggested to distribute patch files and have some
scripts which automatically generate an overlay by doing acpidump +
acpixtract + iasl -d + apply-patch + iasl -ta. This would then automatically
run at boot so that the next boot will have a fixed DSDT. Which is an
interesting concept if anyone is willing to work on it ...

Regards,

Hans




*) While working on fixing something which needed a DSDT override IIRC

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Mika Westerberg]

On Tue, Nov 19, 2019 at 04:38:52PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 19-11-2019 13:57, Mika Westerberg wrote:
> > On Tue, Nov 19, 2019 at 02:44:11PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 19, 2019 at 12:12:35PM +0100, Hans de Goede wrote:
> > > > On 19-11-2019 09:26, Mika Westerberg wrote:
> > > > > On Mon, Nov 18, 2019 at 04:35:56PM +0100, Hans de Goede wrote:
> > > 
> > > > Working around this is not impossible, but it will be quite ugly and given
> > > > the age of the machine IMHO not worth it. I've also found out that I need a
> > > > DSDT override to be able to control the LCD backlight, this is controlled by
> > > > the 1st PWM controller in the SoC LPSS block, which is normally enumerated
> > > > through ACPI but the entire Device (PWM1) {} block is missing from the
> > > > DSDT :|  Adding it from similar hardware fixes things and makes the backlight
> > > > controllable. TL;DR: it seems that this is one of the rare cased where
> > > > people who want to run Linux will need to do a manual DSDT override :|
> > > 
> > > If it's missing it's easy to inject entire block from EFI variable or using
> > > ConfigFS (see meta-acpi project [1] for details).
> > > 
> > > > When they do that override they can also fix the _LID method and
> > > > then re-enable LID functionality on the kernel commandline overriding
> > > > this DMI quirk.
> > > 
> > > Yes, if you override entire DSDT it can be fixed for many bugs at once.
> > > 
> > > > I will probably do a blog post on this (some people have asked me
> > > > to do some blogposts about how to analyze DSDT-s, this will be a nice
> > > > example) and add a link to the DSDT override to the blogpost, I believe
> > > > that this is the best we can do for users of this device.
> > > 
> > > Perhaps above mentioned project somehow can be extended to keep DSDT ASL code
> > > for overriding? Mika?
> > > 
> > > [1]: https://github.com/westeri/meta-acpi/
> > 
> > No objections.
> > 
> > Maybe we should have a mechanism in the kernel that allows you to have
> > ACPI table quirks like this for multiple different systems (based on DMI
> > indentifiers perhaps) inside a single initrd and the kernel then loads
> > tables only matching the running system. That would allow distros to
> > ship these for broken systems.
> 
> I would love to have something like this, but I'm afraid that the distros
> cannot just distribute modified DSDT's. I know we ask people to upload
> acpidump's to bugzilla, etc. all the time. But one can reasonably argue
> that that is fair-use (IANAL, TINLA). OTOH for something to be distributed
> by distros we are going to need something a lot less handwavy wrt
> re-dsitribution of these files, which AFAIK is impossible to get.

Good point.

> I had a discussion about this a while ago at my local hackerspace (*),
> and someone there suggested to distribute patch files and have some
> scripts which automatically generate an overlay by doing acpidump +
> acpixtract + iasl -d + apply-patch + iasl -ta. This would then automatically
> run at boot so that the next boot will have a fixed DSDT. Which is an
> interesting concept if anyone is willing to work on it ...

Indeed interesting idea. Not volunteering to work on it though ;-)

Re: [PATCH] ACPI / button: Add DMI quirk for Acer Switch 10 SW5-032 lid-switch

[Rafael J. Wysocki]

On Monday, November 18, 2019 4:35:56 PM CET Hans de Goede wrote:
> The Acer Switch 10 SW5-032 _LID method is quite broken, it looks like this:
> 
>             Method (_LID, 0, NotSerialized)  // _LID: Lid Status
>             {
>                 If ((STAS & One))
>                 {
>                     Local0 = One
>                     PBCG |= 0x05000000
>                     HMCG |= 0x05000000
>                 }
>                 Else
>                 {
>                     Local0 = Zero
>                     PBCG &= 0xF0FFFFFF
>                     HMCG &= 0xF0FFFFFF
>                 }
> 
>                 ^^PCI0.GFX0.CLID = Local0
>                 Return (Local0)
>             }
> 
> The problem here is the accesses to the PBCG and HMCG, these are the
> pinconf0 registers for the power, resp. the home button GPIO,
> e.g. PBCG is declared as:
> 
>             OperationRegion (PWBT, SystemMemory, 0xFED0E080, 0x10)
>             Field (PWBT, DWordAcc, NoLock, Preserve)
>             {
>                 PBCG,   32,
>                 PBV1,   32,
>                 PBSA,   32,
>                 PBV2,   32
>             }
> 
> Where 0xFED0E000 is the base address of the GPO2 device and 0x80 is
> the offset for the pin used for the powerbutton.
> 
> The problem here is this line in _LID:
>                     PBCG |= 0x05000000
> 
> This changes the trigger flags of the GPIO, changing when it generates
> interrupts. Note it does not clear the original flags. Linux uses an
> edge triggered interrupt on both positive and negative edges. This |=
> adds the BYT_TRIG_LVL flag to this, so now it is turned into a level
> interrupt which fires both when low and high, iow it simply always
> fires leading to an interrupt storm, the tablet immediately waking up
> from suspend again, etc.
> 
> There is nothing we can do to fix this, except for a DSDT override,
> which the user needs to do manually. The only thing we can do is
> never call _LID, which requires disabling the lid-switch functionality
> altogether.
> 
> This commit adds a quirk for this, as no lid-switch function is better
> then the interrupt storm. A user manually applying a DSDT override can
> also override the quirk on the kernel cmdline.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/button.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index d27b01c0323d..b758b45737f5 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -77,6 +77,19 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids);
>  
>  /* Please keep this list sorted alphabetically by vendor and model */
>  static const struct dmi_system_id dmi_lid_quirks[] = {
> +	{
> +		/*
> +		 * Acer Switch 10 SW5-012. _LID method messes with home and
> +		 * power button GPIO IRQ settings causing an interrupt storm on
> +		 * both GPIOs. This is unfixable without a DSDT override, so we
> +		 * have to disable the lid-switch functionality altogether :|
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
> +		},
> +		.driver_data = (void *)(long)ACPI_BUTTON_LID_INIT_DISABLED,
> +	},
>  	{
>  		/*
>  		 * Asus T200TA, _LID keeps reporting closed after every second
> 

Applying as 5.5 material, thanks!