Hi Hans, On 10 Nov 2022 at 15:37, Hans de Goede <hdegoede@xxxxxxxxxx> 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(¶m, 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@xxxxxxxxxx/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
