Perhaps not in the location where I put it though and perhaps it should be done with LSB0 callbacks instead. Although, being done this way allows for it to blend with the suspend sequence. Ideally, the Display Off delay would be blended with userspace going down such that if e.g., there is heavy userspace activity that requires ~2s to freeze, the quirk would add no delay. Instead, it would only add delay if userspace freezes quickly (less than .5s). Same can be said with Sleep Entry and beginning prepare_late, which blocks the EC interrupts (that would need a lot of investigation though). On that note, it seems to me that the Ally has 2 bugs related to the _DSM calls 3 and 4. First bug is that Display On is gated on current firmware and only works when the USB subsystem is powered on. Allegedly, this is fixed on the upcoming firmware but it is not something I have verified personally. I will verify it in 10 days or so, if the new firmware does not fail QA I guess. However, there is a second bug with Display Off in _DSM 4. The controller of the Ally needs time to power off, around 500ms. Otherwise it gets its power clipped and/or does not power off correctly. This causes the issues mentioned in the discussion and I have no indication that this is fixed with newer controller firmware. It is also my understanding that most of the testing of the new firmware happened with the asus-wmi quirk in place, which papers over that issue, so removing the quirk might be premature in any case. We have currently released this patch series in Bazzite and I am happy to report that it completely fixes all controller related issues in the Ally devices and makes them behave exactly as they do in Windows, regardless of firmware and bug for bug. So we will be keeping it around and extending it as appropriate to include the Sleep calls. I am reminded multiple times per week that the Ally has TDP suspend bugs, where if the user is playing a heavy game, the EC of the device tends to get stuck at 6W and fail to respond after waking the device. So moving calls 7, 8 is the natural next step in this investigation. I already have a draft patch on standby, that we plan to AB test soon. > IMHO it would be good to submit a v2 of just patches 1 - 3 run through > checkpatch. Also the commit message of patch 3 should point to the existing > quirk code in asus-wmi.c and mention that then is no longer necessary after > patch 3, then we can discuss what is the best place for these quirks. I did run it through before sending the patch. However, some of the warnings were a bit cryptic to me... I will run it again. I will add a note for asus-wmi on future patch series. First 3 patches of the series are designed to NOOP before patch 4. Did you mean patch 3 (which adds the delay) instead of 4? > Rafael, what do you think about at least taking patches 1 - 3 upstream? > Reading through how Windows handles things making the display on/off > calls before suspending devices sounds like it is the right thing to do > to me. Antheas
