Received-SPF: pass (google.com: domain of linux-kernel+bounces-178516-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <8b690d8f-986a-7806-b274-523b351e6e55@quicinc.com>
Date: Tue, 14 May 2024 15:05:59 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v12 6/6] PCI: qcom: Add OPP support to scale performance
Content-Language: en-US
To: Manivannan Sadhasivam <mani@kernel.org>
CC: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring
	<robh@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Lorenzo Pieralisi
	<lpieralisi@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?=
	<kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>, <johan+linaro@kernel.org>,
        <bmasney@redhat.com>, <djakov@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
        <vireshk@kernel.org>, <quic_vbadigan@quicinc.com>,
        <quic_skananth@quicinc.com>, <quic_nitegupt@quicinc.com>,
        <quic_parass@quicinc.com>, <krzysztof.kozlowski@linaro.org>
References: <20240427-opp_support-v12-0-f6beb0a1f2fc@quicinc.com>
 <20240427-opp_support-v12-6-f6beb0a1f2fc@quicinc.com>
 <20240430052613.GD3301@thinkpad>
 <8b213eba-7ab6-ae9c-7683-937a9d6aaf08@quicinc.com>
 <20240514092838.GE2463@thinkpad>
From: Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
In-Reply-To: <20240514092838.GE2463@thinkpad>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 5/14/2024 2:58 PM, Manivannan Sadhasivam wrote:
> On Thu, May 09, 2024 at 09:21:55PM +0530, Krishna Chaitanya Chundru wrote:
>>
>>
>> On 4/30/2024 10:56 AM, Manivannan Sadhasivam wrote:
>>> On Sat, Apr 27, 2024 at 07:22:39AM +0530, 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@linaro.org>
>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-qcom.c | 81 ++++++++++++++++++++++++++++------
>>>>    1 file changed, 67 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> index 465d63b4be1c..40c875c518d8 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>
>>> [...]
>>>
>>>> @@ -1661,6 +1711,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>>>>    		ret = icc_disable(pcie->icc_cpu);
>>>>    		if (ret)
>>>>    			dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n", ret);
>>>> +
>>>> +		if (!pcie->icc_mem)
>>>> +			dev_pm_opp_set_opp(pcie->pci->dev, NULL);
>>>
>>> At the start of the suspend, there is a call to icc_set_bw() for PCIe-MEM path.
>>> Don't you want to update it too?
>>>
>>> - Mani
>>>
>> if opp is supported we just need to call dev_pm_opp_set_opp() only once
>> which will take care for both PCIe-MEM & CPU-PCIe path.
>> so we are not adding explicitly there.
> 
> No, I was asking you why you are not adding a check for the existing
> icc_set_bw() at the start like you were doing elsewhere.
>Got it I will add the check in the next patch series.
- Krishna Chaitanya.
> - Mani
>