ideapad_laptop tablet mode toggle detection

ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

Hello, I'm interested in detecting tablet mode switching for my Lenovo
Yoga laptop properly in the kernel. Ultimately I'd like to provide the
SW_TABLET_MODE input event.

I have found that there are ACPI events fired when the tablet mode is
toggled (both directions send the same event). Here are the logs:

kernel: ACPI BIOS Error (bug): Could not resolve symbol
[\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND (20221020/psargs-330)
kernel: ACPI Error: Aborting method \_SB.PC00.LPCB.EC0._Q44 due to
previous error (AE_NOT_FOUND) (20221020/psparse-529)
root[10258]: ACPI group/action undefined: 06129D99-6083- / 000000d0

When I looked at the code I found TP_HKEY_EV_TABLET_CHANGED in the
thinkpad_acpi.c driver which seems to have exactly the behavior I'm
looking for but with a different ACPI event number. Would it be
possible to do something like this in the ideapad_laptop driver?

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Tue, Feb 28, 2023 at 8:45 PM Andrew Kallmeyer <kallmeyeras@gmail.com> wrote:
>
> Hello, I'm interested in detecting tablet mode switching for my Lenovo
> Yoga laptop properly in the kernel. Ultimately I'd like to provide the
> SW_TABLET_MODE input event.
>
> I have found that there are ACPI events fired when the tablet mode is
> toggled (both directions send the same event). Here are the logs:
>
> kernel: ACPI BIOS Error (bug): Could not resolve symbol
> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND (20221020/psargs-330)
> kernel: ACPI Error: Aborting method \_SB.PC00.LPCB.EC0._Q44 due to
> previous error (AE_NOT_FOUND) (20221020/psparse-529)
> root[10258]: ACPI group/action undefined: 06129D99-6083- / 000000d0
>
> When I looked at the code I found TP_HKEY_EV_TABLET_CHANGED in the
> thinkpad_acpi.c driver which seems to have exactly the behavior I'm
> looking for but with a different ACPI event number. Would it be
> possible to do something like this in the ideapad_laptop driver?

I've made some progress! After some very helpful pointers about ACPI
from Maximilian Luz I was able to create a small module printks on screen
flip events! It finds the _SB.PC00.LPCB.EC0.VPC0 acpi_handle and calls
acpi_install_notify_handler on it and receives the events.

I'm still wondering about that AE_NOT_FOUND error about the WM00 device
though. It seems that the ACPI expects the kernel to define this WM00
device as an extension point. In the DSDT I found:

External (_SB_.WM00, DeviceObj)
...
Scope (\_SB.PC00.LPCB.EC0)
{

    Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
    {
       P80B = 0x44
       Notify (VPC0, 0x80) // Status Change
       WEID = 0xF4
       Notify (WM00, 0x80) // Status Change
    }
...
    Device (VPC0)
    {
        Name (_HID, "VPC2004")  // _HID: Hardware ID
        Name (_UID, Zero)  // _UID: Unique ID
        Name (_VPC, 0x7C0DE114)
        Name (VPCD, Zero)


Additionally two other _Qxx methods (11 and 12) Notify the same
two devices. These other two are the brightness up and down keys.
The only difference is they set those two variables differently so I'll
have to read those variables as well to differentiate screen flips.
Right now my module actually detects both screen flips and
brightness key presses.

I haven't been able to figure out how to create the WM00 device,
I'm not even sure that's a thing in ACPI. I also haven't seen how
to read those variables. Is it okay to reuse the events sent to this
VPC0 device or am I intercepting the events from some other
functionality? Any pointers would be greatly appreciated.

- Andrew Kallmeyer

Re: ideapad_laptop tablet mode toggle detection

[Maximilian Luz]

Hi,

On 3/4/23 06:46, Andrew Kallmeyer wrote:
> I'm still wondering about that AE_NOT_FOUND error about the WM00 device> though. It seems that the ACPI expects the kernel to define this WM00> device as an extension point. In the DSDT I found:> > External (_SB_.WM00, DeviceObj)
AFAIK this just means that it's external to this table (i.e. the DSDT I
assume), not that the kernel needs to define it (I'm not sure if that's even an
option in the ACPI spec or ACPICA). So it should be in some SSDT or the ACPI
implementation is broken (unless defining devices from the kernel is really an
option and I'm just misinformed).

> Scope (\_SB.PC00.LPCB.EC0)
> {
> 
>      Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
>      {
>         P80B = 0x44
>         Notify (VPC0, 0x80) // Status Change
>         WEID = 0xF4
>         Notify (WM00, 0x80) // Status Change
>      }
> ...
>      Device (VPC0)
>      {
>          Name (_HID, "VPC2004")  // _HID: Hardware ID
>          Name (_UID, Zero)  // _UID: Unique ID
>          Name (_VPC, 0x7C0DE114)
>          Name (VPCD, Zero)
> 
> 
> Additionally two other _Qxx methods (11 and 12) Notify the same
> two devices. These other two are the brightness up and down keys.
> The only difference is they set those two variables differently so I'll
> have to read those variables as well to differentiate screen flips.
> Right now my module actually detects both screen flips and
> brightness key presses.

I believe that makes sense, given the ACPI code.

> I haven't been able to figure out how to create the WM00 device,
> I'm not even sure that's a thing in ACPI. I also haven't seen how
> to read those variables.

You can use acpi_evaluate_object() and acpi_evaluate_object_typed() for that.

> Is it okay to reuse the events sent to this
> VPC0 device or am I intercepting the events from some other
> functionality? Any pointers would be greatly appreciated.

I guess that depends on the VPC0 device. If it doesn't have a driver already
(which you can check by getting the HID of that device from the DSDT and
grep-ing for it in the kernel source), you can write your own driver against
it, install the notify-handler, and do basically whatever you want. You're not
intercepting/blocking anything by that. If there already is a driver, you'll
have to check what that does and if you can integrate your functionality there.

Given it's a Lenovo device and there are some drivers here, maybe it's also
some know interface/structure, but I guess Hans would know more about that than
I do.

Best regards,
Max

Re: ideapad_laptop tablet mode toggle detection

[Armin Wolf]

Am 04.03.23 um 21:14 schrieb Maximilian Luz:

> Hi,
>
> On 3/4/23 06:46, Andrew Kallmeyer wrote:
>> I'm still wondering about that AE_NOT_FOUND error about the WM00
>> device> though. It seems that the ACPI expects the kernel to define
>> this WM00> device as an extension point. In the DSDT I found:> >
>> External (_SB_.WM00, DeviceObj)
> AFAIK this just means that it's external to this table (i.e. the DSDT I
> assume), not that the kernel needs to define it (I'm not sure if
> that's even an
> option in the ACPI spec or ACPICA). So it should be in some SSDT or
> the ACPI
> implementation is broken (unless defining devices from the kernel is
> really an
> option and I'm just misinformed).
>
>> Scope (\_SB.PC00.LPCB.EC0)
>> {
>>
>>      Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
>>      {
>>         P80B = 0x44
>>         Notify (VPC0, 0x80) // Status Change
>>         WEID = 0xF4
>>         Notify (WM00, 0x80) // Status Change
>>      }
>> ...
>>      Device (VPC0)
>>      {
>>          Name (_HID, "VPC2004")  // _HID: Hardware ID
>>          Name (_UID, Zero)  // _UID: Unique ID
>>          Name (_VPC, 0x7C0DE114)
>>          Name (VPCD, Zero)
>>
>>
>> Additionally two other _Qxx methods (11 and 12) Notify the same
>> two devices. These other two are the brightness up and down keys.
>> The only difference is they set those two variables differently so I'll
>> have to read those variables as well to differentiate screen flips.
>> Right now my module actually detects both screen flips and
>> brightness key presses.
>
> I believe that makes sense, given the ACPI code.
>
>> I haven't been able to figure out how to create the WM00 device,
>> I'm not even sure that's a thing in ACPI. I also haven't seen how
>> to read those variables.
>
> You can use acpi_evaluate_object() and acpi_evaluate_object_typed()
> for that.
>
>> Is it okay to reuse the events sent to this
>> VPC0 device or am I intercepting the events from some other
>> functionality? Any pointers would be greatly appreciated.
>
> I guess that depends on the VPC0 device. If it doesn't have a driver
> already
> (which you can check by getting the HID of that device from the DSDT and
> grep-ing for it in the kernel source), you can write your own driver
> against
> it, install the notify-handler, and do basically whatever you want.
> You're not
> intercepting/blocking anything by that. If there already is a driver,
> you'll
> have to check what that does and if you can integrate your
> functionality there.
>
> Given it's a Lenovo device and there are some drivers here, maybe it's
> also
> some know interface/structure, but I guess Hans would know more about
> that than
> I do.
>
> Best regards,
> Max

Hi,

it seems that the VPC0 ACPI device is handled by the already mentioned ideapad-laptop driver,
you probably just have to add the necessary event codes to ideapad_keymap[].

Armin Wolf

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Sat, Mar 4, 2023 at 1:37 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Hi,
>
> it seems that the VPC0 ACPI device is handled by the already mentioned ideapad-laptop driver,
> you probably just have to add the necessary event codes to ideapad_keymap[].
>
> Armin Wolf
>

I read into the ideapad-laptop code some more and found that the
ideapad_acpi_notify function ends up being the exact same handler that
I am installing in my test module. I copied over the read_ec_data code
to read the same variable and found that the screen flip event I'm
looking at is the bit 5 case which simply calls
ideapad_sync_touchpad_state (which shows up on screen in my GNOME
install). It seems this is not quite the P80B = 0x44 variable because
the only bit 5 is set (possibly it's just the first 4?) but it looks
to be enough to detect the screen flip.

So it seems like I would want to add something to send the
SW_TABLET_MODE input event there in that bit 5 case. Presumably this
would be something like the tpacpi_input_send_tabletsw and
hotkey_tablet_mode_notify_change function calls that are done in the
thinkpad-acpi driver. Would that be a reasonable thing to add to the
ideapad driver? I'm not sure how we would know about compatibility
across the other ideapad laptops. It seems I could at least get this
all working with my own module for myself for now at least (maybe a
patch of the ideapad driver because reading that variable in my module
prevents the original driver from reading it).

I did also notice that bit 10 has a comment about a tablet mode
switching signal being unreliable but on my machine bit 10 was not set
for the tablet mode switch event when I checked with my test module.

Thanks,
Andrew Kallmeyer

Re: ideapad_laptop tablet mode toggle detection

[Armin Wolf]

Am 05.03.23 um 06:42 schrieb Andrew Kallmeyer:

> On Sat, Mar 4, 2023 at 1:37 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> Hi,
>>
>> it seems that the VPC0 ACPI device is handled by the already mentioned ideapad-laptop driver,
>> you probably just have to add the necessary event codes to ideapad_keymap[].
>>
>> Armin Wolf
>>
> I read into the ideapad-laptop code some more and found that the
> ideapad_acpi_notify function ends up being the exact same handler that
> I am installing in my test module. I copied over the read_ec_data code
> to read the same variable and found that the screen flip event I'm
> looking at is the bit 5 case which simply calls
> ideapad_sync_touchpad_state (which shows up on screen in my GNOME
> install). It seems this is not quite the P80B = 0x44 variable because
> the only bit 5 is set (possibly it's just the first 4?) but it looks
> to be enough to detect the screen flip.
>
> So it seems like I would want to add something to send the
> SW_TABLET_MODE input event there in that bit 5 case. Presumably this
> would be something like the tpacpi_input_send_tabletsw and
> hotkey_tablet_mode_notify_change function calls that are done in the
> thinkpad-acpi driver. Would that be a reasonable thing to add to the
> ideapad driver? I'm not sure how we would know about compatibility
> across the other ideapad laptops. It seems I could at least get this
> all working with my own module for myself for now at least (maybe a
> patch of the ideapad driver because reading that variable in my module
> prevents the original driver from reading it).
>
> I did also notice that bit 10 has a comment about a tablet mode
> switching signal being unreliable but on my machine bit 10 was not set
> for the tablet mode switch event when I checked with my test module.
>
> Thanks,
> Andrew Kallmeyer

Hi,

could it be that bit 5 is set to disable the touchpad when the device switches
to tablet mode? I suspect that the query handler does the following:
1. Notify VPC0 to disable the touchpad.
2. Notify ACPI WMI, which does submit the necessary scancode for switching to tablet mode.

Could you provide the output of "acpidump"? Because i suspect that the virtual key handling
is done using ACPI WMI, as many modern devices are using this approach. In this case, you
could experiment with ideapad_wmi_notify(), and maybe take a look at ideapad_wmi_ids[].

Armin Wolf

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Hi,
>
> could it be that bit 5 is set to disable the touchpad when the device switches
> to tablet mode? I suspect that the query handler does the following:
> 1. Notify VPC0 to disable the touchpad.
> 2. Notify ACPI WMI, which does submit the necessary scancode for switching to tablet mode.

I think you're right about this notification being for the touchpad,
although at least on my machine
there is no other touchpad switch. So this is identical for my machine
specifically. In this function
from the decompiled ACPI dump you can see VCP0 and WM00 notified:

Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
{
   P80B = 0x44
   Notify (VPC0, 0x80) // Status Change
   WEID = 0xF4
   Notify (WM00, 0x80) // Status Change
}

This WM00 device sounds like the WMI you're talking about, however I'm
getting those errors
about this device not existing in journalctl still. I was asking
before about how to create this
missing device but it's not clear to me if that is possible.

kernel: ACPI BIOS Error (bug): Could not resolve symbol
[\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND

I searched in my .dsl files from the acpidump and wasn't able to find
any of the 3 ideapad_wmi_ids
listed in the driver. Maybe you have an idea of how to interface with
this missing WM00 object though.

>
> Could you provide the output of "acpidump"? Because i suspect that the virtual key handling
> is done using ACPI WMI, as many modern devices are using this approach. In this case, you
> could experiment with ideapad_wmi_notify(), and maybe take a look at ideapad_wmi_ids[].
>
> Armin Wolf

Here is the raw acpidump output: https://la.ask.systems/temp/acpidump.out

- Andrew

Re: ideapad_laptop tablet mode toggle detection

[Armin Wolf]

Am 05.03.23 um 23:59 schrieb Andrew Kallmeyer:

> On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> Hi,
>>
>> could it be that bit 5 is set to disable the touchpad when the device switches
>> to tablet mode? I suspect that the query handler does the following:
>> 1. Notify VPC0 to disable the touchpad.
>> 2. Notify ACPI WMI, which does submit the necessary scancode for switching to tablet mode.
> I think you're right about this notification being for the touchpad,
> although at least on my machine
> there is no other touchpad switch. So this is identical for my machine
> specifically. In this function
> from the decompiled ACPI dump you can see VCP0 and WM00 notified:
>
> Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
> {
>     P80B = 0x44
>     Notify (VPC0, 0x80) // Status Change
>     WEID = 0xF4
>     Notify (WM00, 0x80) // Status Change
> }
>
> This WM00 device sounds like the WMI you're talking about, however I'm
> getting those errors
> about this device not existing in journalctl still. I was asking
> before about how to create this
> missing device but it's not clear to me if that is possible.
>
> kernel: ACPI BIOS Error (bug): Could not resolve symbol
> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND
>
> I searched in my .dsl files from the acpidump and wasn't able to find
> any of the 3 ideapad_wmi_ids
> listed in the driver. Maybe you have an idea of how to interface with
> this missing WM00 object though.

I was combing through the ACPI DSDT table inside the acpidump you provided,
and i found serveral PNP0C14 devices, which hold WMI methods, events and data.

The WMI GUIDs are encoded inside the associated _WDG buffers, you should therefore
only grep for parts of the GUIDs.
For example: GUID 06129D99-6083-4164-81AD-F092F9D773A6 -> grep "0xF0, 0x92"

When feeding the content of the buffers named WQxx to the bmfdec utility, i was able
to extract a description of each WMI object.

On of which (WMIY) is handling the "Lenovo Yoga Mode", which seems to handle the tablet
mode transitions. The MOF data is:

[WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), Description("Lenovo Yoga Mode Change Event"), guid("{06129D99-6083-4164-81AD-F092F9D773A6}")]
class LENOVO_GSENSOR_EVENT : WMIEvent {
   [key, read] string InstanceName;
   [read] boolean Active;
   [WmiDataId(1), read, Description("Lenovo Yoga Mode Change Event")] uint32 ModeDataVal;
};

[WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), Description("LENOVO_GSENSOR_DATA class"), guid("{09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C}")]
class LENOVO_GSENSOR_DATA {
   [key, read] string InstanceName;
   [read] boolean Active;

   [WmiMethodId(1), Implemented, Description("Mode Data")] void GetUsageMode([out, Description("Mode Data")] uint32 Data);
   [WmiMethodId(2), Implemented, Description("Get Xaxis Value")] void GetXaxisValue([out, Description("Get Xaxis Value")] uint32 Data);
   [WmiMethodId(3), Implemented, Description("Get Yaxis Value")] void GetYaxisValue([out, Description("Get Yaxis Value")] uint32 Data);
   [WmiMethodId(4), Implemented, Description("Get Zaxis Value")] void GetZaxisValue([out, Description("Get Zaxis Value")] uint32 Data);
   [WmiMethodId(5), Implemented, Description("Base to Ground")] void GetAngle4Value([out, Description("Base to Ground")] uint32 Data);
   [WmiMethodId(6), Implemented, Description("Screen to Ground")] void GetAngle5Value([out, Description("Screen to Ground")] uint32 Data);
   [WmiMethodId(7), Implemented, Description("Screen to Base")] void GetAngle6Value([out, Description("Screen to Base")] uint32 Data);
};

While looking at _WED (used to get ModeDataVal) and WMAB (handles method calls on LENOVO_GSENSOR_DATA),
i assume that  only the first method (GetUsageMode) is implemented, and that ModeDataVal is used to tell
which value of LENOVO_GSENSOR_DATA has changed (hardwired to 1 on your device).

Maybe you can write a wmi driver which handles both WMI objects, so that you can find out what the values
returned by GetUsageMode mean. With a bit of luck, you can use this to implement tablet mode toggle detection.

BTW, what is the name of your notebook model?

Armin Wolf

>> Could you provide the output of "acpidump"? Because i suspect that the virtual key handling
>> is done using ACPI WMI, as many modern devices are using this approach. In this case, you
>> could experiment with ideapad_wmi_notify(), and maybe take a look at ideapad_wmi_ids[].
>>
>> Armin Wolf
> Here is the raw acpidump output: https://la.ask.systems/temp/acpidump.out
>
> - Andrew

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Sun, Mar 5, 2023 at 5:26 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> I was combing through the ACPI DSDT table inside the acpidump you provided,
> and i found serveral PNP0C14 devices, which hold WMI methods, events and data.

Thanks so much for the help! This is my first time looking at an ACPI
dump so I never would have found this.

> Maybe you can write a wmi driver which handles both WMI objects, so that you can find out what the values
> returned by GetUsageMode mean. With a bit of luck, you can use this to implement tablet mode toggle detection.

I will look into this, this does look more like the proper way to do
it so it can be put in the driver for everyone. I suppose that missing
Notify device is just unrelated then? Does setting the memory mapped
variable just automatically update these WMI devices? I was worried
that the failed notification would make the WMI devices not work, just
guessing based on the WM00 name.

If you have any pointers on where to look to see the kernel functions
involved in WMI that would be appreciated.

> BTW, what is the name of your notebook model?

It is advertised as Lenovo Yoga 7i and the model number is Yoga 7 14IAL7

Re: ideapad_laptop tablet mode toggle detection

[Armin Wolf]

Am 06.03.23 um 02:41 schrieb Andrew Kallmeyer:

> On Sun, Mar 5, 2023 at 5:26 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> I was combing through the ACPI DSDT table inside the acpidump you provided,
>> and i found serveral PNP0C14 devices, which hold WMI methods, events and data.
> Thanks so much for the help! This is my first time looking at an ACPI
> dump so I never would have found this.
>
>> Maybe you can write a wmi driver which handles both WMI objects, so that you can find out what the values
>> returned by GetUsageMode mean. With a bit of luck, you can use this to implement tablet mode toggle detection.
> I will look into this, this does look more like the proper way to do
> it so it can be put in the driver for everyone. I suppose that missing
> Notify device is just unrelated then? Does setting the memory mapped
> variable just automatically update these WMI devices? I was worried
> that the failed notification would make the WMI devices not work, just
> guessing based on the WM00 name.
>
> If you have any pointers on where to look to see the kernel functions
> involved in WMI that would be appreciated.

The modern WMI bus infrastructure introduced new methods for interacting with WMI devices, you can find
them in include/linux/wmi.h. The use of the older methods like wmi_evaluate_method() is deprecated, since
those methods assume that each WMI GUID only appears exactly once in each device, and notifications can
be unreliable. The WMI bus infrastructure does not suffer from this problems.

Maybe you could also add the WMI GUIDs to allow_duplicates[] inside drivers/platform/x86/wmi.c, so that
the WMI core does not complain should Lenovo someday decide to use multiple yoga mode sensors.

Regarding the missing WMI device: it could be that this device was used in another model, and that the
firmware engineers forgot to properly remove it when they developed the firmware for your device.
In such a case a BIOS update might help.

Armin Wolf

>> BTW, what is the name of your notebook model?
> It is advertised as Lenovo Yoga 7i and the model number is Yoga 7 14IAL7

Re: ideapad_laptop tablet mode toggle detection

[Armin Wolf]

Am 06.03.23 um 02:26 schrieb Armin Wolf:

> Am 05.03.23 um 23:59 schrieb Andrew Kallmeyer:
>
>> On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
>>> Hi,
>>>
>>> could it be that bit 5 is set to disable the touchpad when the
>>> device switches
>>> to tablet mode? I suspect that the query handler does the following:
>>> 1. Notify VPC0 to disable the touchpad.
>>> 2. Notify ACPI WMI, which does submit the necessary scancode for
>>> switching to tablet mode.
>> I think you're right about this notification being for the touchpad,
>> although at least on my machine
>> there is no other touchpad switch. So this is identical for my machine
>> specifically. In this function
>> from the decompiled ACPI dump you can see VCP0 and WM00 notified:
>>
>> Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
>> {
>>     P80B = 0x44
>>     Notify (VPC0, 0x80) // Status Change
>>     WEID = 0xF4
>>     Notify (WM00, 0x80) // Status Change
>> }
>>
>> This WM00 device sounds like the WMI you're talking about, however I'm
>> getting those errors
>> about this device not existing in journalctl still. I was asking
>> before about how to create this
>> missing device but it's not clear to me if that is possible.
>>
>> kernel: ACPI BIOS Error (bug): Could not resolve symbol
>> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND
>>
>> I searched in my .dsl files from the acpidump and wasn't able to find
>> any of the 3 ideapad_wmi_ids
>> listed in the driver. Maybe you have an idea of how to interface with
>> this missing WM00 object though.
>
> I was combing through the ACPI DSDT table inside the acpidump you
> provided,
> and i found serveral PNP0C14 devices, which hold WMI methods, events
> and data.
>
> The WMI GUIDs are encoded inside the associated _WDG buffers, you
> should therefore
> only grep for parts of the GUIDs.
> For example: GUID 06129D99-6083-4164-81AD-F092F9D773A6 -> grep "0xF0,
> 0x92"
>
> When feeding the content of the buffers named WQxx to the bmfdec
> utility, i was able
> to extract a description of each WMI object.
>
> On of which (WMIY) is handling the "Lenovo Yoga Mode", which seems to
> handle the tablet
> mode transitions. The MOF data is:
>
> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
> Description("Lenovo Yoga Mode Change Event"),
> guid("{06129D99-6083-4164-81AD-F092F9D773A6}")]
> class LENOVO_GSENSOR_EVENT : WMIEvent {
>   [key, read] string InstanceName;
>   [read] boolean Active;
>   [WmiDataId(1), read, Description("Lenovo Yoga Mode Change Event")]
> uint32 ModeDataVal;
> };
>
> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
> Description("LENOVO_GSENSOR_DATA class"),
> guid("{09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C}")]
> class LENOVO_GSENSOR_DATA {
>   [key, read] string InstanceName;
>   [read] boolean Active;
>
>   [WmiMethodId(1), Implemented, Description("Mode Data")] void
> GetUsageMode([out, Description("Mode Data")] uint32 Data);
>   [WmiMethodId(2), Implemented, Description("Get Xaxis Value")] void
> GetXaxisValue([out, Description("Get Xaxis Value")] uint32 Data);
>   [WmiMethodId(3), Implemented, Description("Get Yaxis Value")] void
> GetYaxisValue([out, Description("Get Yaxis Value")] uint32 Data);
>   [WmiMethodId(4), Implemented, Description("Get Zaxis Value")] void
> GetZaxisValue([out, Description("Get Zaxis Value")] uint32 Data);
>   [WmiMethodId(5), Implemented, Description("Base to Ground")] void
> GetAngle4Value([out, Description("Base to Ground")] uint32 Data);
>   [WmiMethodId(6), Implemented, Description("Screen to Ground")] void
> GetAngle5Value([out, Description("Screen to Ground")] uint32 Data);
>   [WmiMethodId(7), Implemented, Description("Screen to Base")] void
> GetAngle6Value([out, Description("Screen to Base")] uint32 Data);
> };
>
> While looking at _WED (used to get ModeDataVal) and WMAB (handles
> method calls on LENOVO_GSENSOR_DATA),
> i assume that  only the first method (GetUsageMode) is implemented,
> and that ModeDataVal is used to tell
> which value of LENOVO_GSENSOR_DATA has changed (hardwired to 1 on your
> device).
>
> Maybe you can write a wmi driver which handles both WMI objects, so
> that you can find out what the values
> returned by GetUsageMode mean. With a bit of luck, you can use this to
> implement tablet mode toggle detection.
>
> BTW, what is the name of your notebook model?
>
> Armin Wolf
>
Well, it turns out i totally forgot that there exists already a patch which adds support for this:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20221004214332.35934-1-soyer@irl.hu/

Maybe you can get this patch into shape and submit it again?

Armin Wolf

>>> Could you provide the output of "acpidump"? Because i suspect that
>>> the virtual key handling
>>> is done using ACPI WMI, as many modern devices are using this
>>> approach. In this case, you
>>> could experiment with ideapad_wmi_notify(), and maybe take a look at
>>> ideapad_wmi_ids[].
>>>
>>> Armin Wolf
>> Here is the raw acpidump output:
>> https://la.ask.systems/temp/acpidump.out
>>
>> - Andrew

Re: ideapad_laptop tablet mode toggle detection

[Hans de Goede]

Hi,

Armin and Maximilian, thank you for helping Andrew with this.

On 3/6/23 02:41, Armin Wolf wrote:
> Am 06.03.23 um 02:26 schrieb Armin Wolf:
> 
>> Am 05.03.23 um 23:59 schrieb Andrew Kallmeyer:
>>
>>> On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
>>>> Hi,
>>>>
>>>> could it be that bit 5 is set to disable the touchpad when the
>>>> device switches
>>>> to tablet mode? I suspect that the query handler does the following:
>>>> 1. Notify VPC0 to disable the touchpad.
>>>> 2. Notify ACPI WMI, which does submit the necessary scancode for
>>>> switching to tablet mode.
>>> I think you're right about this notification being for the touchpad,
>>> although at least on my machine
>>> there is no other touchpad switch. So this is identical for my machine
>>> specifically. In this function
>>> from the decompiled ACPI dump you can see VCP0 and WM00 notified:
>>>
>>> Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
>>> {
>>>     P80B = 0x44
>>>     Notify (VPC0, 0x80) // Status Change
>>>     WEID = 0xF4
>>>     Notify (WM00, 0x80) // Status Change
>>> }
>>>
>>> This WM00 device sounds like the WMI you're talking about, however I'm
>>> getting those errors
>>> about this device not existing in journalctl still. I was asking
>>> before about how to create this
>>> missing device but it's not clear to me if that is possible.
>>>
>>> kernel: ACPI BIOS Error (bug): Could not resolve symbol
>>> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND
>>>
>>> I searched in my .dsl files from the acpidump and wasn't able to find
>>> any of the 3 ideapad_wmi_ids
>>> listed in the driver. Maybe you have an idea of how to interface with
>>> this missing WM00 object though.
>>
>> I was combing through the ACPI DSDT table inside the acpidump you
>> provided,
>> and i found serveral PNP0C14 devices, which hold WMI methods, events
>> and data.
>>
>> The WMI GUIDs are encoded inside the associated _WDG buffers, you
>> should therefore
>> only grep for parts of the GUIDs.
>> For example: GUID 06129D99-6083-4164-81AD-F092F9D773A6 -> grep "0xF0,
>> 0x92"
>>
>> When feeding the content of the buffers named WQxx to the bmfdec
>> utility, i was able
>> to extract a description of each WMI object.
>>
>> On of which (WMIY) is handling the "Lenovo Yoga Mode", which seems to
>> handle the tablet
>> mode transitions. The MOF data is:
>>
>> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
>> Description("Lenovo Yoga Mode Change Event"),
>> guid("{06129D99-6083-4164-81AD-F092F9D773A6}")]
>> class LENOVO_GSENSOR_EVENT : WMIEvent {
>>   [key, read] string InstanceName;
>>   [read] boolean Active;
>>   [WmiDataId(1), read, Description("Lenovo Yoga Mode Change Event")]
>> uint32 ModeDataVal;
>> };
>>
>> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
>> Description("LENOVO_GSENSOR_DATA class"),
>> guid("{09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C}")]
>> class LENOVO_GSENSOR_DATA {
>>   [key, read] string InstanceName;
>>   [read] boolean Active;
>>
>>   [WmiMethodId(1), Implemented, Description("Mode Data")] void
>> GetUsageMode([out, Description("Mode Data")] uint32 Data);
>>   [WmiMethodId(2), Implemented, Description("Get Xaxis Value")] void
>> GetXaxisValue([out, Description("Get Xaxis Value")] uint32 Data);
>>   [WmiMethodId(3), Implemented, Description("Get Yaxis Value")] void
>> GetYaxisValue([out, Description("Get Yaxis Value")] uint32 Data);
>>   [WmiMethodId(4), Implemented, Description("Get Zaxis Value")] void
>> GetZaxisValue([out, Description("Get Zaxis Value")] uint32 Data);
>>   [WmiMethodId(5), Implemented, Description("Base to Ground")] void
>> GetAngle4Value([out, Description("Base to Ground")] uint32 Data);
>>   [WmiMethodId(6), Implemented, Description("Screen to Ground")] void
>> GetAngle5Value([out, Description("Screen to Ground")] uint32 Data);
>>   [WmiMethodId(7), Implemented, Description("Screen to Base")] void
>> GetAngle6Value([out, Description("Screen to Base")] uint32 Data);
>> };
>>
>> While looking at _WED (used to get ModeDataVal) and WMAB (handles
>> method calls on LENOVO_GSENSOR_DATA),
>> i assume that  only the first method (GetUsageMode) is implemented,
>> and that ModeDataVal is used to tell
>> which value of LENOVO_GSENSOR_DATA has changed (hardwired to 1 on your
>> device).
>>
>> Maybe you can write a wmi driver which handles both WMI objects, so
>> that you can find out what the values
>> returned by GetUsageMode mean. With a bit of luck, you can use this to
>> implement tablet mode toggle detection.
>>
>> BTW, what is the name of your notebook model?
>>
>> Armin Wolf
>>
> Well, it turns out i totally forgot that there exists already a patch which adds support for this:
> https://patchwork.kernel.org/project/platform-driver-x86/patch/20221004214332.35934-1-soyer@irl.hu/
> 
> Maybe you can get this patch into shape and submit it again?

Yes if you can do that, then that would be great.

The end result should really be a patch-series then with the first patch
being the prep patch introducing a new ideapad-laptop.h I suggested
during my review. This patch can have you as the author + your
Signed-off-by.

The 2nd patch would then introduce a new version of Gergo's driver using
the helper functions introduced in the first patch.

For this patch you want to keep Gergo as the author (1), and then add
yourself as co-author by using the following tags at the end of
the commit message:

Co-developed-by: Andrew Kallmeyer <kallmeyeras@gmail.com>
Signed-off-by: Andrew Kallmeyer <kallmeyeras@gmail.com>

Note do NOT add a Signed-off-by for Gergo, he clearly intended to
submit his code under the GPL (see the GPL copyright header in
the new file) so the Signed-off-by missing is fine. And you must
not add Signed-off-by-s for other people. A S-o-b must always
be given by the person themselves.

Regards,

Hans

1) git commit --author="..."






> 
> Armin Wolf
> 
>>>> Could you provide the output of "acpidump"? Because i suspect that
>>>> the virtual key handling
>>>> is done using ACPI WMI, as many modern devices are using this
>>>> approach. In this case, you
>>>> could experiment with ideapad_wmi_notify(), and maybe take a look at
>>>> ideapad_wmi_ids[].
>>>>
>>>> Armin Wolf
>>> Here is the raw acpidump output:
>>> https://la.ask.systems/temp/acpidump.out
>>>
>>> - Andrew
> 

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Mon, Mar 6, 2023 at 12:38 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> Armin and Maximilian, thank you for helping Andrew with this.
>
> On 3/6/23 02:41, Armin Wolf wrote:
> > Am 06.03.23 um 02:26 schrieb Armin Wolf:
> >
> >> Am 05.03.23 um 23:59 schrieb Andrew Kallmeyer:
> >>
> >>> On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
> >>>> Hi,
> >>>>
> >>>> could it be that bit 5 is set to disable the touchpad when the
> >>>> device switches
> >>>> to tablet mode? I suspect that the query handler does the following:
> >>>> 1. Notify VPC0 to disable the touchpad.
> >>>> 2. Notify ACPI WMI, which does submit the necessary scancode for
> >>>> switching to tablet mode.
> >>> I think you're right about this notification being for the touchpad,
> >>> although at least on my machine
> >>> there is no other touchpad switch. So this is identical for my machine
> >>> specifically. In this function
> >>> from the decompiled ACPI dump you can see VCP0 and WM00 notified:
> >>>
> >>> Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
> >>> {
> >>>     P80B = 0x44
> >>>     Notify (VPC0, 0x80) // Status Change
> >>>     WEID = 0xF4
> >>>     Notify (WM00, 0x80) // Status Change
> >>> }
> >>>
> >>> This WM00 device sounds like the WMI you're talking about, however I'm
> >>> getting those errors
> >>> about this device not existing in journalctl still. I was asking
> >>> before about how to create this
> >>> missing device but it's not clear to me if that is possible.
> >>>
> >>> kernel: ACPI BIOS Error (bug): Could not resolve symbol
> >>> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND
> >>>
> >>> I searched in my .dsl files from the acpidump and wasn't able to find
> >>> any of the 3 ideapad_wmi_ids
> >>> listed in the driver. Maybe you have an idea of how to interface with
> >>> this missing WM00 object though.
> >>
> >> I was combing through the ACPI DSDT table inside the acpidump you
> >> provided,
> >> and i found serveral PNP0C14 devices, which hold WMI methods, events
> >> and data.
> >>
> >> The WMI GUIDs are encoded inside the associated _WDG buffers, you
> >> should therefore
> >> only grep for parts of the GUIDs.
> >> For example: GUID 06129D99-6083-4164-81AD-F092F9D773A6 -> grep "0xF0,
> >> 0x92"
> >>
> >> When feeding the content of the buffers named WQxx to the bmfdec
> >> utility, i was able
> >> to extract a description of each WMI object.
> >>
> >> On of which (WMIY) is handling the "Lenovo Yoga Mode", which seems to
> >> handle the tablet
> >> mode transitions. The MOF data is:
> >>
> >> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
> >> Description("Lenovo Yoga Mode Change Event"),
> >> guid("{06129D99-6083-4164-81AD-F092F9D773A6}")]
> >> class LENOVO_GSENSOR_EVENT : WMIEvent {
> >>   [key, read] string InstanceName;
> >>   [read] boolean Active;
> >>   [WmiDataId(1), read, Description("Lenovo Yoga Mode Change Event")]
> >> uint32 ModeDataVal;
> >> };
> >>
> >> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
> >> Description("LENOVO_GSENSOR_DATA class"),
> >> guid("{09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C}")]
> >> class LENOVO_GSENSOR_DATA {
> >>   [key, read] string InstanceName;
> >>   [read] boolean Active;
> >>
> >>   [WmiMethodId(1), Implemented, Description("Mode Data")] void
> >> GetUsageMode([out, Description("Mode Data")] uint32 Data);
> >>   [WmiMethodId(2), Implemented, Description("Get Xaxis Value")] void
> >> GetXaxisValue([out, Description("Get Xaxis Value")] uint32 Data);
> >>   [WmiMethodId(3), Implemented, Description("Get Yaxis Value")] void
> >> GetYaxisValue([out, Description("Get Yaxis Value")] uint32 Data);
> >>   [WmiMethodId(4), Implemented, Description("Get Zaxis Value")] void
> >> GetZaxisValue([out, Description("Get Zaxis Value")] uint32 Data);
> >>   [WmiMethodId(5), Implemented, Description("Base to Ground")] void
> >> GetAngle4Value([out, Description("Base to Ground")] uint32 Data);
> >>   [WmiMethodId(6), Implemented, Description("Screen to Ground")] void
> >> GetAngle5Value([out, Description("Screen to Ground")] uint32 Data);
> >>   [WmiMethodId(7), Implemented, Description("Screen to Base")] void
> >> GetAngle6Value([out, Description("Screen to Base")] uint32 Data);
> >> };
> >>
> >> While looking at _WED (used to get ModeDataVal) and WMAB (handles
> >> method calls on LENOVO_GSENSOR_DATA),
> >> i assume that  only the first method (GetUsageMode) is implemented,
> >> and that ModeDataVal is used to tell
> >> which value of LENOVO_GSENSOR_DATA has changed (hardwired to 1 on your
> >> device).
> >>
> >> Maybe you can write a wmi driver which handles both WMI objects, so
> >> that you can find out what the values
> >> returned by GetUsageMode mean. With a bit of luck, you can use this to
> >> implement tablet mode toggle detection.
> >>
> >> BTW, what is the name of your notebook model?
> >>
> >> Armin Wolf
> >>
> > Well, it turns out i totally forgot that there exists already a patch which adds support for this:
> > https://patchwork.kernel.org/project/platform-driver-x86/patch/20221004214332.35934-1-soyer@irl.hu/
> >
> > Maybe you can get this patch into shape and submit it again?
>
> Yes if you can do that, then that would be great.
>
> The end result should really be a patch-series then with the first patch
> being the prep patch introducing a new ideapad-laptop.h I suggested
> during my review. This patch can have you as the author + your
> Signed-off-by.
>
> The 2nd patch would then introduce a new version of Gergo's driver using
> the helper functions introduced in the first patch.
>
> For this patch you want to keep Gergo as the author (1), and then add
> yourself as co-author by using the following tags at the end of
> the commit message:
>
> Co-developed-by: Andrew Kallmeyer <kallmeyeras@gmail.com>
> Signed-off-by: Andrew Kallmeyer <kallmeyeras@gmail.com>
>
> Note do NOT add a Signed-off-by for Gergo, he clearly intended to
> submit his code under the GPL (see the GPL copyright header in
> the new file) so the Signed-off-by missing is fine. And you must
> not add Signed-off-by-s for other people. A S-o-b must always
> be given by the person themselves.
>
> Regards,
>
> Hans
>
> 1) git commit --author="..."

Hi, Hans, thanks for the great advice! I'm excited to see it's already
so close to being done. I have compiled Gergo's module separately from
the kernel tree and loaded it on my laptop and it works perfectly.

I will follow the review comments on the patch, getting it integrated
with the existing module etc. I will also make sure to sign the
commits as you advise. It will be good to have it working for
everyone. This will be my first kernel patch; I have found the docs
page on submitting a patch but let me know if there's anything I'm
missing when I do it.

- Andrew

Re: ideapad_laptop tablet mode toggle detection

[Andrew Kallmeyer]

On Tue, Mar 7, 2023 at 9:14 PM Andrew Kallmeyer <kallmeyeras@gmail.com> wrote:
>
> I will follow the review comments on the patch, getting it integrated
> with the existing module etc.

I see now that I misunderstood and you were only suggesting sharing
the functions not integrating this into the existing module. I will do
it like you said, you even mentioned in the review that you prefer it
to be a separate module.

- Andrew

Re: ideapad_laptop tablet mode toggle detection

[Hans de Goede]

Hi Andrew,

On 3/8/23 06:14, Andrew Kallmeyer wrote:
> On Mon, Mar 6, 2023 at 12:38 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> Armin and Maximilian, thank you for helping Andrew with this.
>>
>> On 3/6/23 02:41, Armin Wolf wrote:
>>> Am 06.03.23 um 02:26 schrieb Armin Wolf:
>>>
>>>> Am 05.03.23 um 23:59 schrieb Andrew Kallmeyer:
>>>>
>>>>> On Sun, Mar 5, 2023 at 1:40 PM Armin Wolf <W_Armin@gmx.de> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> could it be that bit 5 is set to disable the touchpad when the
>>>>>> device switches
>>>>>> to tablet mode? I suspect that the query handler does the following:
>>>>>> 1. Notify VPC0 to disable the touchpad.
>>>>>> 2. Notify ACPI WMI, which does submit the necessary scancode for
>>>>>> switching to tablet mode.
>>>>> I think you're right about this notification being for the touchpad,
>>>>> although at least on my machine
>>>>> there is no other touchpad switch. So this is identical for my machine
>>>>> specifically. In this function
>>>>> from the decompiled ACPI dump you can see VCP0 and WM00 notified:
>>>>>
>>>>> Method (_Q44, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
>>>>> {
>>>>>     P80B = 0x44
>>>>>     Notify (VPC0, 0x80) // Status Change
>>>>>     WEID = 0xF4
>>>>>     Notify (WM00, 0x80) // Status Change
>>>>> }
>>>>>
>>>>> This WM00 device sounds like the WMI you're talking about, however I'm
>>>>> getting those errors
>>>>> about this device not existing in journalctl still. I was asking
>>>>> before about how to create this
>>>>> missing device but it's not clear to me if that is possible.
>>>>>
>>>>> kernel: ACPI BIOS Error (bug): Could not resolve symbol
>>>>> [\_SB.PC00.LPCB.EC0._Q44.WM00], AE_NOT_FOUND
>>>>>
>>>>> I searched in my .dsl files from the acpidump and wasn't able to find
>>>>> any of the 3 ideapad_wmi_ids
>>>>> listed in the driver. Maybe you have an idea of how to interface with
>>>>> this missing WM00 object though.
>>>>
>>>> I was combing through the ACPI DSDT table inside the acpidump you
>>>> provided,
>>>> and i found serveral PNP0C14 devices, which hold WMI methods, events
>>>> and data.
>>>>
>>>> The WMI GUIDs are encoded inside the associated _WDG buffers, you
>>>> should therefore
>>>> only grep for parts of the GUIDs.
>>>> For example: GUID 06129D99-6083-4164-81AD-F092F9D773A6 -> grep "0xF0,
>>>> 0x92"
>>>>
>>>> When feeding the content of the buffers named WQxx to the bmfdec
>>>> utility, i was able
>>>> to extract a description of each WMI object.
>>>>
>>>> On of which (WMIY) is handling the "Lenovo Yoga Mode", which seems to
>>>> handle the tablet
>>>> mode transitions. The MOF data is:
>>>>
>>>> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
>>>> Description("Lenovo Yoga Mode Change Event"),
>>>> guid("{06129D99-6083-4164-81AD-F092F9D773A6}")]
>>>> class LENOVO_GSENSOR_EVENT : WMIEvent {
>>>>   [key, read] string InstanceName;
>>>>   [read] boolean Active;
>>>>   [WmiDataId(1), read, Description("Lenovo Yoga Mode Change Event")]
>>>> uint32 ModeDataVal;
>>>> };
>>>>
>>>> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"),
>>>> Description("LENOVO_GSENSOR_DATA class"),
>>>> guid("{09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C}")]
>>>> class LENOVO_GSENSOR_DATA {
>>>>   [key, read] string InstanceName;
>>>>   [read] boolean Active;
>>>>
>>>>   [WmiMethodId(1), Implemented, Description("Mode Data")] void
>>>> GetUsageMode([out, Description("Mode Data")] uint32 Data);
>>>>   [WmiMethodId(2), Implemented, Description("Get Xaxis Value")] void
>>>> GetXaxisValue([out, Description("Get Xaxis Value")] uint32 Data);
>>>>   [WmiMethodId(3), Implemented, Description("Get Yaxis Value")] void
>>>> GetYaxisValue([out, Description("Get Yaxis Value")] uint32 Data);
>>>>   [WmiMethodId(4), Implemented, Description("Get Zaxis Value")] void
>>>> GetZaxisValue([out, Description("Get Zaxis Value")] uint32 Data);
>>>>   [WmiMethodId(5), Implemented, Description("Base to Ground")] void
>>>> GetAngle4Value([out, Description("Base to Ground")] uint32 Data);
>>>>   [WmiMethodId(6), Implemented, Description("Screen to Ground")] void
>>>> GetAngle5Value([out, Description("Screen to Ground")] uint32 Data);
>>>>   [WmiMethodId(7), Implemented, Description("Screen to Base")] void
>>>> GetAngle6Value([out, Description("Screen to Base")] uint32 Data);
>>>> };
>>>>
>>>> While looking at _WED (used to get ModeDataVal) and WMAB (handles
>>>> method calls on LENOVO_GSENSOR_DATA),
>>>> i assume that  only the first method (GetUsageMode) is implemented,
>>>> and that ModeDataVal is used to tell
>>>> which value of LENOVO_GSENSOR_DATA has changed (hardwired to 1 on your
>>>> device).
>>>>
>>>> Maybe you can write a wmi driver which handles both WMI objects, so
>>>> that you can find out what the values
>>>> returned by GetUsageMode mean. With a bit of luck, you can use this to
>>>> implement tablet mode toggle detection.
>>>>
>>>> BTW, what is the name of your notebook model?
>>>>
>>>> Armin Wolf
>>>>
>>> Well, it turns out i totally forgot that there exists already a patch which adds support for this:
>>> https://patchwork.kernel.org/project/platform-driver-x86/patch/20221004214332.35934-1-soyer@irl.hu/
>>>
>>> Maybe you can get this patch into shape and submit it again?
>>
>> Yes if you can do that, then that would be great.
>>
>> The end result should really be a patch-series then with the first patch
>> being the prep patch introducing a new ideapad-laptop.h I suggested
>> during my review. This patch can have you as the author + your
>> Signed-off-by.
>>
>> The 2nd patch would then introduce a new version of Gergo's driver using
>> the helper functions introduced in the first patch.
>>
>> For this patch you want to keep Gergo as the author (1), and then add
>> yourself as co-author by using the following tags at the end of
>> the commit message:
>>
>> Co-developed-by: Andrew Kallmeyer <kallmeyeras@gmail.com>
>> Signed-off-by: Andrew Kallmeyer <kallmeyeras@gmail.com>
>>
>> Note do NOT add a Signed-off-by for Gergo, he clearly intended to
>> submit his code under the GPL (see the GPL copyright header in
>> the new file) so the Signed-off-by missing is fine. And you must
>> not add Signed-off-by-s for other people. A S-o-b must always
>> be given by the person themselves.
>>
>> Regards,
>>
>> Hans
>>
>> 1) git commit --author="..."
> 
> Hi, Hans, thanks for the great advice! I'm excited to see it's already
> so close to being done. I have compiled Gergo's module separately from
> the kernel tree and loaded it on my laptop and it works perfectly.
> 
> I will follow the review comments on the patch, getting it integrated
> with the existing module etc. I will also make sure to sign the
> commits as you advise.

Great, thank you for picking op Gergo's work on this!

> It will be good to have it working for
> everyone. This will be my first kernel patch; I have found the docs
> page on submitting a patch but let me know if there's anything I'm
> missing when I do it.

Unfortunately the kernel process is not very newcomer friendly,
so chances are you will make make some mistakes / miss some steps
with your first submission.

Note that that (making some mistakes) is totally fine, the best
advice I can give you is don't be afraid to make mistakes :)

If something needs to be fixed before the patches can be merged
I or another reviewer will gently point that out and you can
prepare a new version addressing any remarks.

The only other advise I have is try to use "git send-email"
to send out the patches, some people copy and paste them
into a regular email client to send them and most email
clients then mangle the patch (change whitespace, wrap
lines, etc.).

Regards,

Hans