[BUG] Black screen 3 minutes after booting, CoffeeLake UHD 630

[headkaze]

I'll see if I can figure how to calculate the base address correctly. Surprised Intel would change something as basic as the BAR1 though.

Did you see my post regarding this? I believe BAR0 and BAR1 need to be combined to make a 64-bit address now.

BTW, was looking at the Clover code, and it seems to have some code to set LEVX/LEVD. Does that code not work? The code in Clover is a bit over complex (eg. device-id is NOT a factor in determining the max, it is only guessed that way in SSDT-PNLF because SSDT-PNLF has no idea which ig-platform-id is being used).

The code works but as soon as the CFL fb kext loads the backlight registers get overwritten. Backlight max is no longer stored in the ig-platform-id list for CFL. Clover does not have access to this data either so it would have to be hard code based on ig-platform-id and for CFL use values already stored in the registers.

 

[RehabMan]

Did you see my post regarding this? I believe BAR0 and BAR1 need to be combined to make a 64-bit address now.

The debug output proves there is no problem accessing the registers.

The code works but as soon as the CFL fb kext loads the backlight registers get overwritten.

Overwritten by what? And why would Apple software overwrite with values that are larger than what is used on real Mac hardware?

I think we still have a mystery to unravel here. Maybe another ACPI table that has this "larger" backlight setup data?

Backlight max is no longer stored in the ig-platform-id list for CFL.

I guess this info in Framebuffer.bt is inaccurate?

struct FramebufferCFL {
 ...
    local uint32_t fBacklightFrequency <read=frequencyToPrintable> = 0 ;
    local uint32_t fBacklightMax <read=frequencyToPrintable> = 0;
};

I see this in WhateverGreen s Clover does not have access to this data either so it would have to be hard code based on ig-platform-id

The problem with Clover code right now is that it is using device-id and ig-platform-id.
It should be using ig-platform-id exclusively.

But yes, WhateverGreen has access to the ig-platform-id, so it won't have to guess based on device-id.

and for CFL use values already stored in the registers.

Yes, just like SSDT-PNLF adjusting current backlight level scaled to new max.

 

[headkaze]

The debug output proves there is no problem accessing the registers.

Must be in the ACPI language to combine them already then. The way I discovered this was IGPU/IODeviceMemory entry in ioreg value was much bigger than the value read from BAR0. So eventually I discovered they needed to be combined.

Overwritten by what? And why would Apple software overwrite with values that are larger than what is used on real Mac hardware?

I think we still have a mystery to unravel here.

Overwritten by the formula that causes the truncation and thus the black screen issue (in AppleIntelFramebufferController::hwSetBacklight and AppleIntelFramebuffer::setAttributeForConnection). There's no mystery at this point I've explained it in great detail. Look at my WEG patch. Look at the methods in AppleIntelCFLGraphicsFramebuffer that I wrap to set the registers with values that do not truncate.

I've already explained that the PWM_FREQ values on Apple hardware do not overflow 32-bits when used to calculate PWM_DUTY so it's a non-issue for their hardware. Apple uses 0x56CE or 0x4571 while our hardware uses 0x1D4C0.

I guess this info in Framebuffer.bt is inaccurate?

struct FramebufferCFL {
 ...
    local uint32_t fBacklightFrequency <read=frequencyToPrintable> = 0 ;
    local uint32_t fBacklightMax <read=frequencyToPrintable> = 0;
};

"local" means it's a local variable and not part of the struct.

The problem with Clover code right now is that it is using device-id and ig-platform-id.
It should be using ig-platform-id exclusively.

You can set the value via IntelMaxValue but I agree it should be set based on ig-platform-id for < CFL at least.

But yes, WhateverGreen has access to the ig-platform-id, so it won't have to guess based on device-id.

Yes, for everything < CFL. But as we've discovered anyway it's not max value that is the issue with CFL. We know it's 0xFFFF. The issue is with PWM_FREQ. But we can just use the value already set from the BIOS which is what Clover does now.

 

[RehabMan]

I've already explained that the PWM_FREQ values on Apple hardware do not overflow 32-bits when used to calculate PWM_DUTY so it's a non-issue for their hardware. Apple uses 0x56CE or 0x4571 while our hardware uses 0x1D4C0.

But SSDT-PNLF.aml has overwritten this 0x1d4c0 value at LEVD with 0xFFFF.
Right?
So how does it get set back to 0x1d4c0?
Or are you referring to some other register that SSDT-PNFL also needs to adjust?

"local" means it's a local variable and not part of the struct.

OK, strange language there (evidently not quite like C).

You can set the value via IntelMaxValue but I agree it should be set based on ig-platform-id for < CFL at least.

Not sure I understand "< CFL".

Yes, for everything < CFL. But as we've discovered anyway it's not max value that is the issue with CFL. We know it's 0xFFFF. The issue is with FREQ. But we can just use the value already set from the BIOS which is what Clover does now.

I will go back and find this FREQ register... then we can set that in SSDT-PNLF as well.

Edit: Does your mention of FREQ correspond to BXT_BLC_PWM_FREQ1? Because I'm already setting that to 0xFFFF in SSDT-PNLF.aml.

 

[headkaze]

But SSDT-PNLF.aml has overwritten this 0x1d4c0 value at LEVD with 0xFFFF.
Right?

That's not right. 0x1d4c0 is correct. Why are you overwriting it with 0xFFFF? I've told you the formula.

Leave PWM_FREQ as 0x1d4c0. Then set PWM_DUTY = BACKLIGHT_LEVEL * PWM_FREQ / 0xFFFF.

So how does it get set back to 0x1d4c0?
Or are you referring to some other register that SSDT-PNFL also needs to adjust?

There are several places in AppleIntelCFLGraphicsFramebuffer where it writes to these registers.

OK, strange language there (evidently not quite like C).

Yeah, it caught me off guard at first too. I think he uses local variables so he doesn't have to write separate routines for every different struct.

Not sure I understand "< CFL".

Just means "less than" CFL. So anything before Coffee Lake.

I will go back and find this FREQ register... then we can set that in SSDT-PNLF as well.

Edit: Does your mention of FREQ correspond to BXT_BLC_PWM_FREQ1? Because I'm already setting that to 0xFFFF in SSDT-PNLF.aml.

Yes, PWM_FREQ == BXT_BLC_PWM_FREQ1 == 0xC8254 and PWM_DUTY == BXT_BLC_PWM_DUTY1 == 0xC8258.

Again don't set BXT_BLC_PWM_FREQ1 to 0xFFFF. It's not right. Leave it to the default value. For CFL you need to set BXT_BLC_PWM_DUTY1 based on that formula I gave.

 

[RehabMan]

That's not right. 0x1d4c0 is correct. Why are you overwriting it with 0xFFFF? I've told you the formula.

Leave PWM_FREQ as 0x1d4c0. Then set PWM_DUTY = BACKLIGHT_LEVEL * PWM_FREQ / 0xFFFF.

We want our PWM_FREQ at 0xFFFF as that is what is expected by AppleBacklight brightness profiles (eg. the list of brightness levels, values for PWM_DUTY is in the range of 0-0xFFFF).

This is always how we have always typically done this patch... we match the values the graphics kexts expect, instead of trying to patch the graphics kexts to use BIOS values (IntelBacklight.kext is able to match BIOS values, but it stopped working for some reason we haven't figured out).

If we want to try to match graphics kexts to BIOS that is a whole different scenario, which requires a completely different approach. That approach is going to be very tricky since the backlight profile data for each backlight level is limited to 16-bits. That is, you can't get the AppleBacklight.kext to go to full brightness as 0xFFFF would be the limit (0xFFFF/0x1d4c0 is only 54%). I would guess this is the reason Apple decided to stick with 0xFFFF.

There are several places in AppleIntelCFLGraphicsFramebuffer where it writes to these registers.

I guess we'll have to find out where it is setting FREQ again after we already set it to 0xFFFF.

Just means "less than" CFL. So anything before Coffee Lake.

Yeah, I get that... I just don't understand why we expect to do something fundamentally different for CFL. We have to live within AppleBacklight limitations.

Yes, PWM_FREQ == BXT_BLC_PWM_FREQ1 == 0xC8254 and PWM_DUTY == BXT_BLC_PWM_DUTY1 == 0xC8258.

OK, great... this ratio (PWM_DUTY/PWM_FREQ) defines the perceived brightness level.

Again don't set BXT_BLC_PWM_FREQ1 to 0xFFFF. It's not right. Leave it to the default value. For CFL you need to set BXT_BLC_PWM_DUTY1 based on that formula I gave.

I don't get this. Why would we set BXT_BLC_PWM_DUTY1 to be smaller than BXT_BLC_PWM_FREQ1 if we expect to retain the same backlight level that the system was started with? You can try it though... easy enough to comment out the relevant lines in SSDT-PNLF and test.

For now, I suggest you guys push the WEG fix. I don't think I'll be getting to the bottom of this until I have the hardware in hand (which will be a while for me, as I'm waiting for something worth buying).

 

[headkaze]

We want our PWM_FREQ at 0xFFFF as that is what is expected by AppleBacklight brightness profiles (eg. the list of brightness levels, values for PWM_DUTY is in the range of 0-0xFFFF).

This is always how we have always typically done this patch... we match the values the graphics kexts expect, instead of trying to patch the graphics kexts to use BIOS values (IntelBacklight.kext is able to match BIOS values, but it stopped working for some reason we haven't figured out).

Just a hunch but the issue in IntelBacklight could be this:

   // setup BAR1 address...
    m_baseMap = m_provider->mapDeviceMemoryWithRegister(kIOPCIConfigBaseAddress0);

I would check that m_baseMap matches the value in IGPU/IODeviceMemory.
m_baseMap should match the first address entry:

"IODeviceMemory" = (({"address"=547876765696,"length"=16777216}),({"address"=547608330240,"length"=268435456}),"IOSubMemoryDescriptor is not serializable")

If not you may need to combine kIOPCIConfigBaseAddress0 / kIOPCIConfigBaseAddress1.

If we want to try to match graphics kexts to BIOS that is a whole different scenario, which requires a completely different approach. That approach is going to be very tricky since the backlight profile data for each backlight level is limited to 16-bits. That is, you can't get the AppleBacklight.kext to go to full brightness as 0xFFFF would be the limit (0xFFFF/0x1d4c0 is only 54%). I would guess this is the reason Apple decided to stick with 0xFFFF.

I don't pretend to understand how PNLF works nor can I confidently read ACPI code. If FB kexts write to these registers (as they do) in places like AppleIntelFramebufferController::hwSetBacklight how does PNLF work at all? PNLF writes to the registers directly right? But aren't you fighting against the FB kext that also writes to these registers? My thought was your PNLF somehow overrides it. That's why I was holding out hope you could come up with a PNLF-only solution to this.

I guess we'll have to find out where it is setting FREQ again after we already set it to 0xFFFF.

There are a few places but AppleIntelFramebufferController::hwSetBacklight and AppleIntelFramebuffer::setAttributeForConnection are the two most important ones. I found once I write back to the registers after these calls it seems to be enough. In fact for hwSetBacklight I don't call the original function at all otherwise you get flickering.

Yeah, I get that... I just don't understand why we expect to do something fundamentally different for CFL. We have to live within AppleBacklight limitations.

I don't really understand why it's changed so much in CFL either.

I don't get this. Why would we set BXT_BLC_PWM_DUTY1 to be smaller than BXT_BLC_PWM_FREQ1 if we expect to retain the same backlight level that the system was started with? You can try it though... easy enough to comment out the relevant lines in SSDT-PNLF and test.

Using the formula BACKLIGHT_LEVEL * PWM_FREQ / 0xFFFF to set max 0xFFFF * 0x1d4c0 / 0xFFFF = 0x1d4c0. Same as the default not smaller.

For now, I suggest you guys push the WEG fix. I don't think I'll be getting to the bottom of this until I have the hardware in hand (which will be a while for me, as I'm waiting for something worth buying).

Are you sure you don't want to have a few people try it out first? Or do you not think a PNLF-only solution is possible now? Do you think the fix should be rolled into AppleBacklightFixup or just left in WEG for now?

 

[RehabMan]

Just a hunch but the issue in IntelBacklight could be this:

   // setup BAR1 address...
    m_baseMap = m_provider->mapDeviceMemoryWithRegister(kIOPCIConfigBaseAddress0);

No.

IntelBacklight.kext actually works when the methods within are called.
Brightness by bergdesign works with IntelBacklight.kext.
But the brightness slider in SysPrefs->Displays does not (it never calls into the kext to change brightness).
Something has changed in a framework somewhere, that makes the SysPrefs stuff fail, but evidently Brightness by bergdesign doesn't use the same framework or the same calls within anway.

I don't pretend to understand how PNLF works nor can I confidently read ACPI code. If FB kexts write to these registers (as they do) in places like AppleIntelFramebufferController::hwSetBacklight how does PNLF work at all? PNLF writes to the registers directly right?

On previous architectures, the kexts are setting the backlight duty during brightness changes and the freq/max on wake from sleep. They are set to the values expected for the given ig-platform-id (as coded in ig-platform table).

But aren't you fighting against the FB kext that also writes to these registers? My thought was your PNLF somehow overrides it.

No.
_INI methods in ACPI are called very early in ACPI initialization, then never called again.

Using the formula BACKLIGHT_LEVEL * PWM_FREQ / 0xFFFF to set max 0xFFFF * 0x1d4c0 / 0xFFFF = 0x1d4c0. Same as the default not smaller.

But then there is no change at all. As we already know, FREQ and DUTY are already both 0x1d4c0.

The entire goal of the _INI code is simply to set the state of the backlight registers (at boot only) to the values that are used by Apple products of the same architecture/same ig-platform-id. If people want only the part of SSDT-PNLF.aml that triggers AppleBacklight to load, they can strip it down or set RMCF.BKLT=0.

Are you sure you don't want to have a few people try it out first? Or do you not think a PNLF-only solution is possible now? Do you think the fix should be rolled into AppleBacklightFixup or just left in WEG for now?

I will leave the SSDT-PNLF code as is. If people want to try it, or attempt modifications of it, that's fine... have at 'er.

The fix that you have should stay in WEG, I think. At least for now.

 

[headkaze]

_INI methods in ACPI are called very early in ACPI initialization, then never called again.

In that case I don't think a PNLF-only solution for CFL is possible.

 

[RehabMan]

In that case I don't think a PNLF-only solution for CFL is possible.

If we could reproduce the startup PWM configuration of a real Mac before the graphics kexts start working with the PWM setup, it would be possible.
But I guess there is something somewhere that is forcing the out-of-range (for Apple) PWM values, and we haven't found where/what it is in order to fix it in SSDT-PNLF.aml (or Clover, or other handy "init" place).