Match the platform by CPU model and attach on acpi with PWRM at
0xfe000000; the PMC is not PCI-enumerable on these platforms.
SLP_S0_RES offsets and granularities follow each generation's PCH
Family Datasheet Volume 2. A mismatch between the LPIT residency
counter address and SLP_S0_RES is reported on attach.
Details
Details
- Reviewers
olce adrian ziaee mashtizadeh_gmail.com
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 76878 Build 73761: arc lint + arc unit
Event Timeline
Comment Actions
manpage changes looks good. i suppose i can test this now that it supports alder lake.
| share/man/man4/intel_pmc.4 | ||
|---|---|---|
| 13–20 | ||
Comment Actions
Yes! Your Alder lake should attach with this diff.
@dteske also has AL from Framework.
| share/man/man4/intel_pmc.4 | ||
|---|---|---|
| 13–20 | this diff need a rebase, that verbose code don't exist in the Diff that was merged. | |
This comment was removed by seuros.
Comment Actions
Generally, you have to test if the driver is already attached. Here, the driver could attach through PCI or through ACPI, but it shouldn't attach through both. This is not supposed to happen, but better be safe than sorry, especially when dealing with hardware. So please add a check to both probe methods that device_get_unit(dev) is 0, else bail out (because there's already one driver/device). Check on acpi_get_handle(dev) still looks like a good idea, even if it does not handle the redundancy anymore.
I'll do an extra pass tomorrow morning.
(As I said in my latest mail, I wanted to see if we could do something cleaner and forward-proof. The majority of this day was spent for this, and unfortunately, it does not appear we can do better. Intel PCH documentation appears to be quite flawed with respect to the PMC: There's no PCI device on one of my laptops (Comet Lake) although there should be according to various datasheets, including those for the chipset that should be in that mobile processor (AFAICT after some deep online research). For future generations, we'll want to infer PWRMBASE from the Shared SRAM device. I checked that there's indeed such a device even on Comet Lake, and dumped the content of the BAR areas, without finding the 0xfe000000 base; that one still can be deduced from the LPIT table. The datasheet descriptions of the BARs appear to be wrong (but have been corrected in later generations).)
| share/man/man4/intel_pmc.4 | ||
|---|---|---|
| 30 | ||
| 92 | long-uptime systems looks too vague. I'd specify the order of magnitude. | |
| sys/dev/intel/intel_pmc.c | ||
| 22–26 | Suggested style changes. Also, Linux has code to switch to the Coffee Lake registers if the Kaby Lake CPU ID is found but the 0x9d21 PCI device is not, so the PMC is probably not PCI-enumerable on a Coffee Lake machine too (if you have such a machine and time and will to check). | |
| 243–252 | I can imagine why you're doing that. I'll later propose some complement to it to avoid problems in virtual machines. | |
| 325–371 | This is a kind of layer violation, parsing of LPIT should go into ACPI proper. But that's fine for now, I'll refactor this later. | |
| 401–405 | So, here, you're giving priority to INTEL_PMC_PWRM_BASE_DEFAULT (0xfe000000), whereas Linux uses this as a last resort if there's a residency counter address from the LPIT. Is that intended? If you prefer to keep it like that, the check should probably consider all valid LPI entries before deciding there's a mismatch. | |
Comment Actions
Note: Ideally, at some point, we should add some way to print the IP blocks per generation (separate change/revision).
| sys/dev/intel/intel_pmc.c | ||
|---|---|---|
| 181 | I guess you don't have the hardware for this one? | |
Comment Actions
I tested this on real hardware, with the exception of a few -* variants, but Linux handles them the same way. (I can also get that hardware in the next days.)
There are more generations described by either Linux or the specs, but I omitted them due to lack of hardware for testing.
The specialised platforms are out of scope except Bartlett Lake which i bought for the RAM not as a daily driver.
| share/man/man4/intel_pmc.4 | ||
|---|---|---|
| 30 | ack | |
| 92 | ack | |
| sys/dev/intel/intel_pmc.c | ||
| 181 | Bartlett Lake ? i do for now. But maybe we should remove it, the motherboard is not generic (DFI). wdyt ? | |
| 401–405 | Yes. That was only way to work on every generation listed here. We could have it like linux way as a fallback( there is a small hang in unsupported platform), or i will could try to find proper way auto detect correct way. I have no preference in driver work. | |
| sys/dev/intel/intel_pmc.c | ||
|---|---|---|
| 181 | Mmm, I'd say, let's keep it, especially if you have the hardware. If someone uses FreeBSD on this one, they might be pleasantly surprised, or in the worst case it crashes and hopefully we will hear about it. It looks like a niche anyway. | |
| 401–405 | Just to check I'm understanding correctly, you're saying that there's at least one generation that needs our static PWRMBASE default in priority over the LPIT content, else it does not work? What is the "small hang" you experienced, and in which context? Not sure how to auto-detect if INTEL_PMC_PWRM_BASE_DEFAULT and the LPIT table are not enough. Do you have an idea in mind? | |