linux-surface / libwacom-surface

Patches to support Microsoft Surface Devices with `libwacom`.
MIT License
24 stars 4 forks source link

Surface Pro 7+ support #12

Closed skgsergio closed 1 year ago

skgsergio commented 1 year ago

I've just installed Linux on my SP7+ and I've noticed that libwacom-surface doesn't have support for it:

sconde@umbriel ~ $ libwacom-list-local-devices 
/dev/input/event256 is a tablet but not supported by libwacom
/dev/input/event27 is a tablet but not supported by libwacom
Failed to find any devices known to libwacom.

I copied the .tablet file for the Surface Pro 7 and adjusted the device ids and seems to work, however I'm not sure how to check if the rest of the properties need adjustment

/usr/share/libwacom/surface-pro7plus.tablet

# This is for the Microsoft Surface Pro 7+

[Device]
Name=Microsoft Surface Pro 7+
Class=PenDisplay
DeviceMatch=virt:045e:0c1a;mei:045e:0c1a
Width=10
Height=6
IntegratedIn=Display;System;

[Features]
Stylus=false # <-- Not sure about this, I mean the stylus works even with this as false and I don't notice any difference setting it to true
Touch=true
Buttons=0
sconde@umbriel ~ $ libwacom-list-local-devices 
/dev/input/event27 is a tablet but not supported by libwacom
devices:
- name: 'Microsoft Surface Pro 7+'
  bus: 'unknown'
  vid: '0x045e'
  pid: '0x0c1a'
  nodes: 
  - /dev/input/event256: 'IPTS Stylus'

image

qzed commented 1 year ago

Honestly, I'm not sure about the values either and we'll have to revisit those before we submit our changes upstream. For now, I think this is fine, but if you want to dig into this and have a look at how this should be done properly, feel free to do so.

One note regarding the data though: The SP7 uses ITHC and not IPTS, so the mei line is wrong and should be pci instead. This is only relevant if IPTSd is not running. Unfortunately, even our patched libwacom doesn't support that yet, so you can't directly test it. I'll draft up some patches to add support for the PCI bus and the SP7+ and notify you here again when I have something to test for you.

qzed commented 1 year ago

Alright, can you git clone https://github.com/linux-surface/libwacom and git checkout feature/sp7+, then build and test it by running the following commands?

meson setup build
meson compile -C build
./build/libwacom-list-local-devices --database ./data

Please also run it once without IPTSd (i.e. after running systemctl stop $(iptsd-find-service)) to make sure that works as well.

StollD commented 1 year ago

One note regarding the data though: The SP7 uses ITHC and not IPTS, so the mei line is wrong and should be pci instead. This is only relevant if IPTSd is not running.

I actually don't know if we would want to match both devices, because the PCI device would stay around even if iptsd is running, so now you have two tablets, but one doesn't work.

And the MEI match is / was meant for the old IPTS driver. The new driver doesn't even set the bus ID, so it shows up as unknown.

So IMHO we should not match by PCI, since the singletouch is only meant to be a fallback, and it would keep the same behavour as IPTS (e.g. only having a match when iptsd is running).

qzed commented 1 year ago

Oh, you're right. I thought the new driver was setting BUS_MEI as well... I've just checked and it looks like auto-rotation works without libwacom support (in fallback mode without IPTS, on Gnome at least) so I guess you're correct on that as well, i.e. we don't need that match.

skgsergio commented 1 year ago

So... what tests should I perform then?

Edit: image

qzed commented 1 year ago

Thanks! That looks about right with the updated files. I'll prepare a new release now.

qzed commented 1 year ago

The repo is currently updating with the new release, so it should be available soon. I'll close this as fixed, but please let me know if there are any issues.

skgsergio commented 1 year ago

The repo is currently updating with the new release, so it should be available soon. I'll close this as fixed, but please let me know if there are any issues.

I'm using Arch so I've got it from AUR as soon as you've updated it. Working.