[v4,0/6] Introduce LMh driver for Qualcomm SoCs

Limits Management Hardware(LMh) is a hardware infrastructure on some
Qualcomm SoCs that can enforce temperature and current limits as programmed
by software for certain IPs like CPU. On many newer SoCs LMh is configured
by firmware/TZ and no programming is needed from the kernel side. But on
certain SoCs like sdm845 the firmware does not do a complete programming of
the h/w block. On such SoCs kernel software has to explicitly set up the
temperature limits and turn on various monitoring and enforcing algorithms
on the hardware.

Introduce support for enabling and programming various limit settings and
monitoring capabilities of Limits Management Hardware(LMh) associated with
cpu clusters. Also introduce support in cpufreq hardware driver to monitor
the interrupt associated with cpu frequency throttling so that this
information can be conveyed to the schdeuler via thermal pressure
interface.

With this patch series following cpu performance improvement(30-70%) is
observed on sdm845. The reasoning here is that without LMh being programmed
properly from the kernel, the default settings were enabling thermal
mitigation for CPUs at too low a temperature (around 70-75 degree C).  This
in turn meant that many a time CPUs were never actually allowed to hit the
maximum possible/required frequencies.

UnixBench whets and dhry (./Run whets dhry)
System Benchmarks Index Score

                Without LMh Support             With LMh Support
1 copy test     1353.7                          1773.2

8 copy tests    4473.6                          7402.3

Sysbench cpu
sysbench cpu --threads=8 --time=60 --cpu-max-prime=100000 run

                Without LMh Support             With LMh Support
Events per
second                  355                             614

Avg Latency(ms)         21.84                           13.02

v3->v4:
	- Rebased to v5.14-rc2.

v2->v3:
        - Included patch adding dt binding documentation for LMh nodes.
        - Rebased to v5.13

Thara Gopinath (6):
  firmware: qcom_scm: Introduce SCM calls to access LMh
  thermal: qcom: Add support for LMh driver
  cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  arm64: dts: qcom: sdm45: Add support for LMh node
  arm64: dts: qcom: sdm845: Remove cpufreq cooling devices for CPU
    thermal zones
  dt-bindings: thermal: Add dt binding for QCOM LMh

 .../devicetree/bindings/thermal/qcom-lmh.yaml | 100 ++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 162 ++----------
 drivers/cpufreq/qcom-cpufreq-hw.c             | 142 +++++++++++
 drivers/firmware/qcom_scm.c                   |  58 +++++
 drivers/firmware/qcom_scm.h                   |   4 +
 drivers/thermal/qcom/Kconfig                  |  10 +
 drivers/thermal/qcom/Makefile                 |   1 +
 drivers/thermal/qcom/lmh.c                    | 232 ++++++++++++++++++
 include/linux/qcom_scm.h                      |  14 ++
 9 files changed, 587 insertions(+), 136 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
 create mode 100644 drivers/thermal/qcom/lmh.c

On 27-07-21, 11:25, Thara Gopinath wrote:
> +static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
> +{

> +	/* In the unlikely case cpufreq is de-registered do not enable polling or h/w interrupt */
> +
> +	spin_lock(&data->throttle_lock);
> +	if (data->cancel_throttle) {
> +		spin_unlock(&data->throttle_lock);
> +		return;
> +	}
> +	spin_unlock(&data->throttle_lock);
> +
> +	/*
> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
> +	 * polling and switch back to interrupt mechanism
> +	 */
> +
> +	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->cpus)))
> +		/* Clear the existing interrupts and enable it back */
> +		enable_irq(data->throttle_irq);
> +	else
> +		mod_delayed_work(system_highpri_wq, &data->throttle_work,
> +				 msecs_to_jiffies(10));
> +}

> +static void qcom_cpufreq_hw_lmh_exit(struct qcom_cpufreq_data *data)
> +{
> +	if (data->throttle_irq <= 0)
> +		return;
> +
> +	spin_lock(&data->throttle_lock);
> +	data->cancel_throttle = true;
> +	spin_unlock(&data->throttle_lock);
> +	cancel_delayed_work_sync(&data->throttle_work);
> +	free_irq(data->throttle_irq, data);
> +}

Lets see if we can still make it break :)

CPU0                                            CPU1

qcom_lmh_dcvs_notify()                          qcom_cpufreq_hw_lmh_exit()

spin_unlock()
                                                spin_lock(),
                                                cancel_throttle = true
                                                spin_unlock()

                                                cancel_delayed_work_sync()
mod_delayed_work()
                                                free_irq()
                                                kfree(data)
qcom_lmh_dcvs_poll()
Uses data.


Sorry, locking is fun :)

On 7/27/21 11:50 PM, Viresh Kumar wrote:
> On 27-07-21, 11:25, Thara Gopinath wrote:
>> +static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
>> +{
> 
>> +	/* In the unlikely case cpufreq is de-registered do not enable polling or h/w interrupt */
>> +
>> +	spin_lock(&data->throttle_lock);
>> +	if (data->cancel_throttle) {
>> +		spin_unlock(&data->throttle_lock);
>> +		return;
>> +	}
>> +	spin_unlock(&data->throttle_lock);
>> +
>> +	/*
>> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
>> +	 * polling and switch back to interrupt mechanism
>> +	 */
>> +
>> +	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->cpus)))
>> +		/* Clear the existing interrupts and enable it back */
>> +		enable_irq(data->throttle_irq);
>> +	else
>> +		mod_delayed_work(system_highpri_wq, &data->throttle_work,
>> +				 msecs_to_jiffies(10));
>> +}
> 
>> +static void qcom_cpufreq_hw_lmh_exit(struct qcom_cpufreq_data *data)
>> +{
>> +	if (data->throttle_irq <= 0)
>> +		return;
>> +
>> +	spin_lock(&data->throttle_lock);
>> +	data->cancel_throttle = true;
>> +	spin_unlock(&data->throttle_lock);
>> +	cancel_delayed_work_sync(&data->throttle_work);
>> +	free_irq(data->throttle_irq, data);
>> +}
> 
> Lets see if we can still make it break :)
> 
> CPU0                                            CPU1
> 
> qcom_lmh_dcvs_notify()                          qcom_cpufreq_hw_lmh_exit()
> 
> spin_unlock()
>                                                  spin_lock(),
>                                                  cancel_throttle = true
>                                                  spin_unlock()
> 
>                                                  cancel_delayed_work_sync()
> mod_delayed_work()
>                                                  free_irq()
>                                                  kfree(data)
> qcom_lmh_dcvs_poll()
> Uses data.
> 
> 
> Sorry, locking is fun :)

Ha! I was too lazy to write this down! So how about I make this a mutex 
and put mod_delayed_work() inside the lock. So it will be something like 
below

qcom_lmh_dcvs_notify()			qcom_cpufreq_hw_lmh_exit()

mutex_lock()				mutex_lock()
   if (data->cancel_throttle) {		cancel_throttle = true
	mutex_unlock()			mutex_unlock()
	return				cancel_delayed_work_sync()
   }					free_irq()
   enable_irq() / mod_delayed_work()
mutex_unlock()

I will let you break it!

>

On 28-07-21, 18:19, Thara Gopinath wrote:
> Ha! I was too lazy to write this down! So how about I make this a mutex and


mutex may not work as you come here from irq.

> put mod_delayed_work() inside the lock. So it will be something like below

> 

> qcom_lmh_dcvs_notify()			qcom_cpufreq_hw_lmh_exit()

> 

> mutex_lock()				mutex_lock()

>   if (data->cancel_throttle) {		cancel_throttle = true

> 	mutex_unlock()			mutex_unlock()

> 	return				cancel_delayed_work_sync()

>   }					free_irq()

>   enable_irq() / mod_delayed_work()

> mutex_unlock()

> 

> I will let you break it!


I can't any further :)

Consider merging below to this patch, it fixes sever other minor
issues I see in the code.

-- 
viresh

