Re: [PATCH V2] arm64: dts: qcom: sc7180: Add bandwidth votes for eMMC and SDcard

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux