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

Use native backlight EC methods #21

Closed
Qonfused opened this issue Jan 2, 2023 · 32 comments · Fixed by #38
Closed

Use native backlight EC methods #21

Qonfused opened this issue Jan 2, 2023 · 32 comments · Fixed by #38
Assignees
Labels
project:UX481 For variants UX481FA (14", 2019) and UX481FL (14", 2019) status:in-progress Implementation is in progress. type:feature New feature request

Comments

@Qonfused
Copy link
Owner

Qonfused commented Jan 2, 2023

The current SSDT-ATKD manually implements many common WMI hotkeys that are redundantly implemented in the Asus-SMC kext (or should be implemented into that project). This includes brightness keys for the primary display and keyboard backlight. Using the WMI/Asus-SMC methods allows for a more standardized implementation less prone to error from model-specific quirks.

Here are the original EC methods, dependent on MSOS returning Windows 8 or higher:
Method (_Q0E, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
{
    If ((MSOS () >= OSW8))
    {
        BRTN (0x87)
    }

    Return (Zero)
}

Method (_Q0F, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
{
    If ((MSOS () >= OSW8))
    {
        BRTN (0x86)
    }

    Return (Zero)
}

There are two main implementations that can resolve this issue:

  • Adding a MSOS fix should allow for brightness keys to work without SSDT-ATKD (using native methods instead). This will also require a PNLF to XNLF to address a pre-existing PNLF variable so that SSDT_PNLF will work properly.
  • The BrightnessKeys kext can also be used to fix backlight keys through the use of a keyboard remap + notifier methods for backlight; Replaces ATKD methods and renames.
@Qonfused Qonfused added the type:feature New feature request label Jan 2, 2023
@Qonfused Qonfused self-assigned this Jan 2, 2023
@Qonfused Qonfused changed the title Use native backlight control Use native EC methods with SSDT-PNLF Jan 2, 2023
@Qonfused Qonfused added type:documentation Improvements or additions to documentation status:in-progress Implementation is in progress. and removed type:documentation Improvements or additions to documentation labels Jan 5, 2023
@Qonfused Qonfused added the project:UX481 For variants UX481FA (14", 2019) and UX481FL (14", 2019) label Jan 12, 2023
@shiecldk
Copy link
Contributor

Hi @Qonfused, thank you very much for the good work on the ASUS dual-screen laptop. Did you fix both the backlight control of the main OLED display and the ScreenPad? I've been away from this project for a whole year and am catching up on what you've done with your 14-inch model. Thanks.

@Qonfused
Copy link
Owner Author

@shiecldk Currently only the screenpad has backlight control implemented (SSDT-SPLC). At current, it's just a re-implementation of the ACPI WMI methods used for controlling the screenpad through ScreenXpert. Might be worth looking into whether there exists a WMI method for OLED backlight.

@Qonfused
Copy link
Owner Author

#4 (comment) Sorry for being off-topic. I can confirm keyboard backlight is not working anymore for me (even after rolling back to my SSDTs) after using @ wern-apfel 's or @ Qonfused 's SSDT for FN keys, i.e. SSDT-ATKD.aml, SSDT-PNLF.aml, SSDT-KBLC.aml, and SSDT-SPLC.aml.

@shiecldk I'll need to find the EC methods (WEBC() not _Qxx) to check if there is something like a different offset for the keyboard backlight. May be worth double-checking on a fresh macOS install with your current implementation and then with the KBLC implementation. I did fix some intermittency issues regarding how the initial backlight value is set when refactoring wern-apfel's implementation, which may still be present on your system. If this is the case, it may only work for only a handful of something like 10-20 reboots, which would likely indicate a regression here.

Your DSDT does appear to have the same WMNB methods for the screenpad's backlight (#6 (comment)), which is what the SPLC ssdt re-implements. Did you notice any issues with the screenpad backlight as well?

@shiecldk
Copy link
Contributor

@Qonfused The attachment is the DSDT for BIOS 305 on UX582. Ok let me check!
UX582 DSDT 305.zip

@shiecldk
Copy link
Contributor

shiecldk commented Jan 27, 2023

With your SSDTs, I can set the ScreenPad backlight with your _Q31 method without a problem and can turn off the ScreenPad with your _Q32 method. All the other FN settings work fine, except the FN+F7 (_Q0D) for the keyboard backlight, and I'm not sure what FN+F10 (_Q17) and FN+F12 (_Q18) are used for. The FN+F4 (_Q0E) and FN+F5 (_Q0F) work fine for brightness control with Lunar.app software brightness control. However, I still don't have the native main OLED display backlight control.

@Qonfused
Copy link
Owner Author

The FN+F10 method may actually be toggling the camera. In my setup, the FN+12 key toggles the battery charging threshold; ASUS has a firmware implementation for limiting battery charging, which I have set to limit to 80% by default.

I documented some of wern's implementation in this commit when reviewing his ACPI: 004cb97.

@shiecldk
Copy link
Contributor

shiecldk commented Jan 27, 2023

#4 (comment) Sorry for being off-topic. I can confirm keyboard backlight is not working anymore for me (even after rolling back to my SSDTs) after using @ wern-apfel 's or @ Qonfused 's SSDT for FN keys, i.e. SSDT-ATKD.aml, SSDT-PNLF.aml, SSDT-KBLC.aml, and SSDT-SPLC.aml.

@shiecldk I'll need to find the EC methods (WEBC() not _Qxx) to check if there is something like a different offset for the keyboard backlight. May be worth double-checking on a fresh macOS install with your current implementation and then with the KBLC implementation. I did fix some intermittency issues regarding how the initial backlight value is set when refactoring wern-apfel's implementation, which may still be present on your system. If this is the case, it may only work for only a handful of something like 10-20 reboots, which would likely indicate a regression here.

Your DSDT does appear to have the same WMNB methods for the screenpad's backlight (#6 (comment)), which is what the SPLC ssdt re-implements. Did you notice any issues with the screenpad backlight as well?

Can you upload your DSDT for comparison? Thanks. I assume all these required only your SSDT-ATKD.aml, SSDT-PNLF.aml, SSDT-KBLC.aml, and SSDT-SPLC.aml? I actually don't have _Q76 in my UX582 DSDT.

@Qonfused
Copy link
Owner Author

Can you upload your DSDT for comparison? Thanks. I assume all these required only your SSDT-ATKD.aml, SSDT-PNLF.aml, SSDT-KBLC.aml, and SSDT-SPLC.aml? I actually don't have _Q76 in my UX582 DSDT.

I have the DSDTs for the UX481FA and UX481FL in the Resources/ACPI directory. I've isolated all the control methods/etc for the keyboard in KBLC, and the screenpad in SPLC; the ATKD SSDT just binds the keyboard EC events to these control methods.

Another thing to note is that as OLED requires per-pixel voltage calculations to offset display brightness, it may not be compatible with Apple's native protocol for OLED/Retina displays. I know that the LG Ultrafine uses the native protocol, but it is engineered specifically for macOS. The more generally compatible approach is to use a gamma table to control backlight, which MonitorControl implements; the Lunar app may be doing this as well.

There aren't any _Q0E/_Q0F EC methods in your DSDT.

@shiecldk
Copy link
Contributor

shiecldk commented Jan 27, 2023

Ah, I forget I have these hot patches now with OpenCore on DSDT, so you should probably look for XQ0E, etc:
Screen Shot 2023-01-27 at 11 44 44 PM

@Qonfused
Copy link
Owner Author

Those are just ACPI renames for if you had _Q0E/_Q0F EC methods in your DSDT. The idea is that you create a new method for controlling them in macOS and just call the renamed methods XQ0E/XQ0F when not using macOS.

@shiecldk
Copy link
Contributor

shiecldk commented Jan 27, 2023

Yeah, I understand. I mean, originally, they were _Q0E, etc before I renamed them to XQ0E, etc with OpenCore. The DSDT I provided was pull directly on macOS after being already patched by OpenCore.

Screen Shot 2023-01-27 at 11 49 19 PM

@Qonfused
Copy link
Owner Author

Ah I didn't realize this was pulled from macOS. I'm not sure that there is a native implementation in your firmware that those EC methods actually call, otherwise backlight would work natively with BrightnessKeys. You could try removing the ACPI renames for _Q0E/_Q0F and apply the MSOS fix I mentioned at the top of this issue if it's just being blocked by that MSOS check.

@shiecldk
Copy link
Contributor

@Qonfused Is there a way to increase the ScreenPad backlight steps to 8 or 16? I see the current steps are 5-6.

@Qonfused
Copy link
Owner Author

Qonfused commented Jan 27, 2023

The intensity value is written as some fraction of 255, so 16 steps technically would work when rounding each step value to 16 (approx 255/16). I was thinking of faking a new PNLF device for the screenpad so that macOS shows a brightness indicator (the same as when changing the internal display's brightness).

@Qonfused
Copy link
Owner Author

Qonfused commented Jan 27, 2023

I'll need to deoptimize when compiling this SSDT, but the 0x33 isn't a magic number: it's just 51=255/5. Here's how I would refactor the SPLC methods to do that:

// Backlight down
Method (SPLD, 0, Serialized)
{
    Local0 = DerefOf (SPBL [One])
    If ((Local0 > Zero))
    {
        SPBL [One] = (Local0 - 0x10) // Use 0x10 for 255/16 ~= 16
        // Add check here to reset negative values to zero
        If ((DerefOf (SPBL [One]) < 0x00)) { SPBL [One] = 0x00 }
        //
        WEBC (0x13, 0x02, SPBL)
    }
}

// Backlight up
Method (SPLU, 0, Serialized)
{
    Local0 = DerefOf (SPBL [One])
    If ((Local0 < 0xFF))
    {
        SPBL [One] = (Local0 + 0x10) // Use 0x10 for 255/16 ~= 16
        // Add check here to reset OOB (>255) values to 0xFF
        If ((DerefOf (SPBL [One]) > 0xFF)) { SPBL [One] = 0xFF }
        //
        WEBC (0x13, 0x02, SPBL)
    }
}

Note that the SPBL value stores the previous/new backlight value at index one of the buffer. The top level if statements would check the previous backlight value for whether backlight is already at min/max brightness.

@shiecldk
Copy link
Contributor

shiecldk commented Jan 27, 2023

Thanks. It looks like there are errors after adding
If ((DerefOf (SPBL [One]) < 0x00)) { SPLB [One] = 0x00 }
and
If ((DerefOf (SPBL [One]) > 0xFF)) { SPLB [One] = 0xFF }
Screen Shot 2023-01-28 at 4 14 41 AM
Are there some syntax errors?

Edit: I solve it now. Seems to be an issue to directly copy from GitHub to MaciASL editor.

@Qonfused
Copy link
Owner Author

Whoops, that's a typo. It's supposed to be SPBL [One] in each of those.

@shiecldk
Copy link
Contributor

I just use 0x11 now since macOS has 16 steps (actually 255/17=15) for brightness control. I think it would be great to implement some brightness indicator since it's a bit hard to check which is the brightest brightness. lol

@shiecldk
Copy link
Contributor

I solved the keyboard backlight issue by disabling the SMCLightSensor.kext as described here (6c1dd86).

Do you have a problem with the FN key lock after waking from sleep? I have to unlock and lock the FN in order to reactive the FN keys.

@Qonfused
Copy link
Owner Author

Qonfused commented Jan 28, 2023

I have the same issue. It's a simple fix, but I haven't addressed it yet. It can be fixed using something similar to the screenpad reset method (SPRS) after sleep. @wern-apfel does this by renaming the NWAK function for sleep to XWAK and resetting the keyboard backlight there, though there may be a better way to do this.

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 1, 2023

Tried reproducing the issue, but it seems my FN lock key works properly before/after sleep. I believe I was thinking of an older issue with my previous comment, though I had thought this was still an issue. @shiecldk Does anything else break after sleep that could be related?

@shiecldk
Copy link
Contributor

shiecldk commented Feb 1, 2023

@Qonfused I haven't tried @wern-apfel 's NWAK fix. I'll try it later to see if it fixes the FN keys after sleep and waking. I can still use the FN keys. It's just if I don't reactivate the FN keys by pressing FN+Esc, it will not work after waking.

Could you explain what these two ACPI patches are used for in OpenCore and if they are necessary?
Screen Shot 2023-02-01 at 10 06 20 PM

Also, for FN+F7 and FN+Display Switch (for ScreenPad Brightness), do you think it would be possible to implement some sort of countdown for pressing the FN keys, so that, for instance, when holding the FN+F7 long enough while changing the keyboard backlight to brighter, it would actually reverse the brightness direction (to the deducing direction)?

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 1, 2023

Those ACPI renames accompany the GPIO patch for the trackpad and primary display. They aren't needed if you remove the _CRS methods in the GPIO SSDT for those devices; they don't currently work atm.

What you're describing there would have to be implemented as part of a kext. You can enable key-repeating for media keys (probably in accessibility settings).

@shiecldk
Copy link
Contributor

shiecldk commented Feb 2, 2023

@Qonfused I added this SSDT-NWAK.aml, and it solves my wake issue with FN keys. However, it generates a new problem: the ScreenPad is in black screen (with the backlight available thought) after boot or wake but it can be reenabled after using the FN+Enable/Disable ScreenPad (_Q32) key. It looks like it's conflicting with your SSDTs. Any ideas on how to solve this?
SSDT-NWAK.aml.zip
Screen Shot 2023-02-02 at 8 57 39 PM

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 2, 2023

These lines disable the screenpad connector:

Local0 = Buffer (0x02)
{
    0x00, 0x00
}
Local0 [One] = 0x00010082
\_SB.PCI0.LPCB.EC0.WEBC (0x13, 0x02, Local0)

@shiecldk
Copy link
Contributor

shiecldk commented Feb 2, 2023

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 2, 2023

Thank you very much. It works now. Have you checked these?
https://github.com/5T33Z0/OC-Little-Translated
https://5t33z0.gitbook.io/oc-litte-translated/
https://github.com/daliansky/OC-little
https://ocbook.tlhub.cn/
They are pretty useful. I'm dropping SSDT-IMEI.aml from my OC config since it's not required based on this instruction: https://github.com/5T33Z0/OC-Little-Translated/tree/2c03786225a580647771c37ce9bbf94696eb43fc/01_Adding_missing_Devices_and_enabling_Features#functional-ssdts

I had posted a link for the 5t33z0 oc-little docs in #7, but I haven't seen the others.

Also adding SSDT-ARTC.aml and SSDT-FWHD.aml based on this: https://github.com/5T33Z0/OC-Little-Translated/tree/2c03786225a580647771c37ce9bbf94696eb43fc/01_Adding_missing_Devices_and_enabling_Features#cosmetic-ssdts-optional

Those fall under cosmetic SSDTs, which even if present on real macs, they aren't necessary for enabling any device/macOS functionality. It may be worth comparing against the MacbookPro16,1 DSDT, which is the closest match to our SMBIOS models. I wouldn't focus on adding any cosmetic ACPI patches unless there may be a functional reason for it.

@shiecldk
Copy link
Contributor

shiecldk commented Feb 4, 2023

I found out it seems the sleep is broken on my ScreenPad after implementing the SSDT-NWAK.aml. I'm trying to see how can I fix this. Do you have some ideas how can be the cause of it? Thanks.

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 4, 2023

Does the screenpad remain black or does it not shut off during sleep?

@shiecldk
Copy link
Contributor

shiecldk commented Feb 4, 2023

Looks like it remains not shut off during sleep.

@Qonfused
Copy link
Owner Author

Qonfused commented Feb 4, 2023

The hall sensor that detects when the lid is closed may not function until the lid is around half a centimeter from the keyboard; I'd check if the primary display and keyboard backlight turn off. The expected behavior is for both displays to turn off (at the same time) shortly before the keyboard backlight.

@shiecldk
Copy link
Contributor

shiecldk commented Feb 7, 2023

Actually, I found out it seems sleep is broken after the SSDT-NWAK.aml implementation. The ScreenPad does not sleep and the laptop still keeps running.

@Qonfused Qonfused changed the title Use native EC methods with SSDT-PNLF Use native backlight EC methods Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project:UX481 For variants UX481FA (14", 2019) and UX481FL (14", 2019) status:in-progress Implementation is in progress. type:feature New feature request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants