Message ID | 1539257761-23023-3-git-send-email-tdas@codeaurora.org (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver | expand |
Quoting Taniya Das (2018-10-11 04:36:01) > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO > > If in doubt, say N. > > +config ARM_QCOM_CPUFREQ_HW > + bool "QCOM CPUFreq HW driver" Is there any reason this can't be a module? > + depends on ARCH_QCOM || COMPILE_TEST > + help > + Support for the CPUFreq HW driver. > + Some QCOM chipsets have a HW engine to offload the steps > + necessary for changing the frequency of the CPUs. Firmware loaded > + in this engine exposes a programming interface to the OS. > + The driver implements the cpufreq interface for this HW engine. > + Say Y if you want to support CPUFreq HW. > + > config ARM_S3C_CPUFREQ > bool > help > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > new file mode 100644 > index 0000000..fe1c264 > --- /dev/null > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -0,0 +1,354 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/cpufreq.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/of_address.h> > +#include <linux/of_platform.h> > + > +#define LUT_MAX_ENTRIES 40U > +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16) > +#define LUT_ROW_SIZE 32 > +#define CLK_HW_DIV 2 > + > +enum { > + REG_ENABLE, > + REG_LUT_TABLE, > + REG_PERF_STATE, > + > + REG_ARRAY_SIZE, > +}; > + > +struct cpufreq_qcom { > + struct cpufreq_frequency_table *table; > + void __iomem *reg_bases[REG_ARRAY_SIZE]; > + cpumask_t related_cpus; > + unsigned int max_cores; > + unsigned long xo_rate; > + unsigned long cpu_hw_rate; > +}; > + > +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { Is this going to change in the future? > + [REG_ENABLE] = 0x0, > + [REG_LUT_TABLE] = 0x110, > + [REG_PERF_STATE] = 0x920, > +}; > + > +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; > + > +static int > +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, > + unsigned int index) > +{ > + struct cpufreq_qcom *c = policy->driver_data; > + > + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); Why can't we avoid the indirection here and store the perf_state pointer in probe? Then we don't have to indirect through a table to perform the register write. > + > + return 0; > +} > + [..] > +static int qcom_resources_init(struct platform_device *pdev) > +{ > + struct device_node *cpu_np; > + struct of_phandle_args args; > + struct clk *clk; > + unsigned int cpu; > + unsigned long xo_rate, cpu_hw_rate; > + int ret; > + > + clk = clk_get(&pdev->dev, "xo"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + xo_rate = clk_get_rate(clk); > + > + clk_put(clk); > + > + clk = clk_get(&pdev->dev, "cpu_clk"); Sad that the name is cpu_clk, instead of something like 'backup' or whatever the name really is in hardware.
Hello Stephen, On 10/18/2018 5:02 AM, Stephen Boyd wrote: > Quoting Taniya Das (2018-10-11 04:36:01) >> --- a/drivers/cpufreq/Kconfig.arm >> +++ b/drivers/cpufreq/Kconfig.arm >> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO >> >> If in doubt, say N. >> >> +config ARM_QCOM_CPUFREQ_HW >> + bool "QCOM CPUFreq HW driver" > > Is there any reason this can't be a module? > We do not have any use cases where we need to support it as module. >> + depends on ARCH_QCOM || COMPILE_TEST >> + help >> + Support for the CPUFreq HW driver. >> + Some QCOM chipsets have a HW engine to offload the steps >> + necessary for changing the frequency of the CPUs. Firmware loaded >> + in this engine exposes a programming interface to the OS. >> + The driver implements the cpufreq interface for this HW engine. >> + Say Y if you want to support CPUFreq HW. >> + >> config ARM_S3C_CPUFREQ >> bool >> help >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c >> new file mode 100644 >> index 0000000..fe1c264 >> --- /dev/null >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >> @@ -0,0 +1,354 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include <linux/cpufreq.h> >> +#include <linux/init.h> >> +#include <linux/kernel.h> >> +#include <linux/of_address.h> >> +#include <linux/of_platform.h> >> + >> +#define LUT_MAX_ENTRIES 40U >> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16) >> +#define LUT_ROW_SIZE 32 >> +#define CLK_HW_DIV 2 >> + >> +enum { >> + REG_ENABLE, >> + REG_LUT_TABLE, >> + REG_PERF_STATE, >> + >> + REG_ARRAY_SIZE, >> +}; >> + >> +struct cpufreq_qcom { >> + struct cpufreq_frequency_table *table; >> + void __iomem *reg_bases[REG_ARRAY_SIZE]; >> + cpumask_t related_cpus; >> + unsigned int max_cores; >> + unsigned long xo_rate; >> + unsigned long cpu_hw_rate; >> +}; >> + >> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { > > Is this going to change in the future? > Yes, they could change and that was the reason to introduce the offsets. This was discussed earlier too with Sudeep and was to add them. >> + [REG_ENABLE] = 0x0, >> + [REG_LUT_TABLE] = 0x110, >> + [REG_PERF_STATE] = 0x920, >> +}; >> + >> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; >> + >> +static int >> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, >> + unsigned int index) >> +{ >> + struct cpufreq_qcom *c = policy->driver_data; >> + >> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); > > Why can't we avoid the indirection here and store the perf_state pointer > in probe? Then we don't have to indirect through a table to perform the > register write. > As the offsets could change and that was the reason to add this. >> + >> + return 0; >> +} >> + > [..] >> +static int qcom_resources_init(struct platform_device *pdev) >> +{ >> + struct device_node *cpu_np; >> + struct of_phandle_args args; >> + struct clk *clk; >> + unsigned int cpu; >> + unsigned long xo_rate, cpu_hw_rate; >> + int ret; >> + >> + clk = clk_get(&pdev->dev, "xo"); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + >> + xo_rate = clk_get_rate(clk); >> + >> + clk_put(clk); >> + >> + clk = clk_get(&pdev->dev, "cpu_clk"); > > Sad that the name is cpu_clk, instead of something like 'backup' or > whatever the name really is in hardware. > Sure, would update it.
Quoting Taniya Das (2018-11-02 20:06:00) > Hello Stephen, > > On 10/18/2018 5:02 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2018-10-11 04:36:01) > >> --- a/drivers/cpufreq/Kconfig.arm > >> +++ b/drivers/cpufreq/Kconfig.arm > >> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO > >> > >> If in doubt, say N. > >> > >> +config ARM_QCOM_CPUFREQ_HW > >> + bool "QCOM CPUFreq HW driver" > > > > Is there any reason this can't be a module? > > > > We do not have any use cases where we need to support it as module. Ok, so it could easily be tristate then? Why not allow it? > > >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > >> new file mode 100644 > >> index 0000000..fe1c264 > >> --- /dev/null > >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > >> @@ -0,0 +1,354 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > >> + */ [...] > >> + > >> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { > > > > Is this going to change in the future? > > > > Yes, they could change and that was the reason to introduce the offsets. > This was discussed earlier too with Sudeep and was to add them. > > >> + [REG_ENABLE] = 0x0, This is only used once? Maybe it could be removed. > >> + [REG_LUT_TABLE] = 0x110, And this is only used during probe to figure out the supported frequencies. So we definitely don't need to store around the registers after probe in an array of iomem pointers. The only one that we need after probe is the one below. > >> + [REG_PERF_STATE] = 0x920, > >> +}; > >> + > >> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; > >> + > >> +static int > >> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, > >> + unsigned int index) > >> +{ > >> + struct cpufreq_qcom *c = policy->driver_data; > >> + > >> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); > > > > Why can't we avoid the indirection here and store the perf_state pointer > > in probe? Then we don't have to indirect through a table to perform the > > register write. > > > > As the offsets could change and that was the reason to add this. With fast switching we can avoid incurring any extra instructions, so please make another iomem pointer in the cpufreq_qcom struct just for writing the index or if possible, just pass the iomem pointer that points to the REG_PERF_STATE as the policy->driver_data variable here. Then we have the address in hand without any extra load. If my understanding is correct, we don't need to keep around anything besides this register address anyway so we should be able to just load it and write it immediately.
On Sat, Nov 03, 2018 at 08:36:00AM +0530, Taniya Das wrote: > Hello Stephen, > > On 10/18/2018 5:02 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2018-10-11 04:36:01) > > > --- a/drivers/cpufreq/Kconfig.arm > > > +++ b/drivers/cpufreq/Kconfig.arm [...] > > > +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { > > > > Is this going to change in the future? > > > > Yes, they could change and that was the reason to introduce the offsets. > This was discussed earlier too with Sudeep and was to add them. Sorry, I didn't like these registers to be coming from DT and I had the same question: will this keep changing ? And IIRC, the answer was yes. But I agree with Stephen, if and when we see the change, you can introduce the array and keep it simple until then. -- Regards, Sudeep
Hello Stephen, Thanks for your comments. On 11/4/2018 9:50 AM, Stephen Boyd wrote: > Quoting Taniya Das (2018-11-02 20:06:00) >> Hello Stephen, >> >> On 10/18/2018 5:02 AM, Stephen Boyd wrote: >>> Quoting Taniya Das (2018-10-11 04:36:01) >>>> --- a/drivers/cpufreq/Kconfig.arm >>>> +++ b/drivers/cpufreq/Kconfig.arm >>>> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO >>>> >>>> If in doubt, say N. >>>> >>>> +config ARM_QCOM_CPUFREQ_HW >>>> + bool "QCOM CPUFreq HW driver" >>> >>> Is there any reason this can't be a module? >>> >> >> We do not have any use cases where we need to support it as module. > > Ok, so it could easily be tristate then? Why not allow it? > I have checked other vendors CPUfreq drivers and those too support only "bool". >> >>>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c >>>> new file mode 100644 >>>> index 0000000..fe1c264 >>>> --- /dev/null >>>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >>>> @@ -0,0 +1,354 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >>>> + */ > [...] >>>> + >>>> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { >>> >>> Is this going to change in the future? >>> >> >> Yes, they could change and that was the reason to introduce the offsets. >> This was discussed earlier too with Sudeep and was to add them. >> >>>> + [REG_ENABLE] = 0x0, > > This is only used once? Maybe it could be removed. > >>>> + [REG_LUT_TABLE] = 0x110, > > And this is only used during probe to figure out the supported > frequencies. So we definitely don't need to store around the registers > after probe in an array of iomem pointers. The only one that we need > after probe is the one below. > >>>> + [REG_PERF_STATE] = 0x920, >>>> +}; >>>> + As these address offsets could change, so I am of the opinion to leave them as it is. >>>> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; >>>> + >>>> +static int >>>> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, >>>> + unsigned int index) >>>> +{ >>>> + struct cpufreq_qcom *c = policy->driver_data; >>>> + >>>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); >>> >>> Why can't we avoid the indirection here and store the perf_state pointer >>> in probe? Then we don't have to indirect through a table to perform the >>> register write. >>> >> >> As the offsets could change and that was the reason to add this. > > With fast switching we can avoid incurring any extra instructions, so > please make another iomem pointer in the cpufreq_qcom struct just for > writing the index or if possible, just pass the iomem pointer that > points to the REG_PERF_STATE as the policy->driver_data variable here. > Then we have the address in hand without any extra load. If my > understanding is correct, we don't need to keep around anything besides > this register address anyway so we should be able to just load it and > write it immediately. > The c->reg_bases[] is just an index to the updated bases addresses. I am not clear as to why it would incur an extra instruction. The below code would already take care of it. + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++) + c->reg_bases[i] = base + offsets[i]; +
On Sun, Nov 11, 2018 at 06:12:29PM +0530, Taniya Das wrote: > Hello Stephen, > > Thanks for your comments. > > On 11/4/2018 9:50 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2018-11-02 20:06:00) > > > Hello Stephen, > > > > > > On 10/18/2018 5:02 AM, Stephen Boyd wrote: > > > > Quoting Taniya Das (2018-10-11 04:36:01) > > > > > --- a/drivers/cpufreq/Kconfig.arm > > > > > +++ b/drivers/cpufreq/Kconfig.arm > > > > > @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO > > > > > > > > > > If in doubt, say N. > > > > > > > > > > +config ARM_QCOM_CPUFREQ_HW > > > > > + bool "QCOM CPUFreq HW driver" > > > > > > > > Is there any reason this can't be a module? > > > > > > > > > > We do not have any use cases where we need to support it as module. > > > > Ok, so it could easily be tristate then? Why not allow it? > > > > I have checked other vendors CPUfreq drivers and those too support only > "bool". That's not entirely correct. Most drivers in Kconfig are 'tristate' and about 50% of those in KConfig.arm are. I'd say make it 'tristate' unless there are good reasons not to do so. > > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > new file mode 100644 > > > > > index 0000000..fe1c264 > > > > > --- /dev/null > > > > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > @@ -0,0 +1,354 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +/* > > > > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > > > > + */ > > [...] > > > > > + > > > > > +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { > > > > > > > > Is this going to change in the future? > > > > > > > > > > Yes, they could change and that was the reason to introduce the offsets. > > > This was discussed earlier too with Sudeep and was to add them. > > > > > > > > + [REG_ENABLE] = 0x0, > > > > This is only used once? Maybe it could be removed. > > > > > > > + [REG_LUT_TABLE] = 0x110, > > > > And this is only used during probe to figure out the supported > > frequencies. So we definitely don't need to store around the registers > > after probe in an array of iomem pointers. The only one that we need > > after probe is the one below. > > > > > > > + [REG_PERF_STATE] = 0x920, > > > > > +}; > > > > > + > > As these address offsets could change, so I am of the opinion to leave them > as it is. As of now there is only one set of offsets. Let's just keep the code simple while this is the case and address different offsets when it is actually needed, as suggested by Stephen and Sudeep. > > > > > +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; > > > > > + > > > > > +static int > > > > > +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, > > > > > + unsigned int index) > > > > > +{ > > > > > + struct cpufreq_qcom *c = policy->driver_data; > > > > > + > > > > > + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); > > > > > > > > Why can't we avoid the indirection here and store the perf_state pointer > > > > in probe? Then we don't have to indirect through a table to perform the > > > > register write. > > > > > > > > > > As the offsets could change and that was the reason to add this. > > > > With fast switching we can avoid incurring any extra instructions, so > > please make another iomem pointer in the cpufreq_qcom struct just for > > writing the index or if possible, just pass the iomem pointer that > > points to the REG_PERF_STATE as the policy->driver_data variable here. > > Then we have the address in hand without any extra load. If my > > understanding is correct, we don't need to keep around anything besides > > this register address anyway so we should be able to just load it and > > write it immediately. > > > > The c->reg_bases[] is just an index to the updated bases addresses. I am not > clear as to why it would incur an extra instruction. > > The below code would already take care of it. > > + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++) > + c->reg_bases[i] = base + offsets[i]; > + From a performance point of view using a direct iomem pointer seems like a micro-optimization that probably doesn't have a measurable impact. However I think the code shouldn't be more complex than necessary, and at this point the indirection isn't needed. Cheers Matthias
Quoting Matthias Kaehlcke (2018-11-15 16:23:37) > On Sun, Nov 11, 2018 at 06:12:29PM +0530, Taniya Das wrote: > > On 11/4/2018 9:50 AM, Stephen Boyd wrote: > > > Quoting Taniya Das (2018-11-02 20:06:00) > > > > On 10/18/2018 5:02 AM, Stephen Boyd wrote: > > > > > Quoting Taniya Das (2018-10-11 04:36:01) > > > > > > --- a/drivers/cpufreq/Kconfig.arm > > > > > > +++ b/drivers/cpufreq/Kconfig.arm > > > > > > @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO > > > > > > > > > > > > If in doubt, say N. > > > > > > > > > > > > +config ARM_QCOM_CPUFREQ_HW > > > > > > + bool "QCOM CPUFreq HW driver" > > > > > > > > > > Is there any reason this can't be a module? > > > > > > > > > > > > > We do not have any use cases where we need to support it as module. > > > > > > Ok, so it could easily be tristate then? Why not allow it? > > > > > > > I have checked other vendors CPUfreq drivers and those too support only > > "bool". > > That's not entirely correct. Most drivers in Kconfig are 'tristate' > and about 50% of those in KConfig.arm are. I'd say make it 'tristate' > unless there are good reasons not to do so. Yes, please make tristate. > > > > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > > new file mode 100644 > > > > > > index 0000000..fe1c264 > > > > > > --- /dev/null > > > > > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > > @@ -0,0 +1,354 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > +/* > > > > > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > > > > > + */ > > > [...] > > > > > > + > > > > > > +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { > > > > > > > > > > Is this going to change in the future? > > > > > > > > > > > > > Yes, they could change and that was the reason to introduce the offsets. > > > > This was discussed earlier too with Sudeep and was to add them. > > > > > > > > > > + [REG_ENABLE] = 0x0, > > > > > > This is only used once? Maybe it could be removed. > > > > > > > > > + [REG_LUT_TABLE] = 0x110, > > > > > > And this is only used during probe to figure out the supported > > > frequencies. So we definitely don't need to store around the registers > > > after probe in an array of iomem pointers. The only one that we need > > > after probe is the one below. > > > > > > > > > + [REG_PERF_STATE] = 0x920, > > > > > > +}; > > > > > > + > > > > As these address offsets could change, so I am of the opinion to leave them > > as it is. > > As of now there is only one set of offsets. Let's just keep the code > simple while this is the case and address different offsets when it is > actually needed, as suggested by Stephen and Sudeep. Yes, please simplify by getting rid of this and not storing anything in the struct that's only used during probe. > > > > > > > With fast switching we can avoid incurring any extra instructions, so > > > please make another iomem pointer in the cpufreq_qcom struct just for > > > writing the index or if possible, just pass the iomem pointer that > > > points to the REG_PERF_STATE as the policy->driver_data variable here. > > > Then we have the address in hand without any extra load. If my > > > understanding is correct, we don't need to keep around anything besides > > > this register address anyway so we should be able to just load it and > > > write it immediately. > > > > > > > The c->reg_bases[] is just an index to the updated bases addresses. I am not > > clear as to why it would incur an extra instruction. > > > > The below code would already take care of it. > > > > + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++) > > + c->reg_bases[i] = base + offsets[i]; > > + > > From a performance point of view using a direct iomem pointer > seems like a micro-optimization that probably doesn't have a > measurable impact. However I think the code shouldn't be more complex > than necessary, and at this point the indirection isn't needed. > Yes it's a micro-optimization for sure, in the task switching path so it may actually be useful. Either way, I think we can greatly simplify by just having the iomem pointer be the only pointer that is stored in the policy driver_data.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 0cd8eb7..c379c7f 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO If in doubt, say N. +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM || COMPILE_TEST + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index c1ffeab..3882908 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c new file mode 100644 index 0000000..fe1c264 --- /dev/null +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -0,0 +1,354 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/cpufreq.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> + +#define LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 +#define CLK_HW_DIV 2 + +enum { + REG_ENABLE, + REG_LUT_TABLE, + REG_PERF_STATE, + + REG_ARRAY_SIZE, +}; + +struct cpufreq_qcom { + struct cpufreq_frequency_table *table; + void __iomem *reg_bases[REG_ARRAY_SIZE]; + cpumask_t related_cpus; + unsigned int max_cores; + unsigned long xo_rate; + unsigned long cpu_hw_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { + [REG_ENABLE] = 0x0, + [REG_LUT_TABLE] = 0x110, + [REG_PERF_STATE] = 0x920, +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; + +static int +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, + unsigned int index) +{ + struct cpufreq_qcom *c = policy->driver_data; + + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); + + return 0; +} + +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) +{ + struct cpufreq_qcom *c; + struct cpufreq_policy *policy; + unsigned int index; + + policy = cpufreq_cpu_get_raw(cpu); + if (!policy) + return 0; + + c = policy->driver_data; + + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]); + index = min(index, LUT_MAX_ENTRIES - 1); + + return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, + unsigned int target_freq) +{ + struct cpufreq_qcom *c = policy->driver_data; + int index; + + index = policy->cached_resolved_idx; + if (index < 0) + return 0; + + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); + + return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) +{ + struct cpufreq_qcom *c; + + c = qcom_freq_domain_map[policy->cpu]; + if (!c) { + pr_err("No scaling support for CPU%d\n", policy->cpu); + return -ENODEV; + } + + cpumask_copy(policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + + return 0; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, + NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK | + CPUFREQ_HAVE_GOVERNOR_PER_POLICY, + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name = "qcom-cpufreq-hw", + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev, + struct cpufreq_qcom *c) +{ + struct device *dev = &pdev->dev; + void __iomem *base; + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq; + + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1, + sizeof(*c->table), GFP_KERNEL); + if (!c->table) + return -ENOMEM; + + base = c->reg_bases[REG_LUT_TABLE]; + + for (i = 0; i < LUT_MAX_ENTRIES; i++) { + data = readl_relaxed(base + i * LUT_ROW_SIZE); + src = (data & GENMASK(31, 30)) >> 30; + lval = data & GENMASK(7, 0); + core_count = CORE_COUNT_VAL(data); + + if (src) + c->table[i].frequency = c->xo_rate * lval / 1000; + else + c->table[i].frequency = c->cpu_hw_rate / 1000; + + cur_freq = c->table[i].frequency; + + dev_dbg(dev, "index=%d freq=%d, core_count %d\n", + i, c->table[i].frequency, core_count); + + if (core_count != c->max_cores) + cur_freq = CPUFREQ_ENTRY_INVALID; + + /* + * Two of the same frequencies with the same core counts means + * end of table. + */ + if (i > 0 && c->table[i - 1].frequency == + c->table[i].frequency && prev_cc == core_count) { + struct cpufreq_frequency_table *prev = &c->table[i - 1]; + + if (prev_freq == CPUFREQ_ENTRY_INVALID) + prev->flags = CPUFREQ_BOOST_FREQ; + break; + } + prev_cc = core_count; + prev_freq = cur_freq; + } + + c->table[i].frequency = CPUFREQ_TABLE_END; + + return 0; +} + +static int qcom_get_related_cpus(int index, struct cpumask *m) +{ + struct device_node *cpu_np; + struct of_phandle_args args; + int cpu, ret; + + for_each_possible_cpu(cpu) { + cpu_np = of_cpu_device_node_get(cpu); + if (!cpu_np) + continue; + + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain", + "#freq-domain-cells", 0, + &args); + of_node_put(cpu_np); + if (ret < 0) + continue; + + if (index == args.args[0]) + cpumask_set_cpu(cpu, m); + } + + return 0; +} + +static int qcom_cpu_resources_init(struct platform_device *pdev, + unsigned int cpu, int index, + unsigned long xo_rate, + unsigned long cpu_hw_rate) +{ + struct cpufreq_qcom *c; + struct resource *res; + struct device *dev = &pdev->dev; + const u16 *offsets; + int ret, i, cpu_r; + void __iomem *base; + + if (qcom_freq_domain_map[cpu]) + return 0; + + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL); + if (!c) + return -ENOMEM; + + offsets = of_device_get_match_data(&pdev->dev); + if (!offsets) + return -EINVAL; + + res = platform_get_resource(pdev, IORESOURCE_MEM, index); + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++) + c->reg_bases[i] = base + offsets[i]; + + /* HW should be in enabled state to proceed */ + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) { + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index); + return -ENODEV; + } + + ret = qcom_get_related_cpus(index, &c->related_cpus); + if (ret) { + dev_err(dev, "Domain-%d failed to get related CPUs\n", index); + return ret; + } + + c->max_cores = cpumask_weight(&c->related_cpus); + if (!c->max_cores) + return -ENOENT; + + c->xo_rate = xo_rate; + c->cpu_hw_rate = cpu_hw_rate; + + ret = qcom_cpufreq_hw_read_lut(pdev, c); + if (ret) { + dev_err(dev, "Domain-%d failed to read LUT\n", index); + return ret; + } + + for_each_cpu(cpu_r, &c->related_cpus) + qcom_freq_domain_map[cpu_r] = c; + + return 0; +} + +static int qcom_resources_init(struct platform_device *pdev) +{ + struct device_node *cpu_np; + struct of_phandle_args args; + struct clk *clk; + unsigned int cpu; + unsigned long xo_rate, cpu_hw_rate; + int ret; + + clk = clk_get(&pdev->dev, "xo"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + xo_rate = clk_get_rate(clk); + + clk_put(clk); + + clk = clk_get(&pdev->dev, "cpu_clk"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV; + + clk_put(clk); + + for_each_possible_cpu(cpu) { + cpu_np = of_cpu_device_node_get(cpu); + if (!cpu_np) { + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n", + cpu); + continue; + } + + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain", + "#freq-domain-cells", 0, + &args); + of_node_put(cpu_np); + if (ret < 0) + return ret; + + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0], + xo_rate, cpu_hw_rate); + if (ret) + return ret; + } + + return 0; +} + +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) +{ + int rc; + + /* Get the bases of cpufreq for domains */ + rc = qcom_resources_init(pdev); + if (rc) { + dev_err(&pdev->dev, "CPUFreq resource init failed\n"); + return rc; + } + + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver); + if (rc) { + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n"); + return rc; + } + + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n"); + + return 0; +} + +static const struct of_device_id qcom_cpufreq_hw_match[] = { + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets }, + {} +}; + +static struct platform_driver qcom_cpufreq_hw_driver = { + .probe = qcom_cpufreq_hw_driver_probe, + .driver = { + .name = "qcom-cpufreq-hw", + .of_match_table = qcom_cpufreq_hw_match, + }, +}; + +static int __init qcom_cpufreq_hw_init(void) +{ + return platform_driver_register(&qcom_cpufreq_hw_driver); +} +subsys_initcall(qcom_cpufreq_hw_init);