Add PowerDownPhy() and PowerUpPhy()

[acidanthera-bot]

@Piker-Alpha opened vit9696/Lilu#27

I read an old note of mine saying something about the fact that the calls to PowerDownPhy() and PowerUpPhy() are missing in SetDPPowerState(). In AppleIntelAzulController::SetDPPowerState() they are called, but they are missing in the Skylake/Kabylake frame buffers.

Thing is. When I boot a Skylake based system then I see warnings in the log stating that the frame buffer attributes can't be read or written to, because it's not woken up yet.

This may very well be the reason why wake after sleep isn't working. Can't we just add something like this in the IGFX plugin?

PowerDownPhy()
SetDPPowerState()
PowerUpPhy()

Possibly with the also missing calls to ReadAUX()

What do you think?

[acidanthera-bot]

@vit9696 commented

Hi,

The idea sounds pretty interesting, although I cannot be sure if it is the cause. I had a talk with lvs, and he also said that it is worth trying. Just wrapping SetDPPowerState should be more than enough, and we have pretty much everything from the arguments to call Power*Phy functions (except the last integer, but it should be 0/1 anyway, could just bruteforce I guess), so it is fairly straight-forward.

The only issue behind trying is that lvs is currently on holidays and I am rather busy irl. Since neither of us has SKL/KBL anyway, it will most likely be faster for you to try.

Vit

[acidanthera-bot]

@Piker-Alpha commented

I cannot even boot with the stock versions of Lilu.kext and IntelGraphicsFixup.kext (both at v1.2.0) but my patched copy of v1.1.3 works.

The last thing that I see is something like IOPlatformPanic -> AppleSMC

[acidanthera-bot]

@vit9696 commented

The easiest question, do you mean that your patched IntelGraphicsFixup 1.1.3 works and the system wakes now or do you mean that IntelGraphicsFixup 1.1.3 works at least somehow?

As for Lilu and latest IntelGraphicsFixup it sounds pretty terrific. They were both tested heavily and are used by many people at the moment, furthermore IntelGraphicsFixup simply got no updates across the releases. It would be nice to discover and fix the issue if it is in Lilu itself. Mind helping me out?
— Do you get a panic or actually a black screen? If a panic, perhaps you could show it to me with keepsyms and so on (in case you have no working nvram you could try this patch?
— Are you sure your kextcache is not corrupt for some weird reason?
— Are Lilu/IGFX injected via a bootloader or if they are in /L/E or /S/L/E do you have LiluFriend configured?
— Might there be any chance for some other Lilu plugin/Lilu kext that was designed to work with Lilu 1.1.x branch?
— Could you boot with -lilulowmem boot argument, this should make Lilu work in a more 1.1.x way?

And… mhm, which OS version is that in fact? 10.13 stable?

[acidanthera-bot]

@Piker-Alpha commented

1.) It was a real panic.
2.) I don't use a prelinkedkernel.
3.) Lilu.kext and IntelGraphicsFixup.kext are injected by RevoBoot from /Extra/Extensions.
4.) No other plugins are used on this system.
5.) -lilulowmem won't change anything (not using Clover).
6.) macOS 10.13 build 17A360a

And nope. I first want to be able to boot with the latest version of Lilu.kext and since that still fails... perhaps due to the fact that I am using RevoBoot without loading the prelinkedkernel on a version of macOS that nobody uses anymore, that also means that I may run into issues from time to time that other people never even noticed. Like ReadAUX() returning a value that even I cannot locate in the kext.

Time to go hunting...

[acidanthera-bot]

@vit9696 commented

Attempts to load prelinkedkernel may actually be a reason why it panics. For example, I could imagine macOS to create a prelinkedkernel, then UUID check will succeed, but in reality prelinkedkernel will not be used and the kexts loaded from prelinkedkernel will have wrong addresses.

I would expect -lilulowmem to actually disable this though. However, to think of it differently, this boot arg did not work for vit9696/Lilu#26, and I wonder if the issues are similar.

You could probably try removing prelinked kernel entries explicitly here or just compile with compression disabled. If it works, then we will somehow have to figure out how to distinguish between used and not used prelinkedkernel from Lilu.

[acidanthera-bot]

@Piker-Alpha commented

I'll check that later today, but first a quick question. Is this really what you want?

https://github.com/vit9696/Lilu/blob/dd03718cec6a625672b930edbb2eea67378d14ba/Lilu/Headers/kern_disasm.hpp#L41-L57
https://github.com/vit9696/Lilu/blob/dd03718cec6a625672b930edbb2eea67378d14ba/Lilu/Headers/kern_config.hpp#L24

I mean the init() and deist() functions are not disabled in 1.1.3-1.1.7 right?

