ideapad-laptop touchpad handling problems, request for help

ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 4379 bytes --]

Hi All,

I'm emailing you all because you have written patches or
reported bugs related to the ideapad-laptop touchpad
handling the past.

1. History
==========

I have done a bit of digging into the history of
the touchpad handling:

What I believe is the troublesome part of the touchpad handling
started in 2012 with:

07a4a4fc83dd ("ideapad: add Lenovo IdeaPad Z570 support (part 2)")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=07a4a4fc83dd

Which added the ideapad_sync_touchpad_state() function which depending
on the result of reading VPCCMD_R_TOUCHPAD send an i8042 command to
enable/disable the aux port of the ps2 controller *and* which sends
KEY_TOUCHPAD_OFF/KEY_TOUCHPAD_ON events to userspace to let userspace
know the state of the touchpad.

The first commit to optionally disable ideapad_sync_touchpad_state()
was actually written by me in 2014, for a "Lenovo Yoga 2 11":

f79a901331a8 ("ideapad-laptop: Disable touchpad interface on Yoga models")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f79a901331a8

The problem on the "Lenovo Yoga 2 11" was a spurious KEY_TOUCHPAD_OFF
event on resume, other then that there were no bad side effects.

This patch got reverted soon afterwards in commit 3b264d279e72 because
it stopped the touchpad enable/disable button from working on
a "Lenovo Yoga 2 13".

Then in 2021 a patch was added to again disable ideapad_sync_touchpad_state()
on some models, this time based on the ACPI HID (ELAN0634) of the touchpad:

d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d69cd7eea93e

And the last couple of weeks the following 2 patches were added to disable
ideapad_sync_touchpad_state() on more models based on DMI ids for the first
patch (already merged) + adding a new ACPI HID for the second patch:

a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c

https://patchwork.kernel.org/project/platform-driver-x86/patch/20221029120311.11152-8-erayorcunus@gmail.com/

As mentioned in the commit msg of d69cd7eea93e ("platform/x86:
ideapad-laptop: Disable touchpad_switch for ELAN0634") part of
the problem is VPCCMD_R_TOUCHPAD returning 0 leading to the aux
ps/2 port getting turned off.

This can be a problem even on devices where the touchpad shows up as
an i2c/smbus device because often on those devices the touchpad is
connected over both ps/2 + i2c and at least for synaptics devices
the touchpad needs to be contacted over i2c within a couple of
100s of ms of doing a ps/2 reset for it to switch to i2c mode.


2. Possible solutions
=====================

1. Do something with BIOS date to only enable touchpad_ctrl_via_ec on
older models. Problem is that BIOS updates happen and those can be
of much later date then the production year of the model

2. Move to an allow list for setting touchpad_ctrl_via_ec to true, given
how soon after my initial patch to disable touchpad_ctrl_via_ec I got
a bug report about this, which even was due to a deny-list DMI entry
not being narrow enough this seems like a bad idea. OTOH missing
the ability to turn the touchpad on/off is less of a big deal
then a non working touchpad. So if we fail to find a better
solution this might be the best thing to do.

3. Since the problems are caused when VPCCMD_R_TOUCHPAD reads as 0 at
boot, causing ideapad_sync_touchpad_state() to turn off the ps/2 aux port
and since the touchpad is normally on at boot, we can check for
VPCCMD_R_TOUCHPAD reading as 0 at boot and if that happens assume that
means touchpad-ctrl via the EC is not available. I have attached
a patch implementing this approach.


3. Please test
==============

If you have ideapads where touchpad_ctrl_via_ec should be 1 because
it is needed to toggle the touchpad on/off with the hotkey.

Or the exact opposite you have ideapads where it should be disabled
because ideapad_sync_touchpad_state() turning off the ps/2 aux port
is causing problems.

Then please give the attached patch a try. Note this applies on
top of Torvald's current master, or on top of 6.0 with :
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
added.

Regards,

Hans


[-- Attachment #2: 0001-platform-x86-ideapad-laptop-Improve-touchpad_ctrl_vi.patch --]
[-- Type: text/x-patch, Size: 3550 bytes --]

From c52f2286e40291cb7337e9e9d7966365828d8d3d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 9 Nov 2022 21:39:37 +0100
Subject: [PATCH] platform/x86: ideapad-laptop: Improve touchpad_ctrl_via_ec
 detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On newer ideapads touchpad ctrl via the EC is not available and allowing
ideapad_sync_touchpad_state() to turn of the PS/2 aux port on a 0 read
from VPCCMD_R_TOUCHPAD stops the touchpad from working.

So far we have been using a deny-list based approach to disable
ideapad_sync_touchpad_state() on models where it is causing issues
based on a mix of touchpad ACPI-HID + DMI string matches. But this
does not work well (it results in a game of whack a mole).

Since ideapad_sync_touchpad_state() causes the problem only when
VPCCMD_R_TOUCHPAD reads 0 and since normally the touchpad is always
on at boot, so VPCCMD_R_TOUCHPAD should read as 1, we can avoid
models on which touchpad_ctrl_via_ec causes problems by only allowing
touchpad_ctrl_via_ec when VPCCMD_R_TOUCHPAD reads non 0 at boot.

Fixes: d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
Fixes: a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
Cc: Maxim Mikityanskiy <maxtram95@gmail.com>
Cc: GOESSEL Guillaume <g_goessel@outlook.com>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Manyi Li <limanyi@uniontech.com>
Cc: Eray Orçunus <erayorcunus@gmail.com>
Cc: Ike Panhc <ike.pan@canonical.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/ideapad-laptop.c | 37 +++++++++------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 33b3dfdd1b08..499e75c84476 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1533,38 +1533,25 @@ static const struct dmi_system_id hw_rfkill_list[] = {
 	{}
 };
 
-static const struct dmi_system_id no_touchpad_switch_list[] = {
-	{
-	.ident = "Lenovo Yoga 3 Pro 1370",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 3"),
-		},
-	},
-	{
-	.ident = "ZhaoYang K4e-IML",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ZhaoYang K4e-IML"),
-		},
-	},
-	{}
-};
-
 static void ideapad_check_features(struct ideapad_private *priv)
 {
 	acpi_handle handle = priv->adev->handle;
 	unsigned long val;
+	int ret;
 
 	priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
 
-	/* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
-	if (acpi_dev_present("ELAN0634", NULL, -1))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else if (dmi_check_system(no_touchpad_switch_list))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else
-		priv->features.touchpad_ctrl_via_ec = 1;
+	/*
+	 * On newer ideapads touchpad ctrl via the EC is not available and
+	 * allowing ideapad_sync_touchpad_state() to turn of the PS/2 aux
+	 * port on a 0 read from VPCCMD_R_TOUCHPAD stops the touchpad from
+	 * working.
+	 *
+	 * Assume that the touchpad is always on at boot and that a 0 read
+	 * from VPCCMD_R_TOUCHPAD means EC touchpad ctrl is not available.
+	 */
+	ret = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &val);
+	priv->features.touchpad_ctrl_via_ec = ret == 0 && val != 0;
 
 	if (!read_ec_data(handle, VPCCMD_R_FAN, &val))
 		priv->features.fan_mode = true;
-- 
2.37.3

Re: ideapad-laptop touchpad handling problems, request for help

[Maxim Mikityanskiy]

On Wed, 9 Nov 2022 at 22:59, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> I'm emailing you all because you have written patches or
> reported bugs related to the ideapad-laptop touchpad
> handling the past.
>
> 1. History
> ==========
>
> I have done a bit of digging into the history of
> the touchpad handling:
>
> What I believe is the troublesome part of the touchpad handling
> started in 2012 with:
>
> 07a4a4fc83dd ("ideapad: add Lenovo IdeaPad Z570 support (part 2)")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=07a4a4fc83dd
>
> Which added the ideapad_sync_touchpad_state() function which depending
> on the result of reading VPCCMD_R_TOUCHPAD send an i8042 command to
> enable/disable the aux port of the ps2 controller *and* which sends
> KEY_TOUCHPAD_OFF/KEY_TOUCHPAD_ON events to userspace to let userspace
> know the state of the touchpad.

For the reference, this cumbersome solution was needed in the first
place, because Z570 has an LED, and EC toggles the LED on each
touchpad toggle button press, but EC only controls the LED, it doesn't
actually disable the touchpad. Normally, when EC doesn't disable the
touchpad, we can send KEY_TOUCHPAD_TOGGLE and leave the action to the
DE. However, as Z570 has this LED, which will get out of sync with the
touchpad state, we can't use KEY_TOUCHPAD_TOGGLE.

That leads to the following idea: if newer Lenovos have issues with
VPCCMD_R_TOUCHPAD and they don't have the touchpad LED, we can just
use KEY_TOUCHPAD_TOGGLE for them. However, if it turns out that some
Lenovo model does actually disable the touchpad in hardware, this also
needs to be taken into account.

However, this idea doesn't answer the question of how to detect such
laptops. I wonder how the Windows driver does it.

By the way, newer Lenovos also have other incompatibilities, for
example, fan modes are no longer relevant on some models, and I don't
know how to detect those either.

>
> The first commit to optionally disable ideapad_sync_touchpad_state()
> was actually written by me in 2014, for a "Lenovo Yoga 2 11":
>
> f79a901331a8 ("ideapad-laptop: Disable touchpad interface on Yoga models")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f79a901331a8
>
> The problem on the "Lenovo Yoga 2 11" was a spurious KEY_TOUCHPAD_OFF
> event on resume, other then that there were no bad side effects.
>
> This patch got reverted soon afterwards in commit 3b264d279e72 because
> it stopped the touchpad enable/disable button from working on
> a "Lenovo Yoga 2 13".
>
> Then in 2021 a patch was added to again disable ideapad_sync_touchpad_state()
> on some models, this time based on the ACPI HID (ELAN0634) of the touchpad:
>
> d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d69cd7eea93e
>
> And the last couple of weeks the following 2 patches were added to disable
> ideapad_sync_touchpad_state() on more models based on DMI ids for the first
> patch (already merged) + adding a new ACPI HID for the second patch:
>
> a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
>
> https://patchwork.kernel.org/project/platform-driver-x86/patch/20221029120311.11152-8-erayorcunus@gmail.com/
>
> As mentioned in the commit msg of d69cd7eea93e ("platform/x86:
> ideapad-laptop: Disable touchpad_switch for ELAN0634") part of
> the problem is VPCCMD_R_TOUCHPAD returning 0 leading to the aux
> ps/2 port getting turned off.
>
> This can be a problem even on devices where the touchpad shows up as
> an i2c/smbus device because often on those devices the touchpad is
> connected over both ps/2 + i2c and at least for synaptics devices
> the touchpad needs to be contacted over i2c within a couple of
> 100s of ms of doing a ps/2 reset for it to switch to i2c mode.
>
>
> 2. Possible solutions
> =====================
>
> 1. Do something with BIOS date to only enable touchpad_ctrl_via_ec on
> older models. Problem is that BIOS updates happen and those can be
> of much later date then the production year of the model
>
> 2. Move to an allow list for setting touchpad_ctrl_via_ec to true, given
> how soon after my initial patch to disable touchpad_ctrl_via_ec I got
> a bug report about this, which even was due to a deny-list DMI entry
> not being narrow enough this seems like a bad idea. OTOH missing
> the ability to turn the touchpad on/off is less of a big deal
> then a non working touchpad. So if we fail to find a better
> solution this might be the best thing to do.
>
> 3. Since the problems are caused when VPCCMD_R_TOUCHPAD reads as 0 at
> boot, causing ideapad_sync_touchpad_state() to turn off the ps/2 aux port
> and since the touchpad is normally on at boot, we can check for
> VPCCMD_R_TOUCHPAD reading as 0 at boot and if that happens assume that
> means touchpad-ctrl via the EC is not available. I have attached
> a patch implementing this approach.

I need to check it, but it's entirely possible that the touchpad state
is preserved after reboot. In that case, reading VPCCMD_R_TOUCHPAD as
0 on boot would be normal. I think it's also possible that the user can
disable the touchpad before Linux starts. This approach looks problematic.

>
>
> 3. Please test
> ==============
>
> If you have ideapads where touchpad_ctrl_via_ec should be 1 because
> it is needed to toggle the touchpad on/off with the hotkey.

I still own my Z570, but it's located in another city. I can do some
testing on it, though. I can also provide the DSDT dump along with the
BIOS version.

Also adding Kryštof to this thread, who is a Lenovo Yoga 720-15ikb user.

It would be perfect if someone could reverse engineer what the Windows
driver does in order to distinguish the old and the new interfaces.
With a proper detection mechanism, we could:

1. Just send KEY_TOUCHPAD_ON/OFF if EC disables the touchpad (any
known models?).

2. Disable the touchpad in the driver if the laptop has the LED and EC
doesn't disable the touchpad (Z570).

3. Just send KEY_TOUCHPAD_TOGGLE if the laptop doesn't have the LED
and EC doesn't disable the touchpad (any known models?).

>
> Or the exact opposite you have ideapads where it should be disabled
> because ideapad_sync_touchpad_state() turning off the ps/2 aux port
> is causing problems.
>
> Then please give the attached patch a try. Note this applies on
> top of Torvald's current master, or on top of 6.0 with :
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
> added.
>
> Regards,
>
> Hans
>

Re: ideapad-laptop touchpad handling problems, request for help

[Eray Orçunus]

Hi all,

I don't know is this correct; but I'll reply to 2 messages, firstly to OP.

On Wed, 9 Nov 2022 at 22:59, Hans de Goede <hdegoede@redhat.com> wrote:
> 3. Please test
> ==============
> 
> If you have ideapads where touchpad_ctrl_via_ec should be 1 because
> it is needed to toggle the touchpad on/off with the hotkey.
> 
> Or the exact opposite you have ideapads where it should be disabled
> because ideapad_sync_touchpad_state() turning off the ps/2 aux port
> is causing problems.
> 
> Then please give the attached patch a try. Note this applies on
> top of Torvald's current master, or on top of 6.0 with :
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
> added.

While I agree with your findings(and thanks for your effort, I also tried
to solve this but later gave up), they doesn't apply to one type of
IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
This is because my IdeaPad has this features:

- i8042.noaux doesn't affect touchpad, and it's connected over i2c
- There is no touchpad LED, and touchpad hotkey only sends "key pressed"
  ACPI event, doesn't do anything else
- VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
- Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
- VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)

So I agree on Maxim's solutions, and I have an idea on how to implement
it, I will explain it below.

On 10 Nov 2022 at 00:39, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
> Normally, when EC doesn't disable the
> touchpad, we can send KEY_TOUCHPAD_TOGGLE and leave the action to the
> DE. However, as Z570 has this LED, which will get out of sync with the
> touchpad state, we can't use KEY_TOUCHPAD_TOGGLE.
> 
> That leads to the following idea: if newer Lenovos have issues with
> VPCCMD_R_TOUCHPAD and they don't have the touchpad LED, we can just
> use KEY_TOUCHPAD_TOGGLE for them. However, if it turns out that some
> Lenovo model does actually disable the touchpad in hardware, this also
> needs to be taken into account.

Completely agreed.

> However, this idea doesn't answer the question of how to detect such
> laptops. I wonder how the Windows driver does it.

I reverse engineered Lenovo Utility/Hotkeys programs of ~5 IdeaPads,
and I can say they send VPCCMD_W_TOUCHPAD unconditionally, without any
check if it's working or not. This includes 720s and 520-15IKB, which
don't have working VPCCMD_W_TOUCHPAD. This is because it's the
working-in-background touchpad helper program catches the hotkey ACPI
event (I think) and handles the situation. This can be confirmed by
killing Lenovo programs on these IdeaPads won't affect the touchpad
toggle hotkey.

> I need to check it, but it's entirely possible that the touchpad state
> is preserved after reboot. In that case, reading VPCCMD_R_TOUCHPAD as
> 0 on boot would be normal. I think it's also possible that the user can
> disable the touchpad before Linux starts. This approach looks problematic.

I agree on we can't rely VPCCMD_R_TOUCHPAD to be 0 on boot, because
it always returns 1 for me, yet I don't have working VPCCMD_W_TOUCHPAD.

> 1. Just send KEY_TOUCHPAD_ON/OFF if EC disables the touchpad (any
> known models?).

If you mean the cases when the laptop toggles touchpad itself when
touchpad hotkey is pressed. Good news is(I hope): I don't think there are
such IdeaPads, yet this still needs confirmation.

If you mean the cases where VPCCMD_W_TOUCHPAD works, I agree with you,
yet we don't know how to find/detect these IdeaPads :/

But if we can assume the IdeaPads produced before 2015 all have PS/2
touchpads (at least the ~10 DSDTs I checked all had one), and/or have
VPCCMD_W_TOUCHPAD, why don't we send both VPCCMD_W_TOUCHPAD and i8042
command when "touchpad" attr set on these IdeaPads? 
~10 pre-2015 IdeaPad DSDTs I checked all either had MSS[0-9] or PS2K
devices representing the touchpad, and I believe they were all using
PS/2 interface. Here are these IdeaPads:

S10-3 (PS2M)
S12 (PS2M)
Y530 (PS2M)
B550 (PS2M)
U510 (MSS*)
G500 (MSS*)
G570 (MSS*)
As an exception; Yoga 910(2016) has both TPD0 and PS2M, so we should
check if PS2M is present/connected I think.

So my suggestion is, if we're sure touchpad is connected over PS/2,
(it's up to you how to detect it, it can be either checking AUX on i8042
or checking MSS[0-9] and PS2K on ACPI) we can expose "touchpad" attr.

Otherwise... I say we can stop using VPCCMD_W_TOUCHPAD. I think we
have no reliable way to check if it works. Maybe we can use i2c APIs
instead to toggle touchpad instead, but I don't know if this can work/
possible at all.
Oh, or maybe we can use a whilelist for that? For the IdeaPads that have
i2c touchpads and also controllable by EC.

> 2. Disable the touchpad in the driver if the laptop has the LED and EC
> doesn't disable the touchpad (Z570).

I think all IdeaPads with touchpad LED use PS/2 for touchpad (because
they're old), so I repeat my recommendation above.

> 3. Just send KEY_TOUCHPAD_TOGGLE if the laptop doesn't have the LED
> and EC doesn't disable the touchpad (any known models?).

I agree. I think mainly 2015+ IdeaPads fall under this category, so my
suggestion is to not expose/use VPCCMD_W_TOUCHPAD on any IdeaPads with i2c
touchpad, and send KEY_TOUCHPAD_TOGGLE instead.

Best,
Eray

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi Eray,

On 11/10/22 13:00, Eray Orçunus wrote:
> Hi all,
> 
> I don't know is this correct; but I'll reply to 2 messages, firstly to OP.

It is fine, no worries :)

> On Wed, 9 Nov 2022 at 22:59, Hans de Goede <hdegoede@redhat.com> wrote:
>> 3. Please test
>> ==============
>>
>> If you have ideapads where touchpad_ctrl_via_ec should be 1 because
>> it is needed to toggle the touchpad on/off with the hotkey.
>>
>> Or the exact opposite you have ideapads where it should be disabled
>> because ideapad_sync_touchpad_state() turning off the ps/2 aux port
>> is causing problems.
>>
>> Then please give the attached patch a try. Note this applies on
>> top of Torvald's current master, or on top of 6.0 with :
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
>> added.
> 
> While I agree with your findings(and thanks for your effort, I also tried
> to solve this but later gave up), they doesn't apply to one type of
> IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
> This is because my IdeaPad has this features:
> 
> - i8042.noaux doesn't affect touchpad, and it's connected over i2c
> - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
>   ACPI event, doesn't do anything else
> - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
> - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
> - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)

So if i8042.noaux does not do anything, then why do you want to add
"SYNA2B33" to the list of ACPI HIDs for which we set:

features.touchpad_ctrl_via_ec=0;

?

IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?

Or are you seeing bad behavior on some other modes? If yes, then what
is the bad behavior you are seeing on other models ?

> So I agree on Maxim's solutions, and I have an idea on how to implement
> it, I will explain it below.
> 
> On 10 Nov 2022 at 00:39, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>> Normally, when EC doesn't disable the
>> touchpad, we can send KEY_TOUCHPAD_TOGGLE and leave the action to the
>> DE. However, as Z570 has this LED, which will get out of sync with the
>> touchpad state, we can't use KEY_TOUCHPAD_TOGGLE.
>>
>> That leads to the following idea: if newer Lenovos have issues with
>> VPCCMD_R_TOUCHPAD and they don't have the touchpad LED, we can just
>> use KEY_TOUCHPAD_TOGGLE for them. However, if it turns out that some
>> Lenovo model does actually disable the touchpad in hardware, this also
>> needs to be taken into account.
> 
> Completely agreed.
> 
>> However, this idea doesn't answer the question of how to detect such
>> laptops. I wonder how the Windows driver does it.
> 
> I reverse engineered Lenovo Utility/Hotkeys programs of ~5 IdeaPads,
> and I can say they send VPCCMD_W_TOUCHPAD unconditionally, without any
> check if it's working or not. This includes 720s and 520-15IKB, which
> don't have working VPCCMD_W_TOUCHPAD. This is because it's the
> working-in-background touchpad helper program catches the hotkey ACPI
> event (I think) and handles the situation. This can be confirmed by
> killing Lenovo programs on these IdeaPads won't affect the touchpad
> toggle hotkey.

Ok, VPCCMD_W_TOUCHPAD with a value of 1 not being harmful
matches with current the ideapad-laptop code which actually does
this when features.touchpad_ctrl_via_ec == 0:

       if (!priv->features.touchpad_ctrl_via_ec)
                write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, 1);

I think that before trying to come up with a solution here,
we first need to identify what part of the code enabled by
touchpad_ctrl_via_ec == 1 is actually causing issues for people.

Unfortunately non of the commit messages of the commits adding
touchpad_ctrl_via_ec=0 quirks actually properly describe the
unwanted behavior users are seeing with touchpad_ctrl_via_ec=1

I'm guessing that this part:

                unsigned char param;
                /*
                 * Some IdeaPads don't really turn off touchpad - they only
                 * switch the LED state. We (de)activate KBC AUX port to turn
                 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
                 * KEY_TOUCHPAD_ON to not to get out of sync with LED
                 */
                i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);

May cause issues on some models. It definitely feels fishy and
I would like to disable this except on models where:

1. There is a LED controlled by some touchpad on/off hotkey; and
2. The EC fails to disable the touchpad itself

Which would currently mean only enable this bit on Maxim's Z570
using a DMI based allow list.

###

I know from experience that this part:

                ideapad_input_report(priv, value ? 67 : 66);

Also is a bit troublesome on some models causing spurious touchpad
on/off OSD notifications after suspend/resume.

I'm not sure what to do there. If the touchpad gets disabled
in hw by the EC itself then sending KEY_TOUCHPAD_TOGGLE seems
wrong, especially since then the userspace touchpad state and
the EC touchpad state can get out of sync...

