acpi_powerres: D3cold support and `acpi_d_state_to_str`
AcceptedPublic
Actions

Authored by obiwac_gmail.com on Wed, Jan 8, 11:07 PM.

Details

Reviewers

jhb
imp
jkim
markj
emaste

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 https://github.com/acpica/acpica/pull/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.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 61584
Build 58468: arc lint + arc unit

Event Timeline

obiwac_gmail.com created this revision.Wed, Jan 8, 11:07 PM

Herald added a subscriber: linuxkpi. · View Herald TranscriptWed, Jan 8, 11:07 PM

obiwac_gmail.com requested review of this revision.Wed, Jan 8, 11:07 PM

Harbormaster completed remote builds in B61584: Diff 148948.Wed, Jan 8, 11:07 PM

obiwac_gmail.com edited the summary of this revision. (Show Details)Wed, Jan 8, 11:11 PM

obiwac_gmail.com edited the summary of this revision. (Show Details)Wed, Jan 8, 11:24 PM

obiwac_gmail.com mentioned this in D48294: acpi_powerres: D3cold support and fix turning off power resources on first D-state switch.

obiwac_gmail.com edited the summary of this revision. (Show Details)Wed, Jan 8, 11:56 PM

obiwac_gmail.com added a child revision: D48385: acpi_powerres: Fix turning off power resources on first D-state switch.Thu, Jan 9, 5:02 PM

obiwac_gmail.com added a child revision: D48386: acpi_powerres: `acpi_pwr_get_state` and getting initial D-state for device.

obiwac_gmail.com removed a child revision: D48386: acpi_powerres: `acpi_pwr_get_state` and getting initial D-state for device.Thu, Jan 9, 5:05 PM

markj added inline comments.Fri, Jan 10, 2:19 PM

sys/dev/acpica/acpi_pci.c
57	Is this removal intentional?
sys/dev/acpica/acpivar.h
494
497

Respond to style comments

Harbormaster completed remote builds in B61665: Diff 149102.Fri, Jan 10, 11:19 PM

obiwac_gmail.com marked an inline comment as not done.Fri, Jan 10, 11:20 PM

obiwac_gmail.com added inline comments.

sys/dev/acpica/acpi_pci.c
57	Yes, my LS was bugging me about these not being used, and as I understand it these are just the autogenerated headers to provide the method interface. If such changes are deemed out of scope I will revert this.

Add PCI_POWERSTATE_D3_COLD to pci(9) manpage

Harbormaster completed remote builds in B61666: Diff 149103.Fri, Jan 10, 11:30 PM

more style(9) closening

Harbormaster completed remote builds in B61668: Diff 149106.Fri, Jan 10, 11:35 PM

This looks okay to me, but I'm not very familiar with this part of the kernel.

Based on the discussion on github, it looks like the unqualified D3 state likely should correspond to D3hot?

sys/dev/acpica/acpi_pci.c
57	It's fine, I'd just suggest noting that explicitly in the description/commit log next time, otherwise it can be hard to tell whether or not the change is intentional.

This revision is now accepted and ready to land.Sat, Jan 11, 3:14 PM

Based on the discussion on github, it looks like the unqualified D3 state likely should correspond to D3hot?

I'm still not sure, I will wait for Saket to follow up on this as he isn't certain either. These definitions need to be sorted out upstream before this can be committed anyway.