On Thu, 10 Nov 2022 at 18:42, Eray Orçunus <erayorcunus@xxxxxxxxx> wrote: > > 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. 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@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
