On Mon, Mar 6, 2023 at 12:38 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxx> 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@xxxxxx/
> >
> > 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@xxxxxxxxx>
> Signed-off-by: Andrew Kallmeyer <kallmeyeras@xxxxxxxxx>
>
> 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
