linux-kernel - Re: [PATCH v14 4/4] PCI: qcom: Add OPP support to scale performance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <650a391a-cb2b-7570-5e0b-adaf7f20151e@quicinc.com>
Date: Wed, 19 Jun 2024 20:01:04 +0530
From: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konrad.dybcio@...aro.org>,
        Rob Herring <robh@...nel.org>,
        "Krzysztof
 Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Manivannan Sadhasivam
	<manivannan.sadhasivam@...aro.org>,
        Lorenzo Pieralisi
	<lpieralisi@...nel.org>,
        Krzysztof Wilczyński
	<kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, <johan+linaro@...nel.org>,
        <bmasney@...hat.com>, <djakov@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <vireshk@...nel.org>, <quic_vbadigan@...cinc.com>,
        <quic_skananth@...cinc.com>, <quic_nitegupt@...cinc.com>,
        <quic_parass@...cinc.com>, <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v14 4/4] PCI: qcom: Add OPP support to scale performance



On 6/14/2024 6:12 PM, Ilpo Järvinen wrote:
> On Sun, 9 Jun 2024, Krishna chaitanya chundru wrote:
> 
>> QCOM Resource Power Manager-hardened (RPMh) is a hardware block which
>> maintains hardware state of a regulator by performing max aggregation of
>> the requests made by all of the clients.
>>
>> PCIe controller can operate on different RPMh performance state of power
>> domain based on the speed of the link. And this performance state varies
>> from target to target, like some controllers support GEN3 in NOM (Nominal)
>> voltage corner, while some other supports GEN3 in low SVS (static voltage
>> scaling).
>>
>> The SoC can be more power efficient if we scale the performance state
>> based on the aggregate PCIe link bandwidth.
>>
>> Add Operating Performance Points (OPP) support to vote for RPMh state based
>> on the aggregate link bandwidth.
>>
>> OPP can handle ICC bw voting also, so move ICC bw voting through OPP
>> framework if OPP entries are present.
>>
>> As we are moving ICC voting as part of OPP, don't initialize ICC if OPP
>> is supported.
>>
>> Before PCIe link is initialized vote for highest OPP in the OPP table,
>> so that we are voting for maximum voltage corner for the link to come up
>> in maximum supported speed.
>>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@...cinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 93 +++++++++++++++++++++++++++-------
>>   1 file changed, 75 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index ff1d891c8b9a..296e2d5036f6 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/init.h>
>>   #include <linux/of.h>
>>   #include <linux/pci.h>
>> +#include <linux/pm_opp.h>
>>   #include <linux/pm_runtime.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/phy/pcie.h>
>> @@ -1404,15 +1405,13 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
>>   	return 0;
>>   }
>>   
>> -static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
>> +static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
>>   {
>> +	int speed, width, ret, freq_mbps;
>>   	struct dw_pcie *pci = pcie->pci;
>> +	unsigned long freq_kbps;
>> +	struct dev_pm_opp *opp;
>>   	u32 offset, status;
>> -	int speed, width;
>> -	int ret;
>> -
>> -	if (!pcie->icc_mem)
>> -		return;
>>   
>>   	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>>   	status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
>> @@ -1424,10 +1423,26 @@ static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
>>   	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
>>   	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
>>   
>> -	ret = icc_set_bw(pcie->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
>> -	if (ret) {
>> -		dev_err(pci->dev, "Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
>> -			ret);
>> +	if (pcie->icc_mem) {
>> +		ret = icc_set_bw(pcie->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
>> +		if (ret) {
>> +			dev_err(pci->dev, "Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
>> +				ret);
>> +		}
>> +	} else {
>> +		freq_mbps = pcie_link_speed_to_mbps(pcie_link_speed[speed]);
>> +		if (freq_mbps < 0)
>> +			return;
>> +
>> +		freq_kbps = freq_mbps * 1000;
> 
> Use define from units.h instead of literal.
> 
>> +		opp = dev_pm_opp_find_freq_exact(pci->dev, freq_kbps * width, true);
>> +		if (!IS_ERR(opp)) {
>> +			ret = dev_pm_opp_set_opp(pci->dev, opp);
>> +			if (ret)
>> +				dev_err(pci->dev, "Failed to set OPP for freq (%ld): %d\n",
>> +					freq_kbps * width, ret);
> 
> Make width unsigned and use %lu ?
> 
Ack to all comments. I will update them in next patch series.

- Krishna chaitanya