mwyborski / Linux-Magic-Trackpad-2-Driver

498 stars 84 forks source link

USB: no movement after suspend-resume cycle #37

Open dos1 opened 5 years ago

dos1 commented 5 years ago

(it probably should get reported to kernel's bugzilla too at this point, but let's do it here first)

When Magic Trackpad 2 is connected via USB, it stops responding after a suspend/resume cycle. Only left button clicking works. I suspect that the multitouch capability needs to be enabled again at this point, but currently the driver does nothing on resume.

makslevental commented 5 years ago

i get similar behavior: wired (usb) works on login screen and then once i login (my desktop manager shows up) i get only left and right click (no motion).

wolftune commented 5 years ago

Maybe relevant: I've found temporary solution for similar case myself at https://www.reddit.com/r/Ubuntu/comments/ahgbdg/apple_magic_trackpad_2_configuration_on_kubuntu/ in terms of the movement problems being related to the pressure settings. I have to use that process to get xinput set-prop to the right details. But since using the Magic Trackpad, suspend and resume can even crash my whole machine and otherwise the settings I want are not persisting correctly.

Note: I'm on KDE Neon with hwe kernel 4.18 + dkms of this driver

I should experiment with bluetooth too, but I've had issues there; overall much less reliable than USB during actual use.

Update: I tried kernel 5.1.15 and still had the no-movement-after-resume problem. I was able to get it back by simply doing on-off on the trackpad switch.

Perhaps the answer involves getting the latest libinput as well (I removed synaptics and found that libinput overall does better). See these recent patches for libinput: https://gitlab.freedesktop.org/libinput/libinput/merge_requests/214

dos1 commented 5 years ago

These patches are mine and are unrelated ;)

With out-of-date stack you can get "movement problems" related to libinput's interpretation of pressure values, but that's not what's reported here - after a suspend/resume cycle the values stop coming from the kernel altogether when connected via USB.

wolftune commented 5 years ago

@dos1 thanks for clarifying, that's disappointing because I have the same problem you reported here and was hoping maybe those patches would help. So, I guess not. This suspend-resume problem remains outstanding.

mistertwo commented 4 years ago

Picked up a MT2 recently, and am running into the same issue over USB. I've tried sending a USBDEVFS_RESET ioctl call to the device to see what would happen (I've seen it as a way to reset USB devices without the physical disconnect/reconnect), and instead of bringing the device back, I'm able to recreate the non-moving cursor without setting the computer to sleep and waking it back up.

