linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver.
@ 2021-07-24  9:19 周琰杰 (Zhou Yanjie)
  2021-07-24 11:16 ` Paul Cercueil
  0 siblings, 1 reply; 3+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-07-24  9:19 UTC (permalink / raw)
  To: rjw, daniel.lezcano
  Cc: paul, linux-pm, linux-mips, linux-kernel, dongsheng.qiu,
	aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou,
	Alex Smith

The JZ4780 has a high overhead to executing a MIPS wait on SMP, as a
core must flush out dirty cache lines from its data cache before doing
so. This is because the core clock is gated during a wait and if the
other core tries to access a dirty line from the waiting core's cache,
it will lock up.

To mitigate some of this impact, this driver provides a simple polling
top level idle state, to try to avoid the cache flushing overhead when
the wait will only be short. The second level state is implemented with
the MIPS wait instruction.

This patch first found in the github repository of CI20, the original
author is Alex Smith. Because there is a chance to cause kernel hang
scenarios which can occur within hours or even within days, so this
patch was abandoned, but now it is determined that this is not the
problem caused by this patch, but caused by the cache driver. With
the new Ingenic specific cache driver, it has been working properly
on CI20 v1 for more than one week.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Alex Smith <alex.smith@imgtec.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 drivers/cpuidle/Kconfig.mips     |  8 +++++
 drivers/cpuidle/Makefile         |  1 +
 drivers/cpuidle/cpuidle-jz4780.c | 74 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-jz4780.c

diff --git a/drivers/cpuidle/Kconfig.mips b/drivers/cpuidle/Kconfig.mips
index c3c011a..4a55d24 100644
--- a/drivers/cpuidle/Kconfig.mips
+++ b/drivers/cpuidle/Kconfig.mips
@@ -16,3 +16,11 @@ config MIPS_CPS_CPUIDLE
 	  Processing System (CPS) architecture. In order to make use of
 	  the deepest idle states you will need to ensure that you are
 	  also using the CONFIG_MIPS_CPS SMP implementation.
+
+config MIPS_JZ4780_CPUIDLE
+	bool "CPU Idle driver for Ingenic JZ4780"
+	depends on MACH_JZ4780 && SMP
+	default y
+	help
+	  Select this option to enable CPU idle state management through
+	  cpuidle for Ingenic JZ4780 platforms.
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 26bbc5e..1dd372f 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE)	+= cpuidle-qcom-spm.o
 ###############################################################################
 # MIPS drivers
 obj-$(CONFIG_MIPS_CPS_CPUIDLE)		+= cpuidle-cps.o
+obj-$(CONFIG_MIPS_JZ4780_CPUIDLE)	+= cpuidle-jz4780.o
 
 ###############################################################################
 # POWERPC drivers
diff --git a/drivers/cpuidle/cpuidle-jz4780.c b/drivers/cpuidle/cpuidle-jz4780.c
new file mode 100644
index 00000000..2025de4
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-jz4780.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4780 CPU idle driver
+ * Copyright (C) 2015 Imagination Technologies
+ * Author: Alex Smith <alex.smith@imgtec.com>
+ * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/sched/idle.h>
+
+#include <asm/idle.h>
+#include <asm/mipsregs.h>
+
+/*
+ * The JZ4780 has a high overhead to entering just the basic MIPS wait on SMP,
+ * due to the requirement to flush out dirty lines from the dcache before
+ * waiting. Therefore, we try to mitigate this overhead by using a simple
+ * polling loop for short waits.
+ */
+static int jz4780_cpuidle_poll_enter(struct cpuidle_device *dev,
+				     struct cpuidle_driver *drv, int index)
+{
+	if (!current_set_polling_and_test())
+		while (!need_resched() && !(read_c0_cause() & read_c0_status() & CAUSEF_IP))
+			cpu_relax();
+
+	current_clr_polling();
+	local_irq_enable();
+
+	return index;
+}
+
+static struct cpuidle_driver jz4780_cpuidle_driver = {
+	.name = "jz4780_cpuidle",
+	.owner = THIS_MODULE,
+	.states = {
+		{
+			.enter = jz4780_cpuidle_poll_enter,
+			.exit_latency = 1,
+			.target_residency = 1,
+			.power_usage = UINT_MAX,
+			.name = "poll",
+			.desc = "polling loop",
+		},
+		{
+			.enter = mips_cpuidle_wait_enter,
+			.exit_latency = 50,
+			.target_residency = 300,
+			.power_usage = UINT_MAX,
+			.name = "wait",
+			.desc = "MIPS wait",
+		},
+	},
+	.state_count = 2,
+};
+
+static int __init jz4780_cpuidle_init(void)
+{
+	int ret;
+
+	ret = cpuidle_register(&jz4780_cpuidle_driver, NULL);
+	if (ret) {
+		pr_err("Failed to register JZ4780 idle driver: %d\n", ret);
+		return ret;
+	}
+
+	pr_info("JZ4780 idle driver registered\n");
+
+	return 0;
+}
+device_initcall(jz4780_cpuidle_init);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver.
  2021-07-24  9:19 [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver 周琰杰 (Zhou Yanjie)
@ 2021-07-24 11:16 ` Paul Cercueil
  2021-07-24 13:36   ` Zhou Yanjie
  0 siblings, 1 reply; 3+ messages in thread
From: Paul Cercueil @ 2021-07-24 11:16 UTC (permalink / raw)
  To: 周琰杰
  Cc: rjw, daniel.lezcano, linux-pm, linux-mips, linux-kernel,
	dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang,
	sernia.zhou, Alex Smith

Hi Zhou,

Le sam., juil. 24 2021 at 17:19:59 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> The JZ4780 has a high overhead to executing a MIPS wait on SMP, as a
> core must flush out dirty cache lines from its data cache before doing
> so. This is because the core clock is gated during a wait and if the
> other core tries to access a dirty line from the waiting core's cache,
> it will lock up.
> 
> To mitigate some of this impact, this driver provides a simple polling
> top level idle state, to try to avoid the cache flushing overhead when
> the wait will only be short. The second level state is implemented 
> with
> the MIPS wait instruction.
> 
> This patch first found in the github repository of CI20, the original
> author is Alex Smith. Because there is a chance to cause kernel hang
> scenarios which can occur within hours or even within days, so this
> patch was abandoned, but now it is determined that this is not the
> problem caused by this patch, but caused by the cache driver. With
> the new Ingenic specific cache driver, it has been working properly
> on CI20 v1 for more than one week.
> 
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>  drivers/cpuidle/Kconfig.mips     |  8 +++++
>  drivers/cpuidle/Makefile         |  1 +
>  drivers/cpuidle/cpuidle-jz4780.c | 74 
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+)
>  create mode 100644 drivers/cpuidle/cpuidle-jz4780.c
> 
> diff --git a/drivers/cpuidle/Kconfig.mips 
> b/drivers/cpuidle/Kconfig.mips
> index c3c011a..4a55d24 100644
> --- a/drivers/cpuidle/Kconfig.mips
> +++ b/drivers/cpuidle/Kconfig.mips
> @@ -16,3 +16,11 @@ config MIPS_CPS_CPUIDLE
>  	  Processing System (CPS) architecture. In order to make use of
>  	  the deepest idle states you will need to ensure that you are
>  	  also using the CONFIG_MIPS_CPS SMP implementation.
> +
> +config MIPS_JZ4780_CPUIDLE
> +	bool "CPU Idle driver for Ingenic JZ4780"
> +	depends on MACH_JZ4780 && SMP
> +	default y
> +	help
> +	  Select this option to enable CPU idle state management through
> +	  cpuidle for Ingenic JZ4780 platforms.
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 26bbc5e..1dd372f 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE)	+= 
> cpuidle-qcom-spm.o
>  
> ###############################################################################
>  # MIPS drivers
>  obj-$(CONFIG_MIPS_CPS_CPUIDLE)		+= cpuidle-cps.o
> +obj-$(CONFIG_MIPS_JZ4780_CPUIDLE)	+= cpuidle-jz4780.o
> 
>  
> ###############################################################################
>  # POWERPC drivers
> diff --git a/drivers/cpuidle/cpuidle-jz4780.c 
> b/drivers/cpuidle/cpuidle-jz4780.c
> new file mode 100644
> index 00000000..2025de4
> --- /dev/null
> +++ b/drivers/cpuidle/cpuidle-jz4780.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * JZ4780 CPU idle driver
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Alex Smith <alex.smith@imgtec.com>
> + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>
> + */
> +
> +#include <linux/cpuidle.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/sched/idle.h>
> +
> +#include <asm/idle.h>
> +#include <asm/mipsregs.h>
> +
> +/*
> + * The JZ4780 has a high overhead to entering just the basic MIPS 
> wait on SMP,
> + * due to the requirement to flush out dirty lines from the dcache 
> before
> + * waiting. Therefore, we try to mitigate this overhead by using a 
> simple
> + * polling loop for short waits.
> + */
> +static int jz4780_cpuidle_poll_enter(struct cpuidle_device *dev,
> +				     struct cpuidle_driver *drv, int index)
> +{
> +	if (!current_set_polling_and_test())
> +		while (!need_resched() && !(read_c0_cause() & read_c0_status() & 
> CAUSEF_IP))
> +			cpu_relax();
> +
> +	current_clr_polling();
> +	local_irq_enable();
> +
> +	return index;
> +}
> +
> +static struct cpuidle_driver jz4780_cpuidle_driver = {
> +	.name = "jz4780_cpuidle",
> +	.owner = THIS_MODULE,
> +	.states = {
> +		{
> +			.enter = jz4780_cpuidle_poll_enter,
> +			.exit_latency = 1,
> +			.target_residency = 1,
> +			.power_usage = UINT_MAX,
> +			.name = "poll",
> +			.desc = "polling loop",
> +		},
> +		{
> +			.enter = mips_cpuidle_wait_enter,
> +			.exit_latency = 50,
> +			.target_residency = 300,
> +			.power_usage = UINT_MAX,
> +			.name = "wait",
> +			.desc = "MIPS wait",
> +		},
> +	},
> +	.state_count = 2,
> +};
> +
> +static int __init jz4780_cpuidle_init(void)
> +{
> +	int ret;
> +
> +	ret = cpuidle_register(&jz4780_cpuidle_driver, NULL);

You're missing something here - you never check that the kernel is 
actually running on a JZ4780.

Cheers,
-Paul

> +	if (ret) {
> +		pr_err("Failed to register JZ4780 idle driver: %d\n", ret);
> +		return ret;
> +	}
> +
> +	pr_info("JZ4780 idle driver registered\n");
> +
> +	return 0;
> +}
> +device_initcall(jz4780_cpuidle_init);
> --
> 2.7.4
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver.
  2021-07-24 11:16 ` Paul Cercueil
