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
Add a new submodule as an alternative to Backlight Registers Fix (BLR), fixing the 3-minute dark screen issue and making Backlight Smoother (BLS) work on mobile Coffee Lake platforms running macOS 13.4 or later #113
Conversation
… backlight related functions by the compiler on macOS 13.4, making the submodules BLR and BLS work again.
when I use -igfxbrs system always in reboot lop |
…fore calling `hwSetBacklight()` in `hwSetPanelPower()`.
Please try the latest version and see if the kernel panic still exists. |
I think this latest commit fixed the problem |
I appreciate your quick confirmation. |
Ouch, this looks nice functional wise, but we will run into so many issues maintaining this that I really would like to find some other approach or at least use some sort of pattern matching to recognise the patch areas. Do you have an idb for the driver, maybe I have some idea if I give it a look… |
AppleIntelCFLGraphicsFramebuffer_134.i64.zip Sure, here you are. Please take your time. I do feel that it is possible to automate the patch, but it would be complicated. |
// | ||
// Supplemental Patch 1: Use WriteRegister32() to modify the register `BXT_BLC_PWM_FREQ1` instead of modifying mapped memory directly | ||
// | ||
// Function: AppleIntelFramebufferController::hwSetBacklight() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppleIntelFramebufferController::hwSetBacklight is rather short, maybe it is more reasonable to solve the field offsets and simply rewrite the function ourselves?
- We do not have Camelia, so Camelia backlight calls should not matter for us.
- We have our own logic for setting backlight based on passed backlight level (
a2
argument), so two other ifs and MMIO ops also do not matter for us.
What we are left with is:
mov [rbx+2E78h], r14d
backlight level set in the end of the function.mov rcx, [rbx+1A08h]
MMIO space.
Perhaps do it the other way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also be smarter and perform offset detection in dynamic.
For this you could:
- Allocate a 16K object A to then use it as a dummy AppleIntelFramebufferController.
- Fill the object with 32-bit 0x00000001 values all over.
- Allocate another 16K object B to then use it as a dummy memory space
- Fill first 0x500 64-bit values of A with pointers to (B - 0xC8254) + 8 * i for MMIO space.
- Disable CamelliaBase::SetDPCDBacklight by temporarily patching it out (e.g. C3).
- Execute AppleIntelFramebufferController::hwSetBacklight with object A and let's say pass 255.
- Find the 32-bit value 0x000000FF in object A, this will be your backlight level offset.
- Find the 32-bit value 0x000000FF in object B, this will be your MMIO base offset, which you should be able to reverse by applying the formula: offset - 4 - B_base.
Now you have both offsets you need, so you can just reimplement AppleIntelFramebufferController::hwSetBacklight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppleIntelFramebufferController::hwSetBacklight is rather short, maybe it is more reasonable to solve the field offsets and simply rewrite the function ourselves?
I carefully revisited hwSetBacklight
and its callers LightUpEDP
and hwSetPanelPower
, and I think that finding the offset of each required member field and rewriting hwSetBacklight
is the right approach to this issue.
We have our own logic for setting backlight based on passed backlight level, so two other ifs and MMIO ops also do not matter for us.
Yes, we do have own logic for updating registers at 0xc8254
and 0xc8258
, but I am not sure which two MMIO ops you referred to do not matter. Here is Apple's implementation and my thoughts are...
- Operations related to Camellia can be removed from our implementation.
- i.e., No need to resolve the member field at
0x2e60
.
- i.e., No need to resolve the member field at
- Operations related to checking whether the register address is valid can be removed.
- i.e., No need to resolve the member field at
0x1d30
.
- i.e., No need to resolve the member field at
- Operations related to fetching the base address of the MMIO region can be removed, because we will invoke the original
WriteRegister32()
explicitly.- i.e., No need to resolve the member field at
0x1a08
.
- i.e., No need to resolve the member field at
- Using Apple's PWM frequency leads to the 3-minute dark screen, so our implementation will use the frequency set by the firmware and thus won't write to the frequency register at 0xc8254.
- i.e., No need to resolve the member field at
0x2e84
.
- i.e., No need to resolve the member field at
- Apple's PWM frequency divider is hard coded to
0xFFFF
and stores in the member field at0x2e80
, but there is no guarantee that the value remains the same forever.- i.e., Need to resolve the member field at
0x2e80
.
- i.e., Need to resolve the member field at
hwSetPanelPower
andLightUpEDP
always invokehwSetBacklight
with the brightness value stored in the member field at0x2e78
, so it's weird thathwSetBacklight
just stores the same value to that member field again, but I think it's better to keep this in our implementation.- i.e., Need to resolve the member field at
0x2e78
.
- i.e., Need to resolve the member field at
IOReturn AppleIntelFramebufferController::hwSetBacklight(UInt32 brightness)
{
// Guard: Check if Camellia is present
// The member field at 0x2e60 has a type of `CamelliaBase*`
if (this->field_0x2e60 != nullptr)
{
this->field_0x2e60->SetDPCDBacklight(brightness);
}
// Guard: Check whether the register address `0xc8254` is valid
// Note that this check is implemented in `WriteRegister32()`
// The member field at 0x1d30 stores the length of the MMIO region,
// initialized by `AppleIntelFramebufferController::start()`
if (0xc8254 <= this->field_0x1d30 - 4)
{
// The member field at 0x1a08 stores the base address of the MMIO region
// The member field at 0x2e84 stores Apple's PWM frequency which is
// either 0x56CE or 0x4571 depending upon whether the raw frequency is used
// (i.e., whether the bit 8 is set in the register `SFUSE_STRAP` at 0xc2014)
// Set the PWM frequency
*reinterpret_cast<UInt32*>(this->field_0x1a08 + 0xc8254) = this->field_0x2e84;
}
// Guard: Check whether the register address `0xc8258` is valid
if (0xc8258 <= this->field_0x1d30 - 4)
{
// The member field at 0x2e80 stores Apple's PWM frequency divider
// which is set to 0xFFFF in `AppleIntelFramebufferController::getOSInformation()`
// Set the new duty value
*reinterpret_cast<UInt32*>(this->field_0x1a08 + 0xc8258) = this->field_0x2e84 * brightness / this->field_0x2e80;
}
// Store the new brightness level to the member field at 0x2e78
this->field_0x2e78 = brightness;
return kIOReturnSuccess;
}
Your proposed approach that uses dummy controller objects is really interesting, but I think it's better to disassemble hwSetBacklight()
, identify the offsets of the divider and the brightness level and implement the backlight registers fix in our hwSetBacklight()
wrapper. Now the problem lies in the caller site. I managed to disassemble those two callers and generate the assembly patch for each of them at runtime so that they will call hwSetBacklight()
explicitly. I have added a new patch submodule to reflect the new changes and can confirm that it works on my machine properly. It would be great if you can review it and provide suggestions on making the new submodule easier to maintain in future. Once you approve the new changes, I will remove the supplemental fix from the WEG.
thank you! this was exactly what i needed |
Hello. How I can convert it to kext? |
…d generates assembly patch so that each function invokes `hwSetBacklight()` explicitly, providing an alternative solution to the backlight issue on Coffee Lake platforms running macOS 13.4.
…a mutable reference.
…it to the register when `hwSetBacklight()` is called.
I can confirm that WriteRegister32 and hwSetBacklight are inlined in the Coffee Lake framebuffer driver shipped by macOS Sonoma 14.0 DP1 as well. Will check if the submodule can identify the offsets correctly on the new system later. |
once again -igfxblt or -igfxblr break on macOS Sonoma 14 I can confirmed because I have running coffe lake graphics I m waiting 3 minute black screen during boot |
@ameenjuz The new patch submodule can identify the offsets and generate the assembly patch for macOS 14.0, so I have removed the strict check of the current kernel version. Please give the latest version a try and see if it fixes the issue on macOS 14.0 properly. |
It's not fix in CPU i5 8265u |
@Edwardwich If you are using the Kaby Lake graphics driver, then as clearly indicated in the main post, this submodule is NOT applicable to your laptop. |
|
tested on i5-8300h, new patch works without any issue, thanks |
+1 Can confirm the new patch to be working on 9750h , had some issues with the 1st patch where the laptop panicked after sleep and restarted , it works perfectly now! |
@0xFireWolf worked on macOS Sonoma 14 and macOS 13.5 Beta 2 |
macOS Ventura 13.5 DP1 and DP2 are not affected, so you only need |
|
I see. Thanks for testing the patch. |
It works for i5-10210U, Sonoma, Dell Vostro 5490. The equivalent for -igfxbrs on DeviceProperties? Thank you! |
@StefanAlMare Search for "New Boot Argument and Device Property" in the main post. |
its not needed on this platform |
Your opinion. Starting with certain beta Ventura I lost backlight the first 3 minutes until today when I compiled whatevergreen from https://github.com/0xFireWolf/WhateverGreen/tree/squashed and applied the DeviceProperties from this page. I'm now waiting for Whatevergreen-master with this submodule inside. |
interesting then i run Sonoma now without any issues without any BLR fixes for this CPU igpu |
Like you said, interesting! Please give me your IGPU config. |
like you said, |
Can you share the files you compiled? I am an outsider. |
You can always find the latest version under each CI run of this PR. For example, you can download the kext (artifacts) at https://github.com/acidanthera/WhateverGreen/actions/runs/5192008217. |
Thank you🙏. |
This looks much better now. I am positive there may be ways to make it even more reliable, but for now let's see for how long it holds. Thanks! |
In version 13.5 pushed on July 25th, - igfxblt failed and - igfxblr returned to normal |
Abstract:
Apple has "accidentally" simplified the implementation of the functions,
ReadRegister32
andWriteRegister32
, in Coffee Lake's framebuffer driver shipped by macOS 13.4, so the compiler chose to inline invocations of those functions as many as possible. As a result,hwSetBacklight()
no longer invokesWriteRegister32
to update backlight registers; instead it modifies the register value via mapped memory directly, making itself an inline helper.LightUpEDP()
andhwSetPanelPower()
that invokehwSetBacklight()
now have the definition ofhwSetBacklight()
embedded in themselves. As such, theWriteRegister32
hooks registered by the Backlight Registers Fix (BLR) and the Backlight Smoother (BLS) submodules no longer work. This PR adds a new patch submodule (BLT), as an alternative to BLR, that reverts the optimizations done by the compiler in aforementioned three functions, thus fixing the 3-minute dark screen issue and making BLS work properly again on Coffee Lake platforms running macOS 13.4.Affected System Versions:
Affected Users:
Users who have laptops using Coffee Lake's graphics driver and running macOS 13.4 and are experiencing the 3-minute dark screen issue should consider to enable this fix.
New Boot Argument and Device Property:
-igfxblt
or the device propertyenable-backlight-registers-alternative-fix
to IGPU.-igfxblt
instead of-igfxblr
to the boot arguments.-igfxblt
and-igfxbls
to the boot arguments.-igfxblt
and-igfxbls
to the boot arguments.Related Issues:
Additional Notes:
WriteRegister32
are not inlined in backlight related functions.0xC8250
due to the space limit.Let me know if there are any errors or typos I made in this PR.
Please take your time to review it. Thank you.
-- FireWolf
This PR has undergone a major revision on Jun 5. You can find the description of the deprecated Backlight Registers Supplemental Fix (BRS) submodule below, but please be aware that BRS will be removed from this PR before merged, and thus the boot argument
-igfxbrs
and the equivalent device propertyenable-backlight-registers-supplemental-fix
will be removed as well.Deprecated description of the Backlight Registers Supplemental Fix (BRS) submodule
Abstract:
Apple has "accidentally" simplified the implementation of the functions,
ReadRegister32
andWriteRegister32
, in Coffee Lake's framebuffer driver shipped by macOS 13.4, so the compiler chose to inline invocations of those functions as many as possible. As a result,hwSetBacklight()
no longer invokesWriteRegister32
to update backlight registers; instead it modifies the register value via mapped memory directly, making itself an inline helper.LightUpEDP()
andhwSetPanelPower()
that invokehwSetBacklight()
now have the definition ofhwSetBacklight()
embedded in themselves. As such, theWriteRegister32
hooks registered by the Backlight Registers Fix (BLR) and the Backlight Smoother (BLS) submodules no longer work. This PR adds a new patch submodule (BRS) that reverts the optimizations done by the compiler in aforementioned three functions, thus making both BLR and BLS work properly on Coffee Lake platforms running macOS 13.4.New Boot Argument and Device Property:
-igfxbrs
or the device propertyenable-backlight-registers-supplemental-fix
to IGPU.-igfxblr
and-igfxbrs
to the boot arguments.-igfxbls
and-igfxbrs
to the boot arguments.Limitations:
Related Issues:
Additional Notes:
WriteRegister32
are not inlined in backlight related functions.0xC8250
due to the space limit.WriterRegister32
, but this new change in the driver to some extent makes these two submodules fragile.Let me know if there are any errors or typos I made in this PR.
Please take your time to review it. Thank you.
-- FireWolf