From 7522c692ed9b5cb76408b220d0d3881c53a186ce Mon Sep 17 00:00:00 2001 From: Lina Iyer Date: Fri, 21 Feb 2020 15:53:46 -0700 Subject: [PATCH 1/2] soc: qcom: rpmh: remove serialization of TCS commands Requests sent to RPMH can be sent as fire-n-forget or response required, with the latter ensuring the command has been completed by the hardware accelerator. Commands in a request with tcs_cmd::wait set, would ensure that those select commands are sent as response required, even though the actual TCS request may be fire-n-forget. Also, commands with .wait flag were also guaranteed to be complete before the following command in the TCS is sent. This means that the next command of the same request blocked until the current request is completed. This could mean waiting for a voltage to settle or series of NOCs be configured before the next command is sent. But drivers using this feature have never cared about the serialization aspect. By not enforcing the serialization we can allow the hardware to run in parallel improving the performance. Let's clarify the usage of this member in the tcs_cmd structure to mean only completion and not serialization. This should also improve the performance of bus requests where changes could happen in parallel. Also, CPU resume from deep idle may see benefits from certain wake requests. Change-Id: I341b9887a53a569a9d5947ed884fc1bf60e7604f Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh-rsc.c | 21 +++++++++------------ include/soc/qcom/tcs.h | 5 +++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index bf57fbcaacb8..3b748385f697 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. */ +/* Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "%s " fmt, KBUILD_MODNAME @@ -282,8 +282,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p) cmd = &req->cmds[j]; sts = read_tcs_reg(drv, RSC_DRV_CMD_STATUS, i, j); if (!(sts & CMD_STATUS_ISSUED) || - ((req->wait_for_compl || cmd->wait) && - !(sts & CMD_STATUS_COMPL))) { + (cmd->wait && !(sts & CMD_STATUS_COMPL))) { pr_err("Incomplete request: %s: addr=%#x data=%#x", drv->name, cmd->addr, cmd->data); err = -EIO; @@ -311,7 +310,6 @@ static irqreturn_t tcs_tx_done(int irq, void *p) skip: /* Reclaim the TCS */ write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i, 0); - write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, i, 0); write_tcs_reg(drv, RSC_DRV_IRQ_CLEAR, 0, BIT(i)); spin_lock(&drv->lock); clear_bit(i, drv->tcs_in_use); @@ -326,23 +324,23 @@ skip: static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id, const struct tcs_request *msg) { - u32 msgid, cmd_msgid; + u32 msgid; + u32 cmd_msgid = CMD_MSGID_LEN | CMD_MSGID_WRITE; u32 cmd_enable = 0; - u32 cmd_complete; struct tcs_cmd *cmd; int i, j; - cmd_msgid = CMD_MSGID_LEN; + /* Convert all commands to RR when the request has wait_for_compl set */ cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0; - cmd_msgid |= CMD_MSGID_WRITE; - - cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, 0); for (i = 0, j = cmd_id; i < msg->num_cmds; i++, j++) { cmd = &msg->cmds[i]; cmd_enable |= BIT(j); - cmd_complete |= cmd->wait << j; msgid = cmd_msgid; + /* + * Additionally, if the cmd->wait is set, make the command + * response reqd even if the overall request was fire-n-forget. + */ msgid |= cmd->wait ? CMD_MSGID_RESP_REQ : 0; write_tcs_cmd(drv, RSC_DRV_CMD_MSGID, tcs_id, j, msgid); @@ -355,7 +353,6 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id, cmd->data, cmd->wait); } - write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, cmd_complete); cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, 0); write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable); } diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h index 23e9b114fd0d..87b57d6d5b24 100644 --- a/include/soc/qcom/tcs.h +++ b/include/soc/qcom/tcs.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. */ #ifndef __SOC_QCOM_TCS_H__ @@ -30,7 +30,7 @@ enum rpmh_state { * * @addr: the address of the resource slv_id:18:16 | offset:0:15 * @data: the resource state request - * @wait: wait for this request to be complete before sending the next + * @wait: Ensure that this command is complete before returning */ struct tcs_cmd { u32 addr; @@ -43,6 +43,7 @@ struct tcs_cmd { * * @state: state for the request. * @wait_for_compl: wait until we get a response from the h/w accelerator + * (sets the cmd->wait for all commmands in the request) * @num_cmds: the number of @cmds in this request * @cmds: an array of tcs_cmds */ From fc520c69712e998b7868dffab86471cdaa2b79e2 Mon Sep 17 00:00:00 2001 From: Lina Iyer Date: Thu, 5 Mar 2020 13:39:08 -0700 Subject: [PATCH 2/2] cpuidle: lpm-levels: convert PSCI return value to boolean correctly LPM driver is concerned with the boolean result of the PSCI call. The PSCI error value is not of consequence for further activity. However, it might be helpful to know what the PSCI driver. So, let's log the return value but use the boolean result elsewhere. Change-Id: Ibbc674803da139aef29121d123625661e26bc75d Signed-off-by: Lina Iyer --- drivers/cpuidle/lpm-levels.c | 32 ++++++++++++++-------- include/trace/events/trace_msm_low_power.h | 6 ++-- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/cpuidle/lpm-levels.c b/drivers/cpuidle/lpm-levels.c index 39a8f78fe9d3..7c4b7cb5a5ef 100644 --- a/drivers/cpuidle/lpm-levels.c +++ b/drivers/cpuidle/lpm-levels.c @@ -1097,10 +1097,10 @@ unlock_and_return: return state_id; } -static bool psci_enter_sleep(struct lpm_cpu *cpu, int idx, bool from_idle) +static int psci_enter_sleep(struct lpm_cpu *cpu, int idx, bool from_idle) { int affinity_level = 0, state_id = 0, power_state = 0; - bool success = false; + int ret, success; /* * idx = 0 is the default LPM state */ @@ -1109,12 +1109,13 @@ static bool psci_enter_sleep(struct lpm_cpu *cpu, int idx, bool from_idle) stop_critical_timings(); cpu_do_idle(); start_critical_timings(); - return true; + return 0; } if (from_idle && cpu->levels[idx].use_bc_timer) { - if (tick_broadcast_enter()) - return success; + ret = tick_broadcast_enter(); + if (ret) + return ret; } state_id = get_cluster_id(cpu->parent, &affinity_level, from_idle); @@ -1126,7 +1127,8 @@ static bool psci_enter_sleep(struct lpm_cpu *cpu, int idx, bool from_idle) 0xdeaffeed, 0xdeaffeed, from_idle); stop_critical_timings(); - success = !psci_cpu_suspend_enter(state_id); + ret = psci_cpu_suspend_enter(state_id); + success = (ret == 0); start_critical_timings(); update_debug_pc_event(CPU_EXIT, state_id, @@ -1135,7 +1137,7 @@ static bool psci_enter_sleep(struct lpm_cpu *cpu, int idx, bool from_idle) if (from_idle && cpu->levels[idx].use_bc_timer) tick_broadcast_exit(); - return success; + return ret; } static int lpm_cpuidle_select(struct cpuidle_driver *drv, @@ -1191,6 +1193,7 @@ static int lpm_cpuidle_enter(struct cpuidle_device *dev, const struct cpumask *cpumask = get_cpu_mask(dev->cpu); ktime_t start = ktime_get(); uint64_t start_time = ktime_to_ns(start), end_time; + int ret = -EBUSY; cpu_prepare(cpu, idx, true); cluster_prepare(cpu->parent, cpumask, idx, true, start_time); @@ -1201,7 +1204,8 @@ static int lpm_cpuidle_enter(struct cpuidle_device *dev, if (need_resched()) goto exit; - success = psci_enter_sleep(cpu, idx, true); + ret = psci_enter_sleep(cpu, idx, true); + success = (ret == 0); exit: end_time = ktime_to_ns(ktime_get()); @@ -1211,7 +1215,7 @@ exit: cpu_unprepare(cpu, idx, true); dev->last_residency = ktime_us_delta(ktime_get(), start); update_history(dev, idx); - trace_cpu_idle_exit(idx, success); + trace_cpu_idle_exit(idx, ret); if (lpm_prediction && cpu->lpm_prediction) { histtimer_cancel(); clusttimer_cancel(); @@ -1225,7 +1229,8 @@ static void lpm_cpuidle_s2idle(struct cpuidle_device *dev, { struct lpm_cpu *cpu = per_cpu(cpu_lpm, dev->cpu); const struct cpumask *cpumask = get_cpu_mask(dev->cpu); - bool success = false; + bool success; + int ret; for (; idx >= 0; idx--) { if (lpm_cpu_mode_allow(dev->cpu, idx, false)) @@ -1239,7 +1244,8 @@ static void lpm_cpuidle_s2idle(struct cpuidle_device *dev, cpu_prepare(cpu, idx, true); cluster_prepare(cpu->parent, cpumask, idx, false, 0); - success = psci_enter_sleep(cpu, idx, false); + ret = psci_enter_sleep(cpu, idx, false); + success = (ret == 0); cluster_unprepare(cpu->parent, cpumask, idx, false, 0, success); cpu_unprepare(cpu, idx, true); @@ -1443,6 +1449,7 @@ static int lpm_suspend_enter(suspend_state_t state) const struct cpumask *cpumask = get_cpu_mask(cpu); int idx; bool success; + int ret; for (idx = lpm_cpu->nlevels - 1; idx >= 0; idx--) { if (lpm_cpu_mode_allow(cpu, idx, false)) @@ -1455,7 +1462,8 @@ static int lpm_suspend_enter(suspend_state_t state) cpu_prepare(lpm_cpu, idx, false); cluster_prepare(cluster, cpumask, idx, false, 0); - success = psci_enter_sleep(lpm_cpu, idx, false); + ret = psci_enter_sleep(lpm_cpu, idx, false); + success = (ret == 0); cluster_unprepare(cluster, cpumask, idx, false, 0, success); cpu_unprepare(lpm_cpu, idx, false); diff --git a/include/trace/events/trace_msm_low_power.h b/include/trace/events/trace_msm_low_power.h index bae810cb4231..755d69a2069f 100644 --- a/include/trace/events/trace_msm_low_power.h +++ b/include/trace/events/trace_msm_low_power.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. */ #undef TRACE_SYSTEM @@ -105,13 +105,13 @@ TRACE_EVENT(cpu_idle_enter, TRACE_EVENT(cpu_idle_exit, - TP_PROTO(int index, bool success), + TP_PROTO(int index, int success), TP_ARGS(index, success), TP_STRUCT__entry( __field(int, index) - __field(bool, success) + __field(int, success) ), TP_fast_assign(