| Message ID | 20210727152512.1098329-1-thara.gopinath@linaro.org |
|---|---|
| Headers | show |
| Series | Introduce LMh driver for Qualcomm SoCs | expand |
On 7/27/21 10:25 AM, Thara Gopinath wrote: > 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 > Tested-by: Steev Klimaszewski <steev@kali.org>
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 7/29/21 2:17 AM, Viresh Kumar wrote: > 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. Hi! So the interrupt handler is a threaded handler. I moved it in v4 since one of the "_opp" api has an underlying mutex and was causing issues. So using a mutex should be pretty safe in this case. > >> 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. IIUC, the main change you are suggesting below is to include enable_irq() / mod_delayed_work() under the spin_lock as well. Is that right ? In which case isn't a mutex better than spinlock? >
On 29-07-21, 07:13, Thara Gopinath wrote: > So the interrupt handler is a threaded handler. I moved it in v4 since one > of the "_opp" api has an underlying mutex and was causing issues. So using a > mutex should be pretty safe in this case. Ahh I see. > IIUC, the main change you are suggesting below is to include enable_irq() / > mod_delayed_work() under the spin_lock as well. Is that right ? In which > case isn't a mutex better than spinlock? Yeah, sure.
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