From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux PM <linux-pm@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux ACPI <linux-acpi@vger.kernel.org>,
Len Brown <len.brown@intel.com>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Len Brown <lenb@kernel.org>
Subject: [PATCH v1 03/10] ACPI: processor: Clean up acpi_processor_evaluate_cst()
Date: Fri, 13 Dec 2019 10:14:46 +0100 [thread overview]
Message-ID: <2158277.Sk0T0YLvXj@kreacher> (raw)
In-Reply-To: <3950312.2WmFeOdZGY@kreacher>
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Clean up acpi_processor_evaluate_cst() in multiple ways:
* Rename current_count to last_index which matches the purpose of
the variable better.
* Consistently use acpi_handle_*() for printing messages and make
the messages cleaner.
* Drop redundant parens and braces.
* Rewrite and clarify comments.
* Rearrange checks and drop the redundant ones.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
No changes from the RFC version.
---
drivers/acpi/processor_idle.c | 114 +++++++++++++++++++-----------------------
1 file changed, 52 insertions(+), 62 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index e92d0e6d4cd1..7c2fe3b2ec31 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -304,29 +304,29 @@ static int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu,
union acpi_object *cst;
acpi_status status;
u64 count;
- int current_count = 0;
+ int last_index = 0;
int i, ret = 0;
status = acpi_evaluate_object(handle, "_CST", NULL, &buffer);
if (ACPI_FAILURE(status)) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No _CST, giving up\n"));
+ acpi_handle_debug(handle, "No _CST\n");
return -ENODEV;
}
cst = buffer.pointer;
- /* There must be at least 2 elements */
- if (!cst || (cst->type != ACPI_TYPE_PACKAGE) || cst->package.count < 2) {
- pr_err("not enough elements in _CST\n");
+ /* There must be at least 2 elements. */
+ if (!cst || cst->type != ACPI_TYPE_PACKAGE || cst->package.count < 2) {
+ acpi_handle_warn(handle, "Invalid _CST output\n");
ret = -EFAULT;
goto end;
}
count = cst->package.elements[0].integer.value;
- /* Validate number of power states. */
+ /* Validate the number of C-states. */
if (count < 1 || count != cst->package.count - 1) {
- pr_err("count given by _CST is not valid\n");
+ acpi_handle_warn(handle, "Inconsistent _CST data\n");
ret = -EFAULT;
goto end;
}
@@ -337,111 +337,101 @@ static int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu,
struct acpi_power_register *reg;
struct acpi_processor_cx cx;
+ /*
+ * If there is not enough space for all C-states, skip the
+ * excess ones and log a warning.
+ */
+ if (last_index >= ACPI_PROCESSOR_MAX_POWER - 1) {
+ acpi_handle_warn(handle,
+ "No room for more idle states (limit: %d)\n",
+ ACPI_PROCESSOR_MAX_POWER - 1);
+ break;
+ }
+
memset(&cx, 0, sizeof(cx));
- element = &(cst->package.elements[i]);
+ element = &cst->package.elements[i];
if (element->type != ACPI_TYPE_PACKAGE)
continue;
if (element->package.count != 4)
continue;
- obj = &(element->package.elements[0]);
+ obj = &element->package.elements[0];
if (obj->type != ACPI_TYPE_BUFFER)
continue;
reg = (struct acpi_power_register *)obj->buffer.pointer;
- if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO &&
- (reg->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE))
- continue;
-
- /* There should be an easy way to extract an integer... */
- obj = &(element->package.elements[1]);
+ obj = &element->package.elements[1];
if (obj->type != ACPI_TYPE_INTEGER)
continue;
cx.type = obj->integer.value;
/*
- * Some buggy BIOSes won't list C1 in _CST -
- * Let acpi_processor_get_power_info_default() handle them later
+ * There are known cases in which the _CST output does not
+ * contain C1, so if the type of the first state found is not
+ * C1, leave an empty slot for C1 to be filled in later.
*/
if (i == 1 && cx.type != ACPI_STATE_C1)
- current_count++;
+ last_index = 1;
cx.address = reg->address;
- cx.index = current_count + 1;
+ cx.index = last_index + 1;
- cx.entry_method = ACPI_CSTATE_SYSTEMIO;
if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE) {
- if (acpi_processor_ffh_cstate_probe
- (cpu, &cx, reg) == 0) {
- cx.entry_method = ACPI_CSTATE_FFH;
+ if (!acpi_processor_ffh_cstate_probe(cpu, &cx, reg)) {
+ /*
+ * In the majority of cases _CST describes C1 as
+ * a FIXED_HARDWARE C-state, but if the command
+ * line forbids using MWAIT, use CSTATE_HALT for
+ * C1 regardless.
+ */
+ if (cx.type == ACPI_STATE_C1 &&
+ boot_option_idle_override == IDLE_NOMWAIT) {
+ cx.entry_method = ACPI_CSTATE_HALT;
+ snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT");
+ } else {
+ cx.entry_method = ACPI_CSTATE_FFH;
+ }
} else if (cx.type == ACPI_STATE_C1) {
/*
- * C1 is a special case where FIXED_HARDWARE
- * can be handled in non-MWAIT way as well.
- * In that case, save this _CST entry info.
- * Otherwise, ignore this info and continue.
+ * In the special case of C1, FIXED_HARDWARE can
+ * be handled by executing the HLT instruction.
*/
cx.entry_method = ACPI_CSTATE_HALT;
snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT");
} else {
continue;
}
- if (cx.type == ACPI_STATE_C1 &&
- (boot_option_idle_override == IDLE_NOMWAIT)) {
- /*
- * In most cases the C1 space_id obtained from
- * _CST object is FIXED_HARDWARE access mode.
- * But when the option of idle=halt is added,
- * the entry_method type should be changed from
- * CSTATE_FFH to CSTATE_HALT.
- * When the option of idle=nomwait is added,
- * the C1 entry_method type should be
- * CSTATE_HALT.
- */
- cx.entry_method = ACPI_CSTATE_HALT;
- snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT");
- }
- } else {
+ } else if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+ cx.entry_method = ACPI_CSTATE_SYSTEMIO;
snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI IOPORT 0x%x",
cx.address);
+ } else {
+ continue;
}
- if (cx.type == ACPI_STATE_C1) {
+ if (cx.type == ACPI_STATE_C1)
cx.valid = 1;
- }
- obj = &(element->package.elements[2]);
+ obj = &element->package.elements[2];
if (obj->type != ACPI_TYPE_INTEGER)
continue;
cx.latency = obj->integer.value;
- obj = &(element->package.elements[3]);
+ obj = &element->package.elements[3];
if (obj->type != ACPI_TYPE_INTEGER)
continue;
- current_count++;
- memcpy(&info->states[current_count], &cx, sizeof(cx));
-
- /*
- * We support total ACPI_PROCESSOR_MAX_POWER - 1
- * (From 1 through ACPI_PROCESSOR_MAX_POWER - 1)
- */
- if (current_count >= (ACPI_PROCESSOR_MAX_POWER - 1)) {
- pr_warn("Limiting number of power states to max (%d)\n",
- ACPI_PROCESSOR_MAX_POWER);
- pr_warn("Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n");
- break;
- }
+ memcpy(&info->states[++last_index], &cx, sizeof(cx));
}
- acpi_handle_info(handle, "Found %d idle states\n", current_count);
+ acpi_handle_info(handle, "Found %d idle states\n", last_index);
- info->count = current_count;
+ info->count = last_index;
end:
kfree(buffer.pointer);
--
2.16.4
next prev parent reply other threads:[~2019-12-13 9:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-13 9:09 [PATCH v1 00/10] cpuidle: intel_idle: Use ACPI _CST to get idle states information Rafael J. Wysocki
2019-12-13 9:11 ` [PATCH v1 01/10] ACPI: processor: Export function to claim _CST control Rafael J. Wysocki
2019-12-13 9:12 ` [PATCH v1 02/10] ACPI: processor: Introduce acpi_processor_evaluate_cst() Rafael J. Wysocki
2019-12-13 9:14 ` Rafael J. Wysocki [this message]
2019-12-13 9:15 ` [PATCH v1 04/10] ACPI: processor: Export acpi_processor_evaluate_cst() Rafael J. Wysocki
2019-12-13 9:16 ` [PATCH v1 05/10] intel_idle: Refactor intel_idle_cpuidle_driver_init() Rafael J. Wysocki
2019-12-13 9:20 ` [PATCH v1 06/10] intel_idle: Use ACPI _CST for processor models without C-state tables Rafael J. Wysocki
2019-12-13 9:22 ` [PATCH v1 07/10] cpuidle: Allow idle states to be disabled by default Rafael J. Wysocki
2019-12-13 9:24 ` [PATCH v1 08/10] intel_idle: Allow ACPI _CST to be used for selected known processors Rafael J. Wysocki
2019-12-13 9:24 ` [PATCH v1 09/10] intel_idle: Add module parameter to prevent ACPI _CST from being used Rafael J. Wysocki
2019-12-13 9:27 ` [PATCH v1 10/10] intel_idle: Use ACPI _CST on server systems Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2158277.Sk0T0YLvXj@kreacher \
--to=rjw@rjwysocki.net \
--cc=len.brown@intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=srinivas.pandruvada@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).