Checked to see if there was anything funky in the /dev/bus/usb/* directory for the trackpad, but nothing stood out before or after sending USBDEVFS_RESET, or after doing a physical disconnect and reconnect to bring it back.

Once I find some time, I'll do a bit more digging and see if I can find out what's going on (or not) after the reset.

dos1 commented 4 years ago

Just a thought: I believe it's more likely that it's a matter of what the device itself sends to the computer rather than some internal state of the driver. Driver is configuring the trackpad on init to enable various features, like multitouch reporting, so this would be my main suspect there.

mistertwo commented 4 years ago

I've dug around a bit last night on ways to trigger the init call within X/xinput, but didn't come up with much. If you (or anyone else reading) happens to have something on how to do that, let me know.

I did notice that mtrack was throwing some stuff into the Xorg log to declare when the device was initialized, turned on, off, etc.

Do you happen to have anything that would point me in a proper direction to see what X may or may not be doing upon wake? I'm wondering if triggering a disconnect/init within X may help.

dos1 commented 4 years ago

Do you have any particular reason to suspect X there? I'd have to recheck to be sure, but as far as I remember the events were ceasing to flow at the kernel level already - I'd definitely start by digging in the kernel driver there.

mistertwo commented 4 years ago

I don't have a reason to suspect X at the moment. I was poking to see if there's another way to restart the connection without physically reconnecting via USB.

So after going for a dig in hid_magicmouse (controls the trackpad as well), it looks like there's a probe function that gets called when the device is plugged in, but there's no suspend or resume calls, which from what I've read so far should mean that those aren't being called at all, and we're leaving the trackpad alone upon suspend.

After scanning the code a bit more, I've noticed that there's a place where there's some MT2 specific processing being done here: https://github.com/torvalds/linux/blob/master/drivers/hid/hid-magicmouse.c#L399

We know the mouse clicks work, so maybe there's something that's not lining up with the calls to input_mt_sync_frame just above the call to input_report_key. I'll check back up on this once I've had some rest.

mwyborski commented 4 years ago

@mistertwo and @dos1 I haven't checked the suspend/resume. But you are right, that during initialization there is sent a USB message to the MT2, which puts it into multi-touch mode. Without multi-touch mode only one touch is recognized and e.g. 2-finger for right-click won't work. So this is problably the issue and the initialization should be called after resume. Also there is no problem, if by circumstances the switch to multi-finger-mode is called several times, so i think the fix should be easy. But right now i don't have the time to dig into this.

mwyborski commented 4 years ago

I might give it a shot sometime in november, but if someone of you want to patch it please go for it. Just tell me, so that we are not doing the same work.

wolftune commented 4 years ago

To clarify my reproduction of this: upon resume, my MT2 can click and can move the cursor when I use significant pressure (sort of like a click+drag). But normal cursor movement doesn't go. I simply toggle the physical on/off switch and everything then gets set correctly. So, I'm just in the habit of doing that toggle as a part of the resume routine.

It would be nice to have it fixed though.

mistertwo commented 4 years ago

@robotrovsky I'll give this a shot tonight, and let y'all know how things go.

mwyborski commented 4 years ago

@mistertwo great! I think this was the switch to multi-touch:

    else if (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
        if (id->vendor == BT_VENDOR_ID_APPLE)
            report = hid_register_report(hdev, HID_INPUT_REPORT,
                TRACKPAD2_BT_REPORT_ID, 0);
        else /* USB_VENDOR_ID_APPLE */
            report = hid_register_report(hdev, HID_INPUT_REPORT,
                TRACKPAD2_USB_REPORT_ID, 0);
    } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
mistertwo commented 4 years ago

Alright, gave this a shot. I've got a lot to learn... :)

Anyways, basics. Wrote a couple of functions that should trigger on suspend and resume:

static int magicmouse_hdev_resume(struct hid_device *hdev)

{
        hid_err(hdev, "magicmouse coming up.\n");
//      hid_err(hdev, "magicmouse id:\n");
//      if (hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
//              hid_err(hdev, "we have a device match!!!");
//      }
        printk(KERN_CRIT "magicmouse coming up.");
        return 0;
}

static int magicmouse_hdev_suspend(struct hid_device *hdev, struct pm_message bbq)
{
        hid_err(hdev, "magicmouse going down.\n");
        printk(KERN_CRIT "magicmouse going down.");
        if (hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
                hid_err(hdev, "we have a device match!!!");
        }

        return 0;
}

static struct hid_driver magicmouse_driver = {
        .name = "magicmouse",
        .id_table = magic_mice,
        .probe = magicmouse_probe,
        .raw_event = magicmouse_raw_event,
        .input_mapping = magicmouse_input_mapping,
        .input_configured = magicmouse_input_configured,
        .resume = magicmouse_hdev_resume,
        .suspend = magicmouse_hdev_suspend,
};

At this moment, the suspend function is being called without hassle. Oddly enough, it shows up when resume should be called (after waking the machine) in dmesg. I get nothing at all from the resume function in dmesg.

With regards to the if test in the suspend function, it's not going to be there in the end, as there would be no reason to check on this during suspend. Since that is the call that's "working" at the moment, I'll move forward on that while sorting out suspend not working or appearing to be called. If anyone has any insight on what I'm not doing right there, let me know.

mistertwo commented 4 years ago

After more digging, found that the reset_resume hook, instead of just resume, would trigger things as expected. Tried the driver with just the block that @robotrovsky mentioned, and that stopped all input from the device. Added the feature code here ( https://github.com/torvalds/linux/blob/master/drivers/hid/hid-magicmouse.c#L649 ) and that brought left clicking back.

Tried adding the code for setting up input ( https://github.com/torvalds/linux/blob/master/drivers/hid/hid-magicmouse.c#L566 ) and that would cause the system to hang on resume.

I'll get this up on to GitHub properly this evening so y'all can see what's going on.

mistertwo commented 4 years ago

Hokay.

https://github.com/mistertwo/magic_trackpad_2_fix/blob/verbose_messaging/linux/drivers/hid/hid-magicmouse.c

So far, it looks like there's something where the size of an incoming report to the magicmouse_raw_event function is incorrect. Here's some logs from the debug statements I threw into the driver, before and after a sleep/suspend session.

before:
[  233.047803] mm_raw_event size: 21
[  233.047804] magicmouse 0003:05AC:0265.0007: trackpad points good
[  233.047805] trackpad emit touch triggered
[  233.047806] magicmouse 0003:05AC:0265.0007: trackpad input_mt_sync_frame triggered?
[  233.047808] magicmouse 0003:05AC:0265.0007: trackpad input_report_key triggered?
[  233.059805] magicmouse 0003:05AC:0265.0007: trackpad touch triggered
[  233.059807] mm_raw_event size: 21
[  233.059809] magicmouse 0003:05AC:0265.0007: trackpad points good
[  233.059809] trackpad emit touch triggered
[  233.059811] magicmouse 0003:05AC:0265.0007: trackpad input_mt_sync_frame triggered?

after:
[  253.582207] mm_raw_event size: 8
[  253.594210] magicmouse 0003:05AC:0265.0007: trackpad touch triggered
[  253.594213] mm_raw_event size: 8
[  253.605207] magicmouse 0003:05AC:0265.0007: trackpad touch triggered
[  253.605210] mm_raw_event size: 8
[  253.617211] magicmouse 0003:05AC:0265.0007: trackpad touch triggered
[  253.617214] mm_raw_event size: 8
[  253.628215] magicmouse 0003:05AC:0265.0007: trackpad touch triggered
[  253.628218] mm_raw_event size: 8
[  253.640213] magicmouse 0003:05AC:0265.0007: trackpad touch triggered

I'm not 100% on where the size of 8 is coming from, but the size of 21, from when things are working, lines up with the number of inputs set up within magicmouse_setup_input for MT2 around line 502.

Problem there is when I tried to run that function within my reset_resume hook, it would stop the desktop from fully waking back up. I don't remember seeing anything in the dmesg logs after rebooting, but I'm down to try it again if need be.

I'm going to try pulling in the call to hid_hw_start and see how that changes things. Let me know if y'all see anything that I should refocus on. Thanks!

mwyborski commented 4 years ago

@mistertwo The size of 8 is coming from simple one finger touch report. After the switcht to multitouch this report size will be 21 and contain all touches. So it shows, that the switch to multitouch hasn't been executed. You need to switch to multi-finger-report in the resume method. I think it is something like the snippet i posted before.

Thanks for your effort! I think you are only missing the switch to multi-finger and it will work. If the switch was successful, it will show 21 bytes before and after.

mistertwo commented 4 years ago

@robotrovsky After doing some more exploratory digging, I've taken a step back to re-focus on the chunk of code that you've mentioned. I was having no luck just running that alone, so I checked into just what's happening with that particular chunk of code.

So, depending on what kind of device we have, and how it's connected, it's making a call to hid_register_report ( https://github.com/torvalds/linux/blob/master/drivers/hid/hid-core.c#L57 ). I can see in this function where we're allocating some memory on the system, and adding the report to the head of an already existing list on the HID device, but I'm not seeing where it's sending a message back to the trackpad to put it into multi-touch mode yet.

So, I've been checking into other calls in the probe function that may push data to the trackpad, and both hid_hw_start, and hid_hw_raw_request seem to fit that bill. I'd taken a look into hid_set_drvdata as well ( https://github.com/torvalds/linux/blob/master/include/linux/device.h#L1229 ) but that just seems to set driver data within the hid_device object, and not necessarily push things back to the trackpad itself.

So, assuming that hid_hw_start isn't what we want (computer locked up last time I included that in the reset_resume function), hid_hw_raw_request, which is called at the bottom of the probe function, is closer to what we want.

I've introduced the feature setting code that calls hid_hw_raw_request, near the bottom of the probe function, to the reset_resume function and I'm now able to report multiple touches (in the logs) after a sleep/resume session! Only problem I have now is to sort out why the cursor still won't move with the trackpad. Relevant snippet from dmesg log:

  208.503824] usb 3-2.2: reset full-speed USB device number 6 using xhci_hcd
[  208.720189] magicmouse coming up.
[  208.720321] magicmouse 0003:05AC:0265.000D: unable to request touch data (-32)
[  208.720325] usbhid 3-2.2:1.0: reset_resume error -32
[  208.724058] magicmouse coming up.
[  208.724457] magicmouse coming up.
[  208.724603] magicmouse 0003:05AC:0265.000F: unable to request touch data (-32)
[  208.724606] usbhid 3-2.2:1.2: reset_resume error -32
[  208.725117] magicmouse coming up.
[  208.725243] magicmouse 0003:05AC:0265.0010: unable to request touch data (-32)
[  208.725245] usbhid 3-2.2:1.3: reset_resume error -32
[  208.814062] usb 3-2.1: reset full-speed USB device number 3 using xhci_hcd

Will come back with an update once I have one.

mistertwo commented 4 years ago

Might have spoken a bit soon. Went through a couple of reboots (done by me) and re-compiled and installed the driver a couple of times to be sure.

The error above still appears, but I'm now able to use the trackpad again after a sleep/resume session. No disconnect required.

Updated code available here: https://github.com/mistertwo/magic_trackpad_2_fix/blob/fresh_start/linux/drivers/hid/hid-magicmouse.c

Should you encounter any system instability, let me know. I'm running Gentoo, and since I've started this, it's been slightly iffy at times. I'll see if I can track down what's going on with the lingering error message in the next day or two.

mwyborski commented 4 years ago

@mistertwo that sounds great. The hid_register_report sends the request for the 10 finger report to the MT2. After the hid_register_report the report size should change from 8 bytes to 21 bytes. Great that you got it running. If you think your patch is production ready i can review it if you like. Then you should use checkpatch.pl from the linux source and commit it according to the guide-lines (text email with patch to the the linux-kernel and linux-input mailinglist). Thank you for your effort!

mistertwo commented 4 years ago

Hey, hopping back onto this. I'm able to generate a clean patch, which is good. I'm digging back into the -32 error that was coming back (tied to a broken pipe, according to the error numbers) from the call within hid_hw_raw_request. Yet the mouse still works.

Since I'd rather not submit something that seems broken, I'm going to take a bit more time to see what's going on in there. Or... would this be something that's more of a "it works better, ship it" kind of thing?

mwyborski commented 4 years ago

@mistertwo if you send the patch in, i think it would be better if you remove the request which returns an error. If you don't know where this is coming from, you can try to send this to mailing-list and ask there.

mistertwo commented 4 years ago

@robotrovsky The particular request that sends the -32 error back is the same one that gets the trackpad working again after wake. I'll post in there and see if anyone knows while I keep things rolling.

mwyborski commented 4 years ago

great job @mistertwo . I will have a look, maybe i see something.... Which baseversion have you used, to make a diff.

mistertwo commented 4 years ago

I based my patch on master on torvalds/linux.git, tagged 5.4. This is all on my fresh_start branch for now.

reznikartem commented 4 years ago

@robotrovsky Hello. Is any chance to see this fix in new linux kernel release? Had this problem with suspend too.