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.

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

Fix Unknown Synopsys component type issue in Comet Lake 2nd #246

Merged
merged 1 commit into from Mar 22, 2020
Merged

Fix Unknown Synopsys component type issue in Comet Lake 2nd #246

merged 1 commit into from Mar 22, 2020

Conversation

startpenghubingzhou
Copy link
Contributor

This commit has fixed the issue #236 . This work is based on https://www.notion.so/Using-VoodooI2C-on-comet-lake-cpu-e-g-i5-10210u-142930887087445eaa533120455da5dc , which will force D0 in Comet Lake. And I completed it to check the platform to avoid this patch to be applied in other platforms. See code comments for more details.

@startpenghubingzhou
Copy link
Contributor Author

startpenghubingzhou commented Feb 18, 2020

@kprinssu As I have mentioned in #236 (comment), there 's two ways to check I have thought: by judging from IOMatchedID , or injecting a flag that marked 10th and judging this flag. But comparing with these two ways I think it's better to check it in code by judging from device name.

@startpenghubingzhou
Copy link
Contributor Author

startpenghubingzhou commented Feb 18, 2020

I also tried a compromise way: Set a flag in _DSM in the touchpad device DSDT code so it will be loaded as a property in the father device of VoodooPCIController. But it 's not easy to get a fatherdevice property for VoodooPCIController, and checking device_name flag is an easy and safe-enough way to do that.

Btw, the test last night showed that checking IONameMatched
is an UNBELIVIABLE way because it sometimes returns like 9dxx not 9d as required.

@startpenghubingzhou startpenghubingzhou requested review from alexandred and removed request for alexandred February 18, 2020 03:31
@ben9923
Copy link
Member

ben9923 commented Mar 15, 2020

Hey, thanks for the PR :)
I do think messing with the registers ourselves isn't necessary, as Apple's kexts can do it for us.

Why not doing something like this after IOPCIDevice::enablePCIPowerManagement?

physical_device.pci_device->setPowerState(kIOPCIDeviceOnState, physical_device.pci_device);

It will call IOPCIBridge::setDevicePowerState, that will call IOPCIDevice:: setPCIPowerState

Should be safe to do in all platforms. If device is already in D0 state the function will just skip it.

@startpenghubingzhou
Copy link
Contributor Author

startpenghubingzhou commented Mar 15, 2020

Hey, thanks for the PR :)
I do think messing with the registers ourselves isn't necessary, as Apple's kexts can do it for us.

Why not doing something like this after IOPCIDevice::enablePCIPowerManagement?

physical_device.pci_device->setPowerState(kIOPCIDeviceOnState, physical_device.pci_device);

It will call IOPCIBridge::setDevicePowerState, that will call IOPCIDevice:: setPCIPowerState

Should be safe to do in all platforms. If device is already in D0 state the function will just skip it.

Thx for your suggestions. As you know, I 'm new to iokit so I don' t really know all the functions. I ll edit it and then send a new commit again. :)

@startpenghubingzhou
Copy link
Contributor Author

startpenghubingzhou commented Mar 15, 2020

Hey, thanks for the PR :)
I do think messing with the registers ourselves isn't necessary, as Apple's kexts can do it for us.

Why not doing something like this after IOPCIDevice::enablePCIPowerManagement?

physical_device.pci_device->setPowerState(kIOPCIDeviceOnState, physical_device.pci_device);

It will call IOPCIBridge::setDevicePowerState, that will call IOPCIDevice:: setPCIPowerState

Should be safe to do in all platforms. If device is already in D0 state the function will just skip it.

Hey dude I have tried this one:

auto pci_device = physical_device.pci_device;
    pci_device->enablePCIPowerManagement(kPCIPMCSPowerStateD0);
    IOReturn ret = pci_device->setPowerState(kIOPCIDeviceOnState, physical_device.pci_device);

    if (ret != kIOReturnSuccess) {
        IOLog("%s::%s Set D0 state failed! Trying to patch it with Comet Lake patch...\n", getName(), physical_device.name);
        uint16_t oldPowerStateWord = pci_device->configRead16(0x80 + 0x4);
        uint16_t newPowerStateWord = (oldPowerStateWord & (~0x3)) | 0x0;
        // Modify 0x80 below to your findings.
        pci_device->configWrite16(0x80 + 0x4, newPowerStateWord);
        IOLog("%s::%s Successfully patched D0 state with Comet Lake patch!\n", getName(), physical_device.name);
    }

This can 't patch comet lake successfully because it seems that IOPCIBridge::setDevicePowerState always return kIOReturnSuccess. Tested in Xiaoxin Pro 13 with comet lake CPU.