>> I need to check it, but it's entirely possible that the touchpad state
>> is preserved after reboot. In that case, reading VPCCMD_R_TOUCHPAD as
>> 0 on boot would be normal. I think it's also possible that the user can
>> disable the touchpad before Linux starts. This approach looks problematic.
> 
> I agree on we can't rely VPCCMD_R_TOUCHPAD to be 0 on boot, because
> it always returns 1 for me, yet I don't have working VPCCMD_W_TOUCHPAD.

Ok, so that idea is a no go then, thanks for testing.

> 
>> 1. Just send KEY_TOUCHPAD_ON/OFF if EC disables the touchpad (any
>> known models?).
> 
> If you mean the cases when the laptop toggles touchpad itself when
> touchpad hotkey is pressed. Good news is(I hope): I don't think there are
> such IdeaPads, yet this still needs confirmation.

At least on Maxim's Z570 the laptop does toggle the value
returned by VPCCMD_R_TOUCHPAD and the LED it self, while at the
same time not actually disabling the touchpad.

> If you mean the cases where VPCCMD_W_TOUCHPAD works, I agree with you,
> yet we don't know how to find/detect these IdeaPads :/
> 
> But if we can assume the IdeaPads produced before 2015 all have PS/2
> touchpads (at least the ~10 DSDTs I checked all had one), and/or have
> VPCCMD_W_TOUCHPAD, why don't we send both VPCCMD_W_TOUCHPAD and i8042
> command when "touchpad" attr set on these IdeaPads? 
> ~10 pre-2015 IdeaPad DSDTs I checked all either had MSS[0-9] or PS2K
> devices representing the touchpad, and I believe they were all using
> PS/2 interface. Here are these IdeaPads:
> 
> S10-3 (PS2M)
> S12 (PS2M)
> Y530 (PS2M)
> B550 (PS2M)
> U510 (MSS*)
> G500 (MSS*)
> G570 (MSS*)
> As an exception; Yoga 910(2016) has both TPD0 and PS2M, so we should
> check if PS2M is present/connected I think.
> 
> So my suggestion is, if we're sure touchpad is connected over PS/2,
> (it's up to you how to detect it, it can be either checking AUX on i8042
> or checking MSS[0-9] and PS2K on ACPI) we can expose "touchpad" attr.
> 
> Otherwise... I say we can stop using VPCCMD_W_TOUCHPAD. I think we
> have no reliable way to check if it works. Maybe we can use i2c APIs
> instead to toggle touchpad instead, but I don't know if this can work/
> possible at all.
> Oh, or maybe we can use a whilelist for that? For the IdeaPads that have
> i2c touchpads and also controllable by EC.
> 
>> 2. Disable the touchpad in the driver if the laptop has the LED and EC
>> doesn't disable the touchpad (Z570).
> 
> I think all IdeaPads with touchpad LED use PS/2 for touchpad (because
> they're old), so I repeat my recommendation above.
> 
>> 3. Just send KEY_TOUCHPAD_TOGGLE if the laptop doesn't have the LED
>> and EC doesn't disable the touchpad (any known models?).
> 
> I agree. I think mainly 2015+ IdeaPads fall under this category, so my
> suggestion is to not expose/use VPCCMD_W_TOUCHPAD on any IdeaPads with i2c
> touchpad, and send KEY_TOUCHPAD_TOGGLE instead.

The problem is this all relies on being able to detect i2c vs ps/2
touchpads which is not as simple as it sounds. Specifically many
new touchpad are connected to both busses at the same time, offering
a ps/2 mode by default for compatibility with older software / os-es
and being able to switch to a modern i2c/smbus mode for better performance.

I've asked Benjamin Tissoires, the kernel expert on this about this
and his answer was that it is almost impossible to determine if
a touchpad is going to be using ps/2 or i2c without first waiting
for the whole driver stack to have initialized and then see which
driver(s) are attached and I guess even then the touchpad might
show up as both ps/2 + i2c with only one of them actually generating
events:

https://lore.kernel.org/linux-input/ae50236e-1ce8-b526-9c17-7bc0463ebb86@redhat.com/T/#u

So based on Benjamin's answer I'm afraid that trying to differentiate
between i2c vs ps2 is not really doable.

###

An alternative proposal for a fix for this
==========================================

0: identify the currently troublesome behavior
---------------------------------------------------

As mentioned above I believe there are 2 potentially
troublesome things done one newer laptops when
touchpad_ctrl_via_ec == 1:

1. i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE)
2. Sending of wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events.

1: Fixing undesirable i8042_command(... I8042_CMD_AUX_DISABLE) calls
--------------------------------------------------------------------

My suggestion is to move to an allow-list for this and for now
populate that list with only the DMI strings for Maxim's Z570 and see
from there.

2: Fixing wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events
------------------------------------------------------------------

There are 2 subcases here:

2.1: Fix sending of KEY_TOUCHPAD_OFF/_ON events at probe/resume
---------------------------------------------------------------

We can simply fix this by giving ideapad_sync_touchpad_state()
a parameter to let it know if events should be send at all and
set that parameter to false when called on probe/resume

2.2: Sending wrong KEY_TOUCHPAD_OFF/_ON events at toggle time
-------------------------------------------------------------

On models where the EC does not control the touchpad at all,
currently we still do ideapad_sync_touchpad_state() and then
send either KEY_TOUCHPAD_OFF or _ON based on the value read
from VPCCMD_R_TOUCHPAD.

But on these models VPCCMD_R_TOUCHPAD always returns 0 or 1,
so we always send KEY_TOUCHPAD_OFF or always send KEY_TOUCHPAD_ON,
instead of toggling the state / asking userspace to toggle
its sw touchpad on/off state.

I believe we can detect this case by checking that
the value read from VPCCMD_R_TOUCHPAD has not changed
despite us receiving a notify with bit 5 being set in
the value read from VPCCMD_R_VPC1.

My suggestion to fix this case is to detect when the value
read from VPCCMD_R_TOUCHPAD does not change and in that
case send KEY_TOUCHPAD_TOGGLE to userspace.

Regards,

Hans


p.s.

If any of you wants to discuss this in realtime you can
find me as hansg on irc in #fedora-laptops on libera.chat
or in #dri-devel on OFTC.

Re: ideapad-laptop touchpad handling problems, request for help

[Eray Orçunus]

Hi Hans,

On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/10/22 13:00, Eray Orçunus wrote:
> > While I agree with your findings(and thanks for your effort, I also tried
> > to solve this but later gave up), they doesn't apply to one type of
> > IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
> > This is because my IdeaPad has this features:
> > 
> > - i8042.noaux doesn't affect touchpad, and it's connected over i2c
> > - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
> >   ACPI event, doesn't do anything else
> > - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
> > - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
> > - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)
> 
> So if i8042.noaux does not do anything, then why do you want to add
> "SYNA2B33" to the list of ACPI HIDs for which we set:
> 
> features.touchpad_ctrl_via_ec=0;
> 
> ?
> 
> IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?
> 
> Or are you seeing bad behavior on some other modes? If yes, then what
> is the bad behavior you are seeing on other models ?

It was just because I didn't want to have a not working "touchpad"
attribute :) I used/still using several GNOME extensions and they show
me "Touchpad" toggle just because I have "touchpad" attribute exposed
there, which is doing nothing, and misleading.

But I would understand if you don't want to touch it at that stage, and
you would rather prefer not working "touchpad" attributes to not
exposed "touchpad" attributes that would have been perfectly working.

> I'm guessing that this part:
> 
>                 unsigned char param;
>                 /*
>                  * Some IdeaPads don't really turn off touchpad - they only
>                  * switch the LED state. We (de)activate KBC AUX port to turn
>                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
>                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
>                  */
>                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
> 
> May cause issues on some models. It definitely feels fishy and
> I would like to disable this except on models where:
> 
> 1. There is a LED controlled by some touchpad on/off hotkey; and
> 2. The EC fails to disable the touchpad itself
> 
> Which would currently mean only enable this bit on Maxim's Z570
> using a DMI based allow list.

Agreed, but do you mean "and" or "or"? I mean we can also include the
models that toggle touchpad value while not really toggling the touchpad
(just as you described below) and don't have a touchpad LED (but I don't
know if such model exists really), this way they won't go out of sync
regardless of is there a touchpad LED or not.

> At least on Maxim's Z570 the laptop does toggle the value
> returned by VPCCMD_R_TOUCHPAD and the LED it self, while at the
> same time not actually disabling the touchpad.
> The problem is this all relies on being able to detect i2c vs ps/2
> touchpads which is not as simple as it sounds. Specifically many
> new touchpad are connected to both busses at the same time, offering
> a ps/2 mode by default for compatibility with older software / os-es
> and being able to switch to a modern i2c/smbus mode for better performance.
> 
> I've asked Benjamin Tissoires, the kernel expert on this about this
> and his answer was that it is almost impossible to determine if
> a touchpad is going to be using ps/2 or i2c without first waiting
> for the whole driver stack to have initialized and then see which
> driver(s) are attached and I guess even then the touchpad might
> show up as both ps/2 + i2c with only one of them actually generating
> events:
> 
> https://lore.kernel.org/linux-input/ae50236e-1ce8-b526-9c17-7bc0463ebb86@redhat.com/T/#u
> 
> So based on Benjamin's answer I'm afraid that trying to differentiate
> between i2c vs ps2 is not really doable.

Thanks for the explanation and the link, but as Benjamin said, I believe
we can use ACPI table for detecting PS/2 devices. I believe the DSDTs
with PS2M(and probably MSS[0-9] too) devices probably have PS/2 touchpad,
and have working EC and i8042 commands. Yet this still needs
confirmation/testing, and I think should be resorted if your suggestion
below won't work - your suggestion looks better and easier.

> 1: Fixing undesirable i8042_command(... I8042_CMD_AUX_DISABLE) calls
> --------------------------------------------------------------------
> 
> My suggestion is to move to an allow-list for this and for now
> populate that list with only the DMI strings for Maxim's Z570 and see
> from there.

Agreed.

> 
> 2: Fixing wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events
> ------------------------------------------------------------------
> 
> There are 2 subcases here:
> 
> 2.1: Fix sending of KEY_TOUCHPAD_OFF/_ON events at probe/resume
> ---------------------------------------------------------------
> 
> We can simply fix this by giving ideapad_sync_touchpad_state()
> a parameter to let it know if events should be send at all and
> set that parameter to false when called on probe/resume

Agreed.

> 2.2: Sending wrong KEY_TOUCHPAD_OFF/_ON events at toggle time
> -------------------------------------------------------------
> 
> On models where the EC does not control the touchpad at all,
> currently we still do ideapad_sync_touchpad_state() and then
> send either KEY_TOUCHPAD_OFF or _ON based on the value read
> from VPCCMD_R_TOUCHPAD.
> 
> But on these models VPCCMD_R_TOUCHPAD always returns 0 or 1,
> so we always send KEY_TOUCHPAD_OFF or always send KEY_TOUCHPAD_ON,
> instead of toggling the state / asking userspace to toggle
> its sw touchpad on/off state.
> 
> I believe we can detect this case by checking that
> the value read from VPCCMD_R_TOUCHPAD has not changed
> despite us receiving a notify with bit 5 being set in
> the value read from VPCCMD_R_VPC1.
> 
> My suggestion to fix this case is to detect when the value
> read from VPCCMD_R_TOUCHPAD does not change and in that
> case send KEY_TOUCHPAD_TOGGLE to userspace.

