Messages in this thread | | | Date | Fri, 06 Aug 2021 12:12:42 +0530 | From | Sibi Sankar <> | Subject | Re: [PATCH 2/4] cpufreq: qcom: Re-arrange register offsets to support per core L3 DCVS |
| |
On 2021-08-05 23:55, Stephen Boyd wrote: > Quoting Sibi Sankar (2021-08-05 10:47:20) >> Stephen, >> >> Thanks for taking time to review >> the series. >> >> On 2021-08-05 00:31, Stephen Boyd wrote: >> > Quoting Sibi Sankar (2021-07-29 11:04:43) >> >> Qualcomm SoCs (starting with SM8350) support per core voting for L3 >> >> cache >> >> frequency. >> > >> > And the L3 cache frequency voting code can't be put into this cpufreq >> > driver? >> >> Yes, it could have gone either into >> the cpufreq driver or l3 interconnect >> provider driver. Taniya/Odelu preferred >> the latter, because of the need for other >> clients to vote for l3 frequencies in >> the future. > > What other clients are those?
https://lore.kernel.org/lkml/20190814152116.GB28465@jcrouse1-lnx.qualcomm.com/
GPU was supposed to be one of the other clients that would vote for l3.
> >> The other option to prevent >> register re-arrangement would involve >> using syscons from the cpufreq node, which >> really wasn't necessary since there >> wasn't any register overlap between the >> two drivers. > > Let's not do that. > >> >> > >> >> So, re-arrange the cpufreq register offsets to allow access for >> >> the L3 interconnect to implement per core control. Also prevent >> >> binding >> >> breakage caused by register offset shuffling by using the >> >> SM8250/SM8350 >> >> EPSS compatible. >> >> >> >> Fixes: 7dbd121a2c58 ("arm64: dts: qcom: sc7280: Add cpufreq hw node") >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> >> --- >> >> drivers/cpufreq/qcom-cpufreq-hw.c | 23 +++++++++++++++++++---- >> >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c >> >> b/drivers/cpufreq/qcom-cpufreq-hw.c >> >> index f86859bf76f1..74ef3b38343b 100644 >> >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c >> >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >> >> @@ -28,6 +28,7 @@ struct qcom_cpufreq_soc_data { >> >> u32 reg_volt_lut; >> >> u32 reg_perf_state; >> >> u8 lut_row_size; >> >> + bool skip_enable; >> >> }; >> >> >> >> struct qcom_cpufreq_data { >> >> @@ -257,19 +258,31 @@ static const struct qcom_cpufreq_soc_data >> >> qcom_soc_data = { >> >> .reg_volt_lut = 0x114, >> >> .reg_perf_state = 0x920, >> >> .lut_row_size = 32, >> >> + .skip_enable = false, >> >> }; >> >> >> >> static const struct qcom_cpufreq_soc_data epss_soc_data = { >> >> + .reg_freq_lut = 0x0, >> >> + .reg_volt_lut = 0x100, >> >> + .reg_perf_state = 0x220, >> >> + .lut_row_size = 4, >> >> + .skip_enable = true, >> >> +}; >> >> + >> >> +static const struct qcom_cpufreq_soc_data epss_sm8250_soc_data = { >> >> .reg_enable = 0x0, >> >> .reg_freq_lut = 0x100, >> >> .reg_volt_lut = 0x200, >> >> .reg_perf_state = 0x320, >> >> .lut_row_size = 4, >> >> + .skip_enable = false, >> >> }; >> >> >> >> static const struct of_device_id qcom_cpufreq_hw_match[] = { >> >> { .compatible = "qcom,cpufreq-hw", .data = &qcom_soc_data }, >> >> { .compatible = "qcom,cpufreq-epss", .data = &epss_soc_data }, >> >> + { .compatible = "qcom,sm8250-cpufreq-epss", .data = >> >> &epss_sm8250_soc_data }, >> >> + { .compatible = "qcom,sm8350-cpufreq-epss", .data = >> >> &epss_sm8250_soc_data }, >> >> {} >> >> }; >> >> MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match); >> >> @@ -334,10 +347,12 @@ static int qcom_cpufreq_hw_cpu_init(struct >> >> cpufreq_policy *policy) >> >> data->res = res; >> >> >> >> /* HW should be in enabled state to proceed */ >> > >> > It looks odd that we're no longer making sure that the clk domain is >> > enabled when we probe the driver. Why is that OK? >> >> On newer EPSS hw it's no longer >> required to perform the additional >> hw enable check. IIRC we don't do >> that on corresponding downstream >> kernels as well. > > It's fairly clear that we no longer perform the additional check. The > question is why that's OK.
Taniya probably would know more about the history behind the change. I'll dig up more info regarding ^^ and update the thread.
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
| |