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

Only parse specific usage in Touchpad/TouchScreen event drivers #59

Merged
merged 5 commits into from
Feb 3, 2022

Conversation

1Revenger1
Copy link
Contributor

@1Revenger1 1Revenger1 commented Jan 25, 2022

This hopefully fixes the comments in #58. I don't have a good way to test, though I'll post a binary below.
I'm not sure this is the final solution. Other solutions would require a lot of work and require keeping track of multiple collections most likely.

Please have a USB moues just in case:
VoodooI2CHID.kext.zip

@kprinssu
Copy link
Collaborator

kprinssu commented Jan 25, 2022

Wonderful work @1Revenger1, thanks for taking the initiative and cleaning up the PR 🎉

@gvkt Can you please give this kext a go and see if it works for you?

@narcyzzo
Copy link

narcyzzo commented Jan 25, 2022

Hello, i tested this, touchpad is recognized in settings but not working.UX534FAC GDX1515 (worked with this VoodooI2CHID https://github.com/VoodooI2C/VoodooI2C/files/7700166/VoodooMod.zip)

@gvkt
Copy link
Contributor

gvkt commented Jan 26, 2022

  1. Booting hangs with the compiled kext above and does not complete.
  2. The code should document that the code corresponding to ANY assumes there is only a single collection (trackpad or touch screen) in the report descriptor and will not likely work if the record descriptor contains more than one collection with common elements (it will cache digitizer objects from different collections overwriting each other to keep only the last one for the common elements). In other words, the current bug will still exist in that case unless there is only one collection in the report descriptor. Until a safer implementation is provided for that case. That bug is hard to detect as I found if it manifests in any future device that is not catered to by the modified specializations.

@gvkt
Copy link
Contributor

gvkt commented Jan 27, 2022

Is there any device where neither the PTP nor TouchScreen drivers would match but Multi-touch driver does? If not, you can just prevent the latter from matching anything and remove the ANY section of the code altogether.

@1Revenger1
Copy link
Contributor Author

1Revenger1 commented Jan 27, 2022

Is there any device where neither the PTP nor TouchScreen drivers would match but Multi-touch driver does? If not, you can just prevent the latter from matching anything and remove the ANY section of the code altogether.

USB trackpads I believe still uses multitouch hid. Should be able to use the precision multitouch driver if https://github.com/VoodooI2C/VoodooI2CHID/pull/48/files is merged in, but there may be other situations I'm not aware of.

Are you able to build the kext yourself? I don't trust my build environment. Been having someone else try it and neither your original PR works nor my PR. If we mess with the probe scores, we can get the Precision event driver to attach, but the trackpad is unresponsive. They're on Monterey so we haven't been able to get any logs (DebugEnhancer doesn't change the size, and msgbuf=1048576 prevents booting).

Edit: Is there any CI that could be added? That'd make this a bit easier.

@gvkt
Copy link
Contributor

gvkt commented Jan 28, 2022

Sorry, I don't have access to a build system at the moment.

Copy link
Collaborator

@kprinssu kprinssu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @1Revenger1 , this PR looks good to me. I'll hold off on merging to ensure there aren't any regressions.

@VoodooI2C VoodooI2C deleted a comment from narcyzzo Jan 30, 2022
@kprinssu
Copy link
Collaborator

kprinssu commented Jan 30, 2022

@narcyzzoYou have already previously mentioned that it does not work, please be more constructive, it clutters the PR and not helpful to testing and finding regressions.

@narcyzzo
Copy link

narcyzzo commented Jan 30, 2022

@narcyzzoYou have already previously mentioned that it does not work, please avoid it does not work. Please be more constructive, it clutters the PR and not helpful to testing and finding regressions.

Device is discovered, in MacOS settings, but touchpad doesnt respond to touches, i will upload an ioreg, how can i obtain logs? i only tried those log show --last boot | grep -i i2c
log.txt
It is possible also to Github Cl will happen?- that can help a lot

@gvkt
Copy link
Contributor

gvkt commented Jan 30, 2022

@narcyzzoYou have already previously mentioned that it does not work, please avoid it does not work. Please be more constructive, it clutters the PR and not helpful to testing and finding regressions.

Device is discovered, in MacOS settings, but touchpad doesnt respond to touches, i will upload an ioreg, how can i obtain logs? i only tried those log show --last boot | grep -i i2c log.txt It is possible also to Github Cl will happen?- that can help a lot

Unfortunately, these changes aren’t enough for laptops this is designed to help as it won’t work out of box without modifying the info.plist. So, most people who need this will report it won’t work.