While this is an awesome idea, what about doing this at boot?
Like we will send 0 first, then check if it reads 0, then send 1,
and confirm if it reads 1. This would be the ultimate solution, and
would also fix my "cosmetic" concerns :)

Best,
Eray

Re: ideapad-laptop touchpad handling problems, request for help

[Maxim Mikityanskiy]

On Thu, 10 Nov 2022 at 18:42, Eray Orçunus <erayorcunus@gmail.com> wrote:
>
> Hi Hans,
>
> On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@redhat.com> wrote:
> > On 11/10/22 13:00, Eray Orçunus wrote:
> > > While I agree with your findings(and thanks for your effort, I also tried
> > > to solve this but later gave up), they doesn't apply to one type of
> > > IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
> > > This is because my IdeaPad has this features:
> > >
> > > - i8042.noaux doesn't affect touchpad, and it's connected over i2c
> > > - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
> > >   ACPI event, doesn't do anything else
> > > - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
> > > - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
> > > - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)
> >
> > So if i8042.noaux does not do anything, then why do you want to add
> > "SYNA2B33" to the list of ACPI HIDs for which we set:
> >
> > features.touchpad_ctrl_via_ec=0;
> >
> > ?
> >
> > IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?
> >
> > Or are you seeing bad behavior on some other modes? If yes, then what
> > is the bad behavior you are seeing on other models ?
>
> It was just because I didn't want to have a not working "touchpad"
> attribute :) I used/still using several GNOME extensions and they show
> me "Touchpad" toggle just because I have "touchpad" attribute exposed
> there, which is doing nothing, and misleading.
>
> But I would understand if you don't want to touch it at that stage, and
> you would rather prefer not working "touchpad" attributes to not
> exposed "touchpad" attributes that would have been perfectly working.
>
> > I'm guessing that this part:
> >
> >                 unsigned char param;
> >                 /*
> >                  * Some IdeaPads don't really turn off touchpad - they only
> >                  * switch the LED state. We (de)activate KBC AUX port to turn
> >                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
> >                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
> >                  */
> >                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
> >
> > May cause issues on some models. It definitely feels fishy and
> > I would like to disable this except on models where:
> >
> > 1. There is a LED controlled by some touchpad on/off hotkey; and
> > 2. The EC fails to disable the touchpad itself
> >
> > Which would currently mean only enable this bit on Maxim's Z570
> > using a DMI based allow list.

A small note on the DMI allow-list: I don't think Z570 is the only
laptop where EC fails to disable the touchpad. While I would like this
hack to affect as few laptops as possible, I would expect that other
similar models produced in the same time period suffer from the same
issue, and I don't think we have the full list of them.

> Agreed, but do you mean "and" or "or"? I mean we can also include the
> models that toggle touchpad value while not really toggling the touchpad
> (just as you described below) and don't have a touchpad LED (but I don't
> know if such model exists really), this way they won't go out of sync
> regardless of is there a touchpad LED or not.
>
> > At least on Maxim's Z570 the laptop does toggle the value
> > returned by VPCCMD_R_TOUCHPAD and the LED it self, while at the
> > same time not actually disabling the touchpad.
> > The problem is this all relies on being able to detect i2c vs ps/2
> > touchpads which is not as simple as it sounds. Specifically many
> > new touchpad are connected to both busses at the same time, offering
> > a ps/2 mode by default for compatibility with older software / os-es
> > and being able to switch to a modern i2c/smbus mode for better performance.
> >
> > I've asked Benjamin Tissoires, the kernel expert on this about this
> > and his answer was that it is almost impossible to determine if
> > a touchpad is going to be using ps/2 or i2c without first waiting
> > for the whole driver stack to have initialized and then see which
> > driver(s) are attached and I guess even then the touchpad might
> > show up as both ps/2 + i2c with only one of them actually generating
> > events:
> >
> > https://lore.kernel.org/linux-input/ae50236e-1ce8-b526-9c17-7bc0463ebb86@redhat.com/T/#u
> >
> > So based on Benjamin's answer I'm afraid that trying to differentiate
> > between i2c vs ps2 is not really doable.
>
> Thanks for the explanation and the link, but as Benjamin said, I believe
> we can use ACPI table for detecting PS/2 devices. I believe the DSDTs
> with PS2M(and probably MSS[0-9] too) devices probably have PS/2 touchpad,
> and have working EC and i8042 commands. Yet this still needs
> confirmation/testing, and I think should be resorted if your suggestion
> below won't work - your suggestion looks better and easier.
>
> > 1: Fixing undesirable i8042_command(... I8042_CMD_AUX_DISABLE) calls
> > --------------------------------------------------------------------
> >
> > My suggestion is to move to an allow-list for this and for now
> > populate that list with only the DMI strings for Maxim's Z570 and see
> > from there.
>
> Agreed.
>
> >
> > 2: Fixing wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events
> > ------------------------------------------------------------------
> >
> > There are 2 subcases here:
> >
> > 2.1: Fix sending of KEY_TOUCHPAD_OFF/_ON events at probe/resume
> > ---------------------------------------------------------------
> >
> > We can simply fix this by giving ideapad_sync_touchpad_state()
> > a parameter to let it know if events should be send at all and
> > set that parameter to false when called on probe/resume
>
> Agreed.
>
> > 2.2: Sending wrong KEY_TOUCHPAD_OFF/_ON events at toggle time
> > -------------------------------------------------------------
> >
> > On models where the EC does not control the touchpad at all,
> > currently we still do ideapad_sync_touchpad_state() and then
> > send either KEY_TOUCHPAD_OFF or _ON based on the value read
> > from VPCCMD_R_TOUCHPAD.
> >
> > But on these models VPCCMD_R_TOUCHPAD always returns 0 or 1,
> > so we always send KEY_TOUCHPAD_OFF or always send KEY_TOUCHPAD_ON,
> > instead of toggling the state / asking userspace to toggle
> > its sw touchpad on/off state.
> >
> > I believe we can detect this case by checking that
> > the value read from VPCCMD_R_TOUCHPAD has not changed
> > despite us receiving a notify with bit 5 being set in
> > the value read from VPCCMD_R_VPC1.
> >
> > My suggestion to fix this case is to detect when the value
> > read from VPCCMD_R_TOUCHPAD does not change and in that
> > case send KEY_TOUCHPAD_TOGGLE to userspace.
>
> While this is an awesome idea, what about doing this at boot?
> Like we will send 0 first, then check if it reads 0, then send 1,
> and confirm if it reads 1. This would be the ultimate solution, and
> would also fix my "cosmetic" concerns :)
>
> Best,
> Eray

Re: ideapad-laptop touchpad handling problems, request for help

[Maxim Mikityanskiy]

On Thu, 10 Nov 2022 at 19:54, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>
> On Thu, 10 Nov 2022 at 18:42, Eray Orçunus <erayorcunus@gmail.com> wrote:
> >
> > Hi Hans,
> >
> > On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@redhat.com> wrote:
> > > On 11/10/22 13:00, Eray Orçunus wrote:
> > > > While I agree with your findings(and thanks for your effort, I also tried
> > > > to solve this but later gave up), they doesn't apply to one type of
> > > > IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
> > > > This is because my IdeaPad has this features:
> > > >
> > > > - i8042.noaux doesn't affect touchpad, and it's connected over i2c
> > > > - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
> > > >   ACPI event, doesn't do anything else
> > > > - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
> > > > - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
> > > > - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)
> > >
> > > So if i8042.noaux does not do anything, then why do you want to add
> > > "SYNA2B33" to the list of ACPI HIDs for which we set:
> > >
> > > features.touchpad_ctrl_via_ec=0;
> > >
> > > ?
> > >
> > > IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?
> > >
> > > Or are you seeing bad behavior on some other modes? If yes, then what
> > > is the bad behavior you are seeing on other models ?
> >
> > It was just because I didn't want to have a not working "touchpad"
> > attribute :) I used/still using several GNOME extensions and they show
> > me "Touchpad" toggle just because I have "touchpad" attribute exposed
> > there, which is doing nothing, and misleading.
> >
> > But I would understand if you don't want to touch it at that stage, and
> > you would rather prefer not working "touchpad" attributes to not
> > exposed "touchpad" attributes that would have been perfectly working.
> >
> > > I'm guessing that this part:
> > >
> > >                 unsigned char param;
> > >                 /*
> > >                  * Some IdeaPads don't really turn off touchpad - they only
> > >                  * switch the LED state. We (de)activate KBC AUX port to turn
> > >                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
> > >                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
> > >                  */
> > >                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
> > >
> > > May cause issues on some models. It definitely feels fishy and
> > > I would like to disable this except on models where:
> > >
> > > 1. There is a LED controlled by some touchpad on/off hotkey; and
> > > 2. The EC fails to disable the touchpad itself
> > >
> > > Which would currently mean only enable this bit on Maxim's Z570
> > > using a DMI based allow list.
>
> A small note on the DMI allow-list: I don't think Z570 is the only
> laptop where EC fails to disable the touchpad. While I would like this
> hack to affect as few laptops as possible, I would expect that other
> similar models produced in the same time period suffer from the same
> issue, and I don't think we have the full list of them.

And another idea: i8042_command(I8042_CMD_AUX_DISABLE) can be replaced
with installing a i8042_platform_filter that will filter out all
I8042_STR_AUXDATA when the touchpad should be disabled. It's
definitely less fishy, we don't touch the KBC, but simply filter out
events in software.