@ 2021-07-24 13:36   ` Zhou Yanjie
  0 siblings, 0 replies; 3+ messages in thread
From: Zhou Yanjie @ 2021-07-24 13:36 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: rjw, daniel.lezcano, linux-pm, linux-mips, linux-kernel,
	dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang,
	sernia.zhou, Alex Smith

Hi Paul,

On 2021/7/24 下午7:16, Paul Cercueil wrote:
> Hi Zhou,
>
> Le sam., juil. 24 2021 at 17:19:59 +0800, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> The JZ4780 has a high overhead to executing a MIPS wait on SMP, as a
>> core must flush out dirty cache lines from its data cache before doing
>> so. This is because the core clock is gated during a wait and if the
>> other core tries to access a dirty line from the waiting core's cache,
>> it will lock up.
>>
>> To mitigate some of this impact, this driver provides a simple polling
>> top level idle state, to try to avoid the cache flushing overhead when
>> the wait will only be short. The second level state is implemented with
>> the MIPS wait instruction.
>>
>> This patch first found in the github repository of CI20, the original
>> author is Alex Smith. Because there is a chance to cause kernel hang
>> scenarios which can occur within hours or even within days, so this
>> patch was abandoned, but now it is determined that this is not the
>> problem caused by this patch, but caused by the cache driver. With
>> the new Ingenic specific cache driver, it has been working properly
>> on CI20 v1 for more than one week.
>>
>> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>  drivers/cpuidle/Kconfig.mips     |  8 +++++
>>  drivers/cpuidle/Makefile         |  1 +
>>  drivers/cpuidle/cpuidle-jz4780.c | 74 
>> ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 83 insertions(+)
>>  create mode 100644 drivers/cpuidle/cpuidle-jz4780.c
>>
>> diff --git a/drivers/cpuidle/Kconfig.mips b/drivers/cpuidle/Kconfig.mips
>> index c3c011a..4a55d24 100644
>> --- a/drivers/cpuidle/Kconfig.mips
>> +++ b/drivers/cpuidle/Kconfig.mips
>> @@ -16,3 +16,11 @@ config MIPS_CPS_CPUIDLE
>>        Processing System (CPS) architecture. In order to make use of
>>        the deepest idle states you will need to ensure that you are
>>        also using the CONFIG_MIPS_CPS SMP implementation.
>> +
>> +config MIPS_JZ4780_CPUIDLE
>> +    bool "CPU Idle driver for Ingenic JZ4780"
>> +    depends on MACH_JZ4780 && SMP
>> +    default y
>> +    help
>> +      Select this option to enable CPU idle state management through
>> +      cpuidle for Ingenic JZ4780 platforms.
>> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
>> index 26bbc5e..1dd372f 100644
>> --- a/drivers/cpuidle/Makefile
>> +++ b/drivers/cpuidle/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE)    += 
>> cpuidle-qcom-spm.o
>>
>> ############################################################################### 
>>
>>  # MIPS drivers
>>  obj-$(CONFIG_MIPS_CPS_CPUIDLE)        += cpuidle-cps.o
>> +obj-$(CONFIG_MIPS_JZ4780_CPUIDLE)    += cpuidle-jz4780.o
>>
>>
>> ############################################################################### 
>>
>>  # POWERPC drivers
>> diff --git a/drivers/cpuidle/cpuidle-jz4780.c 
>> b/drivers/cpuidle/cpuidle-jz4780.c
>> new file mode 100644
>> index 00000000..2025de4
>> --- /dev/null
>> +++ b/drivers/cpuidle/cpuidle-jz4780.c
>> @@ -0,0 +1,74 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * JZ4780 CPU idle driver
>> + * Copyright (C) 2015 Imagination Technologies
>> + * Author: Alex Smith <alex.smith@imgtec.com>
>> + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> + */
>> +
>> +#include <linux/cpuidle.h>
>> +#include <linux/init.h>
>> +#include <linux/sched.h>
>> +#include <linux/sched/idle.h>
>> +
>> +#include <asm/idle.h>
>> +#include <asm/mipsregs.h>
>> +
>> +/*
>> + * The JZ4780 has a high overhead to entering just the basic MIPS 
>> wait on SMP,
>> + * due to the requirement to flush out dirty lines from the dcache 
>> before
>> + * waiting. Therefore, we try to mitigate this overhead by using a 
>> simple
>> + * polling loop for short waits.
>> + */
>> +static int jz4780_cpuidle_poll_enter(struct cpuidle_device *dev,
>> +                     struct cpuidle_driver *drv, int index)
>> +{
>> +    if (!current_set_polling_and_test())
>> +        while (!need_resched() && !(read_c0_cause() & 
>> read_c0_status() & CAUSEF_IP))
>> +            cpu_relax();
>> +
>> +    current_clr_polling();
>> +    local_irq_enable();
>> +
>> +    return index;
>> +}
>> +
>> +static struct cpuidle_driver jz4780_cpuidle_driver = {
>> +    .name = "jz4780_cpuidle",
>> +    .owner = THIS_MODULE,
>> +    .states = {
>> +        {
>> +            .enter = jz4780_cpuidle_poll_enter,
>> +            .exit_latency = 1,
>> +            .target_residency = 1,
>> +            .power_usage = UINT_MAX,
>> +            .name = "poll",
>> +            .desc = "polling loop",
>> +        },
>> +        {
>> +            .enter = mips_cpuidle_wait_enter,
>> +            .exit_latency = 50,
>> +            .target_residency = 300,
>> +            .power_usage = UINT_MAX,
>> +            .name = "wait",
>> +            .desc = "MIPS wait",
>> +        },
>> +    },
>> +    .state_count = 2,
>> +};
>> +
>> +static int __init jz4780_cpuidle_init(void)
>> +{
>> +    int ret;
>> +
>> +    ret = cpuidle_register(&jz4780_cpuidle_driver, NULL);
>
> You're missing something here - you never check that the kernel is 
> actually running on a JZ4780.


Sure, I will try to fix it.


Thanks and best regards!


>
> Cheers,
> -Paul
>
>> +    if (ret) {
>> +        pr_err("Failed to register JZ4780 idle driver: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    pr_info("JZ4780 idle driver registered\n");
>> +
>> +    return 0;
>> +}
>> +device_initcall(jz4780_cpuidle_init);
>> -- 
>> 2.7.4
>>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-07-24 13:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-24  9:19 [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver 周琰杰 (Zhou Yanjie)
2021-07-24 11:16 ` Paul Cercueil
2021-07-24 13:36   ` Zhou Yanjie

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox