Enhanced maximum link rate fix, Fixed the page fault in the LSPCON driver, Submodules refactoring. #72
Conversation
Conflicts: WhateverGreen/kern_igfx.cpp WhateverGreen/kern_igfx.hpp
… to HDMI 2.0 output.
…n SKL, KBL and CFL platforms.
Conflicts: Manual/FAQ.IntelHD.en.md
…e from the adapter as some LSPCON adapters are automatically switched back to LS mode on power off (i.e. Cable removal, sleep/wake up cycle).
There was a problem hiding this comment.
Thanks for the patches, particularly the submodules. Makes it better as we go.
The changes look good. The refactored code was mostly there, so I did not review the it too precisely. Noted a few issues, mostly stylistic.
Also, could you do a history rebase next time, like git rebase origin/master? Would be easier to handle the changes with cleaner commit history.
| @@ -503,7 +437,7 @@ bool IGFX::processKext(KernelPatcher &patcher, size_t index, mach_vm_address_t a | |||
| bool bklKabyFb = realFramebuffer == &kextIntelKBLFb && cflBacklightPatch == CoffeeBacklightPatch::On; | |||
| // Solve ReadRegister32 just once as it is shared | |||
| if (bklCoffeeFb || bklKabyFb || | |||
| RPSControl.enabled || ForceWakeWorkaround.enabled || coreDisplayClockPatch) { | |||
| RPSControl.enabled || ForceWakeWorkaround.enabled || modCoreDisplayClockFix.enabled) { | |||
There was a problem hiding this comment.
This is a bit ugly. Probably we want a separate bool for submodules like requiresGenericRegisterIo. We can make a joint version of this bool in IGFX, assign it in processKernel as an OR of all the submodules values, and then use here.
For now since not all entries are submodules we can assign it some hardcoded entries too. But please add a FIXME.
There was a problem hiding this comment.
Totally agreed. At this moment, backlight fix is the only one that injects code to readRegister32/writeRegister32, while others only need them as helper functions.
The worst case is that multiple fixes request to inject code to those functions, so we probably need something like GenericRegisterModifier to orchestrate injection requests. The same thing might happen in the AUX register case.
For the global controller access, we could apply the same concept as well. Apple has refactored a lot of code from AppleIntelFramebufferController to the new class AppleIntelPort, so a new fix might need the controller instance to fetch the framebuffer instance associated with a port via AppleIntelFramebufferController::getFBFromPort().
|
@vit9696 I think that's all for this PR. I will do a history rebase next time, and I will take a look at the register refactoring (#72 (comment)) once I have time. The current RPS control fix and the force wake workaround are already nicely packed, so I think it would be easy to port them as a submodule. Let me know if there any further changes I need to make before this PR is merged. Thanks for your time. |
|
routeMultiple and solveMultiple should not leave errors after them, so you do not need clearError. The rest looks good. |
Done. |
This pull request contains 2 new features, 1 bug fix, and 5 code refactoring changes on submodules.
>> Enhanced Maximum Link Rate Fix
Abstract:
Users are no longer required to specify a maximum link rate for their builtin display via the
dpcd-max-link-rateproperty. The "invalid" maximum link rate reported by the display panel has a special meaning in the eDP 1.4 standard. eDP 1.4 panels may support more link rates other than traditional ones such as RBR, HBR, HBR2 and HBR3. As such, a table of all supported rates can be found in another DPCD register. This update adds a new probe function to find and check the maximum link rate for eDP 1.4 panels, and hence eliminates the need of a manual value injection. The previous solution is used as a fallback mechanism, in case the probed value is not supported by the graphics driver. Additionally, this fix is extended to support Ice Lake platform.Related Research Article:
Coffee Lake Intel UHD Graphics 630 on macOS Catalina: The ultimate solution to the kernel panic due to division by zero in the framebuffer driver
>> Fixed Page Fault in the LSPCON Driver
Abstract:
ReadAUX()is resolved for the maximum link rate fix but not for the LSPCON driver. As a result, if the maximum link rate fix is disabled, the function pointer isNULLand hence a page fault occurs. LSPCON driver needsReadAUX()to wake up the DisplayPort AUX channel for the adapter. This issue is addressed by resolving the symbol for the LSPCON driver as well.>> Submodules Refactoring
Abstract:
kern_igfx_clock.cppis created to implement these clock-related fixes.kern_igfx_lspcon.cpp/hpp.In general, the size of IGFX is greatly reduced, and it is more clear to identify which functions are routed by a submodule.