[PATCH] nvme: cleanup nvme_configure_apst
Christoph Hellwig
hch at lst.de
Fri Apr 9 10:45:24 BST 2021
Remove a level of indentation from the main code implementating the table
search by using a goto for the APST not supported case. Also move the
main comment above the function.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
drivers/nvme/host/core.c | 148 ++++++++++++++++++---------------------
1 file changed, 69 insertions(+), 79 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 314705da2c1076..78817190099040 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2550,28 +2550,28 @@ static int nvme_configure_acre(struct nvme_ctrl *ctrl)
return ret;
}
+/*
+ * APST (Autonomous Power State Transition) lets us program a table of power
+ * state transitions that the controller will perform automatically.
+ * We configure it with a simple heuristic: we are willing to spend at most 2%
+ * of the time transitioning between power states. Therefore, when running in
+ * any given state, we will enter the next lower-power non-operational state
+ * after waiting 50 * (enlat + exlat) microseconds, as long as that state's exit
+ * latency is under the requested maximum latency.
+ *
+ * We will not autonomously enter any non-operational state for which the total
+ * latency exceeds ps_max_latency_us.
+ *
+ * Users can set ps_max_latency_us to zero to turn off APST.
+ */
static int nvme_configure_apst(struct nvme_ctrl *ctrl)
{
- /*
- * APST (Autonomous Power State Transition) lets us program a
- * table of power state transitions that the controller will
- * perform automatically. We configure it with a simple
- * heuristic: we are willing to spend at most 2% of the time
- * transitioning between power states. Therefore, when running
- * in any given state, we will enter the next lower-power
- * non-operational state after waiting 50 * (enlat + exlat)
- * microseconds, as long as that state's exit latency is under
- * the requested maximum latency.
- *
- * We will not autonomously enter any non-operational state for
- * which the total latency exceeds ps_max_latency_us. Users
- * can set ps_max_latency_us to zero to turn off APST.
- */
-
- unsigned apste;
struct nvme_feat_auto_pst *table;
+ unsigned apste = 0;
u64 max_lat_us = 0;
+ __le64 target = 0;
int max_ps = -1;
+ int state;
int ret;
/*
@@ -2592,83 +2592,73 @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl)
if (!ctrl->apst_enabled || ctrl->ps_max_latency_us == 0) {
/* Turn off APST. */
- apste = 0;
dev_dbg(ctrl->device, "APST disabled\n");
- } else {
- __le64 target = cpu_to_le64(0);
- int state;
-
- /*
- * Walk through all states from lowest- to highest-power.
- * According to the spec, lower-numbered states use more
- * power. NPSS, despite the name, is the index of the
- * lowest-power state, not the number of states.
- */
- for (state = (int)ctrl->npss; state >= 0; state--) {
- u64 total_latency_us, exit_latency_us, transition_ms;
-
- if (target)
- table->entries[state] = target;
-
- /*
- * Don't allow transitions to the deepest state
- * if it's quirked off.
- */
- if (state == ctrl->npss &&
- (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS))
- continue;
+ goto done;
+ }
- /*
- * Is this state a useful non-operational state for
- * higher-power states to autonomously transition to?
- */
- if (!(ctrl->psd[state].flags &
- NVME_PS_FLAGS_NON_OP_STATE))
- continue;
+ /*
+ * Walk through all states from lowest- to highest-power.
+ * According to the spec, lower-numbered states use more power. NPSS,
+ * despite the name, is the index of the lowest-power state, not the
+ * number of states.
+ */
+ for (state = (int)ctrl->npss; state >= 0; state--) {
+ u64 total_latency_us, exit_latency_us, transition_ms;
- exit_latency_us =
- (u64)le32_to_cpu(ctrl->psd[state].exit_lat);
- if (exit_latency_us > ctrl->ps_max_latency_us)
- continue;
+ if (target)
+ table->entries[state] = target;
- total_latency_us =
- exit_latency_us +
- le32_to_cpu(ctrl->psd[state].entry_lat);
+ /*
+ * Don't allow transitions to the deepest state if it's quirked
+ * off.
+ */
+ if (state == ctrl->npss &&
+ (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS))
+ continue;
- /*
- * This state is good. Use it as the APST idle
- * target for higher power states.
- */
- transition_ms = total_latency_us + 19;
- do_div(transition_ms, 20);
- if (transition_ms > (1 << 24) - 1)
- transition_ms = (1 << 24) - 1;
+ /*
+ * Is this state a useful non-operational state for higher-power
+ * states to autonomously transition to?
+ */
+ if (!(ctrl->psd[state].flags & NVME_PS_FLAGS_NON_OP_STATE))
+ continue;
- target = cpu_to_le64((state << 3) |
- (transition_ms << 8));
+ exit_latency_us = (u64)le32_to_cpu(ctrl->psd[state].exit_lat);
+ if (exit_latency_us > ctrl->ps_max_latency_us)
+ continue;
- if (max_ps == -1)
- max_ps = state;
+ total_latency_us = exit_latency_us +
+ le32_to_cpu(ctrl->psd[state].entry_lat);
- if (total_latency_us > max_lat_us)
- max_lat_us = total_latency_us;
- }
+ /*
+ * This state is good. Use it as the APST idle target for
+ * higher power states.
+ */
+ transition_ms = total_latency_us + 19;
+ do_div(transition_ms, 20);
+ if (transition_ms > (1 << 24) - 1)
+ transition_ms = (1 << 24) - 1;
+
+ target = cpu_to_le64((state << 3) | (transition_ms << 8));
+ if (max_ps == -1)
+ max_ps = state;
+ if (total_latency_us > max_lat_us)
+ max_lat_us = total_latency_us;
+ }
- apste = 1;
- if (max_ps == -1) {
- dev_dbg(ctrl->device, "APST enabled but no non-operational states are available\n");
- } else {
- dev_dbg(ctrl->device, "APST enabled: max PS = %d, max round-trip latency = %lluus, table = %*phN\n",
- max_ps, max_lat_us, (int)sizeof(*table), table);
- }
- }
+ if (max_ps == -1)
+ dev_dbg(ctrl->device, "APST enabled but no non-operational states are available\n");
+ else
+ dev_dbg(ctrl->device, "APST enabled: max PS = %d, max round-trip latency = %lluus, table = %*phN\n",
+ max_ps, max_lat_us, (int)sizeof(*table), table);
+ apste = 1;
+done:
ret = nvme_set_features(ctrl, NVME_FEAT_AUTO_PST, apste,
table, sizeof(*table), NULL);
if (ret)
dev_err(ctrl->device, "failed to set APST feature (%d)\n", ret);
-
kfree(table);
return ret;
}
--
2.30.1
More information about the Linux-nvme
mailing list