2024-01-15 10:11:22

by Sunil V L

[permalink] [raw]
Subject: [PATCH v2 -next 0/3] RISC-V: ACPI: Add LPI support

This series adds support for Low Power Idle (LPI) on ACPI based
platforms.

LPI is described in the ACPI spec [1]. RISC-V FFH spec required to
enable this is available at [2].

[1] - https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#lpi-low-power-idle-states
[2] - https://github.com/riscv-non-isa/riscv-acpi-ffh/releases/download/v/riscv-ffh.pdf

Changes since v1:
1) Reordered the commits such that the patch which enables
ACPI_PROCESSOR is the last one in the series.
2) Used GENMASK and other changes to address Drew's comments.
3) Moved common functions required by both DT and ACPI based cpuidle
drivers from the DT driver to common arch/riscv/suspend.c.
4) ACPI cpuidle driver is added under drivers/acpi/riscv
5) Rebased to latest for-next branch of linux-riscv.

Sunil V L (3):
cpuidle: RISC-V: Move few functions to arch/riscv
ACPI: RISC-V: Add LPI driver
ACPI: Enable ACPI_PROCESSOR for RISC-V

arch/riscv/include/asm/suspend.h | 3 ++
arch/riscv/kernel/suspend.c | 47 +++++++++++++++++
drivers/acpi/Kconfig | 2 +-
drivers/acpi/riscv/Makefile | 3 +-
drivers/acpi/riscv/cpuidle.c | 81 +++++++++++++++++++++++++++++
drivers/cpuidle/cpuidle-riscv-sbi.c | 41 +--------------
6 files changed, 135 insertions(+), 42 deletions(-)
create mode 100644 drivers/acpi/riscv/cpuidle.c

--
2.34.1



2024-01-15 10:11:35

by Sunil V L

[permalink] [raw]
Subject: [PATCH v2 -next 1/3] cpuidle: RISC-V: Move few functions to arch/riscv

To support ACPI Low Power Idle (LPI), few functions are required which
are currently static functions in the DT based cpuidle driver. Hence,
move them under arch/riscv so that ACPI driver also can use them.

Signed-off-by: Sunil V L <[email protected]>
---
arch/riscv/include/asm/suspend.h | 3 ++
arch/riscv/kernel/suspend.c | 47 +++++++++++++++++++++++++++++
drivers/cpuidle/cpuidle-riscv-sbi.c | 41 +------------------------
3 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
index 02f87867389a..5c7df5ab7a16 100644
--- a/arch/riscv/include/asm/suspend.h
+++ b/arch/riscv/include/asm/suspend.h
@@ -55,4 +55,7 @@ int hibernate_resume_nonboot_cpu_disable(void);
asmlinkage void hibernate_restore_image(unsigned long resume_satp, unsigned long satp_temp,
unsigned long cpu_resume);
asmlinkage int hibernate_core_restore_code(void);
+bool is_sbi_hsm_supported(void);
+bool sbi_suspend_state_is_valid(u32 state);
+int sbi_suspend(u32 state);
#endif
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
index 239509367e42..a3b2e7e16a98 100644
--- a/arch/riscv/kernel/suspend.c
+++ b/arch/riscv/kernel/suspend.c
@@ -128,4 +128,51 @@ static int __init sbi_system_suspend_init(void)
}

