| Message ID | 20220328112836.2464486-2-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cpufreq: qcom-cpufreq-hw: Fixes to DCVS interrupt handling on EPSS | expand |
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 >
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
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, };
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(+)