devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jun Nie <jun.nie@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
	myungjoo.ham@samsung.com, kyungmin.park@samsung.com,
	cw00.choi@samsung.com, linux-pm@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] PM / devfreq: qcom: Introduce CCI devfreq driver
Date: Thu, 2 Feb 2023 20:55:04 +0800	[thread overview]
Message-ID: <CABymUCMJdRXG3AcLeS18JFuYmCv1kw=rJNkCv8sL7AjPD4ZR+A@mail.gmail.com> (raw)
In-Reply-To: <CAA8EJpqyqC5D+O=KJnuZnWN4BwBOKcquN11nJfEp2WMSmJobBg@mail.gmail.com>

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2023年2月1日周三 21:41写道:
>
> On Wed, 1 Feb 2023 at 13:46, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
> >
> > On 01/02/2023 11:32, Dmitry Baryshkov wrote:
> > > On 01/02/2023 10:02, Jun Nie wrote:
> > >> Cache Coherent Interconnect (CCI) is used by some Qualcomm SoCs. This
> > >> driver is introduced so that its freqency can be adjusted. And regulator
> > >> associated with opp table can be also adjusted accordingly which is
> > >> shared with cpu cluster.
> > >>
> > >> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > >> ---
> > >>   drivers/devfreq/Kconfig    |   9 +++
> > >>   drivers/devfreq/Makefile   |   1 +
> > >>   drivers/devfreq/qcom-cci.c | 162 +++++++++++++++++++++++++++++++++++++
> > >>   3 files changed, 172 insertions(+)
> > >>   create mode 100644 drivers/devfreq/qcom-cci.c
> > >
> > > Could you please describe in some additional details what are you trying
> > > to achieve? Should the CCI frequency be scaled manually or does it
> > > follow the cluster frequency? Do clusters vote on the CCI frequency?
> > >
> > > I'm inclined to ask if it is possible to shift this to the cpufreq OPP
> > > tables?
> > >
> >
> > Might not be so easy to just append CCI opps to the cluster frequency opps
> >
> >                  cci_cache: qcom,cci {
> >                          compatible = "qcom,msm8939-cci";
> >                          clock-names = "devfreq_clk";
> >                          clocks = <&apcs2>;
> >                          governor = "cpufreq";
> >                          operating-points-v2 = <&cci_opp_table>;
> >                          power-domains = <&cpr>;
> >                          power-domain-names = "cpr";
> >                          nvmem-cells = <&cpr_efuse_speedbin_pvs>;
> >                          nvmem-cell-names = "cpr_efuse_speedbin_pvs";
> >                  };
> >
> >                  devfreq-cpufreq {
> >                          cci-cpufreq {
> >                                  target-dev = <&cci_cache>;
> >                                  cpu-to-dev-map-0 =
> >                                          <  200000  200000000 >,
> >                                          <  345600  200000000 >,
> >                                          <  400000  200000000 >,
> >                                          <  533330  297600000 >,
> >                                          <  800000  297600000 >,
> >                                          <  960000  297600000 >,
> >                                          < 1113600  297000000 >,
> >                                          < 1344000  595200000 >,
> >                                          < 1459200  595200000 >,
> >                                          < 1497600  595200000 >,
> >                                          < 1651200  595200000 >;
> >                                  cpu-to-dev-map-4 =
> >                                          <  200000 200000000 >,
> >                                          <  249600 200000000 >,
> >                                          <  499200 297600000 >,
> >                                          <  800000 297600000 >,
> >                                          <  998400 595200000 >,
> >                                          < 1113600 595200000 >;
>
> These should map to existing opp entries.
>
> I ended up doing the interconnect driver that maps a clock to the
> interconnect. Then I can use it in the cpu opp tables.
>
> >                          };
> >                  };
> >
> >          cci_opp_table: cci-opp-table {
> >                  compatible = "operating-points-v2";
> >
> >                  opp-200000000 {
> >                          opp-hz = /bits/ 64 <200000000>;
> >                          opp-supported-hw = <0x3f>;
> >                          required-opps = <&cpr_opp3>;
>
> And these should probably map to max(cpu's CPR opp, CCI's CPR opp).

The plan is opp framework to handle it when CPU opp requires both cpr
power domain
opp and CCI opp. While CCI opp will also requires specific cpr opp. Because CPU
have a opp match table per pvs/speed, while CCI has another match
table, it seems
 impossible to write the cpr opp requirements in the code statically.

>
> >                  };
> >
> >                  opp-297600000 {
> >                          opp-hz = /bits/ 64 <297600000>;
> >                          opp-supported-hw = <0x3f>;
> >                          required-opps = <&cpr_opp12>;
> >                  };
> >
> >                  opp-400000000-cpr14 {
> >                          opp-hz = /bits/ 64 <400000000>;
> >                          opp-supported-hw = <0x1>;
> >                          required-opps = <&cpr_opp14>;
> >                  };
> >
> >                  opp-400000000-cpr15 {
> >                          opp-hz = /bits/ 64 <400000000>;
> >                          opp-supported-hw = <0x3e>;
> >                          required-opps = <&cpr_opp15>;
> >                  };
> >
> >                  opp-595200000 {
> >                          opp-hz = /bits/ 64 <595200000>;
> >                          opp-supported-hw = <0x3f>;
> >                          required-opps = <&cpr_opp17>;
> >                  };
> >          };
> >
> >
> >          cpr_opp_table: cpr-opp-table {
> >                  compatible = "operating-points-v2-qcom-level";
> >
> >                  cpr_opp1: opp1 {
> >                          opp-hz = /bits/ 64 <200000000>;
> >                          opp-level = <1>;
> >                          qcom,opp-fuse-level = <1>;
> >                  };
> >                  cpr_opp2: opp2 {
> >                          opp-hz = /bits/ 64 <345600000>;
> >                          opp-level = <2>;
> >                          qcom,opp-fuse-level = <1>;
> >                  };
> >                  cpr_opp3: opp3 {
> >                          opp-hz = /bits/ 64 <400000000>;
> >                          opp-level = <3>;
> >                          qcom,opp-fuse-level = <1>;
> >                  };
> >                  cpr_opp4: opp4 {
> >                          opp-hz = /bits/ 64 <422400000>;
> >                          opp-level = <4>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp5: opp5 {
> >                          opp-hz = /bits/ 64 <499200000>;
> >                          opp-level = <5>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp6: opp6 {
> >                          opp-hz = /bits/ 64 <533330000>;
> >                          opp-level = <6>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp7: opp7 {
> >                          opp-hz = /bits/ 64 <652800000>;
> >                          opp-level = <7>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp8: opp8 {
> >                          opp-hz = /bits/ 64 <729600000>;
> >                          opp-level = <8>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp9: opp9 {
> >                          opp-hz = /bits/ 64 <800000000>;
> >                          opp-level = <9>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp10: opp10 {
> >                          opp-hz = /bits/ 64 <806400000>;
> >                          opp-level = <10>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp11: opp11 {
> >                          opp-hz = /bits/ 64 <883200000>;
> >                          opp-level = <11>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >                  cpr_opp12: opp12 {
> >                          opp-hz = /bits/ 64 <960000000>;
> >                          opp-level = <12>;
> >                          qcom,opp-fuse-level = <2>;
> >                  };
> >          };
> >
> > ---
> > bod
>
>
>
> --
> With best wishes
> Dmitry

  parent reply	other threads:[~2023-02-02 12:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01  8:02 [PATCH 1/2] dt-bindings: interconnect: Add Qualcomm CCI dt-bindings Jun Nie
2023-02-01  8:02 ` [PATCH 2/2] PM / devfreq: qcom: Introduce CCI devfreq driver Jun Nie
2023-02-01  8:48   ` Krzysztof Kozlowski
2023-02-01 15:15     ` Jun Nie
2023-02-01 10:27   ` Bryan O'Donoghue
2023-02-01 11:02   ` Chanwoo Choi
2023-02-01 15:17     ` Jun Nie
2023-02-01 17:18       ` Dmitry Baryshkov
2023-02-01 11:32   ` Dmitry Baryshkov
2023-02-01 11:46     ` Bryan O'Donoghue
2023-02-01 13:41       ` Dmitry Baryshkov
2023-02-01 14:45         ` Bryan O'Donoghue
2023-02-01 14:58           ` Dmitry Baryshkov
2023-02-01 15:17             ` Bryan O'Donoghue
2023-02-01 17:12               ` Dmitry Baryshkov
2023-02-01 17:16                 ` Bryan O'Donoghue
2023-02-01 17:19                   ` Dmitry Baryshkov
2023-02-02 12:55         ` Jun Nie [this message]
2023-02-01 15:23     ` Jun Nie
2023-02-01  8:43 ` [PATCH 1/2] dt-bindings: interconnect: Add Qualcomm CCI dt-bindings Krzysztof Kozlowski
2023-02-02  9:29   ` Jun Nie
2023-02-02  9:42     ` Krzysztof Kozlowski
2023-02-03  3:45       ` Jun Nie
2023-02-03  7:06         ` Krzysztof Kozlowski
2023-02-01 10:46 ` Chanwoo Choi
2023-02-01 14:23 ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABymUCMJdRXG3AcLeS18JFuYmCv1kw=rJNkCv8sL7AjPD4ZR+A@mail.gmail.com' \
    --to=jun.nie@linaro.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=cw00.choi@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).