[RFC/RFT,5/6] ARM: Exynos: migrate DCSCB to the new MCPM backend abstraction

Message ID alpine.LFD.2.11.1503241913040.27567@knanqh.ubzr
State New
Headers show

Commit Message

Nicolas Pitre March 24, 2015, 11:24 p.m.
On Wed, 18 Mar 2015, Nicolas Pitre wrote:

> The custom suspend callback is removed for this change. That includes
> the dubious call to exynos_cpu_power_up(() that was present at the end
> of exynos_suspend().

After testing on actual hardware, it turns out that this call is 
important.  This patch is therefore amended with the following:



Nicolas

Comments

Daniel Lezcano March 25, 2015, 9:40 a.m. | #1
On 03/25/2015 12:24 AM, Nicolas Pitre wrote:
> On Wed, 18 Mar 2015, Nicolas Pitre wrote:
>
>> The custom suspend callback is removed for this change. That includes
>> the dubious call to exynos_cpu_power_up(() that was present at the end
>> of exynos_suspend().
>
> After testing on actual hardware, it turns out that this call is
> important.  This patch is therefore amended with the following:
>
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index d4bbbfb5fe..9bdf54795f 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -152,6 +152,12 @@ static int exynos_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
>   	return -ETIMEDOUT; /* timeout */
>   }
>
> +static void exynos_cpu_is_up(unsigned int cpu, unsigned int cluster)
> +{
> +	/* especially when resuming: make sure power control is set */
> +	exynos_cpu_powerup(cpu, cluster);
> +}
> +
>   static const struct mcpm_platform_ops exynos_power_ops = {
>   	.cpu_powerup		= exynos_cpu_powerup,
>   	.cluster_powerup	= exynos_cluster_powerup,
> @@ -160,6 +166,7 @@ static const struct mcpm_platform_ops exynos_power_ops = {
>   	.cpu_cache_disable	= exynos_cpu_cache_disable,
>   	.cluster_cache_disable	= exynos_cluster_cache_disable,
>   	.wait_for_powerdown	= exynos_wait_for_powerdown,
> +	.cpu_is_up		= exynos_cpu_is_up,
>   };
>
>   /*
>
> The whole commit now appears as follows in my git tree:
>
> commit 0d86b0b4cf869fa48d96bde231b9d04ea68b6422
> Author: Nicolas Pitre <nicolas.pitre@linaro.org>
> Date:   Mon Mar 16 17:16:07 2015 -0400
>
>      ARM: Exynos: migrate DCSCB to the new MCPM backend abstraction
>
>      The custom suspend callback is removed for this change. The extra call
>      to exynos_cpu_power_up(() that was present at the end of exynos_suspend()
>      is now relocated to the cpu_is_up callback.
>
>      Signed-off-by: Nicolas Pitre <nico@linaro.org>

Tested on exynos5800 (chromebook2).

Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Patch

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index d4bbbfb5fe..9bdf54795f 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -152,6 +152,12 @@  static int exynos_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
 	return -ETIMEDOUT; /* timeout */
 }
 
+static void exynos_cpu_is_up(unsigned int cpu, unsigned int cluster)
+{
+	/* especially when resuming: make sure power control is set */
+	exynos_cpu_powerup(cpu, cluster);
+}
+
 static const struct mcpm_platform_ops exynos_power_ops = {
 	.cpu_powerup		= exynos_cpu_powerup,
 	.cluster_powerup	= exynos_cluster_powerup,
@@ -160,6 +166,7 @@  static const struct mcpm_platform_ops exynos_power_ops = {
 	.cpu_cache_disable	= exynos_cpu_cache_disable,
 	.cluster_cache_disable	= exynos_cluster_cache_disable,
 	.wait_for_powerdown	= exynos_wait_for_powerdown,
+	.cpu_is_up		= exynos_cpu_is_up,
 };
 
 /*

The whole commit now appears as follows in my git tree:

commit 0d86b0b4cf869fa48d96bde231b9d04ea68b6422
Author: Nicolas Pitre <nicolas.pitre@linaro.org>
Date:   Mon Mar 16 17:16:07 2015 -0400

    ARM: Exynos: migrate DCSCB to the new MCPM backend abstraction
    
    The custom suspend callback is removed for this change. The extra call
    to exynos_cpu_power_up(() that was present at the end of exynos_suspend()
    is now relocated to the cpu_is_up callback.
    
    Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index b0d3c2e876..9bdf54795f 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -61,25 +61,7 @@  static void __iomem *ns_sram_base_addr;
 	: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", \
 	  "r9", "r10", "lr", "memory")
 
-/*
- * We can't use regular spinlocks. In the switcher case, it is possible
- * for an outbound CPU to call power_down() after its inbound counterpart
- * is already live using the same logical CPU number which trips lockdep
- * debugging.
- */
-static arch_spinlock_t exynos_mcpm_lock = __ARCH_SPIN_LOCK_UNLOCKED;
-static int
-cpu_use_count[EXYNOS5420_CPUS_PER_CLUSTER][EXYNOS5420_NR_CLUSTERS];
-
-#define exynos_cluster_usecnt(cluster) \
-	(cpu_use_count[0][cluster] +   \
-	 cpu_use_count[1][cluster] +   \
-	 cpu_use_count[2][cluster] +   \
-	 cpu_use_count[3][cluster])
-
-#define exynos_cluster_unused(cluster) !exynos_cluster_usecnt(cluster)
-
-static int exynos_power_up(unsigned int cpu, unsigned int cluster)
+static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
 {
 	unsigned int cpunr = cpu + (cluster * EXYNOS5420_CPUS_PER_CLUSTER);
 
@@ -88,127 +70,65 @@  static int exynos_power_up(unsigned int cpu, unsigned int cluster)
 		cluster >= EXYNOS5420_NR_CLUSTERS)
 		return -EINVAL;
 
-	/*
-	 * Since this is called with IRQs enabled, and no arch_spin_lock_irq
-	 * variant exists, we need to disable IRQs manually here.
-	 */
-	local_irq_disable();
-	arch_spin_lock(&exynos_mcpm_lock);
-
-	cpu_use_count[cpu][cluster]++;
-	if (cpu_use_count[cpu][cluster] == 1) {
-		bool was_cluster_down =
-			(exynos_cluster_usecnt(cluster) == 1);
-
-		/*
-		 * Turn on the cluster (L2/COMMON) and then power on the
-		 * cores.
-		 */
-		if (was_cluster_down)
-			exynos_cluster_power_up(cluster);
-
-		exynos_cpu_power_up(cpunr);
-	} else if (cpu_use_count[cpu][cluster] != 2) {
-		/*
-		 * The only possible values are:
-		 * 0 = CPU down
-		 * 1 = CPU (still) up
-		 * 2 = CPU requested to be up before it had a chance
-		 *     to actually make itself down.
-		 * Any other value is a bug.
-		 */
-		BUG();
-	}
+	exynos_cpu_power_up(cpunr);
+	return 0;
+}
 
-	arch_spin_unlock(&exynos_mcpm_lock);
-	local_irq_enable();
+static int exynos_cluster_powerup(unsigned int cluster)
+{
+	pr_debug("%s: cluster %u\n", __func__, cluster);
+	if (cluster >= EXYNOS5420_NR_CLUSTERS)
+		return -EINVAL;
 
+	exynos_cluster_power_up(cluster);
 	return 0;
 }
 
-/*
- * NOTE: This function requires the stack data to be visible through power down
- * and can only be executed on processors like A15 and A7 that hit the cache
- * with the C bit clear in the SCTLR register.
- */
-static void exynos_power_down(void)
+static void exynos_cpu_powerdown_prepare(unsigned int cpu, unsigned int cluster)
 {
-	unsigned int mpidr, cpu, cluster;
-	bool last_man = false, skip_wfi = false;
-	unsigned int cpunr;
-
-	mpidr = read_cpuid_mpidr();
-	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
-	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
-	cpunr =  cpu + (cluster * EXYNOS5420_CPUS_PER_CLUSTER);
+	unsigned int cpunr = cpu + (cluster * EXYNOS5420_CPUS_PER_CLUSTER);
 
 	pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
 	BUG_ON(cpu >= EXYNOS5420_CPUS_PER_CLUSTER ||
 			cluster >= EXYNOS5420_NR_CLUSTERS);
+	exynos_cpu_power_down(cpunr);
+}
 
-	__mcpm_cpu_going_down(cpu, cluster);
-
-	arch_spin_lock(&exynos_mcpm_lock);
-	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
-	cpu_use_count[cpu][cluster]--;
-	if (cpu_use_count[cpu][cluster] == 0) {
-		exynos_cpu_power_down(cpunr);
-
-		if (exynos_cluster_unused(cluster)) {
-			exynos_cluster_power_down(cluster);
-			last_man = true;
-		}
-	} else if (cpu_use_count[cpu][cluster] == 1) {
-		/*
-		 * A power_up request went ahead of us.
-		 * Even if we do not want to shut this CPU down,
-		 * the caller expects a certain state as if the WFI
-		 * was aborted.  So let's continue with cache cleaning.
-		 */
-		skip_wfi = true;
-	} else {
-		BUG();
-	}
-
-	if (last_man && __mcpm_outbound_enter_critical(cpu, cluster)) {
-		arch_spin_unlock(&exynos_mcpm_lock);
-
-		if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A15) {
-			/*
-			 * On the Cortex-A15 we need to disable
-			 * L2 prefetching before flushing the cache.
-			 */
-			asm volatile(
-			"mcr	p15, 1, %0, c15, c0, 3\n\t"
-			"isb\n\t"
-			"dsb"
-			: : "r" (0x400));
-		}
+static void exynos_cluster_powerdown_prepare(unsigned int cluster)
+{
+	pr_debug("%s: cluster %u\n", __func__, cluster);
+	BUG_ON(cluster >= EXYNOS5420_NR_CLUSTERS);
+	exynos_cluster_power_down(cluster);
+}
 
-		/* Flush all cache levels for this cluster. */
-		exynos_v7_exit_coherency_flush(all);
+static void exynos_cpu_cache_disable(void)
+{
+	/* Disable and flush the local CPU cache. */
+	exynos_v7_exit_coherency_flush(louis);
+}
 
+static void exynos_cluster_cache_disable(void)
+{
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A15) {
 		/*
-		 * Disable cluster-level coherency by masking
-		 * incoming snoops and DVM messages:
+		 * On the Cortex-A15 we need to disable
+		 * L2 prefetching before flushing the cache.
 		 */
-		cci_disable_port_by_cpu(mpidr);
-
-		__mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
-	} else {
-		arch_spin_unlock(&exynos_mcpm_lock);
-
-		/* Disable and flush the local CPU cache. */
-		exynos_v7_exit_coherency_flush(louis);
+		asm volatile(
+		"mcr	p15, 1, %0, c15, c0, 3\n\t"
+		"isb\n\t"
+		"dsb"
+		: : "r" (0x400));
 	}
 
-	__mcpm_cpu_down(cpu, cluster);
-
-	/* Now we are prepared for power-down, do it: */
-	if (!skip_wfi)
-		wfi();
+	/* Flush all cache levels for this cluster. */
+	exynos_v7_exit_coherency_flush(all);
 
-	/* Not dead at this point?  Let our caller cope. */
+	/*
+	 * Disable cluster-level coherency by masking
+	 * incoming snoops and DVM messages:
+	 */
+	cci_disable_port_by_cpu(read_cpuid_mpidr());
 }
 
 static int exynos_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