>
> > Agreed, but do you mean "and" or "or"? I mean we can also include the
> > models that toggle touchpad value while not really toggling the touchpad
> > (just as you described below) and don't have a touchpad LED (but I don't
> > know if such model exists really), this way they won't go out of sync
> > regardless of is there a touchpad LED or not.
> >
> > > At least on Maxim's Z570 the laptop does toggle the value
> > > returned by VPCCMD_R_TOUCHPAD and the LED it self, while at the
> > > same time not actually disabling the touchpad.
> > > The problem is this all relies on being able to detect i2c vs ps/2
> > > touchpads which is not as simple as it sounds. Specifically many
> > > new touchpad are connected to both busses at the same time, offering
> > > a ps/2 mode by default for compatibility with older software / os-es
> > > and being able to switch to a modern i2c/smbus mode for better performance.
> > >
> > > I've asked Benjamin Tissoires, the kernel expert on this about this
> > > and his answer was that it is almost impossible to determine if
> > > a touchpad is going to be using ps/2 or i2c without first waiting
> > > for the whole driver stack to have initialized and then see which
> > > driver(s) are attached and I guess even then the touchpad might
> > > show up as both ps/2 + i2c with only one of them actually generating
> > > events:
> > >
> > > https://lore.kernel.org/linux-input/ae50236e-1ce8-b526-9c17-7bc0463ebb86@redhat.com/T/#u
> > >
> > > So based on Benjamin's answer I'm afraid that trying to differentiate
> > > between i2c vs ps2 is not really doable.
> >
> > Thanks for the explanation and the link, but as Benjamin said, I believe
> > we can use ACPI table for detecting PS/2 devices. I believe the DSDTs
> > with PS2M(and probably MSS[0-9] too) devices probably have PS/2 touchpad,
> > and have working EC and i8042 commands. Yet this still needs
> > confirmation/testing, and I think should be resorted if your suggestion
> > below won't work - your suggestion looks better and easier.
> >
> > > 1: Fixing undesirable i8042_command(... I8042_CMD_AUX_DISABLE) calls
> > > --------------------------------------------------------------------
> > >
> > > My suggestion is to move to an allow-list for this and for now
> > > populate that list with only the DMI strings for Maxim's Z570 and see
> > > from there.
> >
> > Agreed.
> >
> > >
> > > 2: Fixing wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events
> > > ------------------------------------------------------------------
> > >
> > > There are 2 subcases here:
> > >
> > > 2.1: Fix sending of KEY_TOUCHPAD_OFF/_ON events at probe/resume
> > > ---------------------------------------------------------------
> > >
> > > We can simply fix this by giving ideapad_sync_touchpad_state()
> > > a parameter to let it know if events should be send at all and
> > > set that parameter to false when called on probe/resume
> >
> > Agreed.
> >
> > > 2.2: Sending wrong KEY_TOUCHPAD_OFF/_ON events at toggle time
> > > -------------------------------------------------------------
> > >
> > > On models where the EC does not control the touchpad at all,
> > > currently we still do ideapad_sync_touchpad_state() and then
> > > send either KEY_TOUCHPAD_OFF or _ON based on the value read
> > > from VPCCMD_R_TOUCHPAD.
> > >
> > > But on these models VPCCMD_R_TOUCHPAD always returns 0 or 1,
> > > so we always send KEY_TOUCHPAD_OFF or always send KEY_TOUCHPAD_ON,
> > > instead of toggling the state / asking userspace to toggle
> > > its sw touchpad on/off state.
> > >
> > > I believe we can detect this case by checking that
> > > the value read from VPCCMD_R_TOUCHPAD has not changed
> > > despite us receiving a notify with bit 5 being set in
> > > the value read from VPCCMD_R_VPC1.
> > >
> > > My suggestion to fix this case is to detect when the value
> > > read from VPCCMD_R_TOUCHPAD does not change and in that
> > > case send KEY_TOUCHPAD_TOGGLE to userspace.
> >
> > While this is an awesome idea, what about doing this at boot?
> > Like we will send 0 first, then check if it reads 0, then send 1,
> > and confirm if it reads 1. This would be the ultimate solution, and
> > would also fix my "cosmetic" concerns :)
> >
> > Best,
> > Eray

Re: ideapad-laptop touchpad handling problems, request for help

[Eray Orçunus]

Hi,

On 10 Nov 2022 at 21:09, Maxim Mikityanskiy <maxtram95@gmail.com>  wrote:
> A small note on the DMI allow-list: I don't think Z570 is the only
> laptop where EC fails to disable the touchpad. While I would like this
> hack to affect as few laptops as possible, I would expect that other
> similar models produced in the same time period suffer from the same
> issue, and I don't think we have the full list of them.

I just checked Z570 ACPI table, and this is what it does when it receives
VPCCMD_R_TOUCHPAD:

	VDAT = TPEN /* \_SB_.PCI0.LPCB.EC0_.TPEN */
	If ((TPEN == One))
	{
	    GL04 |= 0x02
	}
	Else
	{
	    GL04 &= 0xFD
	}

VDAT is the data returned to user.
So we can say that TPEN is the logical state of touchpad key, and GL04
is state of touchpad LED or series of LEDs.

VPCCMD_W_TOUCHPAD is nulled, it doesn't work.

I also checked which DSDTs I have (13 DSDTs from 2008 to this year)
contain TPEN, and turned out it was only S12, from 2009. It also had
nulled VPCCMD_W_TOUCHPAD, and returns TPEN on VPCCMD_R_TOUCHPAD, except
it doesn't have an LED or GL04.

So, it's possible that we can only check if TPEN exists on ACPI table,
instead of having a white-list.

-eray

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 8063 bytes --]

Hi,

On 11/10/22 17:42, Eray Orçunus wrote:
> Hi Hans,
> 
> On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@redhat.com> wrote:
>> On 11/10/22 13:00, Eray Orçunus wrote:
>>> While I agree with your findings(and thanks for your effort, I also tried
>>> to solve this but later gave up), they doesn't apply to one type of
>>> IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
>>> This is because my IdeaPad has this features:
>>>
>>> - i8042.noaux doesn't affect touchpad, and it's connected over i2c
>>> - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
>>>   ACPI event, doesn't do anything else
>>> - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
>>> - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
>>> - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)
>>
>> So if i8042.noaux does not do anything, then why do you want to add
>> "SYNA2B33" to the list of ACPI HIDs for which we set:
>>
>> features.touchpad_ctrl_via_ec=0;
>>
>> ?
>>
>> IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?
>>
>> Or are you seeing bad behavior on some other modes? If yes, then what
>> is the bad behavior you are seeing on other models ?
> 
> It was just because I didn't want to have a not working "touchpad"
> attribute :) I used/still using several GNOME extensions and they show
> me "Touchpad" toggle just because I have "touchpad" attribute exposed
> there, which is doing nothing, and misleading.

So you are using a gnome-shell extension specifically for ideapads,
which actually uses the sysfs attributes registered by the ideapad-laptop
driver?

I did not expect that. My plan is to pretty much not care about if
the touchpad sysfs attribute works and just document that it may
not work on newer ideapad models.

I believe that the best fix for the ideapad specific extension
might be to just file a bug against the extension and ask it
to remove the touchpad on/off support.

gnome already has a prefectly working touchpad on/off toggle
on the normal control-panel so it is not like removing the
ideapad specific one will cause any loss of functionality.

> But I would understand if you don't want to touch it at that stage, and
> you would rather prefer not working "touchpad" attributes to not
> exposed "touchpad" attributes that would have been perfectly working.

>> I'm guessing that this part:
>>
>>                 unsigned char param;
>>                 /*
>>                  * Some IdeaPads don't really turn off touchpad - they only
>>                  * switch the LED state. We (de)activate KBC AUX port to turn
>>                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
>>                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
>>                  */
>>                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
>>
>> May cause issues on some models. It definitely feels fishy and
>> I would like to disable this except on models where:
>>
>> 1. There is a LED controlled by some touchpad on/off hotkey; and
>> 2. The EC fails to disable the touchpad itself
>>
>> Which would currently mean only enable this bit on Maxim's Z570
>> using a DMI based allow list.
> 
> Agreed, but do you mean "and" or "or"? I mean we can also include the
> models that toggle touchpad value while not really toggling the touchpad
> (just as you described below) and don't have a touchpad LED (but I don't
> know if such model exists really), this way they won't go out of sync
> regardless of is there a touchpad LED or not.

I meant "and" since I was talking about the Z570.

But you are right that if there are other models which do toggle
the value returned when reading from VPCCMD_R_TOUCHPAD, but don't
actually turn the touchpad on/off, that those then will also need
ideapad-laptop to control the ps2 aux port.


>> At least on Maxim's Z570 the laptop does toggle the value
>> returned by VPCCMD_R_TOUCHPAD and the LED it self, while at the
>> same time not actually disabling the touchpad.
>> The problem is this all relies on being able to detect i2c vs ps/2
>> touchpads which is not as simple as it sounds. Specifically many
>> new touchpad are connected to both busses at the same time, offering
>> a ps/2 mode by default for compatibility with older software / os-es
>> and being able to switch to a modern i2c/smbus mode for better performance.
>>
>> I've asked Benjamin Tissoires, the kernel expert on this about this
>> and his answer was that it is almost impossible to determine if
>> a touchpad is going to be using ps/2 or i2c without first waiting
>> for the whole driver stack to have initialized and then see which
>> driver(s) are attached and I guess even then the touchpad might
>> show up as both ps/2 + i2c with only one of them actually generating
>> events:
>>
>> https://lore.kernel.org/linux-input/ae50236e-1ce8-b526-9c17-7bc0463ebb86@redhat.com/T/#u
>>
>> So based on Benjamin's answer I'm afraid that trying to differentiate
>> between i2c vs ps2 is not really doable.
> 
> Thanks for the explanation and the link, but as Benjamin said, I believe
> we can use ACPI table for detecting PS/2 devices. I believe the DSDTs
> with PS2M(and probably MSS[0-9] too) devices probably have PS/2 touchpad,
> and have working EC and i8042 commands. Yet this still needs
> confirmation/testing, and I think should be resorted if your suggestion
> below won't work - your suggestion looks better and easier.

I don't think there are any guarantees that models with touchpads
which support both ps2 + i2c mode won't have the PS2M or MSS[0-9]
devices, especially since the ps2 support is there to e.g. have
a working mouse in the Windows installer before a touchpad specific
i2c driver is loaded / available.  And the windows installer likely
needs the ACPI ps/2 devices to detect the touchpad ...


>> 1: Fixing undesirable i8042_command(... I8042_CMD_AUX_DISABLE) calls
>> --------------------------------------------------------------------
>>
>> My suggestion is to move to an allow-list for this and for now
>> populate that list with only the DMI strings for Maxim's Z570 and see
>> from there.
> 
> Agreed.
> 
>>
>> 2: Fixing wrong/spurious KEY_TOUCHPAD_OFF / KEY_TOUCHPAD_ON events
>> ------------------------------------------------------------------
>>
>> There are 2 subcases here:
>>
>> 2.1: Fix sending of KEY_TOUCHPAD_OFF/_ON events at probe/resume
>> ---------------------------------------------------------------
>>
>> We can simply fix this by giving ideapad_sync_touchpad_state()
>> a parameter to let it know if events should be send at all and
>> set that parameter to false when called on probe/resume
> 
> Agreed.
> 
>> 2.2: Sending wrong KEY_TOUCHPAD_OFF/_ON events at toggle time
>> -------------------------------------------------------------
>>
>> On models where the EC does not control the touchpad at all,
>> currently we still do ideapad_sync_touchpad_state() and then
>> send either KEY_TOUCHPAD_OFF or _ON based on the value read
>> from VPCCMD_R_TOUCHPAD.
>>
>> But on these models VPCCMD_R_TOUCHPAD always returns 0 or 1,
>> so we always send KEY_TOUCHPAD_OFF or always send KEY_TOUCHPAD_ON,
>> instead of toggling the state / asking userspace to toggle
>> its sw touchpad on/off state.
>>
>> I believe we can detect this case by checking that
>> the value read from VPCCMD_R_TOUCHPAD has not changed
>> despite us receiving a notify with bit 5 being set in
>> the value read from VPCCMD_R_VPC1.
>>
>> My suggestion to fix this case is to detect when the value
>> read from VPCCMD_R_TOUCHPAD does not change and in that
>> case send KEY_TOUCHPAD_TOGGLE to userspace.
> 
> While this is an awesome idea, what about doing this at boot?
> Like we will send 0 first, then check if it reads 0, then send 1,
> and confirm if it reads 1. This would be the ultimate solution, and
> would also fix my "cosmetic" concerns :)

Ok, I've attached a patch series implementing this, please let me
know what you think; and if possible please test this.

Regards,

Hans



[-- Attachment #2: 0001-platform-x86-ideapad-laptop-Revert-check-for-touchpa.patch --]
[-- Type: text/x-patch, Size: 2407 bytes --]

From 47f03d8d93e3b1319f6477357755147cb7ff45b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eray=20Or=C3=A7unus?= <erayorcunus@gmail.com>
Date: Sat, 29 Oct 2022 15:03:06 +0300
Subject: [PATCH 1/4] platform/x86: ideapad-laptop: Revert "check for touchpad
 support in _CFG"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Last 8 bit of _CFG started being used in later IdeaPads, thus 30th bit
doesn't always show whether device supports touchpad or touchpad switch.
Remove checking bit 30 of _CFG, so older IdeaPads like S10-3 can switch
touchpad again via touchpad attribute.

This reverts commit b3ed1b7fe378 ("platform/x86: ideapad-laptop: check for
touchpad support in _CFG").

Signed-off-by: Eray Orçunus <erayorcunus@gmail.com>
Link: https://lore.kernel.org/r/20221029120311.11152-2-erayorcunus@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/ideapad-laptop.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 33b3dfdd1b08..870c6f4c4245 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -46,11 +46,10 @@ static const char *const ideapad_wmi_fnesc_events[] = {
 #endif
 
 enum {
-	CFG_CAP_BT_BIT       = 16,
-	CFG_CAP_3G_BIT       = 17,
-	CFG_CAP_WIFI_BIT     = 18,
-	CFG_CAP_CAM_BIT      = 19,
-	CFG_CAP_TOUCHPAD_BIT = 30,
+	CFG_CAP_BT_BIT   = 16,
+	CFG_CAP_3G_BIT   = 17,
+	CFG_CAP_WIFI_BIT = 18,
+	CFG_CAP_CAM_BIT  = 19,
 };
 
 enum {
@@ -371,8 +370,6 @@ static int debugfs_cfg_show(struct seq_file *s, void *data)
 		seq_puts(s, " wifi");
 	if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg))
 		seq_puts(s, " camera");
-	if (test_bit(CFG_CAP_TOUCHPAD_BIT, &priv->cfg))
-		seq_puts(s, " touchpad");
 	seq_puts(s, "\n");
 
 	seq_puts(s, "Graphics: ");
@@ -665,8 +662,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
 	else if (attr == &dev_attr_fn_lock.attr)
 		supported = priv->features.fn_lock;
 	else if (attr == &dev_attr_touchpad.attr)
-		supported = priv->features.touchpad_ctrl_via_ec &&
-			    test_bit(CFG_CAP_TOUCHPAD_BIT, &priv->cfg);
+		supported = priv->features.touchpad_ctrl_via_ec;
 	else if (attr == &dev_attr_usb_charging.attr)
 		supported = priv->features.usb_charging;
 
-- 
2.37.3


[-- Attachment #3: 0002-platform-x86-ideapad-laptop-Refactor-ideapad_sync_to.patch --]
[-- Type: text/x-patch, Size: 2265 bytes --]

From cb8b3117193ffa89a9ba1971640f37f032a6f2ed Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 10 Nov 2022 17:00:00 +0100
Subject: [PATCH 2/4] platform/x86: ideapad-laptop: Refactor
 ideapad_sync_touchpad_state()

Add an error exit for read_ec_data() failing instead of putting the main
body in an if (success) block.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/ideapad-laptop.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 870c6f4c4245..7ab638069df9 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1398,23 +1398,26 @@ static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
 static void ideapad_sync_touchpad_state(struct ideapad_private *priv)
 {
 	unsigned long value;
+	unsigned char param;
+	int ret;
 
 	if (!priv->features.touchpad_ctrl_via_ec)
 		return;
 
 	/* Without reading from EC touchpad LED doesn't switch state */
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) {
-		unsigned char param;
-		/*
-		 * Some IdeaPads don't really turn off touchpad - they only
-		 * switch the LED state. We (de)activate KBC AUX port to turn
-		 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
-		 * KEY_TOUCHPAD_ON to not to get out of sync with LED
-		 */
-		i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
-		ideapad_input_report(priv, value ? 67 : 66);
-		sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
-	}
+	ret = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value);
+	if (ret)
+		return;
+
+	/*
+	 * Some IdeaPads don't really turn off touchpad - they only
+	 * switch the LED state. We (de)activate KBC AUX port to turn
+	 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
+	 * KEY_TOUCHPAD_ON to not to get out of sync with LED
+	 */
+	i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
+	ideapad_input_report(priv, value ? 67 : 66);
+	sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
 }
 
 static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
-- 
2.37.3


[-- Attachment #4: 0003-platform-x86-ideapad-laptop-Do-not-send-KEY_TOUCHPAD.patch --]
[-- Type: text/x-patch, Size: 2588 bytes --]

From 93103ff85ce83f4a21973ee26ba50319f5f8236b Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 10 Nov 2022 17:03:46 +0100
Subject: [PATCH 3/4] platform/x86: ideapad-laptop: Do not send KEY_TOUCHPAD*
 events on probe / resume

The sending of KEY_TOUCHPAD* events is causing spurious touchpad OSD
showing on resume.

Disable the sending of events on probe / resume to fix this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/ideapad-laptop.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 7ab638069df9..0535bb602eee 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1395,7 +1395,7 @@ static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
 /*
  * module init/exit
  */
-static void ideapad_sync_touchpad_state(struct ideapad_private *priv)
+static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_events)
 {
 	unsigned long value;
 	unsigned char param;
@@ -1416,8 +1416,11 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv)
 	 * KEY_TOUCHPAD_ON to not to get out of sync with LED
 	 */
 	i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
-	ideapad_input_report(priv, value ? 67 : 66);
-	sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
+
+	if (send_events) {
+		ideapad_input_report(priv, value ? 67 : 66);
+		sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
+	}
 }
 
 static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
@@ -1458,7 +1461,7 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 			ideapad_sync_rfk_state(priv);
 			break;
 		case 5:
-			ideapad_sync_touchpad_state(priv);
+			ideapad_sync_touchpad_state(priv, true);
 			break;
 		case 4:
 			ideapad_backlight_notify_brightness(priv);
@@ -1645,7 +1648,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 			ideapad_register_rfkill(priv, i);
 
 	ideapad_sync_rfk_state(priv);
