On Sun, 19 Feb 2023 23:17:37 +0100
Lukas Wunner <lukas@xxxxxxxxx> wrote:
> On Sun, Feb 19, 2023 at 12:20:05AM +1100, Orlando Chamberlain wrote:
> > This is needed for interrupts to be cleared correctly on MMIO based
> > gmux's. It is untested if this helps/hinders other gmux types, so
> > currently this is only enabled for the MMIO gmux's.
> >
> > There is also a "GMLV" acpi method, and the "GMSP" method can be
> > called with 1 as its argument, but the purposes of these aren't
> > known and they don't seem to be needed.
>
> GMLV and GMSP access a GPIO on the PCH which is connected to the
> GMUX_INT pin of the gmux microcontroller. I've just verified that
> in the schematics of my MBP9,1.
>
> GMLV reads the value of the GPIO ("level").
> GMSP likely sets the value ("set polarity").
>
> On my MBP9,1 (indexed gmux), if the gmux controller signals an
> interrupt, the platform signals a notification:
>
> Scope (\_GPE)
> {
> Method (_L16, 0, NotSerialized) // _Lxx: Level-Triggered GPE
> {
> Notify (\_SB.PCI0.LPCB.GMUX, 0x80) // Status Change
> }
> }
>
> Comparing this to the MBP13,3 and MBP16,1, the GPE method
> differentiates between the OS type: On Darwin, only a notification
> is signaled, whereas on other OSes, the GPIO's value is read and then
> inverted:
>
> Scope (\_GPE)
> {
> Method (_L15, 0, NotSerialized) // _Lxx: Level-Triggered GPE,
> xx=0x00-0xFF {
> If (OSDW ())
> {
> Notify (\_SB.PCI0.LPCB.GPUC, 0x80) // Status Change
> }
> ElseIf ((\_SB.GGII (0x03000015) == One))
> {
> \_SB.SGII (0x03000015, Zero)
> }
> Else
> {
> \_SB.SGII (0x03000015, One)
> }
> }
> }
>
> Linux masquerades as Darwin, so ends up in the notification-only
> code path.
>
> Does macOS execute the GMSP method as well? Have you disassembled
> the gmux driver? All vital information that belongs in the commit
> message and/or a code comment.
I think it does, if certain based bits in "HWFeatureMask" (which shows
up in `ioreg -l`) are set, but I'm not very good at RE so I don't know
exactly how macOS uses it. The kext is AppleMuxControl2.kext.
>
>
> > +static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data,
> > int arg) +{
> > + acpi_status status = AE_OK;
> > + union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > + struct acpi_object_list arg_list = { 1, &arg0 };
> > +
> > + arg0.integer.value = arg;
> > +
> > + status = acpi_evaluate_object(gmux_data->dhandle, "GMSP",
> > &arg_list, NULL);
>
> Can this be simplified by using acpi_execute_simple_method() or
> one of the other helpers provided by drivers/acpi/utils.c?
>
Yes it can thanks!
>
> > @@ -537,6 +561,8 @@ static void gmux_clear_interrupts(struct
> > apple_gmux_data *gmux_data) /* to clear interrupts write back
> > current status */ status = gmux_interrupt_get_status(gmux_data);
> > gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
> > + if (gmux_data->config->use_acpi_gmsp)
> > + gmux_call_acpi_gmsp(gmux_data, 0);
> > }
>
> I think it would be clearer to check the gmux type directly here,
> so that a casual reader understands that invoking the method is
> necessary on MMIO-accessed GMUXes, but not any of the other types.
> By contrast, with the use_acpi_gmsp one has to look up first which
> of the gmux types sets this to true.
I can do it like that next version.
>
> What happens if GMSP is not executed? Needs to be documented in the
> commit message and/or a code comment!
>
It gets a flood of status=0 interrupts, I'll add that as a comment.
> Thanks,
>
> Lukas
