Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Touchpad dimensions updates #40

Merged
merged 3 commits into from
Aug 6, 2017

Conversation

roadrunner2
Copy link
Contributor

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).

Enable by either by compiling this module with -DDEBUG, or by
configuring the kernel with DYNAMIC_DEBUG (enabled by default on most
distros these days) and then dynamically enabling it with

  echo 'func report_tp_state +p' | sudo tee /sys/kernel/debug/dynamic_debug/control

Move the finger around all edges until no new values are reported in the
kernel logs (dmesg).
The values for MBP14,1 have been explicitly confirmed as being identical
to those of the 13,1 model; the others are assumed to be identical to
their 13,* counterparts too.
@cb22
Copy link
Owner

cb22 commented Jul 24, 2017

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
Copy link
Owner

cb22 commented Jul 24, 2017

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
Copy link
Contributor Author

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.

The original values were lifted from elsewhere and were incorrect. The
current values have been verified by two different parties.
@roadrunner2
Copy link
Contributor Author

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
Copy link
Owner

cb22 commented Jul 25, 2017

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

@roadrunner2
Copy link
Contributor Author

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.

@cb22 cb22 merged commit b16862f into cb22:master Aug 6, 2017
@roadrunner2 roadrunner2 deleted the touchpad-dimensions-updates branch August 6, 2017 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants