Re: [PATCH V2] arm64: dts: qcom: sc7180: Add bandwidth votes for eMMC and SDcard
From: sbhanu
Date: Tue Jul 28 2020 - 07:19:40 EST
On 2020-07-28 00:40, Matthias Kaehlcke wrote:
Hi,
On Mon, Jul 27, 2020 at 12:20:38PM +0530, sbhanu@xxxxxxxxxxxxxx wrote:
On 2020-07-24 22:40, Matthias Kaehlcke wrote:
> Hi Shaik,
>
> On Tue, Jul 21, 2020 at 04:16:21PM +0530, Shaik Sajida Bhanu wrote:
> > From: Pradeep P V K <ppvk@xxxxxxxxxxxxxx>
> >
> > Add the bandwidth domain supporting performance state and
> > the corresponding OPP tables for the sdhc device on sc7180.
> >
> > Signed-off-by: Pradeep P V K <ppvk@xxxxxxxxxxxxxx>
> > Signed-off-by: Shaik Sajida Bhanu <sbhanu@xxxxxxxxxxxxxx>
> > ---
> >
> > Changes since V1:
> > - Incorporated review comments by Bjorn Andersson.
> > ---
> > arch/arm64/boot/dts/qcom/sc7180.dtsi | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > index 68f9894..d78a066 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > @@ -684,6 +684,9 @@
> > clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> > <&gcc GCC_SDCC1_AHB_CLK>;
> > clock-names = "core", "iface";
> > + interconnects = <&aggre1_noc MASTER_EMMC &mc_virt SLAVE_EBI1>,
> > + <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_EMMC_CFG>;
> > + interconnect-names = "sdhc-ddr","cpu-sdhc";
> > power-domains = <&rpmhpd SC7180_CX>;
> > operating-points-v2 = <&sdhc1_opp_table>;
> >
> > @@ -704,11 +707,15 @@
> > opp-100000000 {
> > opp-hz = /bits/ 64 <100000000>;
> > required-opps = <&rpmhpd_opp_low_svs>;
> > + opp-peak-kBps = <100000 100000>;
> > + opp-avg-kBps = <100000 50000>;
> > };
> >
> > opp-384000000 {
> > opp-hz = /bits/ 64 <384000000>;
> > required-opps = <&rpmhpd_opp_svs_l1>;
> > + opp-peak-kBps = <600000 900000>;
> > + opp-avg-kBps = <261438 300000>;
> > };
> > };
> > };
> > @@ -2476,6 +2483,10 @@
> > clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> > <&gcc GCC_SDCC2_AHB_CLK>;
> > clock-names = "core", "iface";
> > +
> > + interconnects = <&aggre1_noc MASTER_SDCC_2 &mc_virt SLAVE_EBI1>,
> > + <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_SDCC_2>;
> > + interconnect-names = "sdhc-ddr","cpu-sdhc";
> > power-domains = <&rpmhpd SC7180_CX>;
> > operating-points-v2 = <&sdhc2_opp_table>;
> >
> > @@ -2489,11 +2500,15 @@
> > opp-100000000 {
> > opp-hz = /bits/ 64 <100000000>;
> > required-opps = <&rpmhpd_opp_low_svs>;
> > + opp-peak-kBps = <160000 100000>;
> > + opp-avg-kBps = <80000 50000>;
> > };
> >
> > opp-202000000 {
> > opp-hz = /bits/ 64 <202000000>;
> > required-opps = <&rpmhpd_opp_svs_l1>;
> > + opp-peak-kBps = <200000 120000>;
> > + opp-avg-kBps = <100000 60000>;
> > };
> > };
> > };
>
> Does the sdhci-msm driver actually have BW scaling support at this
> point?
>
yes
> There is commit 4ece9795be56 ("mmc: sdhci-msm: Add interconnect
> bandwidth scaling support"), whose commit message says "make sure
> interconnect driver is ready before handling interconnect scaling.".
>
> I haven't seen any patch adding the scaling support (supposedly by
> adding dev_pm_opp_set_bw() calls?). Did I miss it? If not it seems
> it would make sense to post it in a series together with this patch,
> as far as I can tell this patch alone does nothing in practical terms.
>
> grep sdhc /sys/kernel/debug/interconnect/interconnect_summary
> 8804000.sdhci 0 0 0
> 7c4000.sdhci 0 0 0
> 7c4000.sdhci 0 0 0
> 8804000.sdhci 0 0 0
> ...
"mmc: sdhci-msm: Use OPP API to set clk/perf
state"(https://lkml.org/lkml/2020/4/8/425) and "mmc: sdhci-msm: Add
interconnect bandwidth scaling
support"(https://lkml.org/lkml/2020/3/12/60)
with these two patches scaling will be supported for sdhci-msm driver.
Are you testing with exactly these patches or with the ones that landed
upstream? At least the second one changed substantially
the values in grep sdhc
/sys/kernel/debug/interconnect/interconnect_summary will be zero
during
device is in suspend state...
Yes, I forgot to mention that I started MMC IO before looking at
'interconnect_summary'.
and the values in grep sdhc
/sys/kernel/debug/interconnect/interconnect_summary during device in
resume
state will be like the following::
cicalhost / # cat /sys/kernel/debug/interconnect/interconnect_summary
| grep
sdh
8804000.sdhci 0 60000 120000
7c4000.sdhci 0 300000 900000
7c4000.sdhci 0 300000 900000
8804000.sdhci 0 60000 120000
8804000.sdhci 0 100000 200000
7c4000.sdhci 0 261438 600000
8804000.sdhci 0 60000 120000
On my system the bandwidth is never set:
3.590152] sdhci_msm 7c4000.sdhci: DBG: old/new frequencies (384000000
Hz) are same, nothing to do
https://elixir.bootlin.com/linux/v5.7.8/source/drivers/opp/core.c#L847
This happens every time, even after the bandwith is set to 0. The
problem
seems to be that opp_table->clk doesn't change for target_freq = 0.
My system is based on v5.4, so it is possible that my kernel is missing
some
relevant patch from upstream.
Hi matthias,
In order to aviod confusion this patch is continuation of the below
patch::
"mmc: sdhci-msm: Add interconnect bandwidth scaling support"
(https://lkml.org/lkml/2020/6/9/160).
Thanks,
sajida