From b6ec26fb963133d16acb473bf4fcda09630b2298 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 2 Dec 2015 14:10:44 +0000 Subject: [PATCH 1/8] ACPI / processor_idle: replace PREFIX with pr_fmt Like few of the other ACPI modules, replace PREFIX with pr_fmt and change all the printk call sites to use pr_* companion functions in processor_idle. Signed-off-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_idle.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 175c86bee3a9..e057aa8fafb0 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -23,6 +23,7 @@ * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ +#define pr_fmt(fmt) "ACPI: " fmt #include #include @@ -43,8 +44,6 @@ #include #endif -#define PREFIX "ACPI: " - #define ACPI_PROCESSOR_CLASS "processor" #define _COMPONENT ACPI_PROCESSOR_COMPONENT ACPI_MODULE_NAME("processor_idle"); @@ -81,9 +80,9 @@ static int set_max_cstate(const struct dmi_system_id *id) if (max_cstate > ACPI_PROCESSOR_MAX_POWER) return 0; - printk(KERN_NOTICE PREFIX "%s detected - limiting to C%ld max_cstate." - " Override with \"processor.max_cstate=%d\"\n", id->ident, - (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1); + pr_notice("%s detected - limiting to C%ld max_cstate." + " Override with \"processor.max_cstate=%d\"\n", id->ident, + (long)id->driver_data, ACPI_PROCESSOR_MAX_POWER + 1); max_cstate = (long)id->driver_data; @@ -351,7 +350,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr) /* There must be at least 2 elements */ if (!cst || (cst->type != ACPI_TYPE_PACKAGE) || cst->package.count < 2) { - printk(KERN_ERR PREFIX "not enough elements in _CST\n"); + pr_err("not enough elements in _CST\n"); ret = -EFAULT; goto end; } @@ -360,7 +359,7 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr) /* Validate number of power states. */ if (count < 1 || count != cst->package.count - 1) { - printk(KERN_ERR PREFIX "count given by _CST is not valid\n"); + pr_err("count given by _CST is not valid\n"); ret = -EFAULT; goto end; } @@ -469,11 +468,9 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr) * (From 1 through ACPI_PROCESSOR_MAX_POWER - 1) */ if (current_count >= (ACPI_PROCESSOR_MAX_POWER - 1)) { - printk(KERN_WARNING - "Limiting number of power states to max (%d)\n", - ACPI_PROCESSOR_MAX_POWER); - printk(KERN_WARNING - "Please increase ACPI_PROCESSOR_MAX_POWER if needed.\n"); + 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; } } @@ -1097,8 +1094,8 @@ int acpi_processor_power_init(struct acpi_processor *pr) retval = cpuidle_register_driver(&acpi_idle_driver); if (retval) return retval; - printk(KERN_DEBUG "ACPI: %s registered with cpuidle\n", - acpi_idle_driver.name); + pr_debug("%s registered with cpuidle\n", + acpi_idle_driver.name); } dev = kzalloc(sizeof(*dev), GFP_KERNEL); From db62fda318a6b9082ee9d230be8b45e56b6545c0 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 17 Feb 2016 11:54:19 +0000 Subject: [PATCH 2/8] ACPI / processor : add support for ACPI0010 processor container ACPI 6.0 adds support for optional processor container device which may contain child objects that are either processor devices or other processor containers. This allows representing hierarchical processor topologies. It is declared using the _HID of ACPI0010. It is an abstract container used to represent CPU topology and should not be used to hotplug purposes. If no matching handler is found for a device in acpi_scan_attach_handler, acpi_bus_attach does a default enumeration for those devices with valid HID in the acpi namespace. This patch adds a scan handler for these ACPI processor containers to avoid default that enumeration and ensures the platform devices are not created for them. Signed-off-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_processor.c | 17 +++++++++++++++++ include/acpi/processor.h | 1 + 2 files changed, 18 insertions(+) diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 6979186dbd4b..b5e54f2da53d 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -514,7 +514,24 @@ static struct acpi_scan_handler processor_handler = { }, }; +static int acpi_processor_container_attach(struct acpi_device *dev, + const struct acpi_device_id *id) +{ + return 1; +} + +static const struct acpi_device_id processor_container_ids[] = { + { ACPI_PROCESSOR_CONTAINER_HID, }, + { } +}; + +static struct acpi_scan_handler processor_container_handler = { + .ids = processor_container_ids, + .attach = acpi_processor_container_attach, +}; + void __init acpi_processor_init(void) { acpi_scan_add_handler_with_hotplug(&processor_handler, "processor"); + acpi_scan_add_handler(&processor_container_handler); } diff --git a/include/acpi/processor.h b/include/acpi/processor.h index 07fb100bcc68..54d7860cac11 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -9,6 +9,7 @@ #define ACPI_PROCESSOR_CLASS "processor" #define ACPI_PROCESSOR_DEVICE_NAME "Processor" #define ACPI_PROCESSOR_DEVICE_HID "ACPI0007" +#define ACPI_PROCESSOR_CONTAINER_HID "ACPI0010" #define ACPI_PROCESSOR_BUSY_METRIC 10 From 504997cf96fabf67b4768b7a8ca1a1b622abd839 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 17 Feb 2016 12:03:23 +0000 Subject: [PATCH 3/8] ACPI / sleep: move acpi_processor_sleep to sleep.c acpi_processor_sleep is neither related nor used by CPUIdle framework. It's used in system suspend/resume path as a syscore operation. It makes more sense to move it to acpi/sleep.c where all the S-state transition (a.k.a. Linux system suspend/hiberate) related code are present. Also make it depend on CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT so that it's not compiled on architecture like ARM64 where S-states are not yet defined in ACPI. Signed-off-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_driver.c | 2 -- drivers/acpi/processor_idle.c | 37 --------------------------------- drivers/acpi/sleep.c | 35 +++++++++++++++++++++++++++++++ include/acpi/processor.h | 8 ------- 4 files changed, 35 insertions(+), 47 deletions(-) diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index 11154a330f07..d2fa8cb82d2b 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -314,7 +314,6 @@ static int __init acpi_processor_driver_init(void) if (result < 0) return result; - acpi_processor_syscore_init(); register_hotcpu_notifier(&acpi_cpu_notifier); acpi_thermal_cpufreq_init(); acpi_processor_ppc_init(); @@ -330,7 +329,6 @@ static void __exit acpi_processor_driver_exit(void) acpi_processor_ppc_exit(); acpi_thermal_cpufreq_exit(); unregister_hotcpu_notifier(&acpi_cpu_notifier); - acpi_processor_syscore_exit(); driver_unregister(&acpi_processor_driver); } diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index e057aa8fafb0..fadce354d2b7 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -31,7 +31,6 @@ #include /* need_resched() */ #include #include -#include #include /* @@ -193,42 +192,6 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr, #endif -#ifdef CONFIG_PM_SLEEP -static u32 saved_bm_rld; - -static int acpi_processor_suspend(void) -{ - acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld); - return 0; -} - -static void acpi_processor_resume(void) -{ - u32 resumed_bm_rld = 0; - - acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld); - if (resumed_bm_rld == saved_bm_rld) - return; - - acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld); -} - -static struct syscore_ops acpi_processor_syscore_ops = { - .suspend = acpi_processor_suspend, - .resume = acpi_processor_resume, -}; - -void acpi_processor_syscore_init(void) -{ - register_syscore_ops(&acpi_processor_syscore_ops); -} - -void acpi_processor_syscore_exit(void) -{ - unregister_syscore_ops(&acpi_processor_syscore_ops); -} -#endif /* CONFIG_PM_SLEEP */ - #if defined(CONFIG_X86) static void tsc_check_state(int state) { diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 9cb975200cac..fbfcce3b5227 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -677,6 +678,39 @@ static void acpi_sleep_suspend_setup(void) static inline void acpi_sleep_suspend_setup(void) {} #endif /* !CONFIG_SUSPEND */ +#ifdef CONFIG_PM_SLEEP +static u32 saved_bm_rld; + +static int acpi_save_bm_rld(void) +{ + acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &saved_bm_rld); + return 0; +} + +static void acpi_restore_bm_rld(void) +{ + u32 resumed_bm_rld = 0; + + acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_RLD, &resumed_bm_rld); + if (resumed_bm_rld == saved_bm_rld) + return; + + acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_RLD, saved_bm_rld); +} + +static struct syscore_ops acpi_sleep_syscore_ops = { + .suspend = acpi_save_bm_rld, + .resume = acpi_restore_bm_rld, +}; + +void acpi_sleep_syscore_init(void) +{ + register_syscore_ops(&acpi_sleep_syscore_ops); +} +#else +static inline void acpi_sleep_syscore_init(void) {} +#endif /* CONFIG_PM_SLEEP */ + #ifdef CONFIG_HIBERNATION static unsigned long s4_hardware_signature; static struct acpi_table_facs *facs; @@ -839,6 +873,7 @@ int __init acpi_sleep_init(void) sleep_states[ACPI_STATE_S0] = 1; + acpi_sleep_syscore_init(); acpi_sleep_suspend_setup(); acpi_sleep_hibernate_setup(); diff --git a/include/acpi/processor.h b/include/acpi/processor.h index 54d7860cac11..6f1805dd5d3c 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -395,14 +395,6 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr) } #endif /* CONFIG_ACPI_PROCESSOR_IDLE */ -#if defined(CONFIG_PM_SLEEP) & defined(CONFIG_ACPI_PROCESSOR_IDLE) -void acpi_processor_syscore_init(void); -void acpi_processor_syscore_exit(void); -#else -static inline void acpi_processor_syscore_init(void) {} -static inline void acpi_processor_syscore_exit(void) {} -#endif - /* in processor_thermal.c */ int acpi_processor_get_limit_info(struct acpi_processor *pr); extern const struct thermal_cooling_device_ops processor_cooling_ops; From ad62e1e67751d21a3e15bca9d3ededc73de2cfa0 Mon Sep 17 00:00:00 2001 From: Ashwin Chaugule Date: Wed, 17 Feb 2016 13:20:59 -0700 Subject: [PATCH 4/8] ACPI / CPPC: Optimize PCC Read Write operations Previously the send_pcc_cmd() code checked if the PCC operation had completed before returning from the function. This check was performed regardless of the PCC op type (i.e. Read/Write). Knowing the type of cmd can be used to optimize the check and avoid needless waiting. e.g. with Write ops, the actual Writing is done before calling send_pcc_cmd(). And the subsequent Writes will check if the channel is free at the entry of send_pcc_cmd() anyway. However, for Read cmds, we need to wait for the cmd completion bit to be flipped, since the actual Read ops follow after returning from the send_pcc_cmd(). So, only do the looping check at the end for Read ops. Also, instead of using udelay() calls, use ktime as a means to check for deadlines. The current deadline in which the Remote should flip the cmd completion bit is defined as N * Nominal latency. Where N is arbitrary and large enough to work on slow emulators and Nominal latency comes from the ACPI table (PCCT). This helps in working around the CONFIG_HZ effects on udelay() and also avoids needing different ACPI tables for Silicon and Emulation platforms. Signed-off-by: Ashwin Chaugule Signed-off-by: Prashanth Prakash Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 101 ++++++++++++++++++++++++++++----------- 1 file changed, 72 insertions(+), 29 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 6730f965b379..026cdd4d491a 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -39,6 +39,7 @@ #include #include +#include #include /* @@ -63,25 +64,53 @@ static struct mbox_chan *pcc_channel; static void __iomem *pcc_comm_addr; static u64 comm_base_addr; static int pcc_subspace_idx = -1; -static u16 pcc_cmd_delay; static bool pcc_channel_acquired; +static ktime_t deadline; /* * Arbitrary Retries in case the remote processor is slow to respond - * to PCC commands. + * to PCC commands. Keeping it high enough to cover emulators where + * the processors run painfully slow. */ #define NUM_RETRIES 500 +static int check_pcc_chan(void) +{ + int ret = -EIO; + struct acpi_pcct_shared_memory __iomem *generic_comm_base = pcc_comm_addr; + ktime_t next_deadline = ktime_add(ktime_get(), deadline); + + /* Retry in case the remote processor was too slow to catch up. */ + while (!ktime_after(ktime_get(), next_deadline)) { + if (readw_relaxed(&generic_comm_base->status) & PCC_CMD_COMPLETE) { + ret = 0; + break; + } + /* + * Reducing the bus traffic in case this loop takes longer than + * a few retries. + */ + udelay(3); + } + + return ret; +} + static int send_pcc_cmd(u16 cmd) { - int retries, result = -EIO; - struct acpi_pcct_hw_reduced *pcct_ss = pcc_channel->con_priv; + int ret = -EIO; struct acpi_pcct_shared_memory *generic_comm_base = (struct acpi_pcct_shared_memory *) pcc_comm_addr; - u32 cmd_latency = pcct_ss->latency; - /* Min time OS should wait before sending next command. */ - udelay(pcc_cmd_delay); + /* + * For CMD_WRITE we know for a fact the caller should have checked + * the channel before writing to PCC space + */ + if (cmd == CMD_READ) { + ret = check_pcc_chan(); + if (ret) + return ret; + } /* Write to the shared comm region. */ writew(cmd, &generic_comm_base->command); @@ -90,31 +119,30 @@ static int send_pcc_cmd(u16 cmd) writew(0, &generic_comm_base->status); /* Ring doorbell */ - result = mbox_send_message(pcc_channel, &cmd); - if (result < 0) { + ret = mbox_send_message(pcc_channel, &cmd); + if (ret < 0) { pr_err("Err sending PCC mbox message. cmd:%d, ret:%d\n", - cmd, result); - return result; + cmd, ret); + return ret; } - /* Wait for a nominal time to let platform process command. */ - udelay(cmd_latency); + /* + * For READs we need to ensure the cmd completed to ensure + * the ensuing read()s can proceed. For WRITEs we dont care + * because the actual write()s are done before coming here + * and the next READ or WRITE will check if the channel + * is busy/free at the entry of this call. + */ + if (cmd == CMD_READ) + ret = check_pcc_chan(); - /* Retry in case the remote processor was too slow to catch up. */ - for (retries = NUM_RETRIES; retries > 0; retries--) { - if (readw_relaxed(&generic_comm_base->status) & PCC_CMD_COMPLETE) { - result = 0; - break; - } - } - - mbox_client_txdone(pcc_channel, result); - return result; + mbox_client_txdone(pcc_channel, ret); + return ret; } static void cppc_chan_tx_done(struct mbox_client *cl, void *msg, int ret) { - if (ret) + if (ret < 0) pr_debug("TX did not complete: CMD sent:%x, ret:%d\n", *(u16 *)msg, ret); else @@ -306,6 +334,7 @@ static int register_pcc_channel(int pcc_subspace_idx) { struct acpi_pcct_hw_reduced *cppc_ss; unsigned int len; + u64 usecs_lat; if (pcc_subspace_idx >= 0) { pcc_channel = pcc_mbox_request_channel(&cppc_mbox_cl, @@ -335,7 +364,14 @@ static int register_pcc_channel(int pcc_subspace_idx) */ comm_base_addr = cppc_ss->base_address; len = cppc_ss->length; - pcc_cmd_delay = cppc_ss->min_turnaround_time; + + /* + * cppc_ss->latency is just a Nominal value. In reality + * the remote processor could be much slower to reply. + * So add an arbitrary amount of wait on top of Nominal. + */ + usecs_lat = NUM_RETRIES * cppc_ss->latency; + deadline = ns_to_ktime(usecs_lat * NSEC_PER_USEC); pcc_comm_addr = acpi_os_ioremap(comm_base_addr, len); if (!pcc_comm_addr) { @@ -604,7 +640,7 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps) (ref_perf->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) || (nom_perf->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM)) { /* Ring doorbell once to update PCC subspace */ - if (send_pcc_cmd(CMD_READ)) { + if (send_pcc_cmd(CMD_READ) < 0) { ret = -EIO; goto out_err; } @@ -662,7 +698,7 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs) if ((delivered_reg->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) || (reference_reg->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM)) { /* Ring doorbell once to update PCC subspace */ - if (send_pcc_cmd(CMD_READ)) { + if (send_pcc_cmd(CMD_READ) < 0) { ret = -EIO; goto out_err; } @@ -713,6 +749,13 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls) spin_lock(&pcc_lock); + /* If this is PCC reg, check if channel is free before writing */ + if (desired_reg->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) { + ret = check_pcc_chan(); + if (ret) + goto busy_channel; + } + /* * Skip writing MIN/MAX until Linux knows how to come up with * useful values. @@ -722,10 +765,10 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls) /* Is this a PCC reg ?*/ if (desired_reg->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) { /* Ring doorbell so Remote can get our perf request. */ - if (send_pcc_cmd(CMD_WRITE)) + if (send_pcc_cmd(CMD_WRITE) < 0) ret = -EIO; } - +busy_channel: spin_unlock(&pcc_lock); return ret; From 77e3d86f2f8c533c9c72fb5585b623b447d9bd57 Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 17 Feb 2016 13:21:00 -0700 Subject: [PATCH 5/8] ACPI / CPPC: optimized cpc_read and cpc_write cpc_read and cpc_write are used while holding the pcc_lock spin_lock, so they need to be as fast as possible. acpi_os_read/write_memory APIs linearly search through a list for cached mapping which is quite expensive. Since the PCC subspace is already mapped into virtual address space during initialization, we can just add the offset and access the necessary CPPC registers. This patch + similar changes to PCC driver reduce the time per freq. transition from around 200us to about 20us for the CPPC cpufreq driver. Signed-off-by: Prashanth Prakash Acked-by: Ashwin Chaugule Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 82 +++++++++++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 026cdd4d491a..6970b44d37d5 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -67,6 +67,9 @@ static int pcc_subspace_idx = -1; static bool pcc_channel_acquired; static ktime_t deadline; +/* pcc mapped address + header size + offset within PCC subspace */ +#define GET_PCC_VADDR(offs) (pcc_comm_addr + 0x8 + (offs)) + /* * Arbitrary Retries in case the remote processor is slow to respond * to PCC commands. Keeping it high enough to cover emulators where @@ -582,29 +585,74 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr) } EXPORT_SYMBOL_GPL(acpi_cppc_processor_exit); -static u64 get_phys_addr(struct cpc_reg *reg) +/* + * Since cpc_read and cpc_write are called while holding pcc_lock, it should be + * as fast as possible. We have already mapped the PCC subspace during init, so + * we can directly write to it. + */ + +static int cpc_read(struct cpc_reg *reg, u64 *val) { - /* PCC communication addr space begins at byte offset 0x8. */ - if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) - return (u64)comm_base_addr + 0x8 + reg->address; - else - return reg->address; + int ret_val = 0; + + *val = 0; + if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) { + void __iomem *vaddr = GET_PCC_VADDR(reg->address); + + switch (reg->bit_width) { + case 8: + *val = readb(vaddr); + break; + case 16: + *val = readw(vaddr); + break; + case 32: + *val = readl(vaddr); + break; + case 64: + *val = readq(vaddr); + break; + default: + pr_debug("Error: Cannot read %u bit width from PCC\n", + reg->bit_width); + ret_val = -EFAULT; + } + } else + ret_val = acpi_os_read_memory((acpi_physical_address)reg->address, + val, reg->bit_width); + return ret_val; } -static void cpc_read(struct cpc_reg *reg, u64 *val) +static int cpc_write(struct cpc_reg *reg, u64 val) { - u64 addr = get_phys_addr(reg); + int ret_val = 0; - acpi_os_read_memory((acpi_physical_address)addr, - val, reg->bit_width); -} + if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) { + void __iomem *vaddr = GET_PCC_VADDR(reg->address); -static void cpc_write(struct cpc_reg *reg, u64 val) -{ - u64 addr = get_phys_addr(reg); - - acpi_os_write_memory((acpi_physical_address)addr, - val, reg->bit_width); + switch (reg->bit_width) { + case 8: + writeb(val, vaddr); + break; + case 16: + writew(val, vaddr); + break; + case 32: + writel(val, vaddr); + break; + case 64: + writeq(val, vaddr); + break; + default: + pr_debug("Error: Cannot write %u bit width to PCC\n", + reg->bit_width); + ret_val = -EFAULT; + break; + } + } else + ret_val = acpi_os_write_memory((acpi_physical_address)reg->address, + val, reg->bit_width); + return ret_val; } /** From 8b0f57889843af6304b80724e36bd3d93b6484b1 Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 17 Feb 2016 13:21:01 -0700 Subject: [PATCH 6/8] mailbox: pcc: optimized pcc_send_data pcc_send_data() can be invoked during the execution of performance critical code as in cppc_cpufreq driver. With acpi_* APIs, the doorbell register accessed in pcc_send_data() if present in system memory will be searched (in cached virt to phys addr mapping), mapped, read/written and then unmapped. These operations take significant amount of time. This patch maps the performance critical doorbell register during init and then reads/writes to it directly using the mapped virtual address. This patch + similar changes to CPPC acpi driver reduce the time per freq. transition from around 200us to about 20us for the CPPC cpufreq driver Signed-off-by: Prashanth Prakash Acked-by: Ashwin Chaugule Signed-off-by: Rafael J. Wysocki --- drivers/mailbox/pcc.c | 111 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 7 deletions(-) diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index 45d85aea9955..ac11a9b395d3 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -63,6 +63,7 @@ #include #include #include +#include #include "mailbox.h" @@ -70,6 +71,9 @@ static struct mbox_chan *pcc_mbox_channels; +/* Array of cached virtual address for doorbell registers */ +static void __iomem **pcc_doorbell_vaddr; + static struct mbox_controller pcc_mbox_ctrl = {}; /** * get_pcc_channel - Given a PCC subspace idx, get @@ -166,6 +170,66 @@ void pcc_mbox_free_channel(struct mbox_chan *chan) } EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); +/* + * PCC can be used with perf critical drivers such as CPPC + * So it makes sense to locally cache the virtual address and + * use it to read/write to PCC registers such as doorbell register + * + * The below read_register and write_registers are used to read and + * write from perf critical registers such as PCC doorbell register + */ +static int read_register(void __iomem *vaddr, u64 *val, unsigned int bit_width) +{ + int ret_val = 0; + + switch (bit_width) { + case 8: + *val = readb(vaddr); + break; + case 16: + *val = readw(vaddr); + break; + case 32: + *val = readl(vaddr); + break; + case 64: + *val = readq(vaddr); + break; + default: + pr_debug("Error: Cannot read register of %u bit width", + bit_width); + ret_val = -EFAULT; + break; + } + return ret_val; +} + +static int write_register(void __iomem *vaddr, u64 val, unsigned int bit_width) +{ + int ret_val = 0; + + switch (bit_width) { + case 8: + writeb(val, vaddr); + break; + case 16: + writew(val, vaddr); + break; + case 32: + writel(val, vaddr); + break; + case 64: + writeq(val, vaddr); + break; + default: + pr_debug("Error: Cannot write register of %u bit width", + bit_width); + ret_val = -EFAULT; + break; + } + return ret_val; +} + /** * pcc_send_data - Called from Mailbox Controller code. Used * here only to ring the channel doorbell. The PCC client @@ -181,21 +245,39 @@ EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); static int pcc_send_data(struct mbox_chan *chan, void *data) { struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv; - struct acpi_generic_address doorbell; + struct acpi_generic_address *doorbell; u64 doorbell_preserve; u64 doorbell_val; u64 doorbell_write; + u32 id = chan - pcc_mbox_channels; + int ret = 0; - doorbell = pcct_ss->doorbell_register; + if (id >= pcc_mbox_ctrl.num_chans) { + pr_debug("pcc_send_data: Invalid mbox_chan passed\n"); + return -ENOENT; + } + + doorbell = &pcct_ss->doorbell_register; doorbell_preserve = pcct_ss->preserve_mask; doorbell_write = pcct_ss->write_mask; /* Sync notification from OS to Platform. */ - acpi_read(&doorbell_val, &doorbell); - acpi_write((doorbell_val & doorbell_preserve) | doorbell_write, - &doorbell); - - return 0; + if (pcc_doorbell_vaddr[id]) { + ret = read_register(pcc_doorbell_vaddr[id], &doorbell_val, + doorbell->bit_width); + if (ret) + return ret; + ret = write_register(pcc_doorbell_vaddr[id], + (doorbell_val & doorbell_preserve) | doorbell_write, + doorbell->bit_width); + } else { + ret = acpi_read(&doorbell_val, doorbell); + if (ret) + return ret; + ret = acpi_write((doorbell_val & doorbell_preserve) | doorbell_write, + doorbell); + } + return ret; } static const struct mbox_chan_ops pcc_chan_ops = { @@ -271,14 +353,29 @@ static int __init acpi_pcc_probe(void) return -ENOMEM; } + pcc_doorbell_vaddr = kcalloc(count, sizeof(void *), GFP_KERNEL); + if (!pcc_doorbell_vaddr) { + kfree(pcc_mbox_channels); + return -ENOMEM; + } + /* Point to the first PCC subspace entry */ pcct_entry = (struct acpi_subtable_header *) ( (unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct)); for (i = 0; i < count; i++) { + struct acpi_generic_address *db_reg; + struct acpi_pcct_hw_reduced *pcct_ss; pcc_mbox_channels[i].con_priv = pcct_entry; pcct_entry = (struct acpi_subtable_header *) ((unsigned long) pcct_entry + pcct_entry->length); + + /* If doorbell is in system memory cache the virt address */ + pcct_ss = (struct acpi_pcct_hw_reduced *)pcct_entry; + db_reg = &pcct_ss->doorbell_register; + if (db_reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) + pcc_doorbell_vaddr[i] = acpi_os_ioremap(db_reg->address, + db_reg->bit_width/8); } pcc_mbox_ctrl.num_chans = count; From beee23aebc6650609ef1547f6d813fa5065f74aa Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 17 Feb 2016 13:21:02 -0700 Subject: [PATCH 7/8] ACPI / CPPC: replace writeX/readX to PCC with relaxed version We do not have a strict read/write order requirement while accessing PCC subspace. The only requirement is all access should be committed before triggering the PCC doorbell to transfer the ownership of PCC to the platform and this requirement is enforced by the PCC driver. Profiling on a many core system shows improvement of about 1.8us on average per freq change request(about 10% improvement on average). Since these operations are executed while holding the pcc_lock, reducing this time helps the CPPC implementation to scale much better as the number of cores increases. Signed-off-by: Prashanth Prakash Acked-by: Ashwin Chaugule Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 6970b44d37d5..8a0911a13599 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -116,10 +116,10 @@ static int send_pcc_cmd(u16 cmd) } /* Write to the shared comm region. */ - writew(cmd, &generic_comm_base->command); + writew_relaxed(cmd, &generic_comm_base->command); /* Flip CMD COMPLETE bit */ - writew(0, &generic_comm_base->status); + writew_relaxed(0, &generic_comm_base->status); /* Ring doorbell */ ret = mbox_send_message(pcc_channel, &cmd); @@ -601,16 +601,16 @@ static int cpc_read(struct cpc_reg *reg, u64 *val) switch (reg->bit_width) { case 8: - *val = readb(vaddr); + *val = readb_relaxed(vaddr); break; case 16: - *val = readw(vaddr); + *val = readw_relaxed(vaddr); break; case 32: - *val = readl(vaddr); + *val = readl_relaxed(vaddr); break; case 64: - *val = readq(vaddr); + *val = readq_relaxed(vaddr); break; default: pr_debug("Error: Cannot read %u bit width from PCC\n", @@ -632,16 +632,16 @@ static int cpc_write(struct cpc_reg *reg, u64 val) switch (reg->bit_width) { case 8: - writeb(val, vaddr); + writeb_relaxed(val, vaddr); break; case 16: - writew(val, vaddr); + writew_relaxed(val, vaddr); break; case 32: - writel(val, vaddr); + writel_relaxed(val, vaddr); break; case 64: - writeq(val, vaddr); + writeq_relaxed(val, vaddr); break; default: pr_debug("Error: Cannot write %u bit width to PCC\n", From f387e5b901adb8352fcdf031bb2c539e390b92e2 Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 17 Feb 2016 13:21:03 -0700 Subject: [PATCH 8/8] ACPI / CPPC: use MRTT/MPAR to decide if/when a req can be sent The ACPI spec defines Minimum Request Turnaround Time(MRTT) and Maximum Periodic Access Rate(MPAR) to prevent the OSPM from sending too many requests than the platform can handle. For further details on these parameters please refer to section 14.1.3 of ACPI 6.0 spec. This patch includes MRTT/MPAR in deciding if or when a CPPC request can be sent to the platform to make sure CPPC implementation is compliant to the spec. Signed-off-by: Prashanth Prakash Acked-by: Ashwin Chaugule Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 56 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 8a0911a13599..8adac69dba3d 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -66,6 +66,7 @@ static u64 comm_base_addr; static int pcc_subspace_idx = -1; static bool pcc_channel_acquired; static ktime_t deadline; +static unsigned int pcc_mpar, pcc_mrtt; /* pcc mapped address + header size + offset within PCC subspace */ #define GET_PCC_VADDR(offs) (pcc_comm_addr + 0x8 + (offs)) @@ -85,6 +86,11 @@ static int check_pcc_chan(void) /* Retry in case the remote processor was too slow to catch up. */ while (!ktime_after(ktime_get(), next_deadline)) { + /* + * Per spec, prior to boot the PCC space wil be initialized by + * platform and should have set the command completion bit when + * PCC can be used by OSPM + */ if (readw_relaxed(&generic_comm_base->status) & PCC_CMD_COMPLETE) { ret = 0; break; @@ -104,6 +110,9 @@ static int send_pcc_cmd(u16 cmd) int ret = -EIO; struct acpi_pcct_shared_memory *generic_comm_base = (struct acpi_pcct_shared_memory *) pcc_comm_addr; + static ktime_t last_cmd_cmpl_time, last_mpar_reset; + static int mpar_count; + unsigned int time_delta; /* * For CMD_WRITE we know for a fact the caller should have checked @@ -115,6 +124,41 @@ static int send_pcc_cmd(u16 cmd) return ret; } + /* + * Handle the Minimum Request Turnaround Time(MRTT) + * "The minimum amount of time that OSPM must wait after the completion + * of a command before issuing the next command, in microseconds" + */ + if (pcc_mrtt) { + time_delta = ktime_us_delta(ktime_get(), last_cmd_cmpl_time); + if (pcc_mrtt > time_delta) + udelay(pcc_mrtt - time_delta); + } + + /* + * Handle the non-zero Maximum Periodic Access Rate(MPAR) + * "The maximum number of periodic requests that the subspace channel can + * support, reported in commands per minute. 0 indicates no limitation." + * + * This parameter should be ideally zero or large enough so that it can + * handle maximum number of requests that all the cores in the system can + * collectively generate. If it is not, we will follow the spec and just + * not send the request to the platform after hitting the MPAR limit in + * any 60s window + */ + if (pcc_mpar) { + if (mpar_count == 0) { + time_delta = ktime_ms_delta(ktime_get(), last_mpar_reset); + if (time_delta < 60 * MSEC_PER_SEC) { + pr_debug("PCC cmd not sent due to MPAR limit"); + return -EIO; + } + last_mpar_reset = ktime_get(); + mpar_count = pcc_mpar; + } + mpar_count--; + } + /* Write to the shared comm region. */ writew_relaxed(cmd, &generic_comm_base->command); @@ -135,9 +179,17 @@ static int send_pcc_cmd(u16 cmd) * because the actual write()s are done before coming here * and the next READ or WRITE will check if the channel * is busy/free at the entry of this call. + * + * If Minimum Request Turnaround Time is non-zero, we need + * to record the completion time of both READ and WRITE + * command for proper handling of MRTT, so we need to check + * for pcc_mrtt in addition to CMD_READ */ - if (cmd == CMD_READ) + if (cmd == CMD_READ || pcc_mrtt) { ret = check_pcc_chan(); + if (pcc_mrtt) + last_cmd_cmpl_time = ktime_get(); + } mbox_client_txdone(pcc_channel, ret); return ret; @@ -375,6 +427,8 @@ static int register_pcc_channel(int pcc_subspace_idx) */ usecs_lat = NUM_RETRIES * cppc_ss->latency; deadline = ns_to_ktime(usecs_lat * NSEC_PER_USEC); + pcc_mrtt = cppc_ss->min_turnaround_time; + pcc_mpar = cppc_ss->max_access_rate; pcc_comm_addr = acpi_os_ioremap(comm_base_addr, len); if (!pcc_comm_addr) {