Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <30aa1bdc-8b77-1077-a0a5-258ea0d27f71@linaro.org>
Date:   Sun, 3 Apr 2022 19:46:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [RFC PATCH 4/4] ufs: set power domain performance state when
 scaling gears
Content-Language: en-US
To:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Nitin Rawat <quic_nitirawa@quicinc.com>,
        Asutosh Das <quic_asutoshd@quicinc.com>
Cc:     Andy Gross <agross@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Avri Altman <avri.altman@wdc.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Bean Huo <beanhuo@micron.com>,
        Bart Van Assche <bvanassche@acm.org>,
        Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        Taniya Das <tdas@codeaurora.org>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-scsi@vger.kernel.org
References: <20220401145820.1003826-1-krzysztof.kozlowski@linaro.org>
 <20220401145820.1003826-5-krzysztof.kozlowski@linaro.org>
 <YkdWvVVp4RloGjkC@ripper>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <YkdWvVVp4RloGjkC@ripper>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 01/04/2022 21:47, Bjorn Andersson wrote:
> On Fri 01 Apr 07:58 PDT 2022, Krzysztof Kozlowski wrote:
> 
>> Scaling gears requires not only scaling clocks, but also voltage levels,
>> e.g. via performance states.
>>
>> USe the provided OPP table, to set proper OPP frequency which through
>> required-opps will trigger performance state change.
>>
> 
> This looks quite nice! Just two questions about the path looking forward.
> 
> If we where to extend the opp core to allow specifying the clock rate
> for some N first clocks (similar to how e.g. regulators are handled) it
> seems possible to extend this to replace the freq-table property as
> well. Would you agree?

Yes, although that might be trickier. The frequency is a key. I'll take
a look whether it could be changed to multiple values like the voltage.

> 
> 
> The other missing required feature (in this area) from the upstream UFS
> driver is the ability of voting for interconnect bandwidth. Based on
> your path it would be trivial to specify different values for the votes
> for each speed, but looking at downstream [1] (each row represents the
> vote for the two paths in KB/s) indicates a more complex relationship
> between gear and voted bandwidth.
> 
> This was the reason I suggested that perhaps we need to key the
> opp-table based on the gear? But I don't think there would be any issue
> detecting this in runtime...
> 
> [1] https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio.dtsi#L1982

It should be doable with current bindings, assuming that gear is some
imaginary frequency. We have two interconnects for UFS (the DDR and CPU)
and OPP bindings allow to specify opp-peak-kBps and opp-avg-kBps for all
of interconnects. IOW, the opp-peak-kBps will have two values and
opp-avg-kBps as well.

What would be still missing is scaling clocks.

interconnects = <&ddr>, <&cpu>
interconnect-names = "ufs-ddr", "cpu-ufs";
opp-table {
  // gear 1 or some core clock frequency?
  opp-1 {
    opp-hz = /bits/ 64 <75000000>, <0>, <0>, <75000000> ....;
    opp-avg-kBps = <922 1000>;
    opp-peak-kBps = <0 0>;
    required-opps = <&rpmpd_opp_low_svs>;
  }
}

arch/arm64/boot/dts/qcom/sdm630.dtsi already uses it.

I think still the problem is scaling of multiple clocks, depending on
the gear.

Best regards,
Krzysztof