Received: by 2002:a05:6358:16cc:b0:ea:6187:17c9 with SMTP id r12csp865954rwl; Thu, 5 Jan 2023 05:48:02 -0800 (PST) X-Google-Smtp-Source: AMrXdXvP7M9HE0HHO8JGF83yaDGck4ZTOcyjFkHGiwPvCnKZop3Ov+7rjDC6W3382x9IRs21/5uk X-Received: by 2002:a05:6a20:1386:b0:9d:efbf:814b with SMTP id w6-20020a056a20138600b0009defbf814bmr101404605pzh.20.1672926482381; Thu, 05 Jan 2023 05:48:02 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672926482; cv=none; d=google.com; s=arc-20160816; b=S7xXPTpavzpy6fpqDj+FYlHy+M6ABiJovZM2rn+A3qkPYEHBn0+wgftESNX4d43Lia hS/BbZivl2PnFGXqivsYzt+7sN1OzHWZuEHnUo5lIRAS/dVN7dgW43Slfcn2jt18Fxy9 LX4Ur80xWRZqUmgxgkBMYXUz8QbIdsYbv0GpPc33yL0R8JTNuMZ2kX6vd9aa5E+3UT8A k+3vGUIVFGe3l+mTtFjWGaFqz875nwrgIjaadJpqluU+LMA+rKjP/GCRDmVslW0YA2nX Quo4HEvb6d2p60kWaeQCIL/MMYUfefCd3nd7y9MkOyxNsjYQfCj0sWVqZDtM6he8JGxs c2hw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=AUq/fH3433jj5mBTye1N9Y+4Kwha1VQSzZyDmC40ABs=; b=BrPxVXeKEJ4Jd7fsE5hUMi0rmi6EJikxd6Cq/KuTbz1kwPiIGg/ElRR+mPOsFRK4pN 6wd3m/+TaYSm3DzNG8Wsa7F2JNE+pUCvDn0tJ/KN7xGlD/Ci7CxIpmXbObzJWSHckj5V AyfC/OaOsEJW2+1VNe4s4BLJ3mMblVAYr2Cby4e2Zg8jDXGZVyUIfe/JsZmkBr3ozKwS 323YNw6R4d1KQc2xdJBSoWX39nxYYBsraz9ylBQyeTTT10XG3WHvXQBJ2wj0n4WFpKyp ch+0XU8rxsUxkROeAWfAV+oKC53KuQGb1nrNkarfExeT6/XXFxKObjVwr9coITBrlHXa lggw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=qqi1KR0C; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 12-20020a63010c000000b004a20e55b475si13096404pgb.294.2023.01.05.05.47.54; Thu, 05 Jan 2023 05:48:02 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=qqi1KR0C; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231876AbjAENhH (ORCPT + 58 others); Thu, 5 Jan 2023 08:37:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52948 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230306AbjAENgt (ORCPT ); Thu, 5 Jan 2023 08:36:49 -0500 Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0062FEAF for ; Thu, 5 Jan 2023 05:36:47 -0800 (PST) Received: by mail-pj1-x1029.google.com with SMTP id v13-20020a17090a6b0d00b00219c3be9830so2028467pjj.4 for ; Thu, 05 Jan 2023 05:36:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=AUq/fH3433jj5mBTye1N9Y+4Kwha1VQSzZyDmC40ABs=; b=qqi1KR0CwVwPialeP/vVz4H2BrZcPEUSCREGYjXQQShJCMoNB0+HQeVXeRubvB1e2e x2RAYpDlaRt3BOKkEyV14HcAeIaAWx9pcf1LhlN2nESIhE581wZYE/TT2RnLx9nQJEj9 1DWbG/O/NH/gLFa1WyB2drG7aqQBPh2NE0AaVKlgVviZmocgRrXY/Wx1SO2+kfvmDVf8 4pquiqpJtZbvh93ky60+1qo/px80CmlvMKaIR14dSQ8HzpjufuHuwpZBJEsxQ865rzGB Tcbkn3pkreOCnOvb/5lpwisXCQvI3NXI1JRIMU45tfX6TjlER6Sbl0OSTg1ytho4tyRC u5NQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=AUq/fH3433jj5mBTye1N9Y+4Kwha1VQSzZyDmC40ABs=; b=cK5ZxuAR+FYWx7/j9Th8xBeTfUAIYrRmy8AOvxn+rl5OqBAKrmmQxz32AWOSv0Rk0J nnEREWQxlyu66WOCynsZSnP/EvoFyuOCHi2VO+wIvyGWSlkZkcx3iVbQ4eBnFMKP8l7q nL2Xuz6rX/MRHxVwY8yRBBS7fVVYEFN0IHbfy0/4Rx/6AcIgG/JvYVBMYK1cEz8RsPFx cmZ0vWP0fLyQ75Nt6c4OuXmud8GpCQx9FOTlfxL7xtqKdlVzA3qIQvufpNYSoh6l387O 53C8Jgcpc5/YvMwFeh+sYNqNcrm3dulDpOb/AO5R/g3ydUxuXoTuZiyGelQIL/pTxckU o36w== X-Gm-Message-State: AFqh2kpt1OSUA6mGllCgyz3+NuflIUyNNk8Z4y8GlW+o7Ym/NS0AxXJP r/XaPB5okMijm/1dFv0/tYuT X-Received: by 2002:a17:903:110d:b0:189:89e2:5406 with SMTP id n13-20020a170903110d00b0018989e25406mr92514518plh.24.1672925807368; Thu, 05 Jan 2023 05:36:47 -0800 (PST) Received: from thinkpad ([27.111.75.153]) by smtp.gmail.com with ESMTPSA id i6-20020a170902c94600b00189651e5c26sm26132627pla.236.2023.01.05.05.36.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Jan 2023 05:36:46 -0800 (PST) Date: Thu, 5 Jan 2023 19:06:39 +0530 From: Manivannan Sadhasivam To: Dhruva Gole Cc: lpieralisi@kernel.org, robh@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org, kw@linux.com, bhelgaas@google.com, linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, quic_krichai@quicinc.com, johan+linaro@kernel.org, steev@kali.org Subject: Re: [PATCH 1/1] PCI: qcom: Add support for system suspend and resume Message-ID: <20230105133639.GC4463@thinkpad> References: <20230103074907.12784-1-manivannan.sadhasivam@linaro.org> <20230103074907.12784-2-manivannan.sadhasivam@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 03, 2023 at 04:46:11PM +0530, Dhruva Gole wrote: > > > On 03/01/23 13:19, Manivannan Sadhasivam wrote: > > During the system suspend, vote for minimal interconnect bandwidth and > > also turn OFF the resources like clock and PHY if there are no active > > devices connected to the controller. For the controllers with active > > devices, the resources are kept ON as removing the resources will > > trigger access violation during the late end of suspend cycle as kernel > > tries to access the config space of PCIe devices to mask the MSIs. > > > > Also, it is not desirable to put the link into L2/L3 state as that > > implies VDD supply will be removed and the devices may go into powerdown > > state. This will affect the lifetime of storage devices like NVMe. > > > > And finally, during resume, turn ON the resources if the controller was > > truly suspended (resources OFF) and update the interconnect bandwidth > > based on PCIe Gen speed. > > > > Suggested-by: Krishna chaitanya chundru > > Signed-off-by: Manivannan Sadhasivam > > --- > > Nice to have another driver added to the list of system suspend > support! > > Acked-by: Dhruva Gole > > > drivers/pci/controller/dwc/pcie-qcom.c | 52 ++++++++++++++++++++++++++ > > 1 file changed, 52 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > index 5696e327795b..48810f1f2dba 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > @@ -227,6 +227,7 @@ struct qcom_pcie { > > struct gpio_desc *reset; > > struct icc_path *icc_mem; > > const struct qcom_pcie_cfg *cfg;qcom_pcie_icc_update > > + bool suspended; > > }; > > #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) > > @@ -1835,6 +1836,52 @@ static int qcom_pcie_remove(struct platform_device *pdev) > > return 0; > > } > > +static int qcom_pcie_suspend_noirq(struct device *dev) > > +{ > > + struct qcom_pcie *pcie = dev_get_drvdata(dev); > > + int ret; > > + > > + ret = icc_set_bw(pcie->icc_mem, 0, 0); > > + if (ret) { > > + dev_err(pcie->pci->dev, "Failed to set interconnect bandwidth: %d\n", ret); > > + return ret; > > + } > > + > > + /* > > + * Turn OFF the resources only for controllers without active PCIe devices. For controllers > > + * with active devices, the resources are kept ON and the link is expected to be in L0/L1 > > + * (sub)states. > > + * > > + * Turning OFF the resources for controllers with active PCIe devices will trigger access > > + * violation during the end of the suspend cycle, as kernel tries to access the PCIe devices > > + * config space for masking MSIs. > > + * > > + * Also, it is not desirable to put the link into L2/L3 state as that implies VDD supply > > + * will be removed and the devices may go into powerdown state. This will affect the > > + * lifetime of the storage devices like NVMe. > > + */ > > + if (!dw_pcie_link_up(pcie->pci)) { > > + qcom_pcie_host_deinit(&pcie->pci->pp); > > + pcie->suspended = true; > > + } > > + > > + return 0; > > +} > > + > > +static int qcom_pcie_resume_noirq(struct device *dev) > > +{ > > + struct qcom_pcie *pcie = dev_get_drvdata(dev); > > + > > + if (pcie->suspended) { > > + qcom_pcie_host_init(&pcie->pci->pp); > > + pcie->suspended = false; > > + } > > + > > + qcom_pcie_icc_update(pcie); > > + > > + return 0; > > +} > > + > > static const struct of_device_id qcom_pcie_match[] = { > > { .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 }, > > { .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 }, > > @@ -1870,12 +1917,17 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0302, qcom_fixup_class); > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1000, qcom_fixup_class); > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class); > > +static const struct dev_pm_ops qcom_pcie_pm_ops = { > > + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_suspend_noirq, qcom_pcie_resume_noirq) > > +}; > > + > > static struct platform_driver qcom_pcie_driver = { > > .probe = qcom_pcie_probe, > > .remove = qcom_pcie_remove, > > .driver = { > > .name = "qcom-pcie", > > .of_match_table = qcom_pcie_match, > > + .pm = &qcom_pcie_pm_ops, > > }, > > }; > > module_platform_driver(qcom_pcie_driver); > > Out of curiosity, were you able to measure how much power you were able > to save after adding suspend support for PCIe? I don't know if clock > gating really saves much amount of power, but yeah its true that we > can't really cut off the power domain entirely in this case. > I did not measure the power consumption and I agree that we won't save much power with setting icc bandwidth to 0. But it is better to have something than nothing. And in the coming days, I have plans to look into other power saving measures also. Thanks, Mani > -- > Thanks and Regards, > Dhruva Gole -- மணிவண்ணன் சதாசிவம்