[v11,04/12] ARM: hisi: enable MCPM implementation

Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
framework to manage power on HiP04 SoC.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/mach-hisi/Makefile   |   1 +
 arch/arm/mach-hisi/platmcpm.c | 351 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 352 insertions(+)
 create mode 100644 arch/arm/mach-hisi/platmcpm.c

On Mon, 14 Jul 2014, Haojian Zhuang wrote:

> +static bool hip04_init_cluster0_snoop(void)
> +{
> +	unsigned long data;
> +
> +	if (!fabric) {
> +		pr_err("failed to find fabric base\n");
> +		return false;
> +	}
> +	data = readl_relaxed(fabric + FAB_SF_MODE);
> +	data |= 1;
> +	writel_relaxed(data, fabric + FAB_SF_MODE);
> +	while (1) {
> +		if (data == readl_relaxed(fabric + FAB_SF_MODE))
> +			break;
> +	}
> +	return true;
> +}

In fact you probably should make this into something like:

static bool hip04_set_cluster_snoop(int cluster, bool active)

Eventually you'll want to turn snoops off when a cluster is completely 
idle.  And this can be used to init snoops on cluster 0 during boot as 
well.

> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +	unsigned long data, mask;
> +
> +	if (!relocation || !sysctrl)
> +		return -ENODEV;
> +	if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> +		return -EINVAL;
> +
> +	spin_lock_irq(&boot_lock);
> +
> +	if (hip04_cpu_table[cluster][cpu]) {
> +		hip04_cpu_table[cluster][cpu]++;
> +		spin_unlock_irq(&boot_lock);
> +		return 0;
> +	}
> +
> +	writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
> +	writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
> +	writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
> +	writel_relaxed(0, relocation + 12);
> +
> +	if (hip04_cluster_down(cluster)) {
> +		data = CLUSTER_DEBUG_RESET_BIT;
> +		writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> +		do {
> +			mask = CLUSTER_DEBUG_RESET_STATUS;
> +			data = readl_relaxed(sysctrl + \
> +					     SC_CPU_RESET_STATUS(cluster));
> +		} while (data & mask);
> +	}
> +
> +	hip04_cpu_table[cluster][cpu]++;
> +
> +	data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> +	       CORE_DEBUG_RESET_BIT(cpu);
> +	writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> +	spin_unlock_irq(&boot_lock);
> +	msleep(POLL_MSEC);

Now that the booting CPU takes cares of snoops for itself, is this delay 
still required?

> +	return 0;
> +}
> +
> +static void hip04_mcpm_power_down(void)
> +{
> +	unsigned int mpidr, cpu, cluster, data = 0;
> +	bool skip_reset = false;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	__mcpm_cpu_going_down(cpu, cluster);
> +
> +	spin_lock(&boot_lock);
> +	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> +	hip04_cpu_table[cluster][cpu]--;
> +	if (hip04_cpu_table[cluster][cpu] == 1) {
> +		/* A power_up request went ahead of us. */
> +		skip_reset = true;
> +	} else if (hip04_cpu_table[cluster][cpu] > 1) {
> +		pr_err("Cluster %d CPU%d boots multiple times\n", cluster, cpu);
> +		BUG();
> +	}
> +
> +	v7_exit_coherency_flush(louis);
> +
> +	__mcpm_cpu_down(cpu, cluster);
> +	spin_unlock(&boot_lock);

You can't touch spinlocks after v7_exit_coherency_flush().  That has to 
come before it.

> +	if (!skip_reset) {
> +		data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> +		       CORE_DEBUG_RESET_BIT(cpu);
> +		writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
> +	}

And the race is still there.  If right before skip_reset is tested the 
code in power_up() is executed, the CPU will then be shut down here 
although it is expected to remain alive.

You are certain no code is ever executed after SC_CPU_RESET_REQ is 
written to? You don't need to execute wfi() for the reset to be 
effective?  If so this write will have to be performed by another CPU 
via a work queue or something unless there is simply no other CPU 
running in the system.


Nicolas

On 14 July 2014 18:12, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Mon, 14 Jul 2014, Haojian Zhuang wrote:
>
>> +static bool hip04_init_cluster0_snoop(void)
>> +{
>> +     unsigned long data;
>> +
>> +     if (!fabric) {
>> +             pr_err("failed to find fabric base\n");
>> +             return false;
>> +     }
>> +     data = readl_relaxed(fabric + FAB_SF_MODE);
>> +     data |= 1;
>> +     writel_relaxed(data, fabric + FAB_SF_MODE);
>> +     while (1) {
>> +             if (data == readl_relaxed(fabric + FAB_SF_MODE))
>> +                     break;
>> +     }
>> +     return true;
>> +}
>
> In fact you probably should make this into something like:
>
> static bool hip04_set_cluster_snoop(int cluster, bool active)
>
> Eventually you'll want to turn snoops off when a cluster is completely
> idle.  And this can be used to init snoops on cluster 0 during boot as
> well.
>
I don't plan to implement power down by resetting cores since the
operation of disabling cluster snoop is not good. It'll make core
hang.

Now I plan to use wfi instead. Only the first time, we need to make cores
out of reset. For the next times, we only need to use IPI to wakeup
secondary cores instead.

>> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
>> +{
>> +     unsigned long data, mask;
>> +
>> +     if (!relocation || !sysctrl)
>> +             return -ENODEV;
>> +     if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
>> +             return -EINVAL;
>> +
>> +     spin_lock_irq(&boot_lock);
>> +
>> +     if (hip04_cpu_table[cluster][cpu]) {
>> +             hip04_cpu_table[cluster][cpu]++;
>> +             spin_unlock_irq(&boot_lock);
>> +             return 0;
>> +     }
>> +
>> +     writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
>> +     writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
>> +     writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>> +     writel_relaxed(0, relocation + 12);
>> +
>> +     if (hip04_cluster_down(cluster)) {
>> +             data = CLUSTER_DEBUG_RESET_BIT;
>> +             writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> +             do {
>> +                     mask = CLUSTER_DEBUG_RESET_STATUS;
>> +                     data = readl_relaxed(sysctrl + \
>> +                                          SC_CPU_RESET_STATUS(cluster));
>> +             } while (data & mask);
>> +     }
>> +
>> +     hip04_cpu_table[cluster][cpu]++;
>> +
>> +     data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> +            CORE_DEBUG_RESET_BIT(cpu);
>> +     writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> +     spin_unlock_irq(&boot_lock);
>> +     msleep(POLL_MSEC);
>
> Now that the booting CPU takes cares of snoops for itself, is this delay
> still required?
>

Oh, this msleep() is removed in the next patch "enable HiP04". I
should move it here instead.

>> +     return 0;
>> +}
>> +
>> +static void hip04_mcpm_power_down(void)
>> +{
>> +     unsigned int mpidr, cpu, cluster, data = 0;
>> +     bool skip_reset = false;
>> +
>> +     mpidr = read_cpuid_mpidr();
>> +     cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> +     cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> +     __mcpm_cpu_going_down(cpu, cluster);
>> +
>> +     spin_lock(&boot_lock);
>> +     BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
>> +     hip04_cpu_table[cluster][cpu]--;
>> +     if (hip04_cpu_table[cluster][cpu] == 1) {
>> +             /* A power_up request went ahead of us. */
>> +             skip_reset = true;
>> +     } else if (hip04_cpu_table[cluster][cpu] > 1) {
>> +             pr_err("Cluster %d CPU%d boots multiple times\n", cluster, cpu);
>> +             BUG();
>> +     }
>> +
>> +     v7_exit_coherency_flush(louis);
>> +
>> +     __mcpm_cpu_down(cpu, cluster);
>> +     spin_unlock(&boot_lock);
>
> You can't touch spinlocks after v7_exit_coherency_flush().  That has to
> come before it.
>
>> +     if (!skip_reset) {
>> +             data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> +                    CORE_DEBUG_RESET_BIT(cpu);
>> +             writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
>> +     }
>
> And the race is still there.  If right before skip_reset is tested the
> code in power_up() is executed, the CPU will then be shut down here
> although it is expected to remain alive.
>
> You are certain no code is ever executed after SC_CPU_RESET_REQ is
> written to? You don't need to execute wfi() for the reset to be
> effective?  If so this write will have to be performed by another CPU
> via a work queue or something unless there is simply no other CPU
> running in the system.
>
I think that wfi is better now. I'll use wfi instead. And we don't
worry about the reset mode any more.

Best Regards
Haojian

On Tue, 15 Jul 2014, Haojian Zhuang wrote:

> On 14 July 2014 18:12, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Mon, 14 Jul 2014, Haojian Zhuang wrote:
> >
> >> +static bool hip04_init_cluster0_snoop(void)
> >> +{
> >> +     unsigned long data;
> >> +
> >> +     if (!fabric) {
> >> +             pr_err("failed to find fabric base\n");
> >> +             return false;
> >> +     }
> >> +     data = readl_relaxed(fabric + FAB_SF_MODE);
> >> +     data |= 1;
> >> +     writel_relaxed(data, fabric + FAB_SF_MODE);
> >> +     while (1) {
> >> +             if (data == readl_relaxed(fabric + FAB_SF_MODE))
> >> +                     break;
> >> +     }
> >> +     return true;
> >> +}
> >
> > In fact you probably should make this into something like:
> >
> > static bool hip04_set_cluster_snoop(int cluster, bool active)
> >
> > Eventually you'll want to turn snoops off when a cluster is completely
> > idle.  And this can be used to init snoops on cluster 0 during boot as
> > well.
> >
> I don't plan to implement power down by resetting cores since the
> operation of disabling cluster snoop is not good. It'll make core
> hang.
> 
> Now I plan to use wfi instead. Only the first time, we need to make cores
> out of reset. For the next times, we only need to use IPI to wakeup
> secondary cores instead.

Well...

If the following conditions are true:

1) snoops can be turned on even for a CPU that is not running, and

2) you do not plan to ever disable snoops after boot, and

3) you do not plan to actually power off CPUs when hotplugged out, and
   only use WFI when they are idle

... then MCPM is providing you absolutely no gain at all.  You might as 
well create your own smp_operations structure directly and skip all the 
extra MCPM complexity you are not benefiting from.

The fundamental reason why MCPM exists is to safely manage cluster wide 
resources in the presence of concurrent powering up and down of CPUs 
when standard race avoidance mechanisms can't be used.  You apparently 
don't have such resources to manage. That certainly will make for 
suboptimal power consumption but there is so far nothing more sensible 
that can be done unless more documentation for this hardware is made 
available.


Nicolas

On 15 July 2014 19:44, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Tue, 15 Jul 2014, Haojian Zhuang wrote:
>
>> On 14 July 2014 18:12, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>> > On Mon, 14 Jul 2014, Haojian Zhuang wrote:
>> >
>> >> +static bool hip04_init_cluster0_snoop(void)
>> >> +{
>> >> +     unsigned long data;
>> >> +
>> >> +     if (!fabric) {
>> >> +             pr_err("failed to find fabric base\n");
>> >> +             return false;
>> >> +     }
>> >> +     data = readl_relaxed(fabric + FAB_SF_MODE);
>> >> +     data |= 1;
>> >> +     writel_relaxed(data, fabric + FAB_SF_MODE);
>> >> +     while (1) {
>> >> +             if (data == readl_relaxed(fabric + FAB_SF_MODE))
>> >> +                     break;
>> >> +     }
>> >> +     return true;
>> >> +}
>> >
>> > In fact you probably should make this into something like:
>> >
>> > static bool hip04_set_cluster_snoop(int cluster, bool active)
>> >
>> > Eventually you'll want to turn snoops off when a cluster is completely
>> > idle.  And this can be used to init snoops on cluster 0 during boot as
>> > well.
>> >
>> I don't plan to implement power down by resetting cores since the
>> operation of disabling cluster snoop is not good. It'll make core
>> hang.
>>
>> Now I plan to use wfi instead. Only the first time, we need to make cores
>> out of reset. For the next times, we only need to use IPI to wakeup
>> secondary cores instead.
>
> Well...
>
> If the following conditions are true:
>
> 1) snoops can be turned on even for a CPU that is not running, and
>
> 2) you do not plan to ever disable snoops after boot, and
>
> 3) you do not plan to actually power off CPUs when hotplugged out, and
>    only use WFI when they are idle
>
> ... then MCPM is providing you absolutely no gain at all.  You might as
> well create your own smp_operations structure directly and skip all the
> extra MCPM complexity you are not benefiting from.
>
> The fundamental reason why MCPM exists is to safely manage cluster wide
> resources in the presence of concurrent powering up and down of CPUs
> when standard race avoidance mechanisms can't be used.  You apparently
> don't have such resources to manage. That certainly will make for
> suboptimal power consumption but there is so far nothing more sensible
> that can be done unless more documentation for this hardware is made
> available.
>
>
> Nicolas

You're right.

Now I tried some code. It seems that power down could work. Let me do
more tests on it.

Best Regards
Haojian