@@ -222,10 +142,8 @@  static int exynos_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
 
 	/* Wait for the core state to be OFF */
 	while (tries--) {
-		if (ACCESS_ONCE(cpu_use_count[cpu][cluster]) == 0) {
-			if ((exynos_cpu_power_state(cpunr) == 0))
-				return 0; /* success: the CPU is halted */
-		}
+		if ((exynos_cpu_power_state(cpunr) == 0))
+			return 0; /* success: the CPU is halted */
 
 		/* Otherwise, wait and retry: */
 		msleep(1);
@@ -234,63 +152,23 @@  static int exynos_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
 	return -ETIMEDOUT; /* timeout */
 }
 
-static void exynos_powered_up(void)
-{
-	unsigned int mpidr, cpu, cluster;
-
-	mpidr = read_cpuid_mpidr();
-	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
-	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
-
-	arch_spin_lock(&exynos_mcpm_lock);
-	if (cpu_use_count[cpu][cluster] == 0)
-		cpu_use_count[cpu][cluster] = 1;
-	arch_spin_unlock(&exynos_mcpm_lock);
-}
-
-static void exynos_suspend(u64 residency)
+static void exynos_cpu_is_up(unsigned int cpu, unsigned int cluster)
 {
-	unsigned int mpidr, cpunr;
-
-	exynos_power_down();
-
-	/*
-	 * Execution reaches here only if cpu did not power down.
-	 * Hence roll back the changes done in exynos_power_down function.
-	 *
-	 * CAUTION: "This function requires the stack data to be visible through
-	 * power down and can only be executed on processors like A15 and A7
-	 * that hit the cache with the C bit clear in the SCTLR register."
-	*/
-	mpidr = read_cpuid_mpidr();
-	cpunr = exynos_pmu_cpunr(mpidr);
-
-	exynos_cpu_power_up(cpunr);
+	/* especially when resuming: make sure power control is set */
+	exynos_cpu_powerup(cpu, cluster);
 }
 
 static const struct mcpm_platform_ops exynos_power_ops = {
-	.power_up		= exynos_power_up,
-	.power_down		= exynos_power_down,
+	.cpu_powerup		= exynos_cpu_powerup,
+	.cluster_powerup	= exynos_cluster_powerup,
+	.cpu_powerdown_prepare	= exynos_cpu_powerdown_prepare,
+	.cluster_powerdown_prepare = exynos_cluster_powerdown_prepare,
+	.cpu_cache_disable	= exynos_cpu_cache_disable,
+	.cluster_cache_disable	= exynos_cluster_cache_disable,
 	.wait_for_powerdown	= exynos_wait_for_powerdown,
-	.suspend		= exynos_suspend,
-	.powered_up		= exynos_powered_up,
+	.cpu_is_up		= exynos_cpu_is_up,
 };
 
-static void __init exynos_mcpm_usage_count_init(void)
-{
-	unsigned int mpidr, cpu, cluster;
-
-	mpidr = read_cpuid_mpidr();
-	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
-	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
-
-	pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
-	BUG_ON(cpu >= EXYNOS5420_CPUS_PER_CLUSTER  ||
-			cluster >= EXYNOS5420_NR_CLUSTERS);
-
-	cpu_use_count[cpu][cluster] = 1;
-}
-
 /*
  * Enable cluster-level coherency, in preparation for turning on the MMU.
  */
@@ -302,19 +180,6 @@  static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
 	"b	cci_enable_port_for_self");
 }
 
-static void __init exynos_cache_off(void)
-{
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A15) {
-		/* disable L2 prefetching on the Cortex-A15 */
-		asm volatile(
-		"mcr	p15, 1, %0, c15, c0, 3\n\t"
-		"isb\n\t"
-		"dsb"
-		: : "r" (0x400));
-	}
-	exynos_v7_exit_coherency_flush(all);
-}
-
 static const struct of_device_id exynos_dt_mcpm_match[] = {
 	{ .compatible = "samsung,exynos5420" },
 	{ .compatible = "samsung,exynos5800" },
@@ -370,13 +235,11 @@  static int __init exynos_mcpm_init(void)
 	 */
 	pmu_raw_writel(EXYNOS5420_SWRESET_KFC_SEL, S5P_PMU_SPARE3);
 
-	exynos_mcpm_usage_count_init();
-
 	ret = mcpm_platform_register(&exynos_power_ops);
 	if (!ret)
 		ret = mcpm_sync_init(exynos_pm_power_up_setup);
 	if (!ret)
-		ret = mcpm_loopback(exynos_cache_off); /* turn on the CCI */
+		ret = mcpm_loopback(exynos_cluster_cache_disable); /* turn on the CCI */
 	if (ret) {
 		iounmap(ns_sram_base_addr);
 		return ret;