The problem is that the touchscreen driver by deault attaches to the trackpad device and doesn’t initialize correctly or parse reports. So trackpad does not respond. You need to insure that the precision trackpad driver attaches to the trackpad device. Read the thread starting at the link below for what I did and how to check it in IoRegistryExplorer.

If you also have a touch screen then just altering the probe priority in info.plist will not work for both.

VoodooI2C/VoodooI2C#485 (comment)

It could be a different problem in your case but I would verify this first.

Not sure how anyone can test for regression or as a solution to their problem without a working build being available. Should probably do a beta release with binaries and have people who don’t need this also test to see it doesn’t affect them.

@1Revenger1
Copy link
Contributor Author

We got it to work, and I feel incredibly dumb.
Turns out the IOMatchCategories from my last PR broke stuff. VoodooI2CHIDDevice was getting interrupts and was calling handleReport, but the interrupt in VoodooI2CHIDPrecisionMultitouch was never called. Looked at the apple source and didn't see anything obvious why the IOMatchCategory would cause issues. Gonna remove those and put another build here I guess.

Btw PR #48 works. The build I was giving @narcyzzo had that change included since I was desperate.

@1Revenger1
Copy link
Contributor Author

Archive 4.zip
This should be good to test.

@gvkt
Copy link
Contributor

gvkt commented Feb 2, 2022

Archive 4.zip This should be good to test.

This does not work out of the box on Asus Zenbook Pro for reasons mentioned above. The Touchscreen event driver attaches to the trackpad and so the trackpad does not respond at all.

Screen Shot 2022-02-02 at 2 09 59 AM

Increasing the probe score for PTP driver solves the problem for the trackpad but then touchscreen event driver does not attach to the main screen but multi-touch driver does and it acts like one giant trackpad (relative co-ordinates) rather than a touch screen. If the laptop does not have a touch screen, then this probe score change is fine.

The only way to solve this for me is to tie the TouchScreen driver to the touch screen with a match for its product ID and not attach to the trackpad.

Similar edits will also need to be done for Asus Zenbook Duo.

With that change, I can report the kexts are working fine with multi-touch and no issues so far.

Just be prepared for reports coming in saying trackpad doesn't work with similar laptops to mine.

Wish somebody had a solution that does not require info.plist edits to control which satellite driver attached to which device.

@narcyzzo
Copy link

narcyzzo commented Feb 2, 2022

@gvkt Yes, i can agree im using those kexts https://github.com/VoodooI2C/VoodooI2CHID/files/7981542/Archive.4.zip with changing Precision probe to 500, and it works, but u say it is not the best solution, can we see your plist changes with product ID? @1Revenger1 still trying to find a solution without plist changes. Asus Zenbook 15 UX534FAC here.

@gvkt
Copy link
Contributor

gvkt commented Feb 2, 2022

UX534FAC

Like I said above, if the laptop does not have a touch screen display (main screen not the Screenpad) like on your model, the probe score change to give priority to PTP is enough. You don't need to do anything else since touch screen driver satellite is not needed.

But if the main display is a touch screen, the probe score change above prevents the touch screen driver to attach to the main display screen. So instead of acting like a touch screen with absolute co-ordinates (cursor moves to where you touch on screen), it acts like a giant trackpad with relative co-ordinates (the cursor does not move to where you touch but moves relative from its current position).

So, instead of changing probe score, I did a match for product id to ensure the touch screen driver matches and attaches to main screen only and the PTP matches and attaches to the screenpad product id. The config.plist change is pictured here

VoodooI2C/VoodooI2C#485 (comment)

Of course, this change is specific to my laptop because the product id might be different for other laptops. ASUS uses multiple vendors and products.

Problem is people with newer ASUS laptops with screenpads who download the kexts will have no idea why their trackpad does not work and they will not know they need to change the info.plist or what to do there. Unless they stumble on threads like this.

@narcyzzo
Copy link

narcyzzo commented Feb 2, 2022

Ah, ok got it now! So yeah, it works but sometimes voodoo just ded beetwen reboots, dont know why and how to debug it.
Zrzut ekranu 2022-02-2 o 16 01 08

@1Revenger1
Copy link
Contributor Author

@gvkt I'm aware that Info.plist edits are still needed. Best that can be done for now I think is to write down that edits are needed somewhere.

Do you have the report descriptors for both the touchscreen and screenpad? I wanted to try and compare them a little and see if there was a better way to detect between the two, or atleast prevent the precision driver attaching to the touchscreen.

@LeeBinder
Copy link

Don't our bootloaders, regardless if OC or Clover, have a section where you can enter kext plist find/ replace patches which are executed on each boot as part of the hotpatch?

@1Revenger1
Copy link
Contributor Author

1Revenger1 commented Feb 2, 2022

@gvkt I just thought of something. We use injector kexts in various other places (such as for broadcom wifi). Might be able to do something similar here, where we create a personality for the screenpad with a high score which just matches the product id.
AsusScreenpad.kext.zip

Not quite sure I got this right tbh, but I think you could do something like the above. It'll make it so you don't need to mess with the Info.plist while updating or testing.

Edit: Use this one instead, added the requirement for it to be present at root.
AsusScreenpad 2.kext.zip
Edit2: This one actually works. Took a few tries.
AsusScreenpad 3.kext.zip

@gvkt
Copy link
Contributor

gvkt commented Feb 3, 2022

The problem is the product id for the trackpad/touch screen can potentially vary from model to model of the laptop depending on the trackpad vendor (Elan or Goodix) and presumably within models/versions of the same vendor. The DSDT for my laptop shows 4 different trackpad models may be used.

The only thing that seems to be a differentiating feature of touch screens vs trackpads in ASUS laptops is that the touch panels have device names like TPLn and the trackpad always seems to have ETPD regardless of whether it is a Goodix screenpad or a simple Elan trackpad.

So what you need in effect for a more generalized solution (albeit depending on the pattern above) is to have the probe score for touch screen driver be higher for TPLn devices and the probe score for PTP driver be higher for ETPD device instead of product id.

Can the above solution be modified to do this instead of the product id? As a separate kext for ASUS, of course. In the worst case, it might require overriding the probe function in the kext code to include this logic. That logic may even depend on a quirk setting or a boot argument.

@1Revenger1
Copy link
Contributor Author

I agree that it may not be the best solution. I wouldn't be super surprised though if the device IDs didn't change a ton. Realistically, the easiest solution right now is to just record all the relevant product IDs and hope there aren't too many. The device ID between your screenpad and @narcyzzo for instance is the same.

I was sort of hoping there'd be a difference in the report descriptors or a value which could be read. Do you have report descriptors from both the screenpad and touchscreen?

@gvkt
Copy link
Contributor

gvkt commented Feb 3, 2022

That is a low sample space deduction! We both happen to have Goodix Screenpads. But if you can include multiple product IDs then you should get the product IDs for the ASUS Duos also which do not have the screenpad and has the same multi-touch and drive attachment problem. Pinging @shiecldk who can provide info on the Duo.

Attached are the parsed report descriptors for Screenpad GDX1515 and Touch Screen Elan90008
ELAN9008_RD.txt
GDX1515_RD.txt

Display Touch Screens not used as trackpads will only have a Touch Screen Collection and correctly get attached to TouchScreen Driver.

Any device that can be used as trackpad (as its only function or multi-function like screenpad) should have the required Mouse collection (if compliant with Windows certification). The PTP driver should have priority over touch screen driver for these devices.

Should also check with the second panel of ASUS Duos vs the first panel to confirm this.

Narrows parsing of record descriptors for the specific mode in which the event driver is attached (trackpad or touch screen).
@kprinssu
Copy link
Collaborator

kprinssu commented Feb 3, 2022

@gvkt @1Revenger1 Code-wise everything looks good to me. I want to merge these changes in, please let me know if they are good to go.

@1Revenger1
Copy link
Contributor Author

I believe it's good to go. Still not certain how to do better matching for Screenpads but that can be a seperate PR.

@kprinssu kprinssu merged commit 7b8ee9f into VoodooI2C:master Feb 3, 2022
@kprinssu
Copy link
Collaborator

kprinssu commented Feb 3, 2022

Thanks @1Revenger1 🎉

@1Revenger1 1Revenger1 deleted the fixParseDescriptors branch February 3, 2022 23:11
@shiecldk
Copy link

shiecldk commented Feb 9, 2022

@gvkt How can I obtain the parsed report from my ZenBook Pro Duo? What infos are needed for this fix? Device-id? Thanks.

@gvkt
Copy link
Contributor

gvkt commented Feb 9, 2022

Followup in this thread for this purpose

VoodooI2C/VoodooI2C#494

Just need the product and vendor id of each of your three touch devices from IORegistry and what satellite drivers needed to be attached to which to make it work for you and what modifications you did to info.plist to make it happen.

@shiecldk
Copy link

shiecldk commented Feb 9, 2022

@gvkt The ids are in here:
ELAN1406, 04F3:3101
ELAN9008, _SB.PCI0.I2C1.TPL0, 04F3:2C56
ELAN9009, _SB.PCI0.I2C1.TPL1, 04F3:2C23

VoodooI2C/VoodooI2C#474 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants