[proposal] EC-mainboard interaction at ASL level

[mrnuke]

This is an ugly one, and we keep having to find different workarounds to make 
this happen. We have the preprocessor, so why not use it?

Define a set of common ACPI method names which the mainboard code should 
define, and the EC code can always use.

* MB_TOGGLE_WLAN() or MB_TOGGLE_WIRELESS()
  Toggle wireless LAN on and off, or wireless LAN and bluetooth
  (respectively). EC calls this on hotkey events.

* MB_INCREASE_BRIGHTNESS() and MB_DECREASE_BRIGHTNESS()
  Increase or decrease screen brightness. EC calls this on hotkey events.

* MB_SWITCH_DISPLAY()
  Switch the active display. EC calls this on hotkey events.

* MB_NOTIFY_POWER_EVENT()
  Handle power state notifications and notify CPU device objects to re-
  evaluate their _PPC and _CST tables.

Of course, we would have the mainboard #define these to an existing method, we 
wouldn't really use these long method names in ASL. Another idea would be to 
standardize on four letter names, but those are not as clear, and hide the 
"MB_" which is there to indicate that the mainboard should provide those.

Notice that these methods are only called on hotkey events. As such, unless 
the user has really fast fingers, there isn't a huge ACPI overhead as opposed 
to setting/clearing the needed bits directly in the caller.

We then extend this to also include ACPI objects for different purposes.

* MB_LID_STATE
  Register/GPIO bit which reads the state of the lid

And we can also define a set of ACPI variables which should always be present, 
for the purpose of ACPI housekeeping.

* LIDS
  Stores the lid state, so that we don't have to read it every time. 
  (remember, reading may involve several port IO operations)
* PWRS
  AC adapter present/not present.

It seems a little convoluted, so if you didn't ACPI before, you'll WTF on this 
one. The end result is that it unifies the interface rather than letting each 
EC define its own interface. It also (hopefully) makes ACPI just a little more 
readable by having a standardized set of pork.

If there aren't any major objections, I'll write this up on the wiki, and 
we'll open the gates to refactoring. I'm interested if there are cases where 
this *cough* API *cough* is not appropriate. It seems to be universally 
efficient.

Alex

[Peter Stuge]

mrnuke wrote:
> Define a set of common ACPI method names which the mainboard code should 
> define, and the EC code can always use.

Thanks for starting this! It is exactly what is needed.
> MB_TOGGLE_WLAN() or MB_TOGGLE_WIRELESS()
> MB_INCREASE_BRIGHTNESS() and MB_DECREASE_BRIGHTNESS()
> MB_SWITCH_DISPLAY()
> MB_NOTIFY_POWER_EVENT()

I additionally have lock, battery, sleep, network, eject and suspend
hotkeys.
> MB_LID_STATE
> LIDS
> PWRS

Battery state(s) too?
> It seems a little convoluted,

Maybe, but it's exactly the right abstraction. Please do go ahead.
> If there aren't any major objections, I'll write this up on the wiki,
> and we'll open the gates to refactoring.

As for the documentation, maybe we should put that in the tree instead?

Thanks

//Peter

[Duncan Laurie]

On Tue, Apr 8, 2014 at 2:08 AM, Peter Stuge  wrote:
> mrnuke wrote:
> > Define a set of common ACPI method names which the mainboard code should
> > define, and the EC code can always use.
>
> Thanks for starting this! It is exactly what is needed.
> > MB_TOGGLE_WLAN() or MB_TOGGLE_WIRELESS()
> > MB_INCREASE_BRIGHTNESS() and MB_DECREASE_BRIGHTNESS()
> > MB_SWITCH_DISPLAY()
> > MB_NOTIFY_POWER_EVENT()
>
> I additionally have lock, battery, sleep, network, eject and suspend
> hotkeys.

There are so many special cases with EC that it could be useful to
have a generic mainboard "EC event" method that any EC _Qxx could call
and pass the event ID to.

The event could be the Qxx number itself (for all the special cases)
or a standard defined event ID like those listed above which could be
> 255 so as not to conflict with possible Qxx IDs.  This limits the

permutations of mainboard EC event handlers and gives the flexibility
to pass all the various EC events to the mainboard for handling.

#define EC_EVENT_LID_CLOSED 0x100

External (_SB.MBEC)

// lid closed
Method (_Q01) {
  If (CondRefOf (_SB.MBEC)) {
    _SB.MBEC (EC_EVENT_LID_CLOSED)
  }
}

// special button handler
Method (_Q14) {
  If (CondRefOf (_SB.MBEC)) {
    _SB.MBEC (0x14)
  }
}

etc..

In the mainboard:

Scope (_SB)
{
  // mainboard event handler
  Method (MBEC, 1, Serialized) {
    Switch (ToInteger (Arg0)) {
      Case (EC_EVENT_LID_CLOSED) { ... }
      Case (0x14) { ... }
    }
  }
}

If we do go the route of having lots of mainboard handlers I think we
should keep the method names consistent with the ACPI spec and use a
mainboard namespace to separate them.  Conditional references are
already a headache in ASL and I think relying on the preprocessor
would make things worse.

Scope (_SB.MB)
  // Lid open
  Method (LIDO) { ... }
  // Lid closed
  Method (LIDC) { ... }
  // Increase brightness
  Method (BRTI) { ... }
}

But even in this form there needs to be a generic method for all the
special cases...
> > MB_LID_STATE
> > LIDS
> > PWRS
>
> Battery state(s) too?

These are actually problematic shadow variables, especially around
suspend/resume.

In order to update the shadow variable state on resume we typically
have the _WAK handler update them from the EC (or a GPIO) but the _WAK
handler is actually executed by the OS asynchronously so there is no
guarantee that the shadow variables are correct for any early methods
(or Notify() handlers) that are executed.

This has bitten me a number of times because it does not always show
up (there are not many dependencies currently) and depending on the
kernel behavior it can be hard to reproduce.  Reading this state
directly from the EC may require a few IO operations, but when dealing
with EC state that can affect the OS it is more important to be
correct than to save a few IO cycles.  (especially since the OS is
already dealing with the overhead of parsing and executing AML)

Rather than defining shadow variables in the NVS it would be safer to
have a (mainboard defined) method that can go and read the state
directly.  This is especially useful if the EC is not the source of
the data -- for example in some mainboards we have a direct gpio for
some state that is typically read from the EC.  This provides the
abstraction of system state without the concerns of stale data.

Method (PWRS) { Return (_SB.PCI.LPC.EC.ACEX) }
Method (LIDS) { Return (_SB.PCI.LPC.EC.LIDS) }

or like Parrot:
Method (LIDS) { Return (GP15) } // GPIO15

-duncan