Re: [sony-laptop] Hardware keys are not properly mapped on Sony VAIO UX VGN-UX390N

Armin Wolf <W_Armin@xxxxxx> · Wed, 19 Jun 2024 14:02:34 +0200

Am 19.06.24 um 10:16 schrieb Mattia Dongili:

On Wed, Jun 19, 2024 at 10:00:18AM +0200, Michał Szczepaniak wrote:
On 19/06/2024 09:54, Mattia Dongili wrote:
On Tue, Jun 18, 2024 at 11:18:12PM +0200, Armin Wolf wrote:
Am 18.06.24 um 15:08 schrieb Michał Szczepaniak:

On 18/06/2024 13:47, Armin Wolf wrote:
Am 18.06.24 um 09:09 schrieb Michał Szczepaniak:

On 18/06/2024 03:24, Mattia Dongili wrote:
On Mon, Jun 17, 2024 at 12:48:13AM +0200, Armin Wolf wrote:
Am 16.06.24 um 22:34 schrieb Michał Szczepaniak:

On 16/06/2024 20:18, Armin Wolf wrote:
Hi,

can you share the output of "acpidump"? The zoom-out button should
report KEY_ZOOMOUT, can you also share the output of dmesg
after loading the driver with the module parameter "debug=1" and
pressing the buttons?
[...]
dmesg:
[   19.108393] [  T475] sony_laptop: detected Type3 model
[   19.108407] [  T475] sony_laptop: Evaluating _STA
[   19.115105] [  T475] sony_laptop: Device disabled
[   19.115115] [  T475] sony_laptop: Evaluating _PRS
[   19.115145] [  T475] sony_laptop: IO1 at 0xc000 (0x20)
[   19.115150] [  T475] sony_laptop: IO1 at 0xc800 (0x20)
[   19.115154] [  T475] sony_laptop: IO1 at 0xd000 (0x20)
[   19.115157] [  T475] sony_laptop: IO1 at 0xd800 (0x20)
[   19.115294] [  T475] input: Sony Vaio Keys as
/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1c/SNY6001:00/input/input6


[   19.115631] [  T475] input: Sony Vaio Jogdial as
/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1c/SNY6001:00/input/input7


[   19.118777] [  T475] sony_laptop: device allocated minor is 123
[   19.118791] [  T475] sony_laptop: I/O port1: 0xc000 (0xc000) +
0x20
[   19.118826] [    C0] sony_laptop: event ([ff] [ff]) at port
0xc000(+0x12)
[   19.118839] [  T475] sony_laptop: IRQ: 6 - triggering: 1 -
polarity: 0 - shr: 0
[   19.118844] [  T475] sony_laptop: Evaluating _SRS
[   19.128310] [    C0] sony_laptop: event ([ff] [ff]) at port
0xc000(+0x12)
[   19.130430] [  T474] input: Power Button as
/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0C:00/input/input8
[   19.136861] [  T475] sony_laptop: sony_pic_call1(0x82): 0x0e0a
[   19.136899] [    C0] sony_laptop: event ([0e] [05]) at port
0xc000(+0x12)
[   19.136905] [    C0] sony_laptop: unknown event ([0e] [05]) at
port
0xc000(+0x12)
[   19.136927] [  T475] sony_laptop: sony_pic_call2(0x81 - 0xff):
0x000e
[   19.136949] [    C0] sony_laptop: event ([00] [05]) at port
0xc000(+0x12)
[   19.136961] [  T475] sony_laptop: sony_pic_call1(0x82): 0x000b
[   19.136988] [    C0] sony_laptop: event ([0e] [05]) at port
0xc000(+0x12)
[   19.136993] [    C0] sony_laptop: unknown event ([0e] [05]) at
port
0xc000(+0x12)
[   19.137161] [  T475] sony_laptop: SPIC setup done.
[   19.137261] [  T475] sony_laptop: method: name: GBRT, args 0
[   19.137268] [  T475] sony_laptop: method: name: SBRT, args 1
[   19.137272] [  T475] sony_laptop: method: name: GPBR, args 0
[   19.137276] [  T475] sony_laptop: method: name: SPBR, args 1
[   19.137281] [  T475] sony_laptop: method: name: PWAK, args 0
[   19.137285] [  T475] sony_laptop: method: name: PWRN, args 0
[   19.137289] [  T475] sony_laptop: method: name: CSXB, args 1
[   19.137293] [  T475] sony_laptop: method: name: GWDP, args 0
[   19.137298] [  T475] sony_laptop: method: name: SLRS, args 1
[   19.137302] [  T475] sony_laptop: method: name: RBMF, args 1
[   19.137306] [  T475] sony_laptop: method: name: RSBI, args 1
[   19.137310] [  T475] sony_laptop: method: name: CBMF, args 1
[   19.137314] [  T475] sony_laptop: method: name: LNPW, args 1
[   19.137319] [  T475] sony_laptop: method: name: GLNP, args 0
[   19.137323] [  T475] sony_laptop: method: name: SCAM, args 1
[   19.137327] [  T475] sony_laptop: method: name: GCAM, args 0
[   19.137340] [  T475] sony_laptop: Found brightness_default
getter:
GPBR
[   19.137388] [  T475] sony_laptop: Found brightness_default
setter:
SPBR
[   19.137402] [  T475] sony_laptop: Found lanpower getter: GLNP
[   19.137406] [  T475] sony_laptop: Found lanpower setter: LNPW
[   19.137423] [  T475] sony_laptop: SNC setup done.


and the 3 buttons zoomin, zoom out, the third one
[  161.975552] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  161.975596] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  161.975681] [    C0] sony_laptop: event ([10] [05]) at port
0xc000(+0x12)
Zoom in

[ 162.154768] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  162.154814] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  162.154880] [    C0] sony_laptop: event ([00] [05]) at port
0xc000(+0x12)
Ignored

[ 163.327457] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  163.327511] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  163.327563] [    C0] sony_laptop: event ([20] [05]) at port
0xc000(+0x12)
Zoom out

[ 163.516819] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  163.516856] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  163.517008] [    C0] sony_laptop: event ([00] [05]) at port
0xc000(+0x12)
ignored

[ 165.206657] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  165.206700] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  165.206805] [    C0] sony_laptop: event ([01] [05]) at port
0xc000(+0x12)
Prog 1

[ 165.365447] [    C0] sony_laptop: event ([5c] [31]) at port
0xc000(+0x12)
[  165.365491] [    C0] sony_laptop: sony_pic_call1(0xa0): 0x5c0a
[  165.365548] [    C0] sony_laptop: event ([00] [05]) at port
0xc000(+0x12)

ignored

Sorry i messed up and didn't use reply all, Im still quite new to
this

That ok, mistakes happen :)

I think the reason for you problem with the zoom-out key is that
when sony-laptop
iterates through the list of possible key responses, it first
matches the definition
for SONYPI_EVENT_PKEY_P1 (0x20), which has the same key data as
SONYPI_EVENT_ZOOM_OUT_PRESSED (also 0x20).

This causes SONYPI_EVENT_PKEY_P1 to be picked instead of
SONYPI_EVENT_ZOOM_OUT_PRESSED.
That's right. The event mask is the same for programmable and zoom
keys,
thus the conflict.

{ 0x05, SONYPI_PKEY_MASK, sonypi_pkeyev },
{ 0x05, SONYPI_ZOOM_MASK, sonypi_zoomev },

I am sending this mail to the maintainer of the sony-laptop driver,
maybe he can help us in this case.
Heh... I actually have a UX ultra portable laptop somewhere (a UX50
IIRC) but I'm not sure it'll even turn on. Those things are like 15~20
years old now.

I don't quite remember the idiosyncrasies of this particular model v/s
other models to be quite frank. On the other hand the module has a
'mask' option that you can use to allow-list only certain sets of
events.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/platform/x86/sony-laptop.c?h=v6.9.5#n94


(I'm glad the help text says "see doc" because I don't see this option
mentioned in the doc...)

The bitmasks are here:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/platform/x86/sony-laptop.c?h=v6.9.5#n3365



Thanks for response but I'm bit confused now, since they have same
event, and i only allow the zoom in/out keys, won't I lose the third
key? Am I missing something?

When using the "mask" module param, you will lose the third key.

I think the underlying issue could be that support for the
problematic 0x20 SONYPI_EVENT_PKEY_P1 was
added in commit 1cae71032183 ("sony-laptop: VGN-A317M hotkey
support"), while support for you VIAO model
was added in commit 3eb8749a3799 ("sony-laptop: add Type4 model").

Commit 1cae71032183 was added after commit 3eb8749a3799, so i think
it will be enough to introduce a
separate copy of sonypi_pkeyev[] without the conflicting 0x20
SONYPI_EVENT_PKEY_P1 definition.
This separate copy can then be used by the type3_events[] definition
(which is used on you model).

If the maintainer agrees with this approach, i can create a patch for
you to test. Are you able to
compile kernel modules on your device?

Thanks,
Armin Wolf

I am using opensuse on the device, I can just add patch in the obs so
yeah rebuilding kernel is no issue, I could do the patch myself but I
don't know how to make it device-specific.

Thanks for help!
Nice, can you test the attached patch and report back if it works?

Thanks,
Armin Wolf
  From 7c44c1d15f859647f19e5e2d9874432bb3a5cb92 Mon Sep 17 00:00:00 2001
From: Armin Wolf <W_Armin@xxxxxx>
Date: Tue, 18 Jun 2024 23:09:36 +0200
Subject: [PATCH] platform/x86: sony-laptop: Fix SONYPI_EVENT_ZOOM_OUT_PRESSED
   on Sony VAIO UX VGN-UX390N

It turns out that on type 3 models, the definitions for the programmable
keys partially conflict with the definitions for the zoom keys.

This causes SONYPI_EVENT_ZOOM_OUT_PRESSED on the Sony VAIO UX VGN-UX390N
to be reported as SONYPI_EVENT_PKEY_P1. Fix this by providing a separate
definition for type3 models without the conflicting key entry.

Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
---
   drivers/platform/x86/sony-laptop.c | 9 ++++++++-
   1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 3e94fdd1ea52..0e1d099ac06a 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -3451,6 +3451,13 @@ static struct sonypi_event sonypi_pkeyev[] = {
   	{ 0, 0 }
   };
+static struct sonypi_event sonypi_pkeyev_type3[] = {
+	{ 0x01, SONYPI_EVENT_PKEY_P1 },
+	{ 0x02, SONYPI_EVENT_PKEY_P2 },
+	{ 0x04, SONYPI_EVENT_PKEY_P3 },
+	{ 0, 0 }
+};
+
   /* The set of possible bluetooth events */
   static struct sonypi_event sonypi_blueev[] = {
   	{ 0x55, SONYPI_EVENT_BLUETOOTH_PRESSED },
@@ -3572,7 +3579,7 @@ static struct sonypi_eventtypes type3_events[] = {
   	{ 0x31, SONYPI_MEMORYSTICK_MASK, sonypi_memorystickev },
   	{ 0x41, SONYPI_BATTERY_MASK, sonypi_batteryev },
   	{ 0x31, SONYPI_PKEY_MASK, sonypi_pkeyev },
-	{ 0x05, SONYPI_PKEY_MASK, sonypi_pkeyev },
+	{ 0x05, SONYPI_PKEY_MASK, sonypi_pkeyev_type3 },
Based on the commits you found, the conflicting event was added for the
VGN-A series (a Type3 model). This change is effectively removing the P1
button handling for them.

See 3eb8749a37990b505ab94466038c067444bbd7eb and later
e93c8a6819b217f4f4a490f67f26e02ff6b23b44: there used to be a Type4 model
that was meant to keep some of the events separate from Type3 models.
Perhaps reintroducing the distinction is going to serve us better in
this case?
The IRQ handler can be shared between Type3/4, but the events
can be in separate arrays, one for type3 and one for type4.

What do you think?

Sounds good, however there seem to be even more conflicting key entries inside the type 3 key definitions.
Since you are the maintainer of the driver, i would prefer if you would handle this.

Thanks,
Armin Wolf

Alternatively you could just swap the pkeysev lists based on the former
type3/4 distinction, i.e.

+       pcidev = pci_get_device(PCI_VENDOR_ID_INTEL,
+                       PCI_DEVICE_ID_INTEL_ICH6_1, NULL);
+       if (pcidev) {
+               dev->control = &spic_types[2];
+               goto out;
+       }
+
+       pcidev = pci_get_device(PCI_VENDOR_ID_INTEL,
+                       PCI_DEVICE_ID_INTEL_ICH7_1, NULL);
+       if (pcidev) {
+               dev->control = &spic_types[3];
+               goto out;
+       }
+
+       pcidev = pci_get_device(PCI_VENDOR_ID_INTEL,
+                       PCI_DEVICE_ID_INTEL_ICH8_4, NULL);
+       if (pcidev) {
+               dev->control = &spic_types[3];
+               goto out;
+       }

(and maybe include also ICH9 as type4).

The output of `lspci` from the UX390 could also help making sure we get
the right distinction.

   	{ 0x05, SONYPI_ZOOM_MASK, sonypi_zoomev },
   	{ 0x05, SONYPI_CAPTURE_MASK, sonypi_captureev },
   	{ 0x05, SONYPI_PKEY_MASK, sonypi_volumeev },
I am here to deliver!

Not sure which one you need so here are the main internal ones (minus wifi
etc)

00:00.0 Host bridge: Intel Corporation Mobile 945GM/PM/GMS, 943/940GML and
945GT Express Memory Controller Hub (rev 03)
00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS,
943/940GML Express Integrated Graphics Controller (rev 03)
00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME,
943/940GML Express Integrated Graphics Controller (rev 03)
00:1b.0 Audio device: Intel Corporation NM10/ICH7 Family High Definition
Audio Controller (rev 02)
00:1c.0 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 1
(rev 02)
00:1c.1 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 2
(rev 02)
00:1d.0 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI
Controller #1 (rev 02)
00:1d.1 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI
Controller #2 (rev 02)
00:1d.2 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI
Controller #3 (rev 02)
00:1d.3 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI
Controller #4 (rev 02)
00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2)
00:1f.0 ISA bridge: Intel Corporation 82801GBM (ICH7-M) LPC Interface Bridge
(rev 02)
00:1f.1 IDE interface: Intel Corporation 82801G (ICH7 Family) IDE Controller
(rev 02)
00:1f.3 SMBus: Intel Corporation NM10/ICH7 Family SMBus Controller (rev 02)
This is good. So ICH7, ICH8 and ICH9 would be type4, while ICH6 remains
as type3.