@kprinssu
Copy link
Collaborator

I spoke with @ben9923 we can consider this a hack and then revert it in the future. We are thinking it's a macOS bug and hopefully it will be fixed in the future.

@startpenghubingzhou Do you have any issues with me merging it in?

@ben9923
Copy link
Member

ben9923 commented Mar 20, 2020

I spoke with @ben9923 we can consider this a hack and then revert it in the future. We are thinking it's a macOS bug and hopefully it will be fixed in the future.

@startpenghubingzhou Do you have any issues with me merging it in?

@startpenghubingzhou macOS 10.15.4 should be out together with the 10th Gen MacBook Air, which means full support for this hardware. Give it a try on it (whether latest beta or once it's released) if you can :)

@startpenghubingzhou
Copy link
Contributor Author

startpenghubingzhou commented Mar 21, 2020

I spoke with @ben9923 we can consider this a hack and then revert it in the future. We are thinking it's a macOS bug and hopefully it will be fixed in the future.

@startpenghubingzhou Do you have any issues with me merging it in?

Nothing if you think this can be a temporary solution, and also, I ll try to explore it deeper to try finding what caused that and find a perfect way to solve that but not just hacking it.:)

@startpenghubingzhou
Copy link
Contributor Author

I spoke with @ben9923 we can consider this a hack and then revert it in the future. We are thinking it's a macOS bug and hopefully it will be fixed in the future.
@startpenghubingzhou Do you have any issues with me merging it in?

@startpenghubingzhou macOS 10.15.4 should be out together with the 10th Gen MacBook Air, which means full support for this hardware. Give it a try on it (whether latest beta or once it's released) if you can :)

Sure if I have time. For these days I m busy working on my homework so I might have enough time to test, but I ll do it as soon as I have the free time. :)

@startpenghubingzhou
Copy link
Contributor Author

I spoke with @ben9923 we can consider this a hack and then revert it in the future. We are thinking it's a macOS bug and hopefully it will be fixed in the future.
@startpenghubingzhou Do you have any issues with me merging it in?

@startpenghubingzhou macOS 10.15.4 should be out together with the 10th Gen MacBook Air, which means full support for this hardware. Give it a try on it (whether latest beta or once it's released) if you can :)

@ben9923 also I asked somebody tested in 10.15.4 beta and it seems that it still need a patch of this.

@kprinssu
Copy link
Collaborator

Sounds good, we can revisit the issue at some later point in time. I am going to be merging this into master and get a new build going.

@kprinssu kprinssu merged commit e493a02 into VoodooI2C:master Mar 22, 2020
@marianopela
Copy link
Contributor

@ben9923 also I asked somebody tested in 10.15.4 beta and it seems that it still need a patch of this.

I confirm it, not working on beta 6

@thefiredragon
Copy link

thefiredragon commented Mar 25, 2020

Can confirm on macOS 10.15.4 trackpad is not working at the moment.
Any updates here?

@thefiredragon
Copy link

on 10.15.3 Trackpad was working with the patch posted on the issues.
After upgrade to 10.15.4 trackpad works after sleep I noticed

@marianopela
Copy link
Contributor

marianopela commented Mar 25, 2020

Can confirm on macOS 10.15.4 trackpad is not working at the moment.
Any updates here?

Working for me. Polling mode

Interrupts never worked for me

@kprinssu
Copy link
Collaborator

@thefiredragon @marianopela can you please carry the discussion over into #236? It makes it much easier to track everything in one location rather multiple.

@thefiredragon
Copy link

@thefiredragon @marianopela can you please carry the discussion over into #236? It makes it much easier to track everything in one location rather multiple.

We can,

for me it's working after second restart and clean kextcache. Also after an reboot.

@startpenghubingzhou
Copy link
Contributor Author

@thefiredragon @marianopela can you please carry the discussion over into #236? It makes it much easier to track everything in one location rather multiple.

We can,

for me it's working after second restart and clean kextcache. Also after an reboot.

This may caused by a kextcache problem and have no connection with this patch.

@Trilis29

This comment has been minimized.

@marianopela
Copy link
Contributor

@Trilis29 this is not the right place to discuss. Just use latest release, which should be fine on your hardware. You’re probably missing XOSI patch or something

@Trilis29

This comment has been minimized.

@kprinssu
Copy link
Collaborator

kprinssu commented May 6, 2020

@Trilis29 stop commenting here. The changes are merged, you probably have another issue. Read https://voodooi2c.github.io/ and https://voodooi2c.github.io/#Troubleshooting/Troubleshooting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants