Message ID | 20221108154037.111794-4-manivannan.sadhasivam@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2787208wru; Tue, 8 Nov 2022 07:46:06 -0800 (PST) X-Google-Smtp-Source: AMsMyM4Gf8adz+C25Vpsl38YQsFv3opoXeIkA03IMPeHn8NiN5lrP7JxGIN31Q7/h7Sf7ULCMSj4 X-Received: by 2002:a17:903:1381:b0:186:8bdb:6865 with SMTP id jx1-20020a170903138100b001868bdb6865mr55998255plb.166.1667922366260; Tue, 08 Nov 2022 07:46:06 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1667922366; cv=none; d=google.com; s=arc-20160816; b=cEZWGzxdsjkxp+x1/vXgPct7RqNbUVtL6vtHmmmEmuokAyaRJtvC/Z0eZTreeSn4AZ MVoCFGrwxtjwecqcQyydr6GxH4wrRhQTrmcl9m0BxyKeg9zV3HiBxAAj83uEnzJeyAx0 F6gaOAZpB+ZcPKo7zP85oVRTupB8HC2IEU3CYh4ygqwr+RVe0Empo+vo7ol8kdnrpJRg JMarLfN8GT3VUyleQk/7slKu8r5hwZbHMjfamwoYTxN6UI4aQp+8R1tLxVxF4aJcktq6 OE84T6JfQrlE2vlHlTOH6q8vDWPBX8/IEKWARMJCmhyOaiIxA2Ta89MWwXGawf8Pz97J X0Tw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=yIl+Qt8RB0v0fumM9NIaZyrAwuRsmhqUeWJ9GHQ1hx0=; b=pnnbUJBFtY4GqmJZfM8OhHoZjsW8ENlBekehpj/Fkb0Y9QTEIVEUf7kBi/p7sw+uCZ PGMN5zBP6yw3+QpckT2438c4S6Wox1gYae6LPauDfsC4/Bi2VuH3sm9U0KqmZeSb9y6V v+MByHS3wLJysl8bsz8nEHOGFkAoSjH+x93gOQYb0DTLiDk4z7zm74MU2TOQI7o+3Fal Tas9iYfSGbB6rmTzFuxrj/24k2Epf2RjCh1kYVhLkkjY48vae/txijBGme7/BqgXEje3 4M1bRY8N3ogvx14Ntbv7V35aueBGj7BKzAmktbhShZLwUBVHD3b18W8BLMlZEzb2y+/I m+Wg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=tCARyLvz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id d6-20020a170903230600b00180d51f70cfsi16717449plh.107.2022.11.08.07.45.52; Tue, 08 Nov 2022 07:46:06 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=tCARyLvz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234489AbiKHPle (ORCPT <rfc822;tony84727@gmail.com> + 99 others); Tue, 8 Nov 2022 10:41:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59490 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234302AbiKHPlX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 8 Nov 2022 10:41:23 -0500 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 49DBC5C769 for <linux-kernel@vger.kernel.org>; Tue, 8 Nov 2022 07:41:22 -0800 (PST) Received: by mail-wr1-x42f.google.com with SMTP id k8so21639080wrh.1 for <linux-kernel@vger.kernel.org>; Tue, 08 Nov 2022 07:41:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=yIl+Qt8RB0v0fumM9NIaZyrAwuRsmhqUeWJ9GHQ1hx0=; b=tCARyLvz99VJwDDOt8Xky84bnUUHQRpdVlzGY8N2/kstISvgFZ+XRmF3dnucaExqjU gNoMznVSOT/l6qRF6bSuD5jGOUxvgL5DQu9fGg5ChzhmOpWBGmA/BbS1g5mEHxdrYwJ/ 3zfsblPBHjc1Q+F8VJQX86okwjgtp9PDxP6WwNgVDYi0Sjmcc1DzH0iZofsJTiMc0XX5 nLAnCvaUJcoAcPZAm61QUewMNYtiJCJqvqO/OgUIhWUGXHtk/ixGsh8f+TKuuHjnWQvP QurTk0FpZ1lY1NntzdpbWCdYzPID6aKOgnwDTfMpcv9eRHouTlhzK/FnCIFI1bon23yk CrVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yIl+Qt8RB0v0fumM9NIaZyrAwuRsmhqUeWJ9GHQ1hx0=; b=NcgoE8nZdG1QBEMA+L6ZgkLVFxc50V9pG8tYsJ47MKoLRUP0zbfk1qGA0eTbM+MPBy +zOtry6r3ClicK2s/jsDVvgfP8VCfxaiEdYbO+t9c/oBOLZrkz/psFsK9XEHsvH95ydt 5dj0rUKpToit1ICl+Ac8BlBoQZPbiIUcxAaTfSW+pJovkHL/tZMfDjiY/RFngqzZsX8h 4g2BFzmkU6Lp80pACVrDzFmIq8LgycXZ9sgR6mBPW0XOux0hlAsJtgmTYKHnBtRVxpis IfRA2dMV19ILmC6YDnmYzXOVb1RGsNh6DGSXorZHDDjo56VAbG41gbuyTq3sI1/1FqXb D7UA== X-Gm-Message-State: ACrzQf0CP0AZGg0MWmPytyPpKCSRPbl1+jGQ1K5RXbc4Ii8JaeLmC7T4 FzUUmy2KJJWAqN1roW4lACnH X-Received: by 2002:adf:d08f:0:b0:238:55af:b5db with SMTP id y15-20020adfd08f000000b0023855afb5dbmr607597wrh.97.1667922080766; Tue, 08 Nov 2022 07:41:20 -0800 (PST) Received: from localhost.localdomain ([117.207.25.46]) by smtp.gmail.com with ESMTPSA id e4-20020adff344000000b002364c77bc96sm10906899wrp.33.2022.11.08.07.41.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Nov 2022 07:41:19 -0800 (PST) From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> To: andersson@kernel.org, viresh.kumar@linaro.org, krzysztof.kozlowski+dt@linaro.org, rafael@kernel.org, robh+dt@kernel.org Cc: johan@kernel.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Subject: [PATCH v5 3/3] cpufreq: qcom-hw: Add CPU clock provider support Date: Tue, 8 Nov 2022 21:10:37 +0530 Message-Id: <20221108154037.111794-4-manivannan.sadhasivam@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221108154037.111794-1-manivannan.sadhasivam@linaro.org> References: <20221108154037.111794-1-manivannan.sadhasivam@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748943363071097576?= X-GMAIL-MSGID: =?utf-8?q?1748943363071097576?= |
Series |
qcom-cpufreq-hw: Add CPU clock provider support
|
|
Commit Message
Manivannan Sadhasivam
Nov. 8, 2022, 3:40 p.m. UTC
Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU
cores. But this relationship is not represented with the clk framework
so far.
So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the
clock producer/consumer relationship cleaner and is also useful for CPU
related frameworks like OPP to know the frequency at which the CPUs are
running.
The clock frequency provided by the driver is for each frequency domain.
We cannot get the frequency of each CPU core because, not all platforms
support per-core DCVS feature.
Also the frequency supplied by the driver is the actual frequency that
comes out of the EPSS/OSM block after the DCVS operation. This frequency is
not same as what the CPUFreq framework has set but it is the one that gets
supplied to the CPUs after throttling by LMh.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 43 +++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
Comments
On Tue, Nov 08, 2022 at 09:10:37PM +0530, Manivannan Sadhasivam wrote: > Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU > cores. But this relationship is not represented with the clk framework > so far. > > So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the > clock producer/consumer relationship cleaner and is also useful for CPU > related frameworks like OPP to know the frequency at which the CPUs are > running. > > The clock frequency provided by the driver is for each frequency domain. > We cannot get the frequency of each CPU core because, not all platforms > support per-core DCVS feature. > > Also the frequency supplied by the driver is the actual frequency that > comes out of the EPSS/OSM block after the DCVS operation. This frequency is > not same as what the CPUFreq framework has set but it is the one that gets > supplied to the CPUs after throttling by LMh. > OK now I see more info here. How different is this value from the one returned by qcom_cpufreq_hw_get() ?
Hi, On Tue, Nov 08, 2022 at 09:10:37PM +0530, Manivannan Sadhasivam wrote: > Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU > cores. But this relationship is not represented with the clk framework > so far. > > So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the > clock producer/consumer relationship cleaner and is also useful for CPU > related frameworks like OPP to know the frequency at which the CPUs are > running. > > The clock frequency provided by the driver is for each frequency domain. > We cannot get the frequency of each CPU core because, not all platforms > support per-core DCVS feature. > > Also the frequency supplied by the driver is the actual frequency that > comes out of the EPSS/OSM block after the DCVS operation. This frequency is > not same as what the CPUFreq framework has set but it is the one that gets > supplied to the CPUs after throttling by LMh. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 43 +++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 5e0598730a04..86bb11de347f 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -4,6 +4,7 @@ > */ > > #include <linux/bitfield.h> > +#include <linux/clk-provider.h> > #include <linux/cpufreq.h> > #include <linux/init.h> > #include <linux/interconnect.h> > @@ -54,6 +55,7 @@ struct qcom_cpufreq_data { > bool cancel_throttle; > struct delayed_work throttle_work; > struct cpufreq_policy *policy; > + struct clk_hw cpu_clk; > > bool per_core_dcvs; > > @@ -615,8 +617,20 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = { > .ready = qcom_cpufreq_ready, > }; > > +static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) > +{ > + struct qcom_cpufreq_data *data = container_of(hw, struct qcom_cpufreq_data, cpu_clk); > + > + return qcom_lmh_get_throttle_freq(data); > +} > + > +static const struct clk_ops qcom_cpufreq_hw_clk_ops = { > + .recalc_rate = qcom_cpufreq_hw_recalc_rate, > +}; > + > static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > { > + struct clk_hw_onecell_data *clk_data; > struct device *dev = &pdev->dev; > struct device *cpu_dev; > struct clk *clk; > @@ -659,8 +673,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > > qcom_cpufreq.soc_data = of_device_get_match_data(dev); > > + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_domains), GFP_KERNEL); > + if (!clk_data) > + return -ENOMEM; > + > + clk_data->num = num_domains; > + > for (i = 0; i < num_domains; i++) { > struct qcom_cpufreq_data *data = &qcom_cpufreq.data[i]; > + struct clk_init_data init = {}; > + const char *clk_name; > struct resource *res; > void __iomem *base; > > @@ -672,6 +694,27 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > > data->base = base; > data->res = res; > + > + /* Register CPU clock for each frequency domain */ > + clk_name = devm_kasprintf(dev, GFP_KERNEL, "qcom_cpufreq%d", i); > + init.name = clk_name; nit: 'clk_name' isn't really needed, the result of devm_kasprintf() could be assigned directly to 'init.name'. 'init' could be renamed to 'clk_init' if the purpose of using 'clk_name' is to make clear that this is the name of a clock. > + init.flags = CLK_GET_RATE_NOCACHE; > + init.ops = &qcom_cpufreq_hw_clk_ops; > + data->cpu_clk.init = &init; > + > + ret = devm_clk_hw_register(dev, &data->cpu_clk); > + if (ret < 0) { > + dev_err(dev, "Failed to register Qcom CPUFreq clock\n"); > + return ret; > + } > + > + clk_data->hws[i] = &data->cpu_clk; > + } > + > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data); > + if (ret < 0) { > + dev_err(dev, "Failed to add Qcom CPUFreq clock provider\n"); > + return ret; > } > > ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver); > -- > 2.25.1 >
On Tue, Nov 08, 2022 at 03:57:17PM +0000, Sudeep Holla wrote: > On Tue, Nov 08, 2022 at 09:10:37PM +0530, Manivannan Sadhasivam wrote: > > Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU > > cores. But this relationship is not represented with the clk framework > > so far. > > > > So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the > > clock producer/consumer relationship cleaner and is also useful for CPU > > related frameworks like OPP to know the frequency at which the CPUs are > > running. > > > > The clock frequency provided by the driver is for each frequency domain. > > We cannot get the frequency of each CPU core because, not all platforms > > support per-core DCVS feature. > > > > Also the frequency supplied by the driver is the actual frequency that > > comes out of the EPSS/OSM block after the DCVS operation. This frequency is > > not same as what the CPUFreq framework has set but it is the one that gets > > supplied to the CPUs after throttling by LMh. > > > > OK now I see more info here. How different is this value from the one > returned by qcom_cpufreq_hw_get() ? > qcom_cpufreq_hw_get() returns the frequency that got programmed by the cpufreq core. But that frequency is not necessarily the one that gets delivered to the CPU cores because the EPSS/OSM hardware block may vary the frequency after the DCVS operation. So this frequency is the final one that gets delivered to the CPU cores. Thanks, Mani > -- > Regards, > Sudeep
On Tue, Nov 08, 2022 at 06:27:36PM +0000, Matthias Kaehlcke wrote: > Hi, > > On Tue, Nov 08, 2022 at 09:10:37PM +0530, Manivannan Sadhasivam wrote: > > Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU > > cores. But this relationship is not represented with the clk framework > > so far. > > > > So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the > > clock producer/consumer relationship cleaner and is also useful for CPU > > related frameworks like OPP to know the frequency at which the CPUs are > > running. > > > > The clock frequency provided by the driver is for each frequency domain. > > We cannot get the frequency of each CPU core because, not all platforms > > support per-core DCVS feature. > > > > Also the frequency supplied by the driver is the actual frequency that > > comes out of the EPSS/OSM block after the DCVS operation. This frequency is > > not same as what the CPUFreq framework has set but it is the one that gets > > supplied to the CPUs after throttling by LMh. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/cpufreq/qcom-cpufreq-hw.c | 43 +++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > index 5e0598730a04..86bb11de347f 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > @@ -4,6 +4,7 @@ > > */ > > > > #include <linux/bitfield.h> > > +#include <linux/clk-provider.h> > > #include <linux/cpufreq.h> > > #include <linux/init.h> > > #include <linux/interconnect.h> > > @@ -54,6 +55,7 @@ struct qcom_cpufreq_data { > > bool cancel_throttle; > > struct delayed_work throttle_work; > > struct cpufreq_policy *policy; > > + struct clk_hw cpu_clk; > > > > bool per_core_dcvs; > > > > @@ -615,8 +617,20 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = { > > .ready = qcom_cpufreq_ready, > > }; > > > > +static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) > > +{ > > + struct qcom_cpufreq_data *data = container_of(hw, struct qcom_cpufreq_data, cpu_clk); > > + > > + return qcom_lmh_get_throttle_freq(data); > > +} > > + > > +static const struct clk_ops qcom_cpufreq_hw_clk_ops = { > > + .recalc_rate = qcom_cpufreq_hw_recalc_rate, > > +}; > > + > > static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > > { > > + struct clk_hw_onecell_data *clk_data; > > struct device *dev = &pdev->dev; > > struct device *cpu_dev; > > struct clk *clk; > > @@ -659,8 +673,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > > > > qcom_cpufreq.soc_data = of_device_get_match_data(dev); > > > > + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_domains), GFP_KERNEL); > > + if (!clk_data) > > + return -ENOMEM; > > + > > + clk_data->num = num_domains; > > + > > for (i = 0; i < num_domains; i++) { > > struct qcom_cpufreq_data *data = &qcom_cpufreq.data[i]; > > + struct clk_init_data init = {}; > > + const char *clk_name; > > struct resource *res; > > void __iomem *base; > > > > @@ -672,6 +694,27 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) > > > > data->base = base; > > data->res = res; > > + > > + /* Register CPU clock for each frequency domain */ > > + clk_name = devm_kasprintf(dev, GFP_KERNEL, "qcom_cpufreq%d", i); > > + init.name = clk_name; > > nit: 'clk_name' isn't really needed, the result of devm_kasprintf() could be > assigned directly to 'init.name'. 'init' could be renamed to 'clk_init' if > the purpose of using 'clk_name' is to make clear that this is the name of a > clock. > Ack. Thanks, Mani > > + init.flags = CLK_GET_RATE_NOCACHE; > > + init.ops = &qcom_cpufreq_hw_clk_ops; > > + data->cpu_clk.init = &init; > > + > > + ret = devm_clk_hw_register(dev, &data->cpu_clk); > > + if (ret < 0) { > > + dev_err(dev, "Failed to register Qcom CPUFreq clock\n"); > > + return ret; > > + } > > + > > + clk_data->hws[i] = &data->cpu_clk; > > + } > > + > > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data); > > + if (ret < 0) { > > + dev_err(dev, "Failed to add Qcom CPUFreq clock provider\n"); > > + return ret; > > } > > > > ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver); > > -- > > 2.25.1 > >
On Wed, Nov 09, 2022 at 01:19:08PM +0530, Manivannan Sadhasivam wrote: [...] > qcom_cpufreq_hw_get() returns the frequency that got programmed by the cpufreq > core. But that frequency is not necessarily the one that gets delivered to the > CPU cores because the EPSS/OSM hardware block may vary the frequency after the > DCVS operation. > > So this frequency is the final one that gets delivered to the CPU cores. > OK, thanks for the info. Just wondering if there is any issue making qcom_cpufreq_hw_get() return this value instead of all these complexity. I think the DT binding is too confusing as cpufreq-dt uses that to manage DVFS which this one uses it. If possible we should just make cpufreq_get(cpu) return the value you need and use the same where ever you need. Sorry if I am missing something obvious but I am struggling to see that.
On Wed, Nov 09, 2022 at 11:08:31AM +0000, Sudeep Holla wrote: > On Wed, Nov 09, 2022 at 01:19:08PM +0530, Manivannan Sadhasivam wrote: > > [...] > > > qcom_cpufreq_hw_get() returns the frequency that got programmed by the cpufreq > > core. But that frequency is not necessarily the one that gets delivered to the > > CPU cores because the EPSS/OSM hardware block may vary the frequency after the > > DCVS operation. > > > > So this frequency is the final one that gets delivered to the CPU cores. > > > > OK, thanks for the info. Just wondering if there is any issue making > qcom_cpufreq_hw_get() return this value instead of all these complexity. > I think the DT binding is too confusing as cpufreq-dt uses that to manage > DVFS which this one uses it. > > If possible we should just make cpufreq_get(cpu) return the value you need > and use the same where ever you need. Sorry if I am missing something obvious > but I am struggling to see that. > I don't think using the final DCVS frequency would be applicable for cpufreq core. cpufreq core sets the desired frequency in the form of index using the target_index() callback and the qcom-cpufreq-hw driver uses that index directly to select the specific entry in the hardware LUT (Look Up Table). Then with get() callback, the frequency will be returned based on the LUT index read from the hardware. In this case, the frequency is going to be static (i.e, what gets set by the cpufreq core will be the same). I believe this is what the API also expects. In the case of qcom_lmh_get_throttle_freq(), the frequency is going to be dynamic (i.e changes with every internal DCVS operation). But this is exactly what the OPP core expects with clk_get_rate() of CPU clock, so using qcom_lmh_get_throttle_freq() makes sense there. Thanks, Mani > -- > Regards, > Sudeep
On Wed, Nov 09, 2022 at 06:05:26PM +0530, Manivannan Sadhasivam wrote: > I don't think using the final DCVS frequency would be applicable for cpufreq > core. > > cpufreq core sets the desired frequency in the form of index using the > target_index() callback and the qcom-cpufreq-hw driver uses that index directly > to select the specific entry in the hardware LUT (Look Up Table). > > Then with get() callback, the frequency will be returned based on the LUT index > read from the hardware. In this case, the frequency is going to be static > (i.e, what gets set by the cpufreq core will be the same). I believe this is > what the API also expects. > I guessed so and hence thought of asking. Is the cpufreq_get() API expected to return something close to what was set or is it expected to return the real set h/w value if and when possible. I wanted to confirm if that is the expectation from the cpufreq core or is it just the way qcom cpufreq-hw driver(probably many others too) work today. > In the case of qcom_lmh_get_throttle_freq(), the frequency is going to be > dynamic (i.e changes with every internal DCVS operation). But this is exactly > what the OPP core expects with clk_get_rate() of CPU clock, so using > qcom_lmh_get_throttle_freq() makes sense there. > OK, the reason I ask is that IIRC the ACPI CPPC driver might get the exact delivered frequency rather than something based on the set value, so it shouldn't be a requirement but I may be wrong. Viresh, thoughts ? -- Regards, Sudeep
On 09-11-22, 16:47, Sudeep Holla wrote: > Is the cpufreq_get() API expected > to return something close to what was set or is it expected to return the > real set h/w value if and when possible. The real frequency the hardware is running at.
On Mon, Nov 14, 2022 at 08:46:58AM +0530, Viresh Kumar wrote: > On 09-11-22, 16:47, Sudeep Holla wrote: > > Is the cpufreq_get() API expected > > to return something close to what was set or is it expected to return the > > real set h/w value if and when possible. > > The real frequency the hardware is running at. > Oh... In that case, qcom-cpufreq-hw driver is not returning the real frequency but instead whatever set by the cpufreq core previously using target_index(). Should I fix it too in the next version of this series? Thanks, Mani > -- > viresh
On 14-11-22, 11:55, Manivannan Sadhasivam wrote: > On Mon, Nov 14, 2022 at 08:46:58AM +0530, Viresh Kumar wrote: > > On 09-11-22, 16:47, Sudeep Holla wrote: > > > Is the cpufreq_get() API expected > > > to return something close to what was set or is it expected to return the > > > real set h/w value if and when possible. > > > > The real frequency the hardware is running at. > > > > Oh... In that case, qcom-cpufreq-hw driver is not returning the real frequency > but instead whatever set by the cpufreq core previously using target_index(). > > Should I fix it too in the next version of this series? Yes.
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 5e0598730a04..86bb11de347f 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -4,6 +4,7 @@ */ #include <linux/bitfield.h> +#include <linux/clk-provider.h> #include <linux/cpufreq.h> #include <linux/init.h> #include <linux/interconnect.h> @@ -54,6 +55,7 @@ struct qcom_cpufreq_data { bool cancel_throttle; struct delayed_work throttle_work; struct cpufreq_policy *policy; + struct clk_hw cpu_clk; bool per_core_dcvs; @@ -615,8 +617,20 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = { .ready = qcom_cpufreq_ready, }; +static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) +{ + struct qcom_cpufreq_data *data = container_of(hw, struct qcom_cpufreq_data, cpu_clk); + + return qcom_lmh_get_throttle_freq(data); +} + +static const struct clk_ops qcom_cpufreq_hw_clk_ops = { + .recalc_rate = qcom_cpufreq_hw_recalc_rate, +}; + static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) { + struct clk_hw_onecell_data *clk_data; struct device *dev = &pdev->dev; struct device *cpu_dev; struct clk *clk; @@ -659,8 +673,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) qcom_cpufreq.soc_data = of_device_get_match_data(dev); + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_domains), GFP_KERNEL); + if (!clk_data) + return -ENOMEM; + + clk_data->num = num_domains; + for (i = 0; i < num_domains; i++) { struct qcom_cpufreq_data *data = &qcom_cpufreq.data[i]; + struct clk_init_data init = {}; + const char *clk_name; struct resource *res; void __iomem *base; @@ -672,6 +694,27 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) data->base = base; data->res = res; + + /* Register CPU clock for each frequency domain */ + clk_name = devm_kasprintf(dev, GFP_KERNEL, "qcom_cpufreq%d", i); + init.name = clk_name; + init.flags = CLK_GET_RATE_NOCACHE; + init.ops = &qcom_cpufreq_hw_clk_ops; + data->cpu_clk.init = &init; + + ret = devm_clk_hw_register(dev, &data->cpu_clk); + if (ret < 0) { + dev_err(dev, "Failed to register Qcom CPUFreq clock\n"); + return ret; + } + + clk_data->hws[i] = &data->cpu_clk; + } + + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data); + if (ret < 0) { + dev_err(dev, "Failed to add Qcom CPUFreq clock provider\n"); + return ret; } ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver);