actypes: Distinguish between D3hot/cold, and default ACPI_STATE_D3 to D3cold.#993
Conversation
|
Hi @obiwac This would need to be discussed first with a BIOS architect as well as an OS expert from my end. I would believe that the default D3 state would be D3Hot and not D3Cold. The reason is that not all devices have the capability to transition to D3Cold whereas D3 "soft-off" is usually interpreted as D3Hot which is assumed to be supported in all devices. (Source: ACPI Spec) https://uefi.org/specs/ACPI/6.4_A/07_Power_and_Performance_Mgmt.html#device-power-management-objects |
|
Thanks for your reply and for looking into this, Saket!
This is also what I initially thought, but some places seem to imply the opposite ("[...] can be in any device state (D0, D1, D2, D3hot, or D3)." from 7.4.2.1). Also, when _PS3 is used to put a device into D3, a device is in a state where it can be completely powered off (7.3.11), but D3hot needs to have the _PR3 power resources still on if I'm understanding this right and OSPM can choose whether or not _PS3 does D3hot or D3(cold?) (also 7.3.11). |
|
You make a good point and even I was a little confused when I read the entire section yesterday. It is still not clear to me yet, hence I need some advice on both BIOS/Firmware and OS fronts before I can say anything further. The ACPI Spec can be vague at times and it is not too uncommon to come across conflicting info (with opposing interpretations across platforms/OS/architectures/BIOS versions etc.) in different sections of the spec language, this is why there seems to be some incompatibility between acpi.sys and ACPICA/iASL at times, which has been a pain point in recent times. I will discuss this promptly when I am officially back to work and involved actively in meetings and get back to you! It would be great for Linux and FreeBSD (even NetBSD etc.) to be in sync regarding the default D3 settings! As for its place to be decided under ACPICA or at the OSPM or BIOS level would be a separate discussion (although ACPICA can make an executive decision in this case), but OSPM drivers or BIOS versions can always modify/override their own default settings according to the appropriateness of the underlying arch/platform/HW-reduced ACPI/specific device needs etc. among a dozen of factors! |
|
Alright, looking forward to hearing back. Thanks again for taking the time to look into this! |
|
Hey @SaketADumbre ! Hope all is well. Do you have an update on this? |
|
Hi @obiwac thanks for your patience all this while, I really appreciate it! I'm officially back to work this week and I'm hoping to get to this by the end of the week with some feedback from discussions with other key stakeholders to see what they think. |
|
I noticed your awesome FOSDEM presentation BTW @obiwac and I still have to discuss this topic with the Linux PM maintainer and a firmware architect, but I will update you in this regard as soon as I can. I usually don't like to make things complicated, but what do you think of this piece of logic? - if d3cold is defined for a platform then we can assign the default d3 state to d3cold (for optimal power savings) else we assign default d3 state to d3hot (for platforms that don't yet support d3cold). |
|
Thanks 😄 Maybe I am misunderstanding your idea, but on a given platform which supports D3cold (i.e. which has devices who have the |
|
Hi again @SaketADumbre ! Do you have any news on this? |
|
This topic is under discussion internally within Intel and I have asked feedback from the Linux and Windows ACPI experts for their take on this. I will get back to you when I have any news on this, sorry it is taking a lot longer than I anticipated, but there is potential fixing that needs to be done in the ACPI spec first to make it clear under what circumstances exactly can any platform and a parent or child device use D3Cold vs D3Hot. |
|
Sorry it is taking so long, but we are still investigating D3Cold (and especially _PR3) behavior on different OS and the Windows expert at Intel was out for a couple of weeks on vacation but is back and investigating more details for Rafael and I to look at. I will let you know once we have something tangible and have discussed next steps for remedy. Thanks a lot, this is a great start to a very positive discussion on the D3Cold power savings on various platforms (OS). |
|
Hey @SaketADumbre, do you have any udpates on this? We have a release target we'd like to hit. Specifically, we'd ideally like to get this sorted before the slush starting August 8th. |
|
Thanks for bringing up your intended timeline, it gives me time to clear this up and get it resolved within (hopefully) a month or so from now. I still haven't heard back from people regarding this topic in a while, but as soon as I am back to work in 8 more days, I will prioritize those discussions and have some sort of plan to tackle this issue. The unclear working for _PR3 under an OSPM is Windows for now and there are various cases that need consideration like the absence of D3Cold state in child devices, parent behavior, hot plugging a new device etc. just to mention a few edge/corner cases right off the top of my head. I will provide you with more context and update when I have those discussions and bring it up with stakeholders from various OS, platform firmware architects, ACPI Spec maintainers etc. |
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
|
Hi @SaketADumbre, our code slush is a little over two weeks away and I am curious if you have an update. |
|
这是来自QQ邮箱的假期自动回复邮件。
您好，我最近正在休假中，无法亲自回复您的邮件。我将在假期结束后，尽快给您回复。
|
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
|
I have some good news to share! I can approve this PR as it is in this form and I was able to get the clearance from my review team consisting of the lead platform firmware architect, the Linux Power Management maintainer and the ACPI Windows Principal Engineer. I am not in my home office right now, but I will merge this PR early next week. I really appreciate your patience in this regard, but it is very important to us that everyone from both the BIOS and OS worlds were aligned with this change request and as long as ACPICA itself wasn't doing the Power Management but rather enabling support for it on both sides! Thanks a lot! |
|
TYSM Saket! That's great to hear! |
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
Summary: This patch separates `ACPI_STATE_D3` into `ACPI_STATE_D3_HOT` and `ACPI_STATE_D3_COLD`. The previous `ACPI_STATE_D3` is now defined as `ACPI_STATE_D3_COLD`. The same distinction has been made between `PCI_POWERSTATE_D3_HOT` and `PCI_POWERSTATE_D3_COLD`, as they're defined by ACPI (and are asserted to be the same). D3cold is essentially the same as D3hot except the power resources are turned off. Support for D3cold has been added to `acpi_pwr_switch_consumer`. Currently I am awaiting information on whether or not `ACPI_STATE_D3` should actually be D3cold or D3hot instead, at which point these macros will be upstreamed and I'll update this revision. See acpica/acpica#993. `acpi_d_state_to_str` replaces the `printf("D%d", d_state)` pattern, allowing for "D3hot" and "D3cold" strings to be printed instead. This was split out from D48294. Sponsored by: The FreeBSD Foundation Test Plan: I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine. I still need to make 100% sure this doesn't break anything with PCI. Reviewers: jhb, imp, jkim, markj, emaste Reviewed By: markj Subscribers: garga, ziaee, #linuxkpi Differential Revision: https://reviews.freebsd.org/D48384
|
LGTM, thanks a lot! |
|
Thanks for merging this! Any way you could make a release so we can pull these changes into our tree? |
|
I was planning on doing a new ACPICA release next week. Would that work for you? I am dealing with some other tasks this week and setting up new systems in the lab and my cube, so I might not have time for a new release this week. |
4 participants
Working on implementing D3cold support for FreeBSD (D48294), I ran into the issue of ACPICA not having constants to distinguish between D3hot and D3cold states. I wasn't sure exactly what
ACPI_STATE_D3should default to (it is unclear for me if D3 for a device which doesn't have a_PR3object is equivalent to D3hot or D3cold), so I decided to mirror what Linux does for this (ininclude/acpi/actypes.h):This PR is me trying to upstream those changes/open the discussion on what these constants should be named or if they should be distinguished at ACPICA's level at all. I think they should be at least distinguished as other objects evaluate to 4 to signify D3cold, e.g. _SxW.
I have checked that these constants aren't used anywhere else in ACPICA.