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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 23 commits into from
Jun 10, 2023
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9e3e2eb
BLRS: Add the definition of the assembly patch descriptor.
0xFireWolf Jun 1, 2023
ac8bfe5
BRS: Add a new patch submodule that reverts the optimizations done in…
0xFireWolf Jun 1, 2023
45eccad
Project: Update the change log.
0xFireWolf Jun 1, 2023
c18444e
Project: Add the boot argument and the device property to README.
0xFireWolf Jun 1, 2023
8147b64
BRS: Fix typos in the documentation.
0xFireWolf Jun 1, 2023
ba3ca8e
BRS: Fix the indentation in the documentation.
0xFireWolf Jun 1, 2023
f191739
Project: Revise the changeling.
0xFireWolf Jun 1, 2023
039806f
BRS: Preserve the base address of the MMIO region stored in `%rcx` be…
0xFireWolf Jun 1, 2023
9304cd6
BLR: Simplify the implementation.
0xFireWolf Jun 4, 2023
35ac5b1
BLT: Add a new submodule that analyzes backlight related functions an…
0xFireWolf Jun 5, 2023
6575154
BLT: Use `getMember()` to update the member field because it returns …
0xFireWolf Jun 5, 2023
496d07a
IGFX: Mark `BRS` deprecated.
0xFireWolf Jun 5, 2023
a4dded9
BLT: Preserve the PWM frequency set by the system firmware and write …
0xFireWolf Jun 5, 2023
4caf663
BLT: Fix a typo.
0xFireWolf Jun 6, 2023
ea6d7e8
BLT: Now available as of macOS 13.4 since macOS 14.0 DP1 contains the…
0xFireWolf Jun 6, 2023
6c1294a
IGFX: Remove the deprecated Backlight Registers Supplemental Fix (BRS…
0xFireWolf Jun 6, 2023
8017057
BLT: Add the description of the submodule to the file-level abstract.
0xFireWolf Jun 6, 2023
f5ce20e
Project: Revise the change log.
0xFireWolf Jun 6, 2023
dfe7b7c
Project: Update the README.
0xFireWolf Jun 6, 2023
198e479
BLT: Revise the abstract of the patch submodule.
0xFireWolf Jun 6, 2023
5c2cae8
Project: Update the manual to document the new Backlight Registers Al…
0xFireWolf Jun 6, 2023
d0037fd
Project: Update the README.
0xFireWolf Jun 6, 2023
8432fbc
Merge branch 'master' into squashed
0xFireWolf Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
WhateverGreen Changelog
=======================
#### v1.6.5
- Revert the optimizations done by the compiler in backlight related functions to make Backlight Registers Fix (BLR) and Backlight Smoother (BLR) work on macOS 13.4. (by @0xFireWolf)
0xFireWolf marked this conversation as resolved.
Show resolved Hide resolved

#### v1.6.4
- Fixed Radeon RX 5500 XT identification regression

Expand Down
33 changes: 32 additions & 1 deletion WhateverGreen/kern_igfx.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1600,6 +1600,36 @@ class IGFX {
void processFramebufferKext(KernelPatcher &patcher, size_t index, mach_vm_address_t address, size_t size) override;
} modBacklightRegistersFix;

/**
* A submodule to revert inlined invocations of backlight-related functions, making the submodules, BLR and BLS, work properly on macOS 13.4.
*
* @note Supported Platforms: CFL.
*/
class BacklightRegistersSupplementalFix: public PatchSubmodule {
/**
* Get assembly patches for the current framebuffer driver
*
* @param index The bundle index of the current framebuffer driver
* @return An array of assembly patches on success, `nullptr` if the given framebuffer driver is not supported.
*/
const AssemblyPatch *getPatches(size_t index) const;

/**
* Apply the given assembly patches to the current framebuffer driver
*
* @param patcher The kernel patcher
* @param index The bundle index of the current framebuffer driver
* @param patches An array of assembly patches, terminated by a NULL patch
*/
bool applyPatches(KernelPatcher &patcher, size_t index, const AssemblyPatch *patches NONNULL) const;

public:
// MARK: Patch Submodule IMP
void init() override;
void processKernel(KernelPatcher &patcher, DeviceInfo *info) override;
void processFramebufferKext(KernelPatcher &patcher, size_t index, mach_vm_address_t address, size_t size) override;
} modBacklightRegistersSupplementalFix;

/**
* Brightness request event source needs access to the original WriteRegister32 function
*/
Expand Down Expand Up @@ -1835,7 +1865,7 @@ class IGFX {
/**
* A collection of submodules
*/
PatchSubmodule *submodules[20] = {
PatchSubmodule *submodules[21] = {
&modDVMTCalcFix,
&modDPCDMaxLinkRateFix,
&modCoreDisplayClockFix,
Expand All @@ -1852,6 +1882,7 @@ class IGFX {
&modPAVPDisabler,
&modReadDescriptorPatch,
&modBacklightRegistersFix,
&modBacklightRegistersSupplementalFix,
&modBacklightSmoother,
&modFramebufferDebugSupport,
&modMaxPixelClockOverride,
Expand Down
205 changes: 204 additions & 1 deletion WhateverGreen/kern_igfx_backlight.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,209 @@
/// 2. Backlight smoother that makes brightness transitions smoother on IVB+.
///

//
// MARK: - Backlight Registers Fix (Supplemental)
//

static constexpr const char* kHwSetBacklightSymbol = "__ZN31AppleIntelFramebufferController14hwSetBacklightEj";
static constexpr const char* kLightUpEDPSymbol = "__ZN31AppleIntelFramebufferController10LightUpEDPEP21AppleIntelFramebufferP21AppleIntelDisplayPathPK29IODetailedTimingInformationV2";
static constexpr const char* kHwSetPanelPowerSymbol = "__ZN31AppleIntelFramebufferController15hwSetPanelPowerEj";

//
// Supplemental Patch 1: Use WriteRegister32() to modify the register `BXT_BLC_PWM_FREQ1` instead of modifying mapped memory directly
//
// Function: AppleIntelFramebufferController::hwSetBacklight()
Copy link
Collaborator

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?

Copy link
Collaborator

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:

  1. Allocate a 16K object A to then use it as a dummy AppleIntelFramebufferController.
  2. Fill the object with 32-bit 0x00000001 values all over.
  3. Allocate another 16K object B to then use it as a dummy memory space
  4. Fill first 0x500 64-bit values of A with pointers to (B - 0xC8254) + 8 * i for MMIO space.
  5. Disable CamelliaBase::SetDPCDBacklight by temporarily patching it out (e.g. C3).
  6. Execute AppleIntelFramebufferController::hwSetBacklight with object A and let's say pass 255.
  7. Find the 32-bit value 0x000000FF in object A, this will be your backlight level offset.
  8. 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.

Copy link
Contributor Author

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.
  • Operations related to checking whether the register address is valid can be removed.
    • i.e., No need to resolve the member field at 0x1d30.
  • 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.
  • 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.
  • Apple's PWM frequency divider is hard coded to 0xFFFF and stores in the member field at 0x2e80, but there is no guarantee that the value remains the same forever.
    • i.e., Need to resolve the member field at 0x2e80.
  • hwSetPanelPower and LightUpEDP always invoke hwSetBacklight with the brightness value stored in the member field at 0x2e78, so it's weird that hwSetBacklight 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.
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.

// Offset: 53
// Version: macOS 13.4
// Platform: CFL
//
// Find: movl 0x2e84(%rbx), %eax // Fetch the register value (this->field_0x2e84)
// movq 0x1a08(%rbx), %rcx // Fetch the base address of the MMIO region
// movl %eax, 0xc8254(%rcx) // Write the register value to the register at 0xc8254
//
// Replace: movl 0x2e84(%rbx), %edx // The register value is the 3rd argument
// movl $0xc8254, %esi // The register address is the 2nd argument
// movq %rbx, %rdi // The implicit controller instance is the 1st argument
// call 0x146d98f6 // Call AppleIntelFramebufferController::WriteRegister(address, value)
// nop // Spare byte
//
//
static constexpr uint8_t kHwSetBacklightPatch1_CFL_134[] = {
0x8B, 0x93, 0x84, 0x2E, 0x00, 0x00, 0xBE, 0x54, 0x82, 0x0C, 0x00, 0x48, 0x89, 0xDF, 0xE8, 0x00, 0xB4, 0xFE, 0xFF
};
static constexpr size_t kHwSetBacklightOffset1_CFL_134 = 53;

//
// Supplemental Patch 2: Use WriteRegister32() to modify the register `BXT_BLC_PWM_DUTY1` instead of modifying mapped memory directly
//
// Function: AppleIntelFramebufferController::hwSetBacklight()
// Offset: 108
// Version: macOS 13.4
// Platform: CFL
//
// Find: movq 0x1a08(%rbx), %rcx // Fetch the base address of the MMIO region
// movl %eax, 0xc8258(%rcx) // Write the register value to the register at 0xc8258
//
// Replace: movl %eax, %edx // The register value is the 3rd argument
// movl $0xc8258, %esi // The register address is the 2nd argument
// call 0x146d98f6 // Call AppleIntelFramebufferController::WriteRegister(address, value)
// nop // Spare byte
//
// Note that this patch does not need to set the controller instance, because %rdi is set by the first patch.
//
static constexpr uint8_t kHwSetBacklightPatch2_CFL_134[] = {
0x89, 0xC2, 0xBE, 0x58, 0x82, 0x0C, 0x00, 0xE8, 0xD0, 0xB3, 0xFE, 0xFF, 0x90
};
static constexpr size_t kHwSetBacklightOffset2_CFL_134 = 108;

//
// Supplemental Patch 3: Revert inlined hwSetBacklight() invocation
//
// Function: AppleIntelFramebufferController::LightUpEDP()
// Offset: 488
// Version: macOS 13.4
// Platform: CFL
//
// Find: movq 0x2e60(%r15), %rdi // Beginning of the inlined function call
// testq %rdi, %rdi // %r15 stores the implicit controller instance
// je loc_146f78b6
// movl 0x2e78(%r15), %esi
//
// Replace: movl 0x2e78(%r15), %esi // Fetch the target backlight level which is the 2nd argument
// movq %r15, %rdi // The implicit controller instance is the 1st argument
// call 0x146ee4ae // Call AppleIntelFramebufferController::hwSetBacklight(level)
// jmp 0x146f7920 // Jump to the end of inlined function call
//
static constexpr uint8_t kLightUpEDPPatch_CFL_134[] = {
0x41, 0x8B, 0xB7, 0x78, 0x2E, 0x00, 0x00, 0x4C, 0x89, 0xFF, 0xE8, 0x01, 0x6C, 0xFF, 0xFF, 0xEB, 0x71
};
static constexpr size_t kLightUpEDPOffset_CFL_134 = 488;

//
// Supplemental Patch 4: Revert inlined hwSetBacklight() invocation
//
// Function: AppleIntelFramebufferController::hwSetPanelPower()
// Offset: 1505
// Version: macOS 13.4
// Platform: CFL
//
// Find: leal 0xfff37da7(%rax), %edx // Beginning of the inlined function call
// cmpl $0xfff37daa, %edx // %r12 stores the implicit controller instance
// ja loc_146ea9e5
// movl 0x2e84(%r12), %eax
//
// Replace: movl 0x2e78(%r12), %esi // Fetch the target backlight level which is the 2nd argument
// movq %r12, %rdi // The implicit controller instance is the 1st argument
// call 0x146ee4ae // Call AppleIntelFramebufferController::hwSetBacklight(level)
// jmp 0x146eaa1c // Jump to the end of inlined function call
//
static constexpr uint8_t kHwSetPanelPowerPatch_CFL_134[] = {
0x41, 0x8B, 0xB4, 0x24, 0x78, 0x2E, 0x00, 0x00, 0x4C, 0x89, 0xE7, 0xE8, 0xDD, 0x3A, 0x00, 0x00, 0xEB, 0x49
};
static constexpr size_t kHwSetPanelPowerOffset_CFL_134 = 1505;

//
// Supplemental patches for the Coffee Lake framebuffer driver on macOS 13.4
//
static AssemblyPatch kBacklightSupplementalPatches_CFL_134[] = {
{kHwSetBacklightSymbol, kHwSetBacklightOffset1_CFL_134, kHwSetBacklightPatch1_CFL_134},
{kHwSetBacklightSymbol, kHwSetBacklightOffset2_CFL_134, kHwSetBacklightPatch2_CFL_134},
{kLightUpEDPSymbol, kLightUpEDPOffset_CFL_134, kLightUpEDPPatch_CFL_134},
{kHwSetPanelPowerSymbol, kHwSetPanelPowerOffset_CFL_134, kHwSetPanelPowerPatch_CFL_134},
{nullptr, 0, nullptr, 0},
};

const AssemblyPatch *IGFX::BacklightRegistersSupplementalFix::getPatches(size_t index) const {
// Fetch the current framebuffer driver
// Guard: Only CFL is supported at this moment (ICL driver does not have this issue, KBL driver is hard to fix)
if (auto framebuffer = callbackIGFX->getRealFramebuffer(index); framebuffer != &kextIntelCFLFb) {
SYSLOG("igfx", "BRS: Only CFL is supported at this moment.");
return nullptr;
}

// Guard: Verify the kernel major version
if (getKernelVersion() != KernelVersion::Ventura) {
SYSLOG("igfx", "BRS: Only macOS Ventura 13.4.x is supported at this moment.");
return nullptr;
}

// Guard: Verify the kernel minor version
switch (getKernelMinorVersion()) {
case 5: // macOS 13.4.x
return kBacklightSupplementalPatches_CFL_134;

default:
SYSLOG("igfx", "BRS: Only macOS Ventura 13.4 is supported at this moment.");
return nullptr;
}
}

bool IGFX::BacklightRegistersSupplementalFix::applyPatches(KernelPatcher &patcher, size_t index, const AssemblyPatch *patches) const {
// Apply each assembly patch
for (const AssemblyPatch *patch = patches; patch->symbol != nullptr; patch += 1) {
// Guard: Resolve the symbol of the function to be patched
auto address = patcher.solveSymbol(index, patch->symbol);
if (address == 0) {
SYSLOG("igfx", "BRS: Failed to resolve the symbol %s. The supplement fix will not work properly.", patch->symbol);
patcher.clearError();
return false;
}

// Guard: Apply the assembly patch
if (patcher.routeBlock(address + patch->offset, patch->patch, patch->patchSize) != 0) {
SYSLOG("igfx", "BRS: Failed to apply the assembly patch to the function %s. The supplement fix will not work properly.", patch->symbol);
patcher.clearError();
return false;
}

DBGLOG("igfx", "BRS: Applied the assembly patch to the function %s successfully.", patch->symbol);
}

return true;
}

void IGFX::BacklightRegistersSupplementalFix::init() {
// We only need to patch the framebuffer driver
requiresPatchingFramebuffer = true;
}

void IGFX::BacklightRegistersSupplementalFix::processKernel(KernelPatcher &patcher, DeviceInfo *info) {
enabled = checkKernelArgument("-igfxbrs");
if (!enabled)
enabled = info->videoBuiltin->getProperty("enable-backlight-registers-supplemental-fix") != nullptr;
if (!enabled)
return;
}

void IGFX::BacklightRegistersSupplementalFix::processFramebufferKext(KernelPatcher &patcher, size_t index, mach_vm_address_t address, size_t size) {
//
// Apple has "accidentally" simplified the implementaion of the functions, Read/WriteRegister, 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 invokes
// `WriteRegister` to update backlight registers; instead it modifies the register value via mapped memory directly, making itself an inline helper.
// `LightUpEDP()` and `hwSetPanelPower()` that invoke `hwSetBacklight()` now have the definition of `hwSetBacklight()` embedded in themselves.
// As such, the `WriteRegister` hooks registered by the Backlight Registers Fix (BLR) and Backlight Smoother (BLS) submodules no longer work.
// This patch submdoule (BRS) reverts the optimizations done by the compiler in aforementioned three functions, thus making BLR and BLS work properly on macOS 13.4.
//
// - FireWolf
// - 2023.06
//
auto self = &callbackIGFX->modBacklightRegistersSupplementalFix;

// Guard: Fetch the assembly patches for the current framebuffer driver
auto patches = self->getPatches(index);
if (patches == nullptr) {
SYSLOG("igfx", "BRS: Your current kernel version and/or platform is not supported.");
return;
}

// Guard: Apply the assemble patches to the current framebuffer driver
if (self->applyPatches(patcher, index, patches)) {
DBGLOG("igfx", "BRS: All assembly patches have been applied to the current framebuffer driver.");
} else {
SYSLOG("igfx", "BRS: Failed to apply some of the assembly patches to the current framebuffer driver.");
}
}

//
// MARK: - Backlight Registers Fix
//
Expand Down Expand Up @@ -83,7 +286,7 @@ void IGFX::BacklightRegistersFix::processFramebufferKext(KernelPatcher &patcher,
callbackIGFX->modMMIORegistersWriteSupport.replacerList.add(&dCFLPWMFreq1);
callbackIGFX->modMMIORegistersWriteSupport.replacerList.add(&dCFLPWMDuty1);
} else {
SYSLOG("igfx", "BLS: [ERR!] Found an unsupported platform. Will not perform any injections.");
SYSLOG("igfx", "BLR: [ERR!] Found an unsupported platform. Will not perform any injections.");
}
}

Expand Down
38 changes: 38 additions & 0 deletions WhateverGreen/kern_igfx_backlight.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,44 @@ static constexpr uint32_t BXT_BLC_PWM_DUTY1 = 0xC8258;
static constexpr uint32_t SFUSE_STRAP = 0xC2014;
static constexpr uint32_t SFUSE_STRAP_RAW_FREQUENCY = 1 << 8;

/**
* Represents an assembly patch
*/
struct AssemblyPatch {
/**
* The symbol of the function to be patched
*/
const char *symbol {nullptr};

/**
* The offset, relative to the address of the function, from which to patch the assembly code
*/
size_t offset {0};

/**
* The assembly patch
*/
const uint8_t *patch {nullptr};

/**
* The number of bytes in the assembly patch
*/
size_t patchSize {0};

/**
* Create an assembly patch
*/
template <size_t N>
constexpr AssemblyPatch(const char *symbol, size_t offset, const uint8_t (&patch)[N])
: symbol(symbol), offset(offset), patch(patch), patchSize(N) {}

/**
* Create an assembly patch
*/
constexpr AssemblyPatch(const char *symbol, size_t offset, const uint8_t *patch, size_t patchSize)
: symbol(symbol), offset(offset), patch(patch), patchSize(patchSize) {}
};

/**
* Ice Lake freq
* Copied from `AppleIntelFramebufferController::start()` function
Expand Down