* [PATCH 0/2] OPP: Disallow "opp-hz" property without a corresponding clk @ 2022-11-21 6:57 Viresh Kumar 2022-11-21 6:57 ` [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Register config_clks helper Viresh Kumar 2022-11-21 6:57 ` [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar 0 siblings, 2 replies; 18+ messages in thread From: Viresh Kumar @ 2022-11-21 6:57 UTC (permalink / raw) To: Manivannan Sadhasivam, Andy Gross, Bjorn Andersson, Konrad Dybcio, Nishanth Menon, Rafael J. Wysocki, Stephen Boyd, Viresh Kumar, Viresh Kumar Cc: linux-pm, Vincent Guittot, johan, krzysztof.kozlowski+dt, linux-arm-msm, linux-kernel This removes the special code added by the commit 2083da24eb56 ("OPP: Allow multiple clocks for a device"), to make it work for Qcom SoC. In qcom-cpufreq-hw driver, we want to skip clk configuration that happens via dev_pm_opp_set_opp(), but still want the OPP core to parse the "opp-hz" property so we can use the freq based OPP helpers. The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we no longer need the special handling of the same in the OPP core. Update Qcom driver as well to disallow freq update to the clk. Mani, can you give this a try again please, thanks ? I have only compile tested this. Viresh Kumar (2): cpufreq: qcom-cpufreq-hw: Register config_clks helper OPP: Disallow "opp-hz" property without a corresponding clk drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++++++++--- drivers/opp/core.c | 14 ------------- drivers/opp/debugfs.c | 2 +- 3 files changed, 32 insertions(+), 18 deletions(-) -- 2.31.1.272.g89b43f80a514 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Register config_clks helper 2022-11-21 6:57 [PATCH 0/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar @ 2022-11-21 6:57 ` Viresh Kumar 2022-11-21 6:57 ` [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar 1 sibling, 0 replies; 18+ messages in thread From: Viresh Kumar @ 2022-11-21 6:57 UTC (permalink / raw) To: Manivannan Sadhasivam, Andy Gross, Bjorn Andersson, Konrad Dybcio, Rafael J. Wysocki, Viresh Kumar Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon, johan, krzysztof.kozlowski+dt, linux-arm-msm, linux-kernel In qcom-cpufreq-hw driver, we want to skip clk configuration that happens via dev_pm_opp_set_opp(), but still want the OPP core to parse the "opp-hz" property so we can use the freq based OPP helpers. The OPP core provides support for the platforms to provide config_clks helpers now, lets use that to provide an empty callback to skip clock configuration. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 1bd1e9ae5308..5f8079898940 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -51,6 +51,7 @@ struct qcom_cpufreq_data { */ struct mutex throttle_lock; int throttle_irq; + int opp_token; char irq_name[15]; bool cancel_throttle; struct delayed_work throttle_work; @@ -58,7 +59,6 @@ struct qcom_cpufreq_data { struct clk_hw cpu_clk; bool per_core_dcvs; - struct freq_qos_request throttle_freq_req; }; @@ -197,6 +197,15 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, return policy->freq_table[index].frequency; } +static int qcom_cpufreq_hw_config_clks(struct device *dev, + struct opp_table *opp_table, + struct dev_pm_opp *opp, void *data, + bool scaling_down) +{ + /* We want to skip clk configuration via dev_pm_opp_set_opp() */ + return 0; +} + static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, struct cpufreq_policy *policy) { @@ -208,11 +217,23 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, int ret; struct qcom_cpufreq_data *drv_data = policy->driver_data; const struct qcom_cpufreq_soc_data *soc_data = qcom_cpufreq.soc_data; + const char * const clk_names[] = { NULL, NULL }; + struct dev_pm_opp_config config = { + .clk_names = clk_names, + .config_clks = qcom_cpufreq_hw_config_clks, + }; table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL); if (!table) return -ENOMEM; + ret = dev_pm_opp_set_config(cpu_dev, &config); + if (ret < 0) { + dev_err(cpu_dev, "Failed to set OPP config: %d\n", ret); + goto free_table; + } + drv_data->opp_token = ret; + ret = dev_pm_opp_of_add_table(cpu_dev); if (!ret) { /* Disable all opps and cross-validate against LUT later */ @@ -227,8 +248,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, } } else if (ret != -ENODEV) { dev_err(cpu_dev, "Invalid opp table in device tree\n"); - kfree(table); - return ret; + goto clear_config; } else { policy->fast_switch_possible = true; icc_scaling_enabled = false; @@ -296,6 +316,13 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, dev_pm_opp_set_sharing_cpus(cpu_dev, policy->cpus); return 0; + +clear_config: + dev_pm_opp_clear_config(drv_data->opp_token); + +free_table: + kfree(table); + return ret; } static void qcom_get_related_cpus(int index, struct cpumask *m) @@ -594,6 +621,7 @@ static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) dev_pm_opp_remove_all_dynamic(cpu_dev); dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); qcom_cpufreq_hw_lmh_exit(data); + dev_pm_opp_clear_config(data->opp_token); kfree(policy->freq_table); kfree(data); iounmap(base); -- 2.31.1.272.g89b43f80a514 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 6:57 [PATCH 0/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar 2022-11-21 6:57 ` [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Register config_clks helper Viresh Kumar @ 2022-11-21 6:57 ` Viresh Kumar 2022-11-21 7:22 ` Johan Hovold 1 sibling, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2022-11-21 6:57 UTC (permalink / raw) To: Manivannan Sadhasivam, Viresh Kumar, Nishanth Menon, Stephen Boyd Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, johan, krzysztof.kozlowski+dt, linux-kernel This removes the special code added by the commit 2083da24eb56 ("OPP: Allow multiple clocks for a device"), to make it work for Qcom SoC. In qcom-cpufreq-hw driver, we want to skip clk configuration that happens via dev_pm_opp_set_opp(), but still want the OPP core to parse the "opp-hz" property so we can use the freq based OPP helpers. The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we no longer need the special handling of the same in the OPP core. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/core.c | 14 -------------- drivers/opp/debugfs.c | 2 +- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index e87567dbe99f..b7158d33c13d 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1384,20 +1384,6 @@ static struct opp_table *_update_opp_table_clk(struct device *dev, } if (ret == -ENOENT) { - /* - * There are few platforms which don't want the OPP core to - * manage device's clock settings. In such cases neither the - * platform provides the clks explicitly to us, nor the DT - * contains a valid clk entry. The OPP nodes in DT may still - * contain "opp-hz" property though, which we need to parse and - * allow the platform to find an OPP based on freq later on. - * - * This is a simple solution to take care of such corner cases, - * i.e. make the clk_count 1, which lets us allocate space for - * frequency in opp->rates and also parse the entries in DT. - */ - opp_table->clk_count = 1; - dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret); return opp_table; } diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c index 96a30a032c5f..402c507edac7 100644 --- a/drivers/opp/debugfs.c +++ b/drivers/opp/debugfs.c @@ -138,7 +138,7 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table) * - For some devices rate isn't available or there are multiple, use * index instead for them. */ - if (likely(opp_table->clk_count == 1 && opp->rates[0])) + if (likely(opp_table->clk_count == 1)) id = opp->rates[0]; else id = _get_opp_count(opp_table); -- 2.31.1.272.g89b43f80a514 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 6:57 ` [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar @ 2022-11-21 7:22 ` Johan Hovold 2022-11-21 7:38 ` Viresh Kumar 2022-11-21 7:39 ` Manivannan Sadhasivam 0 siblings, 2 replies; 18+ messages in thread From: Johan Hovold @ 2022-11-21 7:22 UTC (permalink / raw) To: Viresh Kumar Cc: Manivannan Sadhasivam, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Mon, Nov 21, 2022 at 12:27:48PM +0530, Viresh Kumar wrote: > This removes the special code added by the commit 2083da24eb56 ("OPP: > Allow multiple clocks for a device"), to make it work for Qcom SoC. > > In qcom-cpufreq-hw driver, we want to skip clk configuration that > happens via dev_pm_opp_set_opp(), but still want the OPP core to parse > the "opp-hz" property so we can use the freq based OPP helpers. > > The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we > no longer need the special handling of the same in the OPP core. Didn't this affect also sc8280xp? Perhaps you can hold off with applying this one for a bit until the needed devicetree changes are in linux-next for all the affected platforms. (It looks like Mani's series only updated sm8450 and I guess Bjorn hasn't picked up that one up yet either.) Johan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:22 ` Johan Hovold @ 2022-11-21 7:38 ` Viresh Kumar 2022-11-21 7:42 ` Johan Hovold 2022-11-21 7:44 ` Manivannan Sadhasivam 2022-11-21 7:39 ` Manivannan Sadhasivam 1 sibling, 2 replies; 18+ messages in thread From: Viresh Kumar @ 2022-11-21 7:38 UTC (permalink / raw) To: Johan Hovold Cc: Manivannan Sadhasivam, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On 21-11-22, 08:22, Johan Hovold wrote: > On Mon, Nov 21, 2022 at 12:27:48PM +0530, Viresh Kumar wrote: > > This removes the special code added by the commit 2083da24eb56 ("OPP: > > Allow multiple clocks for a device"), to make it work for Qcom SoC. > > > > In qcom-cpufreq-hw driver, we want to skip clk configuration that > > happens via dev_pm_opp_set_opp(), but still want the OPP core to parse > > the "opp-hz" property so we can use the freq based OPP helpers. > > > > The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we > > no longer need the special handling of the same in the OPP core. > > Didn't this affect also sc8280xp? I assumed Mani fixed all affected Qcom SoCs :( > Perhaps you can hold off with applying > this one for a bit until the needed devicetree changes are in linux-next > for all the affected platforms. Sure. > (It looks like Mani's series only updated sm8450 and I guess Bjorn > hasn't picked up that one up yet either.) I applied that series today to my cpufreq/arm/linux-next, since it had cpufreq updates too and these patches are rebased on top of them. -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:38 ` Viresh Kumar @ 2022-11-21 7:42 ` Johan Hovold 2022-11-21 8:30 ` Viresh Kumar 2022-11-21 7:44 ` Manivannan Sadhasivam 1 sibling, 1 reply; 18+ messages in thread From: Johan Hovold @ 2022-11-21 7:42 UTC (permalink / raw) To: Viresh Kumar Cc: Manivannan Sadhasivam, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Mon, Nov 21, 2022 at 01:08:17PM +0530, Viresh Kumar wrote: > On 21-11-22, 08:22, Johan Hovold wrote: > > On Mon, Nov 21, 2022 at 12:27:48PM +0530, Viresh Kumar wrote: > > > This removes the special code added by the commit 2083da24eb56 ("OPP: > > > Allow multiple clocks for a device"), to make it work for Qcom SoC. > > > > > > In qcom-cpufreq-hw driver, we want to skip clk configuration that > > > happens via dev_pm_opp_set_opp(), but still want the OPP core to parse > > > the "opp-hz" property so we can use the freq based OPP helpers. > > > > > > The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we > > > no longer need the special handling of the same in the OPP core. > > > > Didn't this affect also sc8280xp? > > I assumed Mani fixed all affected Qcom SoCs :( I think he may have been waiting for his suggested solution to be accepted before updating all the affected dtsi:s. > > Perhaps you can hold off with applying > > this one for a bit until the needed devicetree changes are in linux-next > > for all the affected platforms. > > Sure. Thanks. > > (It looks like Mani's series only updated sm8450 and I guess Bjorn > > hasn't picked up that one up yet either.) > > I applied that series today to my cpufreq/arm/linux-next, since it had > cpufreq updates too and these patches are rebased on top of them. Ok, I was expected the devicetree update to through Bjorn's tree as I imagine there may be conflicts otherwise. What was the intention here, Mani? Johan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:42 ` Johan Hovold @ 2022-11-21 8:30 ` Viresh Kumar 2022-11-22 13:26 ` Manivannan Sadhasivam 0 siblings, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2022-11-21 8:30 UTC (permalink / raw) To: Johan Hovold Cc: Manivannan Sadhasivam, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On 21-11-22, 08:42, Johan Hovold wrote: > Ok, I was expected the devicetree update to through Bjorn's tree as I > imagine there may be conflicts otherwise. Dropped the DT patch now. Mani, I believe the first patch in this series is still required. Without that, when the cpufreq driver calls dev_pm_opp_set_opp(), the OPP core may end up calling clk_set_rate(), which shouldn't be done in case of Qcom SoCs. I am not sure though, how it will work with sc8280xp. Can you please check ? -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 8:30 ` Viresh Kumar @ 2022-11-22 13:26 ` Manivannan Sadhasivam 2022-11-24 4:23 ` Viresh Kumar 0 siblings, 1 reply; 18+ messages in thread From: Manivannan Sadhasivam @ 2022-11-22 13:26 UTC (permalink / raw) To: Viresh Kumar Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Mon, Nov 21, 2022 at 02:00:36PM +0530, Viresh Kumar wrote: > On 21-11-22, 08:42, Johan Hovold wrote: > > Ok, I was expected the devicetree update to through Bjorn's tree as I > > imagine there may be conflicts otherwise. > > Dropped the DT patch now. > > Mani, I believe the first patch in this series is still required. > Without that, when the cpufreq driver calls dev_pm_opp_set_opp(), the > OPP core may end up calling clk_set_rate(), which shouldn't be done in > case of Qcom SoCs. > If there is no .set_rate() callback implemented by the clock provider, it won't hurt, right? Thanks, Mani > I am not sure though, how it will work with sc8280xp. Can you please > check ? > > -- > viresh -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-22 13:26 ` Manivannan Sadhasivam @ 2022-11-24 4:23 ` Viresh Kumar 2022-11-24 5:24 ` Manivannan Sadhasivam 0 siblings, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2022-11-24 4:23 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On 22-11-22, 18:56, Manivannan Sadhasivam wrote: > If there is no .set_rate() callback implemented by the clock provider, it won't > hurt, right? It shouldn't, I guess. Well, in that case, is the first patch even required ? Maybe we should keep it, this makes clear that we won't even call set_rate(), irrespective of the face that it is implemented or not. Also, the clk provider may not be part of this file later on, for other SoC versions, and it is better in that case too. -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-24 4:23 ` Viresh Kumar @ 2022-11-24 5:24 ` Manivannan Sadhasivam 0 siblings, 0 replies; 18+ messages in thread From: Manivannan Sadhasivam @ 2022-11-24 5:24 UTC (permalink / raw) To: Viresh Kumar Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Thu, Nov 24, 2022 at 09:53:04AM +0530, Viresh Kumar wrote: > On 22-11-22, 18:56, Manivannan Sadhasivam wrote: > > If there is no .set_rate() callback implemented by the clock provider, it won't > > hurt, right? > > It shouldn't, I guess. Well, in that case, is the first patch even > required ? Maybe we should keep it, this makes clear that we won't > even call set_rate(), irrespective of the face that it is implemented > or not. > I don't think that detail is required to be made explicit. If someone cares, they can easlily find out by glancing through the OPP code. So IMO, we don't need patch 1/2. > Also, the clk provider may not be part of this file later on, for > other SoC versions, and it is better in that case too. > We cannot predict what the HW guys will come up with ;) But as said above, I don't think it is necessary to to make it explicit. Thanks, Mani > -- > viresh -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:38 ` Viresh Kumar 2022-11-21 7:42 ` Johan Hovold @ 2022-11-21 7:44 ` Manivannan Sadhasivam 1 sibling, 0 replies; 18+ messages in thread From: Manivannan Sadhasivam @ 2022-11-21 7:44 UTC (permalink / raw) To: Viresh Kumar Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Mon, Nov 21, 2022 at 01:08:17PM +0530, Viresh Kumar wrote: > On 21-11-22, 08:22, Johan Hovold wrote: > > On Mon, Nov 21, 2022 at 12:27:48PM +0530, Viresh Kumar wrote: > > > This removes the special code added by the commit 2083da24eb56 ("OPP: > > > Allow multiple clocks for a device"), to make it work for Qcom SoC. > > > > > > In qcom-cpufreq-hw driver, we want to skip clk configuration that > > > happens via dev_pm_opp_set_opp(), but still want the OPP core to parse > > > the "opp-hz" property so we can use the freq based OPP helpers. > > > > > > The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we > > > no longer need the special handling of the same in the OPP core. > > > > Didn't this affect also sc8280xp? > > I assumed Mani fixed all affected Qcom SoCs :( > > > Perhaps you can hold off with applying > > this one for a bit until the needed devicetree changes are in linux-next > > for all the affected platforms. > > Sure. > > > (It looks like Mani's series only updated sm8450 and I guess Bjorn > > hasn't picked up that one up yet either.) > > I applied that series today to my cpufreq/arm/linux-next, since it had > cpufreq updates too and these patches are rebased on top of them. > Ah, I thought you applied only cpufreq patches. DTS and bindings patches are supposed to go through Bjorn's tree. Can you please drop them? Thanks, Mani > -- > viresh -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:22 ` Johan Hovold 2022-11-21 7:38 ` Viresh Kumar @ 2022-11-21 7:39 ` Manivannan Sadhasivam 2023-01-25 4:21 ` Viresh Kumar 1 sibling, 1 reply; 18+ messages in thread From: Manivannan Sadhasivam @ 2022-11-21 7:39 UTC (permalink / raw) To: Johan Hovold Cc: Viresh Kumar, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On Mon, Nov 21, 2022 at 08:22:01AM +0100, Johan Hovold wrote: > On Mon, Nov 21, 2022 at 12:27:48PM +0530, Viresh Kumar wrote: > > This removes the special code added by the commit 2083da24eb56 ("OPP: > > Allow multiple clocks for a device"), to make it work for Qcom SoC. > > > > In qcom-cpufreq-hw driver, we want to skip clk configuration that > > happens via dev_pm_opp_set_opp(), but still want the OPP core to parse > > the "opp-hz" property so we can use the freq based OPP helpers. > > > > The DT for Qcom SoCs is fixed now and contain a valid clk entry, and we > > no longer need the special handling of the same in the OPP core. > > Didn't this affect also sc8280xp? Perhaps you can hold off with applying > this one for a bit until the needed devicetree changes are in linux-next > for all the affected platforms. > > (It looks like Mani's series only updated sm8450 and I guess Bjorn > hasn't picked up that one up yet either.) > That's right. I have proposed to do the similar change to other SoCs as well once the series was completely merged. I thought of doing so for 6.3. Btw, there seems to be one more candidate: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi#n2537 Looks like newer SoCs that has the GMU within the GPU block doesn't have clock property. This is because, GMU is the one supplying clocks to the GPU unlike the old SoCs where the clocks used to come from GCC itself. But we do have a GMU devicetree node, so it should be a matter of adding the clock provider support as done for cpufreq and represent it in devicetree. I'll ping Rob Clark and see how to get it done. Thanks, Mani > Johan -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2022-11-21 7:39 ` Manivannan Sadhasivam @ 2023-01-25 4:21 ` Viresh Kumar 2023-02-16 6:47 ` Manivannan Sadhasivam 0 siblings, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2023-01-25 4:21 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel On 21-11-22, 13:09, Manivannan Sadhasivam wrote: > That's right. I have proposed to do the similar change to other SoCs as well > once the series was completely merged. I thought of doing so for 6.3. > > Btw, there seems to be one more candidate: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi#n2537 > > Looks like newer SoCs that has the GMU within the GPU block doesn't have clock > property. This is because, GMU is the one supplying clocks to the GPU unlike the > old SoCs where the clocks used to come from GCC itself. > > But we do have a GMU devicetree node, so it should be a matter of adding the > clock provider support as done for cpufreq and represent it in devicetree. > > I'll ping Rob Clark and see how to get it done. Any update on this Mani ? I want to get the hack removed if possible. -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2023-01-25 4:21 ` Viresh Kumar @ 2023-02-16 6:47 ` Manivannan Sadhasivam 2023-10-11 5:48 ` Viresh Kumar 0 siblings, 1 reply; 18+ messages in thread From: Manivannan Sadhasivam @ 2023-02-16 6:47 UTC (permalink / raw) To: Viresh Kumar Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel, robdclark + Rob Clark On Wed, Jan 25, 2023 at 09:51:45AM +0530, Viresh Kumar wrote: > On 21-11-22, 13:09, Manivannan Sadhasivam wrote: > > That's right. I have proposed to do the similar change to other SoCs as well > > once the series was completely merged. I thought of doing so for 6.3. > > > > Btw, there seems to be one more candidate: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi#n2537 > > > > Looks like newer SoCs that has the GMU within the GPU block doesn't have clock > > property. This is because, GMU is the one supplying clocks to the GPU unlike the > > old SoCs where the clocks used to come from GCC itself. > > > > But we do have a GMU devicetree node, so it should be a matter of adding the > > clock provider support as done for cpufreq and represent it in devicetree. > > > > I'll ping Rob Clark and see how to get it done. > > Any update on this Mani ? I want to get the hack removed if possible. > Hi Viresh, Sorry for the delay. I've submitted the dts changes [1] to handle the CPU clocks for the rest of the Qcom SoCs. For the Qcom GPUs, I've CCed Rob Clark who is the maintainer. Rob, here is the background on the issue that is being discussed in this thread: Viresh submitted a series [2] back in July to improve the OPP framework, but that ended up breaking cpufreq on multiple Qcom SoCs. After investigation, it was found that the series was expecting the clocks supplied to the OPP end devices like CPUs/GPUs to be modeled in DT. But on Qcom platforms even though the clocks for these nodes are supplied by a separate entity, like CPUFreq (EPSS/OSM) for CPUs and GMU for GPUs, there was no clock property present in the respective nodes. And these nodes are using OPP table to switch frequencies dynamically. While the series was merged with a hack that still allows the OPP nodes without clock property in DT, we came to an agreement that the clock hierarchy should be modeled properly. So I submitted a series [3] that added clock provider support to cpufreq driver and sourced the clock from cpufreq node to CPU nodes in DT. Likewise, it should be handled for the adreno GPUs whose clock is managed by GMU on newer SoCs. Can you take a look at this? Thanks, Mani [1] https://lore.kernel.org/linux-arm-msm/20230215070400.5901-1-manivannan.sadhasivam@linaro.org/ [2] https://lore.kernel.org/lkml/cover.1657003420.git.viresh.kumar@linaro.org/ [3] https://lore.kernel.org/linux-arm-msm/20221117053145.10409-1-manivannan.sadhasivam@linaro.org/ > -- > viresh -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2023-02-16 6:47 ` Manivannan Sadhasivam @ 2023-10-11 5:48 ` Viresh Kumar 2023-11-15 6:32 ` Viresh Kumar 0 siblings, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2023-10-11 5:48 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel, robdclark On 16-02-23, 12:17, Manivannan Sadhasivam wrote: > Sorry for the delay. I've submitted the dts changes [1] to handle the CPU clocks > for the rest of the Qcom SoCs. > > For the Qcom GPUs, I've CCed Rob Clark who is the maintainer. > > Rob, here is the background on the issue that is being discussed in this > thread: > > Viresh submitted a series [2] back in July to improve the OPP framework, but > that ended up breaking cpufreq on multiple Qcom SoCs. After investigation, it > was found that the series was expecting the clocks supplied to the OPP end > devices like CPUs/GPUs to be modeled in DT. But on Qcom platforms even though > the clocks for these nodes are supplied by a separate entity, like CPUFreq > (EPSS/OSM) for CPUs and GMU for GPUs, there was no clock property present in > the respective nodes. And these nodes are using OPP table to switch frequencies > dynamically. > > While the series was merged with a hack that still allows the OPP nodes without > clock property in DT, we came to an agreement that the clock hierarchy should > be modeled properly. > > So I submitted a series [3] that added clock provider support to cpufreq driver > and sourced the clock from cpufreq node to CPU nodes in DT. > > Likewise, it should be handled for the adreno GPUs whose clock is managed by > GMU on newer SoCs. Can you take a look at this? Any update on this ? -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2023-10-11 5:48 ` Viresh Kumar @ 2023-11-15 6:32 ` Viresh Kumar 2023-11-15 7:55 ` Manivannan Sadhasivam 0 siblings, 1 reply; 18+ messages in thread From: Viresh Kumar @ 2023-11-15 6:32 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel, robdclark On 11-10-23, 11:18, Viresh Kumar wrote: > On 16-02-23, 12:17, Manivannan Sadhasivam wrote: > > Sorry for the delay. I've submitted the dts changes [1] to handle the CPU clocks > > for the rest of the Qcom SoCs. > > > > For the Qcom GPUs, I've CCed Rob Clark who is the maintainer. > > > > Rob, here is the background on the issue that is being discussed in this > > thread: > > > > Viresh submitted a series [2] back in July to improve the OPP framework, but > > that ended up breaking cpufreq on multiple Qcom SoCs. After investigation, it > > was found that the series was expecting the clocks supplied to the OPP end > > devices like CPUs/GPUs to be modeled in DT. But on Qcom platforms even though > > the clocks for these nodes are supplied by a separate entity, like CPUFreq > > (EPSS/OSM) for CPUs and GMU for GPUs, there was no clock property present in > > the respective nodes. And these nodes are using OPP table to switch frequencies > > dynamically. > > > > While the series was merged with a hack that still allows the OPP nodes without > > clock property in DT, we came to an agreement that the clock hierarchy should > > be modeled properly. > > > > So I submitted a series [3] that added clock provider support to cpufreq driver > > and sourced the clock from cpufreq node to CPU nodes in DT. > > > > Likewise, it should be handled for the adreno GPUs whose clock is managed by > > GMU on newer SoCs. Can you take a look at this? > > Any update on this ? Mani, Ping. -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2023-11-15 6:32 ` Viresh Kumar @ 2023-11-15 7:55 ` Manivannan Sadhasivam 2023-11-15 8:43 ` Dmitry Baryshkov 0 siblings, 1 reply; 18+ messages in thread From: Manivannan Sadhasivam @ 2023-11-15 7:55 UTC (permalink / raw) To: Viresh Kumar Cc: Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel, robdclark, dmitry.baryshkov + Dmitry On Wed, Nov 15, 2023 at 12:02:01PM +0530, Viresh Kumar wrote: > On 11-10-23, 11:18, Viresh Kumar wrote: > > On 16-02-23, 12:17, Manivannan Sadhasivam wrote: > > > Sorry for the delay. I've submitted the dts changes [1] to handle the CPU clocks > > > for the rest of the Qcom SoCs. > > > > > > For the Qcom GPUs, I've CCed Rob Clark who is the maintainer. > > > > > > Rob, here is the background on the issue that is being discussed in this > > > thread: > > > > > > Viresh submitted a series [2] back in July to improve the OPP framework, but > > > that ended up breaking cpufreq on multiple Qcom SoCs. After investigation, it > > > was found that the series was expecting the clocks supplied to the OPP end > > > devices like CPUs/GPUs to be modeled in DT. But on Qcom platforms even though > > > the clocks for these nodes are supplied by a separate entity, like CPUFreq > > > (EPSS/OSM) for CPUs and GMU for GPUs, there was no clock property present in > > > the respective nodes. And these nodes are using OPP table to switch frequencies > > > dynamically. > > > > > > While the series was merged with a hack that still allows the OPP nodes without > > > clock property in DT, we came to an agreement that the clock hierarchy should > > > be modeled properly. > > > > > > So I submitted a series [3] that added clock provider support to cpufreq driver > > > and sourced the clock from cpufreq node to CPU nodes in DT. > > > > > > Likewise, it should be handled for the adreno GPUs whose clock is managed by > > > GMU on newer SoCs. Can you take a look at this? > > > > Any update on this ? > > Mani, > > Ping. > Dmitry, can you please look into this? Please read my above reply to Rob to get the background. - Mani > -- > viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk 2023-11-15 7:55 ` Manivannan Sadhasivam @ 2023-11-15 8:43 ` Dmitry Baryshkov 0 siblings, 0 replies; 18+ messages in thread From: Dmitry Baryshkov @ 2023-11-15 8:43 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Viresh Kumar, Johan Hovold, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, Vincent Guittot, Rafael J. Wysocki, andersson, krzysztof.kozlowski+dt, linux-kernel, robdclark, freedreno On Wed, 15 Nov 2023 at 09:55, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > + Dmitry > > On Wed, Nov 15, 2023 at 12:02:01PM +0530, Viresh Kumar wrote: > > On 11-10-23, 11:18, Viresh Kumar wrote: > > > On 16-02-23, 12:17, Manivannan Sadhasivam wrote: > > > > Sorry for the delay. I've submitted the dts changes [1] to handle the CPU clocks > > > > for the rest of the Qcom SoCs. > > > > > > > > For the Qcom GPUs, I've CCed Rob Clark who is the maintainer. > > > > > > > > Rob, here is the background on the issue that is being discussed in this > > > > thread: > > > > > > > > Viresh submitted a series [2] back in July to improve the OPP framework, but > > > > that ended up breaking cpufreq on multiple Qcom SoCs. After investigation, it > > > > was found that the series was expecting the clocks supplied to the OPP end > > > > devices like CPUs/GPUs to be modeled in DT. But on Qcom platforms even though > > > > the clocks for these nodes are supplied by a separate entity, like CPUFreq > > > > (EPSS/OSM) for CPUs and GMU for GPUs, there was no clock property present in > > > > the respective nodes. And these nodes are using OPP table to switch frequencies > > > > dynamically. > > > > > > > > While the series was merged with a hack that still allows the OPP nodes without > > > > clock property in DT, we came to an agreement that the clock hierarchy should > > > > be modeled properly. > > > > > > > > So I submitted a series [3] that added clock provider support to cpufreq driver > > > > and sourced the clock from cpufreq node to CPU nodes in DT. > > > > > > > > Likewise, it should be handled for the adreno GPUs whose clock is managed by > > > > GMU on newer SoCs. Can you take a look at this? > > > > > > Any update on this ? > > > > Mani, > > > > Ping. > > > > Dmitry, can you please look into this? Please read my above reply to Rob > to get the background. The issue is that we don't have an actual clock that corresponds to the GPU frequency. Not even a read-only one. Can we get away by manually setting config_clocks()? Also could you please remind me, can we sleep inside the config_clks() callback? -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-11-15 8:44 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-21 6:57 [PATCH 0/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar 2022-11-21 6:57 ` [PATCH 1/2] cpufreq: qcom-cpufreq-hw: Register config_clks helper Viresh Kumar 2022-11-21 6:57 ` [PATCH 2/2] OPP: Disallow "opp-hz" property without a corresponding clk Viresh Kumar 2022-11-21 7:22 ` Johan Hovold 2022-11-21 7:38 ` Viresh Kumar 2022-11-21 7:42 ` Johan Hovold 2022-11-21 8:30 ` Viresh Kumar 2022-11-22 13:26 ` Manivannan Sadhasivam 2022-11-24 4:23 ` Viresh Kumar 2022-11-24 5:24 ` Manivannan Sadhasivam 2022-11-21 7:44 ` Manivannan Sadhasivam 2022-11-21 7:39 ` Manivannan Sadhasivam 2023-01-25 4:21 ` Viresh Kumar 2023-02-16 6:47 ` Manivannan Sadhasivam 2023-10-11 5:48 ` Viresh Kumar 2023-11-15 6:32 ` Viresh Kumar 2023-11-15 7:55 ` Manivannan Sadhasivam 2023-11-15 8:43 ` Dmitry Baryshkov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.