-	ideapad_sync_touchpad_state(priv);
+	ideapad_sync_touchpad_state(priv, false);
 
 	err = ideapad_dytc_profile_init(priv);
 	if (err) {
@@ -1747,7 +1750,7 @@ static int ideapad_acpi_resume(struct device *dev)
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 
 	ideapad_sync_rfk_state(priv);
-	ideapad_sync_touchpad_state(priv);
+	ideapad_sync_touchpad_state(priv, false);
 
 	if (priv->dytc)
 		dytc_profile_refresh(priv);
-- 
2.37.3


[-- Attachment #5: 0004-platform-x86-ideapad-laptop-Rework-touchpad-control-.patch --]
[-- Type: text/x-patch, Size: 6736 bytes --]

From b4833f5308a0e90afc0ef4a6ec57b804a89c4a87 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 10 Nov 2022 19:37:26 +0100
Subject: [PATCH 4/4] platform/x86: ideapad-laptop: Rework touchpad control
 code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Recently there have been multiple patches to disable the ideapad-laptop's
touchpad control code, because it is causing issues on various laptops,
see the Fixes tags for this commit.

The turning on/off of the ps2 aux port was added specifically for
the IdeaPad Z570, where the EC does toggle the touchpad on/off LED and
toggles the value returned by reading VPCCMD_R_TOUCHPAD, but it does not
actually turn on/off the touchpad.

The ideapad-laptop code really should not be messing with the i8042
controller on all devices just for this special case.

Drop the touchpad_ctrl_via_ec flag and add a DMI based allow-list for
devices which need this workaround, populating it with just the
Ideapad Z570 for now.

A related problem is that on recent Ideapad models the EC does not control
the touchpad at all. Check for this by checking if the value read from
VPCCMD_R_TOUCHPAD actually changes when receiving a touchpad-toggle hotkey
event; and if it does not change send KEY_TOUCHPAD_TOGGLE to userspace to
let userspace enable/disable the touchpad in software.

Fixes: d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
Fixes: a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
Cc: Maxim Mikityanskiy <maxtram95@gmail.com>
Cc: GOESSEL Guillaume <g_goessel@outlook.com>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Manyi Li <limanyi@uniontech.com>
Cc: Eray Orçunus <erayorcunus@gmail.com>
Cc: Ike Panhc <ike.pan@canonical.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/ideapad-laptop.c | 56 +++++++++++----------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 0535bb602eee..a529ebc26e5e 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -129,6 +129,7 @@ struct ideapad_private {
 	struct ideapad_dytc_priv *dytc;
 	struct dentry *debug;
 	unsigned long cfg;
+	unsigned long r_touchpad_val;
 	const char *fnesc_guid;
 	struct {
 		bool conservation_mode    : 1;
@@ -137,8 +138,8 @@ struct ideapad_private {
 		bool fn_lock              : 1;
 		bool hw_rfkill_switch     : 1;
 		bool kbd_bl               : 1;
-		bool touchpad_ctrl_via_ec : 1;
 		bool usb_charging         : 1;
+		bool ctrl_ps2_aux_port    : 1;
 	} features;
 	struct {
 		bool initialized;
@@ -661,8 +662,6 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
 		supported = priv->features.fan_mode;
 	else if (attr == &dev_attr_fn_lock.attr)
 		supported = priv->features.fn_lock;
-	else if (attr == &dev_attr_touchpad.attr)
-		supported = priv->features.touchpad_ctrl_via_ec;
 	else if (attr == &dev_attr_usb_charging.attr)
 		supported = priv->features.usb_charging;
 
@@ -1082,6 +1081,7 @@ static const struct key_entry ideapad_keymap[] = {
 	{ KE_KEY,  65, { KEY_PROG4 } },
 	{ KE_KEY,  66, { KEY_TOUCHPAD_OFF } },
 	{ KE_KEY,  67, { KEY_TOUCHPAD_ON } },
+	{ KE_KEY,  68, { KEY_TOUCHPAD_TOGGLE } },
 	{ KE_KEY, 128, { KEY_ESC } },
 	{ KE_END },
 };
@@ -1401,9 +1401,6 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
 	unsigned char param;
 	int ret;
 
-	if (!priv->features.touchpad_ctrl_via_ec)
-		return;
-
 	/* Without reading from EC touchpad LED doesn't switch state */
 	ret = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value);
 	if (ret)
@@ -1412,15 +1409,26 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
 	/*
 	 * Some IdeaPads don't really turn off touchpad - they only
 	 * switch the LED state. We (de)activate KBC AUX port to turn
-	 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
-	 * KEY_TOUCHPAD_ON to not to get out of sync with LED
+	 * the touchpad on and off.
 	 */
-	i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
+	if (priv->features.ctrl_ps2_aux_port)
+		i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
 
 	if (send_events) {
-		ideapad_input_report(priv, value ? 67 : 66);
-		sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
+		/*
+		 * On older models the EC controls the touchpad and toggles it
+		 * on/off itself, in this case we report KEY_TOUCHPAD_ON/_OFF.
+		 * If the EC did not toggle, report KEY_TOUCHPAD_TOGGLE.
+		 */
+		if (value != priv->r_touchpad_val) {
+			ideapad_input_report(priv, value ? 67 : 66);
+			sysfs_notify(&priv->platform_device->dev.kobj, NULL, "touchpad");
+		} else {
+			ideapad_input_report(priv, 68);
+		}
 	}
+
+	priv->r_touchpad_val = value;
 }
 
 static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
@@ -1535,19 +1543,12 @@ static const struct dmi_system_id hw_rfkill_list[] = {
 	{}
 };
 
-static const struct dmi_system_id no_touchpad_switch_list[] = {
-	{
-	.ident = "Lenovo Yoga 3 Pro 1370",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 3"),
-		},
-	},
+static const struct dmi_system_id ctrl_ps2_aux_port_list[] = {
 	{
-	.ident = "ZhaoYang K4e-IML",
+	/* Lenovo Ideapad Z570 */
 	.matches = {
 		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ZhaoYang K4e-IML"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "Ideapad Z570"),
 		},
 	},
 	{}
@@ -1559,14 +1560,7 @@ static void ideapad_check_features(struct ideapad_private *priv)
 	unsigned long val;
 
 	priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
-
-	/* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
-	if (acpi_dev_present("ELAN0634", NULL, -1))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else if (dmi_check_system(no_touchpad_switch_list))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else
-		priv->features.touchpad_ctrl_via_ec = 1;
+	priv->features.ctrl_ps2_aux_port = dmi_check_system(ctrl_ps2_aux_port_list);
 
 	if (!read_ec_data(handle, VPCCMD_R_FAN, &val))
 		priv->features.fan_mode = true;
@@ -1639,10 +1633,6 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	if (!priv->features.hw_rfkill_switch)
 		write_ec_cmd(priv->adev->handle, VPCCMD_W_RF, 1);
 
-	/* The same for Touchpad */
-	if (!priv->features.touchpad_ctrl_via_ec)
-		write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, 1);
-
 	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		if (test_bit(ideapad_rfk_data[i].cfgbit, &priv->cfg))
 			ideapad_register_rfkill(priv, i);
-- 
2.37.3

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi,

On 11/10/22 18:54, Maxim Mikityanskiy wrote:
> On Thu, 10 Nov 2022 at 18:42, Eray Orçunus <erayorcunus@gmail.com> wrote:
>>

<snip>

>>> I'm guessing that this part:
>>>
>>>                 unsigned char param;
>>>                 /*
>>>                  * Some IdeaPads don't really turn off touchpad - they only
>>>                  * switch the LED state. We (de)activate KBC AUX port to turn
>>>                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
>>>                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
>>>                  */
>>>                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
>>>
>>> May cause issues on some models. It definitely feels fishy and
>>> I would like to disable this except on models where:
>>>
>>> 1. There is a LED controlled by some touchpad on/off hotkey; and
>>> 2. The EC fails to disable the touchpad itself
>>>
>>> Which would currently mean only enable this bit on Maxim's Z570
>>> using a DMI based allow list.
> 
> A small note on the DMI allow-list: I don't think Z570 is the only
> laptop where EC fails to disable the touchpad. While I would like this
> hack to affect as few laptops as possible, I would expect that other
> similar models produced in the same time period suffer from the same
> issue, and I don't think we have the full list of them.

I realize that we may need to grow the list over time, but I would
rather do that, then keep doing the i8042 poking on almost all
the models even where it is not necessary and whee it is potentially
causing issues.

Regards,

Hans

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi,

On 11/10/22 19:09, Maxim Mikityanskiy wrote:
> On Thu, 10 Nov 2022 at 19:54, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>>
>> On Thu, 10 Nov 2022 at 18:42, Eray Orçunus <erayorcunus@gmail.com> wrote:
>>>
>>> Hi Hans,
>>>
>>> On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> On 11/10/22 13:00, Eray Orçunus wrote:
>>>>> While I agree with your findings(and thanks for your effort, I also tried
>>>>> to solve this but later gave up), they doesn't apply to one type of
>>>>> IdeaPads (like my 520-15IKB), so I'm sure this patch won't work on me.
>>>>> This is because my IdeaPad has this features:
>>>>>
>>>>> - i8042.noaux doesn't affect touchpad, and it's connected over i2c
>>>>> - There is no touchpad LED, and touchpad hotkey only sends "key pressed"
>>>>>   ACPI event, doesn't do anything else
>>>>> - VPCCMD_W_TOUCHPAD does nothing (also confirmed on Windows)
>>>>> - Sending 1 to VPCCMD_W_TOUCHPAD on boot is not needed
>>>>> - VPCCMD_R_TOUCHPAD always returns 1 (this is interesting)
>>>>
>>>> So if i8042.noaux does not do anything, then why do you want to add
>>>> "SYNA2B33" to the list of ACPI HIDs for which we set:
>>>>
>>>> features.touchpad_ctrl_via_ec=0;
>>>>
>>>> ?
>>>>
>>>> IOW what bad effects / behavior are you seeing with touchpad_ctrl_via_ec=1?
>>>>
>>>> Or are you seeing bad behavior on some other modes? If yes, then what
>>>> is the bad behavior you are seeing on other models ?
>>>
>>> It was just because I didn't want to have a not working "touchpad"
>>> attribute :) I used/still using several GNOME extensions and they show
>>> me "Touchpad" toggle just because I have "touchpad" attribute exposed
>>> there, which is doing nothing, and misleading.
>>>
>>> But I would understand if you don't want to touch it at that stage, and
>>> you would rather prefer not working "touchpad" attributes to not
>>> exposed "touchpad" attributes that would have been perfectly working.
>>>
>>>> I'm guessing that this part:
>>>>
>>>>                 unsigned char param;
>>>>                 /*
>>>>                  * Some IdeaPads don't really turn off touchpad - they only
>>>>                  * switch the LED state. We (de)activate KBC AUX port to turn
>>>>                  * touchpad off and on. We send KEY_TOUCHPAD_OFF and
>>>>                  * KEY_TOUCHPAD_ON to not to get out of sync with LED
>>>>                  */
>>>>                 i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
>>>>
>>>> May cause issues on some models. It definitely feels fishy and
>>>> I would like to disable this except on models where:
>>>>
>>>> 1. There is a LED controlled by some touchpad on/off hotkey; and
>>>> 2. The EC fails to disable the touchpad itself
>>>>
>>>> Which would currently mean only enable this bit on Maxim's Z570
>>>> using a DMI based allow list.
>>
>> A small note on the DMI allow-list: I don't think Z570 is the only
>> laptop where EC fails to disable the touchpad. While I would like this
>> hack to affect as few laptops as possible, I would expect that other
>> similar models produced in the same time period suffer from the same
>> issue, and I don't think we have the full list of them.
> 
> And another idea: i8042_command(I8042_CMD_AUX_DISABLE) can be replaced
> with installing a i8042_platform_filter that will filter out all
> I8042_STR_AUXDATA when the touchpad should be disabled. It's
> definitely less fishy, we don't touch the KBC, but simply filter out
> events in software.

That can still cause issues for dual-mode (i2c + ps2) touchpads which
initially get probed in ps2 mode and then switched over to i2c mode later.

Often this switching over needs to be redone at resume time too,
so for this to work we really need the ps2 aux port to fully work.

I do agree using a platform filter would be somewhat cleaner, but
what we have now has been in use for a long time now without issues,
so I'm in favor of an if it aint broken don't fix approach here.

That is on devices where we need to actually do something to disable
ps2 aux traffic, lets keep the current approach. While try to just not
mess with the i8042 controller *at all* on other devices.

Regards,

Hans

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi Eray,

On 11/10/22 19:47, Eray Orçunus wrote:
> Hi,
> 
> On 10 Nov 2022 at 21:09, Maxim Mikityanskiy <maxtram95@gmail.com>  wrote:
>> A small note on the DMI allow-list: I don't think Z570 is the only
>> laptop where EC fails to disable the touchpad. While I would like this
>> hack to affect as few laptops as possible, I would expect that other
>> similar models produced in the same time period suffer from the same
>> issue, and I don't think we have the full list of them.
> 
> I just checked Z570 ACPI table, and this is what it does when it receives
> VPCCMD_R_TOUCHPAD:
> 
> 	VDAT = TPEN /* \_SB_.PCI0.LPCB.EC0_.TPEN */
> 	If ((TPEN == One))
> 	{
> 	    GL04 |= 0x02
> 	}
> 	Else
> 	{
> 	    GL04 &= 0xFD
> 	}
> 
> VDAT is the data returned to user.
> So we can say that TPEN is the logical state of touchpad key, and GL04
> is state of touchpad LED or series of LEDs.
> 
> VPCCMD_W_TOUCHPAD is nulled, it doesn't work.
> 
> I also checked which DSDTs I have (13 DSDTs from 2008 to this year)
> contain TPEN, and turned out it was only S12, from 2009. It also had
> nulled VPCCMD_W_TOUCHPAD, and returns TPEN on VPCCMD_R_TOUCHPAD, except
> it doesn't have an LED or GL04.
> 
> So, it's possible that we can only check if TPEN exists on ACPI table,
> instead of having a white-list.

Hmm, lets keep that idea in case it turns out the allow-list based
approach turns out to cause issue/grow out of control. I would rather
not rely on ACPI variables having a specific name for something like
this, but you might be on to something.

I have checked my own collection of acpidumps and the only DSDT
which I have which has a TPEN variable is for an Ideapad D330.

Funny enough that is a 2-in-1 with a detachable USB keyboard.
I would not be surprised if the ideapad driver does not load
on those devices at all since there are no embedded-controller
handled hotkeys, as the keyboard is fully USB/HID and not
connected to the EC at all.

I'll send you the acpidump in a private email.

Regards,

Hans

Re: ideapad-laptop touchpad handling problems, request for help

[Maxim Mikityanskiy]

On Thu, 10 Nov 2022 at 21:18, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Eray,
>
> On 11/10/22 19:47, Eray Orçunus wrote:
> > Hi,
> >
> > On 10 Nov 2022 at 21:09, Maxim Mikityanskiy <maxtram95@gmail.com>  wrote:
> >> A small note on the DMI allow-list: I don't think Z570 is the only
> >> laptop where EC fails to disable the touchpad. While I would like this
> >> hack to affect as few laptops as possible, I would expect that other
> >> similar models produced in the same time period suffer from the same
> >> issue, and I don't think we have the full list of them.
> >
> > I just checked Z570 ACPI table, and this is what it does when it receives
> > VPCCMD_R_TOUCHPAD:
> >
> >       VDAT = TPEN /* \_SB_.PCI0.LPCB.EC0_.TPEN */
> >       If ((TPEN == One))
> >       {
> >           GL04 |= 0x02
> >       }
> >       Else
> >       {
> >           GL04 &= 0xFD
> >       }
> >
> > VDAT is the data returned to user.
> > So we can say that TPEN is the logical state of touchpad key, and GL04
> > is state of touchpad LED or series of LEDs.
> >
> > VPCCMD_W_TOUCHPAD is nulled, it doesn't work.
> >
> > I also checked which DSDTs I have (13 DSDTs from 2008 to this year)
> > contain TPEN, and turned out it was only S12, from 2009. It also had
> > nulled VPCCMD_W_TOUCHPAD, and returns TPEN on VPCCMD_R_TOUCHPAD, except
> > it doesn't have an LED or GL04.
> >
> > So, it's possible that we can only check if TPEN exists on ACPI table,
> > instead of having a white-list.
>
> Hmm, lets keep that idea in case it turns out the allow-list based
> approach turns out to cause issue/grow out of control. I would rather
> not rely on ACPI variables having a specific name for something like
> this, but you might be on to something.

Maybe also add a module parameter to force the i8042 workaround? This
change is likely to break the touchpad toggle for some devices similar
to Z570 (maybe Z580? Y580? idk), so people would at least have an
option to force enable it using the module parameter, before the
relevant entry gets added to the DMI table, the patch gets merged, and
the kernel gets updated.

Another note on the comments in patch 4:

        /*
         * Some IdeaPads don't really turn off touchpad - they only
         * switch the LED state. We (de)activate KBC AUX port to turn
         * the touchpad on and off.
         */
               /*
                * On older models the EC controls the touchpad and toggles it
                * on/off itself, in this case we report KEY_TOUCHPAD_ON/_OFF.
                * If the EC did not toggle, report KEY_TOUCHPAD_TOGGLE.
                */

These two comments would sound confusing to me if I read them without
having context on Ideapads. I mean this part where "EC controls the
touchpad and toggles it itself", but at the same time these laptops
"don't really turn off the touchpad". Could you rephrase it somehow,
so that it would be clear that there are models where the EC cares
about the touchpad because it toggles *either* the touchpad or at
least its LED?

> I have checked my own collection of acpidumps and the only DSDT
> which I have which has a TPEN variable is for an Ideapad D330.
>
> Funny enough that is a 2-in-1 with a detachable USB keyboard.
> I would not be surprised if the ideapad driver does not load
> on those devices at all since there are no embedded-controller
> handled hotkeys, as the keyboard is fully USB/HID and not
> connected to the EC at all.
>
> I'll send you the acpidump in a private email.
>
> Regards,
>
> Hans
>
>
>
>

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi,

On 11/11/22 12:44, Maxim Mikityanskiy wrote:
> On Thu, 10 Nov 2022 at 21:18, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Eray,
>>
>> On 11/10/22 19:47, Eray Orçunus wrote:
>>> Hi,
>>>
>>> On 10 Nov 2022 at 21:09, Maxim Mikityanskiy <maxtram95@gmail.com>  wrote:
>>>> A small note on the DMI allow-list: I don't think Z570 is the only
>>>> laptop where EC fails to disable the touchpad. While I would like this
>>>> hack to affect as few laptops as possible, I would expect that other
>>>> similar models produced in the same time period suffer from the same
>>>> issue, and I don't think we have the full list of them.
>>>
>>> I just checked Z570 ACPI table, and this is what it does when it receives
>>> VPCCMD_R_TOUCHPAD:
>>>
>>>       VDAT = TPEN /* \_SB_.PCI0.LPCB.EC0_.TPEN */
>>>       If ((TPEN == One))
>>>       {
>>>           GL04 |= 0x02
>>>       }
>>>       Else
>>>       {
>>>           GL04 &= 0xFD
>>>       }
>>>
>>> VDAT is the data returned to user.
>>> So we can say that TPEN is the logical state of touchpad key, and GL04
>>> is state of touchpad LED or series of LEDs.
>>>
>>> VPCCMD_W_TOUCHPAD is nulled, it doesn't work.
>>>
>>> I also checked which DSDTs I have (13 DSDTs from 2008 to this year)
>>> contain TPEN, and turned out it was only S12, from 2009. It also had
>>> nulled VPCCMD_W_TOUCHPAD, and returns TPEN on VPCCMD_R_TOUCHPAD, except
>>> it doesn't have an LED or GL04.
>>>
>>> So, it's possible that we can only check if TPEN exists on ACPI table,
>>> instead of having a white-list.
>>
>> Hmm, lets keep that idea in case it turns out the allow-list based
>> approach turns out to cause issue/grow out of control. I would rather
>> not rely on ACPI variables having a specific name for something like
>> this, but you might be on to something.
> 
> Maybe also add a module parameter to force the i8042 workaround? This
> change is likely to break the touchpad toggle for some devices similar
> to Z570 (maybe Z580? Y580? idk), so people would at least have an
> option to force enable it using the module parameter, before the
> relevant entry gets added to the DMI table, the patch gets merged, and
> the kernel gets updated.

That is a good idea. I'm about to send out this as a proper patch-series
and I've added a module parameter to the version.

> Another note on the comments in patch 4:
> 
>         /*
>          * Some IdeaPads don't really turn off touchpad - they only
>          * switch the LED state. We (de)activate KBC AUX port to turn
>          * the touchpad on and off.
>          */
>                /*
>                 * On older models the EC controls the touchpad and toggles it
>                 * on/off itself, in this case we report KEY_TOUCHPAD_ON/_OFF.
>                 * If the EC did not toggle, report KEY_TOUCHPAD_TOGGLE.
>                 */
> 
> These two comments would sound confusing to me if I read them without
> having context on Ideapads. I mean this part where "EC controls the
> touchpad and toggles it itself", but at the same time these laptops
> "don't really turn off the touchpad". Could you rephrase it somehow,
> so that it would be clear that there are models where the EC cares
> about the touchpad because it toggles *either* the touchpad or at
> least its LED?

To me the comments are clear the code under the first comment
block starts with a "if (priv->features.ctrl_ps2_aux_port)" that
+ the comment to me makes it clear this is a workaround for
some models where the EC fails to turn off the touchpad itself.

Where as the second comment is just about which key-presses are
being reported and why. With that said suggestions / patches to
improve the comments are certainly welcome.

BTW did you ever try simply writing back the value returned from
VPCCMD_R_TOUCHPAD to VPCCMD_W_TOUCHPAD ?

Eray has been looking at what the Windows tools do and according
to Eray they always call VPCCMD_W_TOUCHPAD on touchpad toggle
events. So maybe just writing back the value is enough to actually
disable the touchpad ?

Regards,

Hans

Re: ideapad-laptop touchpad handling problems, request for help

[Eray Orçunus]

On Thu, 16 Nov 2022 at 18:19, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,

Hi, sorry for the late reply.

> On 11/11/22 12:44, Maxim Mikityanskiy wrote:
> > Maybe also add a module parameter to force the i8042 workaround? This
> > change is likely to break the touchpad toggle for some devices similar
> > to Z570 (maybe Z580? Y580? idk), so people would at least have an
> > option to force enable it using the module parameter, before the
> > relevant entry gets added to the DMI table, the patch gets merged, and
> > the kernel gets updated.
> 
> That is a good idea. I'm about to send out this as a proper patch-series
> and I've added a module parameter to the version.

Alright, I will continue the discussion from there then.

> BTW did you ever try simply writing back the value returned from
> VPCCMD_R_TOUCHPAD to VPCCMD_W_TOUCHPAD ?
> 
> Eray has been looking at what the Windows tools do and according
> to Eray they always call VPCCMD_W_TOUCHPAD on touchpad toggle
> events. So maybe just writing back the value is enough to actually
> disable the touchpad ?

It isn't, as I said before, VPCCMD_W_TOUCHPAD is nulled/empty on Z570 and
S12, so it won't work.

Best,
Eray

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi,

On 11/16/22 16:34, Eray Orçunus wrote:
> On Thu, 16 Nov 2022 at 18:19, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
> 
> Hi, sorry for the late reply.
> 
>> On 11/11/22 12:44, Maxim Mikityanskiy wrote:
>>> Maybe also add a module parameter to force the i8042 workaround? This
>>> change is likely to break the touchpad toggle for some devices similar
>>> to Z570 (maybe Z580? Y580? idk), so people would at least have an
>>> option to force enable it using the module parameter, before the
>>> relevant entry gets added to the DMI table, the patch gets merged, and
>>> the kernel gets updated.
>>
>> That is a good idea. I'm about to send out this as a proper patch-series
>> and I've added a module parameter to the version.
> 
> Alright, I will continue the discussion from there then.
> 
>> BTW did you ever try simply writing back the value returned from
>> VPCCMD_R_TOUCHPAD to VPCCMD_W_TOUCHPAD ?
>>
>> Eray has been looking at what the Windows tools do and according
>> to Eray they always call VPCCMD_W_TOUCHPAD on touchpad toggle
>> events. So maybe just writing back the value is enough to actually
>> disable the touchpad ?
> 
> It isn't, as I said before, VPCCMD_W_TOUCHPAD is nulled/empty on Z570 and
> S12, so it won't work.

A right, I forgot that you mentioned that, sorry.

Maxim, that means there is no need to try doing this.

Regards,

Hans

Re: ideapad-laptop touchpad handling problems, request for help

[Jiaxun Yang]

在2022年11月9日十一月 下午8:59，Hans de Goede写道：
> Hi All,
>
> I'm emailing you all because you have written patches or
> reported bugs related to the ideapad-laptop touchpad
> handling the past.

Hi all,

Sorry for chime in this old thread, but I'm able to get some input from a
Lenovo engineer.

Quoting him:

"For newer Lenovo laptops we use TPRD/TPWR methods under touchpad's ACPI I2C HID
device to sync touchpad state with EC. TPRD will return current touchpad state known
by EC and OS can use TPWR to switch EC's touchpad state. This state will be used by EC
to control LEDs and touchpad power saving signals."

According to my understanding we only need replace read write to VPCCMD_R_TOUCHPAD
with calling TPRD/TPWR methods to get all new ideapads work.

As per my reverse engineering on ASLs this method actually reads a flag from EC's
LPC memory space and it do work on some early ideapads (The earliest one I can
trace is Ideapad 320-15ISK which is released on 2018).

I'm going to try to implement it in kernel. Though I haven't decide which part of
driver should handle this, as those methods are under ACPI I2C HID device perhaps
we should put this function under i2c-hid-acpi driver. However as the method is very
ideated specific, putting in ideapad-acpi seems more reasonable... Any thoughts?

He also told me how to get VPC2004 device's BIOS interface version, though the
differences between versions remains unclear to me. I think we can expose it
in dmesg and debugfs to help with future debugging and hopefully reduce the amount
of DMI quirks.

Thanks.

-- 
- Jiaxun

Re: ideapad-laptop touchpad handling problems, request for help

[Hans de Goede]

Hi,

On 1/11/23 17:38, Jiaxun Yang wrote:
> 
> 
> 在2022年11月9日十一月 下午8:59，Hans de Goede写道：
>> Hi All,
>>
>> I'm emailing you all because you have written patches or
>> reported bugs related to the ideapad-laptop touchpad
>> handling the past.
> 
> Hi all,
> 
> Sorry for chime in this old thread, but I'm able to get some input from a
> Lenovo engineer.
> 
> Quoting him:
> 
> "For newer Lenovo laptops we use TPRD/TPWR methods under touchpad's ACPI I2C HID
> device to sync touchpad state with EC. TPRD will return current touchpad state known
> by EC and OS can use TPWR to switch EC's touchpad state. This state will be used by EC
> to control LEDs and touchpad power saving signals."
> 
> According to my understanding we only need replace read write to VPCCMD_R_TOUCHPAD
> with calling TPRD/TPWR methods to get all new ideapads work.
> 
> As per my reverse engineering on ASLs this method actually reads a flag from EC's
> LPC memory space and it do work on some early ideapads (The earliest one I can
> trace is Ideapad 320-15ISK which is released on 2018).
> 
> I'm going to try to implement it in kernel. Though I haven't decide which part of
> driver should handle this, as those methods are under ACPI I2C HID device perhaps
> we should put this function under i2c-hid-acpi driver. However as the method is very
> ideated specific, putting in ideapad-acpi seems more reasonable... Any thoughts?

This really should be handled in ideapad-acpi, but that means figuring out a way
to find the touchpad device in ideapad-apci. For starters you can search for
ACPI devs which match:

static const struct acpi_device_id i2c_hid_acpi_match[] = {
        {"ACPI0C50", 0 },
        {"PNP0C50", 0 },
        { },
};

And then check if they have the TPRD/TPWR methods. This does risk piking
another device then the actual touchpad though, most likely a touchscreen
(in case where the ACPI tables accidentally also have TPRD/TPWR methods
on the touchscreen).

Once you have find the right ACPI device, we can call the methods
(when found) from:

ideapad_sync_touchpad_state()

Note that only ever reads the touchpad state... 

> He also told me how to get VPC2004 device's BIOS interface version, though the
> differences between versions remains unclear to me. I think we can expose it
> in dmesg and debugfs to help with future debugging and hopefully reduce the amount
> of DMI quirks.

Yes reading + logging (in dmesg) the BIOS interface version would be good.

Thank you for all your work on this!

Regards,

Hans