-------------------------8<-------------------------

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 3cc27d9e2ed1..4261299fa9e5 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -38,13 +38,17 @@ struct qcom_cpufreq_soc_data {
 struct qcom_cpufreq_data {
        void __iomem *base;
        struct resource *res;
-       struct delayed_work throttle_work;
        const struct qcom_cpufreq_soc_data *soc_data;
-       struct cpufreq_policy *policy;
-       /* Lock to synchronize between de-init sequence and re-starting LMh polling/interrupts */
+
+       /*
+        * Lock to synchronize between de-init sequence and re-starting LMh
+        * polling/interrupts.
+        */
        spinlock_t throttle_lock;
        int throttle_irq;
        bool cancel_throttle;
+       struct delayed_work throttle_work;
+       struct cpufreq_policy *policy;
 };

 static unsigned long cpu_hw_rate, xo_rate;
@@ -271,10 +275,11 @@ static unsigned int qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)

 static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
 {
+       unsigned long max_capacity, capacity, freq_hz, throttled_freq;
        struct cpufreq_policy *policy = data->policy;
+       int cpu = cpumask_first(policy->cpus);
+       struct device *dev = get_cpu_device(cpu);
        struct dev_pm_opp *opp;
-       struct device *dev;
-       unsigned long max_capacity, capacity, freq_hz, throttled_freq;
        unsigned int freq;

        /*
@@ -284,7 +289,6 @@ static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
        freq = qcom_lmh_get_throttle_freq(data);
        freq_hz = freq * HZ_PER_KHZ;

-       dev = get_cpu_device(cpumask_first(policy->cpus));
        opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
        if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
                opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
@@ -293,34 +297,37 @@ static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)

        /* Update thermal pressure */

-       max_capacity = arch_scale_cpu_capacity(cpumask_first(policy->cpus));
+       max_capacity = arch_scale_cpu_capacity(cpu);
        capacity = mult_frac(max_capacity, throttled_freq, policy->cpuinfo.max_freq);
+
        /* Don't pass boost capacity to scheduler */
        if (capacity > max_capacity)
                capacity = max_capacity;

        arch_set_thermal_pressure(policy->cpus, max_capacity - capacity);

-       /* In the unlikely case cpufreq is de-registered do not enable polling or h/w interrupt */
-
+       /*
+        * In the unlikely case, where the policy is going away, do not enable
+        * polling or h/w interrupt.
+        */
        spin_lock(&data->throttle_lock);
-       if (data->cancel_throttle) {
-               spin_unlock(&data->throttle_lock);
-               return;
-       }
-       spin_unlock(&data->throttle_lock);
+
+       if (data->cancel_throttle)
+               goto out;

        /*
-        * If h/w throttled frequency is higher than what cpufreq has requested for, stop
-        * polling and switch back to interrupt mechanism
+        * If h/w throttled frequency is higher than what cpufreq has requested
+        * for, then stop polling and switch back to interrupt mechanism.
         */

-       if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->cpus)))
-               /* Clear the existing interrupts and enable it back */
+       if (throttled_freq >= qcom_cpufreq_hw_get(cpu)) {
                enable_irq(data->throttle_irq);
-       else
+       } else {
                mod_delayed_work(system_highpri_wq, &data->throttle_work,
                                 msecs_to_jiffies(10));
+       }
+out:
+       spin_unlock(&data->throttle_lock);
 }

 static void qcom_lmh_dcvs_poll(struct work_struct *work)
@@ -328,7 +335,6 @@ static void qcom_lmh_dcvs_poll(struct work_struct *work)
        struct qcom_cpufreq_data *data;

        data = container_of(work, struct qcom_cpufreq_data, throttle_work.work);
-
        qcom_lmh_dcvs_notify(data);
 }

@@ -407,6 +413,7 @@ static void qcom_cpufreq_hw_lmh_exit(struct qcom_cpufreq_data *data)
        spin_lock(&data->throttle_lock);
        data->cancel_throttle = true;
        spin_unlock(&data->throttle_lock);
+
        cancel_delayed_work_sync(&data->throttle_work);
        free_irq(data->throttle_irq, data);
 }