search

cb22 / macbook12-spi-driver

WIP input driver for the SPI touchpad / keyboard found in the 12" MacBook (MacBook8,1 + MacBook9,1)
GNU General Public License v2.0
298 stars 103 forks source link

Touchpad dimensions updates #40

Closed roadrunner2 closed 7 years ago

roadrunner2 commented 7 years ago

The 3 changes here are actually independent, but are all related to touchpad dimensions, so I thought it might be easier if I sent them as one pull request.

The logging of touchpad dimesions is a bit more complicated using dynamic-debug instead of simple #ifdef's, but I think the advantage of being able to give folks a simple echo command to run to turn it on instead of asking them recompile outweighs the uglyness - this should be especially true once the module is mainlined and folks get it as part of their distro's kernel.

I'm still not happy about the MB touchpad dimesions. As I mention in the commit message, the fact that the old values and those reported for the MB10,1 have noticeably different aspect ratios doesn't sit well (see also my comments in #9). It could also be the dimensions for the older MB's are wrong, though I assume you acquired them correctly (but maybe a quick double check using the logging added in this pull request might not hurt).

cb22 commented 7 years ago

The logging of touchpad dimesions is a bit more complicated using dynamic-debug instead of simple #ifdef's, but I think the advantage of being able to give folks a simple echo command to run to turn it on instead of asking them recompile outweighs the uglyness - this should be especially true once the module is mainlined and folks get it as part of their distro's kernel.

Agreed.

I'm still not happy about the MB touchpad dimesions. As I mention in the commit message, the fact that the old values and those reported for the MB10,1 have noticeably different aspect ratios doesn't sit well (see also my comments in #9). It could also be the dimensions for the older MB's are wrong, though I assume you acquired them correctly (but maybe a quick double check using the logging added in this pull request might not hurt).

Actually, I'm not entirely sure they are correct myself. I might have just lifted them straight from the BCM5974 driver in my initial excitement at getting something working.

I'll run this branch on my MB9,1 and report back shortly 👍

cb22 commented 7 years ago

So I just ran it on my 9,1 which reports

 -5087 5579 -182 6089

Compared to the current values of

-4828, 5345, -203, 6803

So those current values in the code are definitely off. The ones I get are basically the same as what you've got for the 10,1

spectacle m25314

roadrunner2 commented 7 years ago

Ah, this is good! I would then suggest we pick the largest dimensions found (it can be tricky to trigger those "pixels" at the very edge) and use them for all MB's, i.e. the measurements you got.

Also, can you confirm the physical dimensions of your trackpad? The were reported as 70mm x 112mm.

roadrunner2 commented 7 years ago

Btw., I just realized I managed to mess up the last patch in this series. Anyway, I reworked and repushed the last patch so it now uses your above reported values for all MacBook's.

cb22 commented 7 years ago

I can confirm the dimensions as 70mm x 112mm (measured using some calipers) 👍

roadrunner2 commented 7 years ago

Thanks. So that makes the x resolution 95 and the y resolution 90 - I still think it's odd for these two values to be so different here when they are only 1 or 2 points different for all other Mac's, but I guess this must just be something with this touchpad. Anyway, I've updated my evdev hwdb override appropriately now, and will submit it upstream soon.