clamshellClosed spam- high CPU usage

[ellie-idb]

On battery, I've noticed that the console is filled with clamshell updates
PMRD: clamshell closed 0, disabled 0, desktopMode 0, ac 0 sleepDisabled 0
which pins the CPU at 10-20% continually and does not allow the processor to ramp back down. I've found that modifying the _LID area to be as follows fixes this, and is still functional (laptop sleeps when lid closed, awakens when lid is opened, etc)

    Scope (_SB)
    {
        Device (LID)
        {
            Name (_OLD, One) // assuming everything else.. the lid should start open?
            Name (_HID, EisaId ("PNP0C0D") /* Lid Device */)  // _HID: Hardware ID
            Method (_LID, 0, NotSerialized)  // _LID: Lid Status
            {
                Local0 = One
                Local0 = ^^PCI0.LPCB.EC0.RPIN (0x05, 0x06)
                If ((Local0 == 0x55))
                {
                    Local0 = Zero
                }
                Else
                {
                    Local0 = One
                }

                Return (Local0)
            }
        }

        Device (PNLF)
        {
            Name (_HID, EisaId ("APP0002"))  // _HID: Hardware ID
            Name (_CID, "backlight")  // _CID: Compatible ID
            Name (_UID, 0x0A)  // _UID: Unique ID
            Name (_STA, 0x0B)  // _STA: Status
        }

        Device (PWRB)
        {
            Name (_HID, EisaId ("PNP0C0C") /* Power Button Device */)  // _HID: Hardware ID
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0B)
            }
        }
    }

    Scope (_SB.PCI0.LPCB.EC0)
    {
        Method (_Q81, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
        {
            Local0 = ^^^^LID._LID ()
            If ((Local0 == Zero))
            {
                ADBG ("LID-OFF")
                SGOV (0x02030009, Zero)
                SGOV (0x02060000, Zero)
            }
            Else
            {
                ADBG ("LID-ON")
                SGOV (0x02030009, One)
                SGOV (0x02060000, One)
                Notify (ALSD, 0x80) // Status Change
            }

            If ((^^^^LID._OLD != Local0))
            {
                Notify (LID, 0x80) // Status Change
                ^^^^LID._OLD = Local0
            }
        }
    }
``

[ellie-idb]

It may be just me in particular, as I run OpenCore, but this patch should probably be integrated in.

[kvn1351]

It may be just me in particular, as I run OpenCore, but this patch should probably be integrated in.

Unrelated: May you send me a copy of your OC folder? I have been trying to make it work for a long time and I'm stuck on a panic..

[ellie-idb]

@kvn1351 Emailed you back. For anyone else trying to run OpenCore, this is my OC folder which works for me.
OC.zip

[kvn1351]

On battery, I've noticed that the console is filled with clamshell updates
PMRD: clamshell closed 0, disabled 0, desktopMode 0, ac 0 sleepDisabled 0
which pins the CPU at 10-20% continually and does not allow the processor to ramp back down. I've found that modifying the _LID area to be as follows fixes this, and is still functional (laptop sleeps when lid closed, awakens when lid is opened, etc)

    Scope (_SB)
    {
        Device (LID)
        {
            Name (_HID, EisaId ("PNP0C0D") /* Lid Device */)  // _HID: Hardware ID
            Method (_LID, 0, NotSerialized)  // _LID: Lid Status
            {
                Local0 = One
                Local0 = ^^PCI0.LPCB.EC0.RPIN (0x05, 0x06)
                If ((Local0 == 0x55))
                {
                    Local0 = Zero
                }
                Else
                {
                    Local0 = One
                }

                Return (Local0)
            }
        }

        Device (PNLF)
        {
            Name (_HID, EisaId ("APP0002"))  // _HID: Hardware ID
            Name (_CID, "backlight")  // _CID: Compatible ID
            Name (_UID, 0x0A)  // _UID: Unique ID
            Name (_STA, 0x0B)  // _STA: Status
        }

        Device (PWRB)
        {
            Name (_HID, EisaId ("PNP0C0C") /* Power Button Device */)  // _HID: Hardware ID
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0B)
            }
        }
    }

    Scope (_SB.PCI0.LPCB.EC0)
    {
        Method (_Q81, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
        {
            // main change, only notifies if state changes rather then whenever queried
            Local1 = ^^^IGPU.CLID /* External reference */
            Local0 = ^^^^LID._LID ()
            If ((Local0 == Zero))
            {
                ADBG ("LID-OFF")
                SGOV (0x02030009, Zero)
                SGOV (0x02060000, Zero)
            }
            Else
            {
                ADBG ("LID-ON")
                SGOV (0x02030009, One)
                SGOV (0x02060000, One)
                Notify (ALSD, 0x80) // Status Change
            }

            If ((Local1 != Local0))
            {
                Notify (LID, 0x80) // Status Change
                ^^^IGPU.CLID = Local0
            }
        }
    }
``

I can confirm the very same happening to me, with a different OC setup. Anyone on Clover experiencing the same?

Also, could you (@hatf0) maybe create a SSDT out of this? I have tried to find some documentation but there isn't much.

[kvn1351]

Ok so I have read the relevant parts of the ACPI documentation and the one from Intel and I don't really understand why you've done this bit here:
If ((Local1 != Local0)) { Notify (LID, 0x80) // Status Change ^^^IGPU.CLID = Local0 }

CLID and _LID aren't returning the same type of data. CLID returns one of four different states while _LID only indicates if the lid is opened or closed with either 0 or 1.

Hence, as far as I understand it, while it may be working, isn't performing the correct logic.

However, I know almost nothing at all about this so maybe I'm completely wrong. @gnodipac886 any ideas?

Edit: Like, to notify only on a state change; why not remove the entire last block (the if statement from I've mentioned above)?

[ellie-idb]

@kvn1351 Oops. I forgot what a variable was for a hot second, and I was using CLID as a pseudo-one. I've updated the patch above to reflect that.

[ellie-idb]

I tried to make it into a SSDT, try this.

DefinitionBlock ("", "SSDT", 2, "hack", "_LID", 0x0)
{
    
    External (ADBG, MethodObj)
    External (SGOV, MethodObj)
    External (ALSD, DeviceObj)
    External (_SB.PCI0.LPCB.EC0, DeviceObj)
    External (_SB.PCI0.LPCB.EC0.RPIN, MethodObj) 
    External (_SB.PCI0.IGPU.CLID, IntObj)
      
    Scope (_SB) {
    Device (LID)
    {
            Name (_OLD, One) // assuming everything else.. the lid should start open?
            Name (_HID, EisaId ("PNP0C0D") /* Lid Device */)  // _HID: Hardware ID
            Method (_LID, 0, NotSerialized)  // _LID: Lid Status
            {
                Local0 = One
                Local0 = ^^PCI0.LPCB.EC0.RPIN (0x05, 0x06)
                If ((Local0 == 0x55))
                {
                    Local0 = Zero
                }
                Else
                {
                    Local0 = One
                }
                
                ^^PCI0.IGPU.CLID = Local0
                Return (Local0)
            }
    }

        Device (PNLF)
        {
            Name (_HID, EisaId ("APP0002"))  // _HID: Hardware ID
            Name (_CID, "backlight")  // _CID: Compatible ID
            Name (_UID, 0x0A)  // _UID: Unique ID
            Name (_STA, 0x0B)  // _STA: Status
        }

        Device (PWRB)
        {
            Name (_HID, EisaId ("PNP0C0C") /* Power Button Device */)  // _HID: Hardware ID
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0B)
            }
        }
    }

    Scope (_SB.PCI0.LPCB.EC0)
    {
        Method (_Q81, 0, NotSerialized) 
        {
                        Local0 = ^^^^LID._LID ()
            If ((Local0 == Zero))
            {
                ADBG ("LID-OFF")
                SGOV (0x02030009, Zero)
                SGOV (0x02060000, Zero)
            }
            Else
            {
                ADBG ("LID-ON")
                SGOV (0x02030009, One)
                SGOV (0x02060000, One)
                Notify (ALSD, 0x80) // Status Change
            }

            If ((^^^^LID._OLD != Local0))
            {
                Notify (LID, 0x80) // Status Change
                ^^^^LID._OLD = Local0
            }
        }
    }
}

[kvn1351]

So, once again I have to state that I'm a complete noob when it comes to this but according to my current understanding of ACPI, the following ssdt would yield collisions because the method _Q81 and the device LID are already defined.

I think you'd have to clover rename _Q81 to something else like XQ81. That would make room in the namespace for _Q81 to be injected.

Now, I don't have the original dsl at hand, but the inclusion of CLID is your idea right? If so, according to the intel spec, I don't see what benefit that would bring us.

Also, I don't understand the use of _OLD, because it would always be equal to 1. Thus the if statement at the very end would always be 1 != Local0. And if this works for you, it means that when Local0 isn't equal to 1 it should Notify, else it shouldn't. But that's already defined in the conditional statement right above it.

So, all that has to be done here, if I'm reasoning correctly, is to remove the last if statement and nothing else. Cause if I remember correctly, Q81 notified at the end of the conditionals right? So we just need to remove that Notify.

Here's my untested ssdt, could someone please check? I don't have access to my Matebook for the rest of the week.

// Untested, may not work. Based on lacking knowledge of ACPI
DefinitionBlock ("", "SSDT", 2, "hack", "_LID", 0x0)
{
    External (ADBG, MethodObj)
    External (SGOV, MethodObj)
    External (ALSD, DeviceObj)
    External (_SB.PCI0.LPCB.EC0, DeviceObj)
    External (_SB.LID, DeviceObj)
    External (_SB.LID._LID, MethodObj)

    Scope (_SB.PCI0.LPCB.EC0)
    {
        //Requires clover patch "_Q81 to _XQ81"
        Method (_Q81, 0, NotSerialized) 
        {
            Local0 = \_SB.LID._LID ()
            If ((Local0 == Zero))
            {
                ADBG ("LID-OFF")
                SGOV (0x02030009, Zero)
                SGOV (0x02060000, Zero)
            }
            Else
            {
                ADBG ("LID-ON")
                SGOV (0x02030009, One)
                SGOV (0x02060000, One)
                Notify (ALSD, 0x80) // Status Change
            }
        }
    }
}

The opencore rename would be:
find 5f513831
replace 58513831

The clover rename would be:
find X1E4MQ==
replace WFE4MQ==

[kvn1351]

Also, I can confirm that the very same logs appear with Clover as I tested yesterday. But I didn't notice any CPU spiked as you mentioned. I didn't perform any extensive testing though.

[ellie-idb]

_XQ81 would most likely to be renamed, I did not think of that.

As well, the _OLD would not remain One. By calling _LID, it queries the GPIO state of the hall effect sensor (RPIN) every time the EC is queried (don't exactly know when macOS does that), and notify only if the sensor state is changed. The main reason for the spam- at least, from what I can tell, are the continual Notify calls. If we're able to throttle it and reduce the Notify calls to happen whenever the state changes, this should somewhat help with the spam. The purpose of _OLD is to save the previous state.

[kvn1351]

_XQ81 would most likely to be renamed, I did not think of that.

As well, the _OLD would not remain One. By calling _LID, it queries the GPIO state of the hall effect sensor (RPIN) every time the EC is queried (don't exactly know when macOS does that), and notify only if the sensor state is changed. The main reason for the spam- at least, from what I can tell, are the continual Notify calls. If we're able to throttle it and reduce the Notify calls to happen whenever the state changes, this should somewhat help with the spam. The purpose of _OLD is to save the previous state.

Oh, I see, I didn't notice the _OLD assignment in the last conditional. Also, it has to be XQ81 in order to match the number of bytes to replace. My replacements are correct but I forgot to remove the _ from my comment. I've edited that.

What I still don't understand though is why _OLD has to be in LID.

[kvn1351]

DefinitionBlock ("", "SSDT", 2, "hack", "_LID", 0x00000000)
{
   External (_SB_.LID_, DeviceObj)
   External (_SB_.LID_._LID, MethodObj)    // 0 Arguments
   External (_SB_.PCI0.LPCB.EC0_, DeviceObj)
   External (ADBG, MethodObj)    // 1 Arguments
   External (ALSD, DeviceObj)
   External (SGOV, MethodObj)    // 2 Arguments

   Scope (_SB.PCI0.LPCB.EC0)
   {
       Name (_OLD, One)
       Method (_Q81, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
       {
           Local0 = \_SB.LID._LID ()
           If ((Local0 == Zero))
           {
               ADBG ("LID-OFF")
               SGOV (0x02030009, Zero)
               SGOV (0x02060000, Zero)
           }
           Else
           {
               ADBG ("LID-ON")
               SGOV (0x02030009, One)
               SGOV (0x02060000, One)
               Notify (ALSD, 0x80) // Status Change
           }

           If ((_OLD != Local0))
           {
               Notify (LID, 0x80) // Status Change
               _OLD = Local0
           }
       }
   }
}

find 5f513831
replace 58513831
OR
find X1E4MQ==
replace WFE4MQ==

@hatf0 like this perhaps?

[kvn1351]

The issue still persists with your dsdt mod and even by patching out Q81 completely. I have now tied it down to being VirtualSMC causing the issue - or enabling it.

With FakeSMC the issue is gone. This isn't a solution though. @gnodipac886 Any ideas?

[mircoianese]

The issue still persists with your dsdt mod and even by patching out Q81 completely. I have now tied it down to being VirtualSMC causing the issue - or enabling it.

With FakeSMC the issue is gone. This isn't a solution though. @gnodipac886 Any ideas?

I'm randomly experiencing the same issue with 100%+ CPU usage... did you come to a conclusion? I switched to FakeSMC also at the moment... but lost the Ambient Light Sensor (any way to get it back?).
Thanks