[acidanthera-bot]

@vit9696 commented

Yes, this is correct. When not using advanced disassembly Disassembler is stateless, I saved function naming for ABI compatibility, but I still should update the comments. Thanks for noticing this.

[acidanthera-bot]

@vit9696 commented

While I am not sure that it would fix your issues, it is probably worth trying the latest commit I added =)

[acidanthera-bot]

@Piker-Alpha commented

1.) Tried with the suggested compression disabled. Still panics.

2.) Used your latest work. Still panics.

3.) I also tried the latest version of AppleALC.kext with Lilu.kext (without IntelGraphicsFixup.kext) and the last thing that I see before the panics is: "missing OSPrelinkKextCount property!".
Which is good. Meaning that usingPrelinkedCache() returns false. This also means that the panic is Lilu.kext related.

4.) I added keepsyms=1 and saw three _ZN... (I think) symbols from Lilu.kext but the output is unreadable. It's just too fast.

5.) With only Lilu.kext it stops when it should enter the desktop. The good news is that it doesn't panic and/or hang. Remove desktop should probably also work.

Same thing happens when I use an older version of IntelGraphicsFixup.kext in combination with Lilu.kext.

[acidanthera-bot]

@vit9696 commented

Hmm, in my opinion, patch the kernel (that's for release):

73 41 42 4d 69 31 58 77 0a  → 53 49 50 45 51 46 33 44 0a                     

And let's see where it panics in this case With debug=0x100 keepsyms=1 it should give us a clear picture after all… Use a debug one please, and upload a binary in case there are issues determining the offsets.

[acidanthera-bot]

@vit9696 commented

Actually I made a mistake in the previous commit, so apart from the kernel patch you will also need to recompile with latest.

[acidanthera-bot]

@Piker-Alpha commented

I pulled the latest commit so that is fine, but I searched for 73 41 42 4d 69 31 58 77 0a and 0a 77 58 31 69 4d 42 41 73 in the kernel of 10.13.1 (Build 17B48) and it's not there.

[acidanthera-bot]

@vit9696 commented

Hi, it looks like I pasted something wrong for some reason.

8A 02 84 C0 74 44 → 8A 02 84 C0 EB 44

I don't have 10.13.1 but it is there for 10.13.0 and 10.13.2 at least.

[acidanthera-bot]

@Piker-Alpha commented

Ok. Patched. But I already installed 10.13.1 and now it boots properly. Without the panic.

One thing though. With the stock Lilu.kext (debug) and without the prelinkedkernel loaded I see this in the log:

unable to load missing 0 image com.apple.driver.AppleIntelHD4000Graphics from prelink
unable to load missing 0 image com.apple.driver.AppleIntelHD5000Graphics from prelink
unable to load missing 0 image com.apple.driver.AppleIntelSKLGraphics from prelink
unable to load missing 0 image com.apple.driver.AppleIntelSKLGraphicsFramebuffer from prelink
unable to load missing 0 image com.apple.driver.AppleIntelKBLGraphics from prelink
unable to load missing 0 image com.apple.driver.AppleIntelKBLGraphicsFramebuffer from prelink
unable to load missing 0 image com.apple.iokit.IOGraphicsFamily from prelink

I think that you may want to change this:

kern_return_t MachInfo::init(const char * const paths[], size_t num, MachInfo *prelink, bool fsfallback) {
	kern_return_t error = KERN_FAILURE;
../..
	// Attempt to get linkedit from prelink
	if (prelink && objectId) {

I think that prelink should be false/null here.

[acidanthera-bot]

@vit9696 commented

Nice to hear. You are right about the warning, this code was not expecting no prelinked cache in normal case. Patched it up :)

[acidanthera-bot]

@Piker-Alpha commented

Thanks.

I must be doing something stupid here:

uint32_t IGFX::readAUX(void * framebufferController, void * AppleIntelFramebuffer, unsigned int unk2, unsigned short unk3, void * out_buffer, void * AppleIntelDisplayPath)
{
	uint32_t retValue = 0;
	if (callbackIgfx && callbackIgfx->orgReadAUX)
	{
		retValue = callbackIgfx->orgReadAUX(framebufferController, AppleIntelFramebuffer, unk2, unk3, out_buffer, AppleIntelDisplayPath);
		printf("igfx : readAUX retValue = 0x%x\n", retValue);
	}
	return retValue;
}

Because all calls return 0xe00002d6

[acidanthera-bot]

@vit9696 commented

Are you sure? The code looks absolutely valid to me. Perhaps you do not notice the successful codes? It looks like this error is normally returned when it fails to read from the register, which implies to me that it may think it is "time out/busy", and thus could happen pretty frequently.

Also, since the original SetDPPowerState also contains a write before read and in fact it is a rather common pattern in the other functions as well… I wonder if we either need to write something first or not focus on aux commands at all and try if it works as is?

[acidanthera-bot]

@Piker-Alpha commented

Yes. I am sure. If retValue == 0 then it prints/logs a few more lines. Which don't show up anywhere.

Also. There are only two different calls being made to readAUX()

kernel: (kernel) igfx : readAUX retValue = 0xe00002d6, unk2 = 0x0, unk3 = 0x1
kernel: (kernel) igfx : readAUX retValue = 0xe00002d6, unk2 = 0x600, unk3 = 0x1

A lot of them but only these two. That can't be good. I also added this little fellow:

uint32_t IGFX::getDPCDInfo(void * framebufferController, void * AppleIntelFramebuffer, void * AppleIntelDisplayPath)
{
	uint32_t retValue = 0;
	if (callbackIgfx && callbackIgfx->orgGetDPCDInfo)
	{
		retValue = callbackIgfx->orgGetDPCDInfo(framebufferController, AppleIntelFramebuffer, AppleIntelDisplayPath);
		printf("igfx : getDPCDInfo retValue = 0x%x\n", retValue);
	}
	
	// return 0 = black screen/no hang.
	return retValue;
}

Note the text. Even with your computeLaneCount()

[acidanthera-bot]

@vit9696 commented

Do you currently use HDMI or DP? Also, are these commands happening at boot or at wake? I checked the disasm a little further and funcs like GetOnlineInfo kind of imply that GetDPCDInfo is expected to fail when using a HDMI display (which makes sense to me).

0x600 DPCD register appears to be pretty special, it contains the actual powerstate of the display. There is a good picture on the 19th page of this pdf for DP newbies like I am. Additionally, I stumbled across this Intel pdf. And now I have a strange deja vu against what we see in Apple drivers and this docs (e.g. ReadAUX is btc_dptx_aux_read and WriteAUX is btc_dptx_aux_write and so on).

All in all I am starting to agree that Apple drivers may actually not set a correct state on sleep/wake, which results in black screen, however, the fact that aux commands fail implies that we have borked display initialisation. Therefore just doing PowerUpPhy several times might be a cure (well, a pretty straight-forward idea at least). I wonder if it works for Apple due to some ACPI code or maybe their special GOP that kicks the display earlier and they are good enough to work.

[acidanthera-bot]

@Piker-Alpha commented

I use DVI and that is why the reads/writes fail (the driver only supports eDP, iDP and DP).
The log output is after a (cold/warm) boot.

[acidanthera-bot]

@vit9696 commented

Hmm, but in this case it means that it is most likely the same for e.g. Azul GPUs. And in this case PowerUpPhy/PowerDownPhy are also never called on them, since they are guarded by successful ReadAUX calls.

[acidanthera-bot]

@vit9696 commented

Ok, I did some checks, and it appears that PowerDownPhy() / PowerUpPhy() are not the case. They seem to be dedicated to DP support, and pretty much unused on Azul just like I said. What may be are two things:

After I implemented GuC support in IntelGraphicsFixup, I discovered that reference firmware scheduler (IGScheduler4) is half-complete, in particular its sleep wake part, which I think may be the key to the problem.

Another issue to kick in is AGDC or rather its AGDP part. I explored the computeLaneCount issue further, and came to a conclusion that Apple driver does support HDMI (and DVI) displays, but it does not support boot displays that are not DP-like. computeLaneCount is not meant to be invoked for anything but DP, and it actually is guarded:

    if ( ctrl->fAGDCCallback )
    {
      if ( !detailsAGDC.modeType ) // This must be 2, and it is for displays other than boot display
      {
        if ( this->fDisplayPath[0]->fPortType != 3 )// LVDS
        {
          laneCount = 0;
          if ( !AppleIntelFramebufferController::ComputeLaneCount(
                  ctrl,
                  &descr→timingInfo.detailedInfo.v2,
                  this->fBitsPerPixel,
                  this->fAvailableLaneCount,    // 0
                  &laneCount)
            || (laneCount > this->fMaxLaneCount) )
          {
            return kIOReturnUnsupported;
          }
        }
      }
    }

But this guard fails, because the structure returned by AppleGraphicsDevicePolicy::VendorEventHandler method 10, which is not described in 10.9 SDK (last SDK with at least something in regards to AGDP), contains 0 in what I called modeType. This appears to be the default for all boot displays in accelerated drivers. And for a 2nd display it returns 2 just as "normal". 10.13.4 kind of patched it by an extra HDMI 2.0 check, but it is hardly related to the problem.

I guess I will close this due to the circumstances, but perhaps it could work as a hint at least.