arch_initcall(sbi_system_suspend_init);
+
+static int sbi_suspend_finisher(unsigned long suspend_type,
+ unsigned long resume_addr,
+ unsigned long opaque)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
+ suspend_type, resume_addr, opaque, 0, 0, 0);
+
+ return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
+}
+
+int sbi_suspend(u32 state)
+{
+ if (state & SBI_HSM_SUSP_NON_RET_BIT)
+ return cpu_suspend(state, sbi_suspend_finisher);
+ else
+ return sbi_suspend_finisher(state, 0, 0);
+}
+
+bool sbi_suspend_state_is_valid(u32 state)
+{
+ if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_RET_PLATFORM)
+ return false;
+ if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
+ return false;
+ return true;
+}
+
+bool is_sbi_hsm_supported(void)
+{
+ /*
+ * The SBI HSM suspend function is only available when:
+ * 1) SBI version is 0.3 or higher
+ * 2) SBI HSM extension is available
+ */
+ if (sbi_spec_version < sbi_mk_version(0, 3) ||
+ !sbi_probe_extension(SBI_EXT_HSM)) {
+ pr_info("HSM suspend not available\n");
+ return false;
+ }
+
+ return true;
+}
#endif /* CONFIG_RISCV_SBI */
diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c
index e8094fc92491..a7f06242f67b 100644
--- a/drivers/cpuidle/cpuidle-riscv-sbi.c
+++ b/drivers/cpuidle/cpuidle-riscv-sbi.c
@@ -73,26 +73,6 @@ static inline bool sbi_is_domain_state_available(void)
return data->available;
}

-static int sbi_suspend_finisher(unsigned long suspend_type,
- unsigned long resume_addr,
- unsigned long opaque)
-{
- struct sbiret ret;
-
- ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
- suspend_type, resume_addr, opaque, 0, 0, 0);
-
- return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
-}
-
-static int sbi_suspend(u32 state)
-{
- if (state & SBI_HSM_SUSP_NON_RET_BIT)
- return cpu_suspend(state, sbi_suspend_finisher);
- else
- return sbi_suspend_finisher(state, 0, 0);
-}
-
static __cpuidle int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)
{
@@ -206,17 +186,6 @@ static const struct of_device_id sbi_cpuidle_state_match[] = {
{ },
};

-static bool sbi_suspend_state_is_valid(u32 state)
-{
- if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
- state < SBI_HSM_SUSPEND_RET_PLATFORM)
- return false;
- if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
- state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
- return false;
- return true;
-}
-
static int sbi_dt_parse_state_node(struct device_node *np, u32 *state)
{
int err = of_property_read_u32(np, "riscv,sbi-suspend-param", state);
@@ -607,16 +576,8 @@ static int __init sbi_cpuidle_init(void)
int ret;
struct platform_device *pdev;

- /*
- * The SBI HSM suspend function is only available when:
- * 1) SBI version is 0.3 or higher
- * 2) SBI HSM extension is available
- */
- if ((sbi_spec_version < sbi_mk_version(0, 3)) ||
- !sbi_probe_extension(SBI_EXT_HSM)) {
- pr_info("HSM suspend not available\n");
+ if (!is_sbi_hsm_supported())
return 0;
- }

ret = platform_driver_register(&sbi_cpuidle_driver);
if (ret)
--
2.34.1


2024-01-15 10:11:54

by Sunil V L

[permalink] [raw]
Subject: [PATCH v2 -next 2/3] ACPI: RISC-V: Add LPI driver

Enable Low Power Idle (LPI) based cpuidle driver for RISC-V platforms.
It depends on SBI HSM calls for idle state transitions.

Signed-off-by: Sunil V L <[email protected]>
---
drivers/acpi/riscv/Makefile | 3 +-
drivers/acpi/riscv/cpuidle.c | 81 ++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 drivers/acpi/riscv/cpuidle.c

diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 8b3b126e0b94..7309d92dd477 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += rhct.o
+obj-y += rhct.o
+obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
diff --git a/drivers/acpi/riscv/cpuidle.c b/drivers/acpi/riscv/cpuidle.c
new file mode 100644
index 000000000000..052ec3942902
--- /dev/null
+++ b/drivers/acpi/riscv/cpuidle.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024, Ventana Micro Systems Inc
+ * Author: Sunil V L <[email protected]>
+ *
+ */
+
+#include <linux/acpi.h>
+#include <acpi/processor.h>
+#include <linux/cpu_pm.h>
+#include <linux/cpuidle.h>
+#include <linux/suspend.h>
+#include <asm/cpuidle.h>
+#include <asm/sbi.h>
+#include <asm/suspend.h>
+
+#define RISCV_FFH_LPI_TYPE_MASK GENMASK_ULL(63, 60)
+#define RISCV_FFH_LPI_RSVD_MASK GENMASK_ULL(59, 32)
+
+#define RISCV_FFH_LPI_TYPE_SBI BIT_ULL(60)
+
+static int acpi_cpu_init_idle(unsigned int cpu)
+{
+ int i;
+ struct acpi_lpi_state *lpi;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+
+ if (unlikely(!pr || !pr->flags.has_lpi))
+ return -EINVAL;
+
+ if (!is_sbi_hsm_supported())
+ return -ENODEV;
+
+ if (pr->power.count <= 1)
+ return -ENODEV;
+
+ for (i = 1; i < pr->power.count; i++) {
+ u32 state;
+
+ lpi = &pr->power.lpi_states[i];
+
+ /*
+ * Validate Entry Method as per FFH spec.
+ * bits[63:60] should be 0x1
+ * bits[59:32] should be 0x0
+ * bits[31:0] represent a SBI power_state
+ */
+ if (((lpi->address & RISCV_FFH_LPI_TYPE_MASK) != RISCV_FFH_LPI_TYPE_SBI) ||
+ (lpi->address & RISCV_FFH_LPI_RSVD_MASK)) {
+ pr_warn("Invalid LPI entry method %#llx\n", lpi->address);
+ return -EINVAL;
+ }
+
+ state = lpi->address;
+ if (!sbi_suspend_state_is_valid(state)) {
+ pr_warn("Invalid SBI power state %#x\n", state);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+ return acpi_cpu_init_idle(cpu);
+}
+
+int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
+{
+ u32 state = lpi->address;
+
+ if (state & SBI_HSM_SUSP_NON_RET_BIT)
+ return CPU_PM_CPU_IDLE_ENTER_PARAM(sbi_suspend,
+ lpi->index,
+ state);
+ else
+ return CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(sbi_suspend,
+ lpi->index,
+ state);
+}
--
2.34.1


2024-01-15 10:12:19

by Sunil V L

[permalink] [raw]
Subject: [PATCH v2 -next 3/3] ACPI: Enable ACPI_PROCESSOR for RISC-V

The ACPI processor driver is not currently enabled for RISC-V.
This is required to enable CPU related functionalities like
LPI and CPPC. Hence, enable ACPI_PROCESSOR for RISC-V.

Signed-off-by: Sunil V L <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
drivers/acpi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index f819e760ff19..9a920752171c 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -282,7 +282,7 @@ config ACPI_CPPC_LIB

config ACPI_PROCESSOR
tristate "Processor"
- depends on X86 || ARM64 || LOONGARCH
+ depends on X86 || ARM64 || LOONGARCH || RISCV
select ACPI_PROCESSOR_IDLE
select ACPI_CPU_FREQ_PSS if X86 || LOONGARCH
select THERMAL
--
2.34.1


2024-01-15 15:23:26

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 -next 1/3] cpuidle: RISC-V: Move few functions to arch/riscv

On Mon, Jan 15, 2024 at 03:40:54PM +0530, Sunil V L wrote:
> To support ACPI Low Power Idle (LPI), few functions are required which
> are currently static functions in the DT based cpuidle driver. Hence,
> move them under arch/riscv so that ACPI driver also can use them.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> arch/riscv/include/asm/suspend.h | 3 ++
> arch/riscv/kernel/suspend.c | 47 +++++++++++++++++++++++++++++
> drivers/cpuidle/cpuidle-riscv-sbi.c | 41 +------------------------
> 3 files changed, 51 insertions(+), 40 deletions(-)
>
> diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
> index 02f87867389a..5c7df5ab7a16 100644
> --- a/arch/riscv/include/asm/suspend.h
> +++ b/arch/riscv/include/asm/suspend.h
> @@ -55,4 +55,7 @@ int hibernate_resume_nonboot_cpu_disable(void);
> asmlinkage void hibernate_restore_image(unsigned long resume_satp, unsigned long satp_temp,
> unsigned long cpu_resume);
> asmlinkage int hibernate_core_restore_code(void);
> +bool is_sbi_hsm_supported(void);
> +bool sbi_suspend_state_is_valid(u32 state);
> +int sbi_suspend(u32 state);
> #endif
> diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> index 239509367e42..a3b2e7e16a98 100644
> --- a/arch/riscv/kernel/suspend.c
> +++ b/arch/riscv/kernel/suspend.c
> @@ -128,4 +128,51 @@ static int __init sbi_system_suspend_init(void)
> }
>
> arch_initcall(sbi_system_suspend_init);
> +
> +static int sbi_suspend_finisher(unsigned long suspend_type,
> + unsigned long resume_addr,
> + unsigned long opaque)
> +{
> + struct sbiret ret;
> +
> + ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
> + suspend_type, resume_addr, opaque, 0, 0, 0);
> +
> + return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
> +}
> +
> +int sbi_suspend(u32 state)

Now that this is exported, I'd name it riscv_sbi_suspend.

> +{
> + if (state & SBI_HSM_SUSP_NON_RET_BIT)
> + return cpu_suspend(state, sbi_suspend_finisher);
> + else
> + return sbi_suspend_finisher(state, 0, 0);
> +}
> +
> +bool sbi_suspend_state_is_valid(u32 state)

Also riscv_ prefix here.

> +{
> + if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
> + state < SBI_HSM_SUSPEND_RET_PLATFORM)
> + return false;
> + if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
> + state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
> + return false;
> + return true;
> +}
> +
> +bool is_sbi_hsm_supported(void)

This I would rename to riscv_sbi_hsm_is_supported

> +{
> + /*
> + * The SBI HSM suspend function is only available when:
> + * 1) SBI version is 0.3 or higher
> + * 2) SBI HSM extension is available
> + */
> + if (sbi_spec_version < sbi_mk_version(0, 3) ||
> + !sbi_probe_extension(SBI_EXT_HSM)) {
> + pr_info("HSM suspend not available\n");
> + return false;
> + }
> +
> + return true;
> +}
> #endif /* CONFIG_RISCV_SBI */
> diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c
> index e8094fc92491..a7f06242f67b 100644
> --- a/drivers/cpuidle/cpuidle-riscv-sbi.c
> +++ b/drivers/cpuidle/cpuidle-riscv-sbi.c
> @@ -73,26 +73,6 @@ static inline bool sbi_is_domain_state_available(void)
> return data->available;
> }
>
> -static int sbi_suspend_finisher(unsigned long suspend_type,
> - unsigned long resume_addr,
> - unsigned long opaque)
> -{
> - struct sbiret ret;
> -
> - ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
> - suspend_type, resume_addr, opaque, 0, 0, 0);
> -
> - return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
> -}
> -
> -static int sbi_suspend(u32 state)
> -{
> - if (state & SBI_HSM_SUSP_NON_RET_BIT)
> - return cpu_suspend(state, sbi_suspend_finisher);
> - else
> - return sbi_suspend_finisher(state, 0, 0);
> -}
> -
> static __cpuidle int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> @@ -206,17 +186,6 @@ static const struct of_device_id sbi_cpuidle_state_match[] = {
> { },
> };
>
> -static bool sbi_suspend_state_is_valid(u32 state)
> -{
> - if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
> - state < SBI_HSM_SUSPEND_RET_PLATFORM)
> - return false;
> - if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
> - state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
> - return false;
> - return true;
> -}
> -
> static int sbi_dt_parse_state_node(struct device_node *np, u32 *state)
> {
> int err = of_property_read_u32(np, "riscv,sbi-suspend-param", state);
> @@ -607,16 +576,8 @@ static int __init sbi_cpuidle_init(void)
> int ret;
> struct platform_device *pdev;
>
> - /*
> - * The SBI HSM suspend function is only available when:
> - * 1) SBI version is 0.3 or higher
> - * 2) SBI HSM extension is available
> - */
> - if ((sbi_spec_version < sbi_mk_version(0, 3)) ||
> - !sbi_probe_extension(SBI_EXT_HSM)) {
> - pr_info("HSM suspend not available\n");
> + if (!is_sbi_hsm_supported())
> return 0;
> - }
>
> ret = platform_driver_register(&sbi_cpuidle_driver);
> if (ret)
> --
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <[email protected]>

2024-01-15 15:26:04

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 -next 2/3] ACPI: RISC-V: Add LPI driver

On Mon, Jan 15, 2024 at 03:40:55PM +0530, Sunil V L wrote:
> Enable Low Power Idle (LPI) based cpuidle driver for RISC-V platforms.
> It depends on SBI HSM calls for idle state transitions.
>
> Signed-off-by: Sunil V L <[email protected]>
> ---
> drivers/acpi/riscv/Makefile | 3 +-
> drivers/acpi/riscv/cpuidle.c | 81 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+), 1 deletion(-)
> create mode 100644 drivers/acpi/riscv/cpuidle.c
>
> diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> index 8b3b126e0b94..7309d92dd477 100644
> --- a/drivers/acpi/riscv/Makefile
> +++ b/drivers/acpi/riscv/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-y += rhct.o
> +obj-y += rhct.o
> +obj-$(CONFIG_ACPI_PROCESSOR_IDLE) += cpuidle.o
> diff --git a/drivers/acpi/riscv/cpuidle.c b/drivers/acpi/riscv/cpuidle.c
> new file mode 100644
> index 000000000000..052ec3942902
> --- /dev/null
> +++ b/drivers/acpi/riscv/cpuidle.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024, Ventana Micro Systems Inc
> + * Author: Sunil V L <[email protected]>
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <acpi/processor.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/cpuidle.h>
> +#include <linux/suspend.h>
> +#include <asm/cpuidle.h>
> +#include <asm/sbi.h>
> +#include <asm/suspend.h>
> +
> +#define RISCV_FFH_LPI_TYPE_MASK GENMASK_ULL(63, 60)
> +#define RISCV_FFH_LPI_RSVD_MASK GENMASK_ULL(59, 32)
> +
> +#define RISCV_FFH_LPI_TYPE_SBI BIT_ULL(60)
> +
> +static int acpi_cpu_init_idle(unsigned int cpu)
> +{
> + int i;
> + struct acpi_lpi_state *lpi;
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> +
> + if (unlikely(!pr || !pr->flags.has_lpi))
> + return -EINVAL;
> +
> + if (!is_sbi_hsm_supported())
> + return -ENODEV;
> +
> + if (pr->power.count <= 1)
> + return -ENODEV;
> +
> + for (i = 1; i < pr->power.count; i++) {
> + u32 state;
> +
> + lpi = &pr->power.lpi_states[i];
> +
> + /*
> + * Validate Entry Method as per FFH spec.
> + * bits[63:60] should be 0x1
> + * bits[59:32] should be 0x0
> + * bits[31:0] represent a SBI power_state
> + */
> + if (((lpi->address & RISCV_FFH_LPI_TYPE_MASK) != RISCV_FFH_LPI_TYPE_SBI) ||
> + (lpi->address & RISCV_FFH_LPI_RSVD_MASK)) {
> + pr_warn("Invalid LPI entry method %#llx\n", lpi->address);
> + return -EINVAL;
> + }
> +
> + state = lpi->address;
> + if (!sbi_suspend_state_is_valid(state)) {
> + pr_warn("Invalid SBI power state %#x\n", state);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> + return acpi_cpu_init_idle(cpu);
> +}
> +
> +int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
> +{
> + u32 state = lpi->address;
> +
> + if (state & SBI_HSM_SUSP_NON_RET_BIT)
> + return CPU_PM_CPU_IDLE_ENTER_PARAM(sbi_suspend,
> + lpi->index,
> + state);
> + else
> + return CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(sbi_suspend,
> + lpi->index,
> + state);
> +}
> --
> 2.34.1
>

Reviewed-by: Andrew Jones <[email protected]>