[PATCH 0/2] cpufreq: qcom-cpufreq-hw: Fixes to DCVS interrupt handling on EPSS

[PATCH 0/2] cpufreq: qcom-cpufreq-hw: Fixes to DCVS interrupt handling on EPSS

[Vladimir Zapolskiy]

The series contains of two critical fixes for QCOM EPSS cpufreq-hw
driver, the fixes correct runtime issues discovered on newer supported
QCOM platforms such as SM8250 and SM8450.

The changes are based on next-20220328 tag.

Vladimir Zapolskiy (2):
  cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts
  cpufreq: qcom-cpufreq-hw: Fix throttle frequency value on EPSS platforms

 drivers/cpufreq/qcom-cpufreq-hw.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

-- 
2.33.0

[PATCH 1/2] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Vladimir Zapolskiy]

It's noted that dcvs interrupts are not self-clearing, thus an interrupt
handler runs constantly, which leads to a severe regression in runtime.
To fix the problem an explicit write to clear interrupt register is
required.

Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index f9d593ff4718..53954e5086e0 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -24,6 +24,8 @@
 #define CLK_HW_DIV			2
 #define LUT_TURBO_IND			1
 
+#define GT_IRQ_STATUS			BIT(2)
+
 #define HZ_PER_KHZ			1000
 
 struct qcom_cpufreq_soc_data {
@@ -31,6 +33,7 @@ struct qcom_cpufreq_soc_data {
 	u32 reg_dcvs_ctrl;
 	u32 reg_freq_lut;
 	u32 reg_volt_lut;
+	u32 reg_intr_clr;
 	u32 reg_current_vote;
 	u32 reg_perf_state;
 	u8 lut_row_size;
@@ -350,6 +353,9 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
 	disable_irq_nosync(c_data->throttle_irq);
 	schedule_delayed_work(&c_data->throttle_work, 0);
 
+	writel_relaxed(GT_IRQ_STATUS,
+		       c_data->base + c_data->soc_data->reg_intr_clr);
+
 	return IRQ_HANDLED;
 }
 
@@ -368,6 +374,7 @@ static const struct qcom_cpufreq_soc_data epss_soc_data = {
 	.reg_dcvs_ctrl = 0xb0,
 	.reg_freq_lut = 0x100,
 	.reg_volt_lut = 0x200,
+	.reg_intr_clr = 0x308,
 	.reg_perf_state = 0x320,
 	.lut_row_size = 4,
 };
-- 
2.33.0

[PATCH 2/2] cpufreq: qcom-cpufreq-hw: Fix throttle frequency value on EPSS platforms

[Vladimir Zapolskiy]

On QCOM platforms with EPSS flavour of cpufreq IP a throttled frequency is
obtained from another register REG_DOMAIN_STATE, thus the helper function
qcom_lmh_get_throttle_freq() should be modified accordingly, as for now
it returns gibberish since .reg_current_vote is unset for EPSS hardware.

Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 53954e5086e0..3156d79ef39e 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -30,6 +30,7 @@
 
 struct qcom_cpufreq_soc_data {
 	u32 reg_enable;
+	u32 reg_domain_state;
 	u32 reg_dcvs_ctrl;
 	u32 reg_freq_lut;
 	u32 reg_volt_lut;
@@ -283,11 +284,16 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
 	}
 }
 
-static unsigned int qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)
+static unsigned long qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)
 {
-	unsigned int val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
+	unsigned int lval;
 
-	return (val & 0x3FF) * 19200;
+	if (data->soc_data->reg_current_vote)
+		lval = readl_relaxed(data->base + data->soc_data->reg_current_vote) & 0x3ff;
+	else
+		lval = readl_relaxed(data->base + data->soc_data->reg_domain_state) & 0xff;
+
+	return lval * xo_rate;
 }
 
 static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
@@ -297,14 +303,12 @@ static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
 	struct device *dev = get_cpu_device(cpu);
 	unsigned long freq_hz, throttled_freq;
 	struct dev_pm_opp *opp;
-	unsigned int freq;
 
 	/*
 	 * Get the h/w throttled frequency, normalize it using the
 	 * registered opp table and use it to calculate thermal pressure.
 	 */
-	freq = qcom_lmh_get_throttle_freq(data);
-	freq_hz = freq * HZ_PER_KHZ;
+	freq_hz = qcom_lmh_get_throttle_freq(data);
 
 	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
 	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
@@ -371,6 +375,7 @@ static const struct qcom_cpufreq_soc_data qcom_soc_data = {
 
 static const struct qcom_cpufreq_soc_data epss_soc_data = {
 	.reg_enable = 0x0,
+	.reg_domain_state = 0x20,
 	.reg_dcvs_ctrl = 0xb0,
 	.reg_freq_lut = 0x100,
 	.reg_volt_lut = 0x200,
-- 
2.33.0

Re: [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Bjorn Andersson]

On Mon 28 Mar 04:28 PDT 2022, Vladimir Zapolskiy wrote:

> It's noted that dcvs interrupts are not self-clearing, thus an interrupt
> handler runs constantly, which leads to a severe regression in runtime.
> To fix the problem an explicit write to clear interrupt register is
> required.
> 
> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index f9d593ff4718..53954e5086e0 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -24,6 +24,8 @@
>  #define CLK_HW_DIV			2
>  #define LUT_TURBO_IND			1
>  
> +#define GT_IRQ_STATUS			BIT(2)
> +
>  #define HZ_PER_KHZ			1000
>  
>  struct qcom_cpufreq_soc_data {
> @@ -31,6 +33,7 @@ struct qcom_cpufreq_soc_data {
>  	u32 reg_dcvs_ctrl;
>  	u32 reg_freq_lut;
>  	u32 reg_volt_lut;
> +	u32 reg_intr_clr;
>  	u32 reg_current_vote;
>  	u32 reg_perf_state;
>  	u8 lut_row_size;
> @@ -350,6 +353,9 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
>  	disable_irq_nosync(c_data->throttle_irq);
>  	schedule_delayed_work(&c_data->throttle_work, 0);
>  

This should only be done if reg_intr_clr != 0 (as it is for OSM).

Other than that, I think looks good.

Regards,
Bjorn

> +	writel_relaxed(GT_IRQ_STATUS,
> +		       c_data->base + c_data->soc_data->reg_intr_clr);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -368,6 +374,7 @@ static const struct qcom_cpufreq_soc_data epss_soc_data = {
>  	.reg_dcvs_ctrl = 0xb0,
>  	.reg_freq_lut = 0x100,
>  	.reg_volt_lut = 0x200,
> +	.reg_intr_clr = 0x308,
>  	.reg_perf_state = 0x320,
>  	.lut_row_size = 4,
>  };
> -- 
> 2.33.0
> 

Re: [PATCH 2/2] cpufreq: qcom-cpufreq-hw: Fix throttle frequency value on EPSS platforms

[Bjorn Andersson]

On Mon 28 Mar 04:28 PDT 2022, Vladimir Zapolskiy wrote:

> On QCOM platforms with EPSS flavour of cpufreq IP a throttled frequency is
> obtained from another register REG_DOMAIN_STATE, thus the helper function
> qcom_lmh_get_throttle_freq() should be modified accordingly, as for now
> it returns gibberish since .reg_current_vote is unset for EPSS hardware.
> 

Perhaps add a paragraph here to mention that you're replacing
19200 * HZ_PER_KHZ with xo_rate in this patch as well?

> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 53954e5086e0..3156d79ef39e 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -30,6 +30,7 @@
>  
>  struct qcom_cpufreq_soc_data {
>  	u32 reg_enable;
> +	u32 reg_domain_state;
>  	u32 reg_dcvs_ctrl;
>  	u32 reg_freq_lut;
>  	u32 reg_volt_lut;
> @@ -283,11 +284,16 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
>  	}
>  }
>  
> -static unsigned int qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)
> +static unsigned long qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)
>  {
> -	unsigned int val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
> +	unsigned int lval;
>  
> -	return (val & 0x3FF) * 19200;
> +	if (data->soc_data->reg_current_vote)
> +		lval = readl_relaxed(data->base + data->soc_data->reg_current_vote) & 0x3ff;
> +	else
> +		lval = readl_relaxed(data->base + data->soc_data->reg_domain_state) & 0xff;
> +
> +	return lval * xo_rate;
>  }
>  
>  static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
> @@ -297,14 +303,12 @@ static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
>  	struct device *dev = get_cpu_device(cpu);
>  	unsigned long freq_hz, throttled_freq;
>  	struct dev_pm_opp *opp;
> -	unsigned int freq;
>  
>  	/*
>  	 * Get the h/w throttled frequency, normalize it using the
>  	 * registered opp table and use it to calculate thermal pressure.
>  	 */
> -	freq = qcom_lmh_get_throttle_freq(data);
> -	freq_hz = freq * HZ_PER_KHZ;
> +	freq_hz = qcom_lmh_get_throttle_freq(data);
>  
>  	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
>  	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
> @@ -371,6 +375,7 @@ static const struct qcom_cpufreq_soc_data qcom_soc_data = {
>  
>  static const struct qcom_cpufreq_soc_data epss_soc_data = {
>  	.reg_enable = 0x0,
> +	.reg_domain_state = 0x20,
>  	.reg_dcvs_ctrl = 0xb0,
>  	.reg_freq_lut = 0x100,
>  	.reg_volt_lut = 0x200,
> -- 
> 2.33.0
> 

Re: [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Vladimir Zapolskiy]

Hi Bjorn,

On 3/31/22 21:29, Bjorn Andersson wrote:
> On Mon 28 Mar 04:28 PDT 2022, Vladimir Zapolskiy wrote:
> 
>> It's noted that dcvs interrupts are not self-clearing, thus an interrupt
>> handler runs constantly, which leads to a severe regression in runtime.
>> To fix the problem an explicit write to clear interrupt register is
>> required.
>>
>> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   drivers/cpufreq/qcom-cpufreq-hw.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index f9d593ff4718..53954e5086e0 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -24,6 +24,8 @@
>>   #define CLK_HW_DIV			2
>>   #define LUT_TURBO_IND			1
>>   
>> +#define GT_IRQ_STATUS			BIT(2)
>> +
>>   #define HZ_PER_KHZ			1000
>>   
>>   struct qcom_cpufreq_soc_data {
>> @@ -31,6 +33,7 @@ struct qcom_cpufreq_soc_data {
>>   	u32 reg_dcvs_ctrl;
>>   	u32 reg_freq_lut;
>>   	u32 reg_volt_lut;
>> +	u32 reg_intr_clr;
>>   	u32 reg_current_vote;
>>   	u32 reg_perf_state;
>>   	u8 lut_row_size;
>> @@ -350,6 +353,9 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
>>   	disable_irq_nosync(c_data->throttle_irq);
>>   	schedule_delayed_work(&c_data->throttle_work, 0);
>>   
> 
> This should only be done if reg_intr_clr != 0 (as it is for OSM).

thank you for review, but I believe here the status shall be read out from
another register INTR_STATUS rather than INTR_CLR, the bitfield is the same.

> Other than that, I think looks good.
> 
> Regards,
> Bjorn
> 
>> +	writel_relaxed(GT_IRQ_STATUS,
>> +		       c_data->base + c_data->soc_data->reg_intr_clr);
>> +
>>   	return IRQ_HANDLED;
>>   }
>>   
>> @@ -368,6 +374,7 @@ static const struct qcom_cpufreq_soc_data epss_soc_data = {
>>   	.reg_dcvs_ctrl = 0xb0,
>>   	.reg_freq_lut = 0x100,
>>   	.reg_volt_lut = 0x200,
>> +	.reg_intr_clr = 0x308,
>>   	.reg_perf_state = 0x320,
>>   	.lut_row_size = 4,
>>   };
>> -- 
>> 2.33.0
>>

--
Best wishes,
Vladimir