Message ID | 1698767186-5046-2-git-send-email-quic_msarkar@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b90f:0:b0:403:3b70:6f57 with SMTP id t15csp332110vqg; Tue, 31 Oct 2023 08:47:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHNmwByXHWLUJCfTwg1pvP0+PJRB9g54oK8WRR0PWAiD0KtMyQ8lgFewWn7diJ3EtN9R7LW X-Received: by 2002:a05:6a00:1ad3:b0:68f:f741:57a1 with SMTP id f19-20020a056a001ad300b0068ff74157a1mr16521998pfv.7.1698767222668; Tue, 31 Oct 2023 08:47:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698767222; cv=none; d=google.com; s=arc-20160816; b=tLmU8X7LP/+J7MrP2Tx/sJUT6oktN8CXRbmHf2/r8BgWSsSjU9D5TsWkin85G2iqTA UbBgeLMQ5E0y+qp25vHQw5wmQh1QbmWMlD2EkJPs1W+tVd7MdUlMtwQD5Bg6Qfittsgf viONWt+0bfvl1+sGZu2h7RVg8y0EzzoNzxPSPBBZt2c2UaBVTawkJc+6NhMiSPaQmQYq eNEFexbAKxdxIPgWDxueldcpLAExecciseF3mHrkjJMzRUqjgplq7afWL8qQMGrpdgzc v5RpyQM/B9cjgtZx8Hj1fvrd1s6KS2bgY6r04QUkfHgJp1Gs598TN7ajR1i/SfEFmPyU LE7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from:dkim-signature; bh=cSWatxjmwn7/qjnN6C+sO+PMXsQJIfxUwRwfDKvrNcM=; fh=Pk1D+TX8j7yMhBoMDvb6Z0dI8+uPCwWwrh/XHSBJc1k=; b=DVGnbtyH3kRMpGn0GpY/jVGtX0lfiP57V9l3A2Q/gWBW+pIVHNtJcUEXpYW7Ed+QUI Sd9PFooYwl827ZVg1sGGZif5QDRy9J1LD9mj2IOJgvBFPxxJbOzPTE10d9W+6GwrJ7Hb OvCkjW+d60CYf+QvhBzEwXoG4p35++0oSv1CiYmBCi88h0/ljvQ7//xMEKuS/EpZaKBR P1iw4md4iGC9gL29fu9sc85kpPznQYSIcB5t53qF6Sv96HGhenMT8bJ5QqMUPUj0G+h+ KJOcOomTrP0ZHwrUC7Q3FIw3hNzCPbJ1BGstffm2XhSK0S8gqidLUmJBa2ofclk/7GDA Xrlw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=NsU3oOXi; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from lipwig.vger.email (lipwig.vger.email. [23.128.96.33]) by mx.google.com with ESMTPS id i64-20020a639d43000000b0057795cb4f16si1179376pgd.684.2023.10.31.08.47.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Oct 2023 08:47:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=NsU3oOXi; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 7EEEB802F576; Tue, 31 Oct 2023 08:47:00 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345108AbjJaPqx (ORCPT <rfc822;chrisjones.unixmen@gmail.com> + 33 others); Tue, 31 Oct 2023 11:46:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42388 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232936AbjJaPqr (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 31 Oct 2023 11:46:47 -0400 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C6512C9; Tue, 31 Oct 2023 08:46:44 -0700 (PDT) Received: from pps.filterd (m0279871.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 39VEYNe8023284; Tue, 31 Oct 2023 15:46:36 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references; s=qcppdkim1; bh=cSWatxjmwn7/qjnN6C+sO+PMXsQJIfxUwRwfDKvrNcM=; b=NsU3oOXiE9cBVUPA2DqgTip9nfsVlnflCOV3uGWyy72y8G40ExOlqZNka7b+l4Z5Olqd it3z3Mwdi9qE4ApuzNP9SMkHMg2mk6wUv1cg60sOv5SRRzxZnUaxuZQ1pE9TqHw81BLt CkS84utDoDDpOeiDVLyvQJXckFp2lhaHfU9VirXuFf++dY0OK/dXSXeNRxqCLGhBjR2Z cqSMlUB8LgSZDDsZ79mb2TjK091ug4G3AZTtBAf6DtJTv6QVehrxmoJFGpPr2FFaBtI9 LYmIjyU/h14/qamJWRMiMTuIfEn3YCE8ZhI42a5JEBgfjTfHUeT4I7BDHETGAy6YKwlW Rw== Received: from apblrppmta02.qualcomm.com (blr-bdr-fw-01_GlobalNAT_AllZones-Outside.qualcomm.com [103.229.18.19]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3u2tpx1sky-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 31 Oct 2023 15:46:36 +0000 Received: from pps.filterd (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 39VFkHpt006276; Tue, 31 Oct 2023 15:46:33 GMT Received: from pps.reinject (localhost [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTP id 3u0uckx0uh-1; Tue, 31 Oct 2023 15:46:33 +0000 Received: from APBLRPPMTA02.qualcomm.com (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 39VFkXhw006369; Tue, 31 Oct 2023 15:46:33 GMT Received: from hu-sgudaval-hyd.qualcomm.com (hu-msarkar-hyd.qualcomm.com [10.213.111.194]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTP id 39VFkWgx006365; Tue, 31 Oct 2023 15:46:33 +0000 Received: by hu-sgudaval-hyd.qualcomm.com (Postfix, from userid 3891782) id 34D4A4BFB; Tue, 31 Oct 2023 21:16:32 +0530 (+0530) From: Mrinmay Sarkar <quic_msarkar@quicinc.com> To: agross@kernel.org, andersson@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, konrad.dybcio@linaro.org, mani@kernel.org, robh+dt@kernel.org Cc: quic_shazhuss@quicinc.com, quic_nitegupt@quicinc.com, quic_ramkri@quicinc.com, quic_nayiluri@quicinc.com, dmitry.baryshkov@linaro.org, robh@kernel.org, quic_krichai@quicinc.com, quic_vbadigan@quicinc.com, quic_parass@quicinc.com, quic_schintav@quicinc.com, quic_shijjose@quicinc.com, Mrinmay Sarkar <quic_msarkar@quicinc.com>, Lorenzo Pieralisi <lpieralisi@kernel.org>, =?utf-8?q?Krzysztof_Wilczy=C5=84?= =?utf-8?q?ski?= <kw@linux.com>, Bjorn Helgaas <bhelgaas@google.com>, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Subject: [PATCH v1 1/3] PCI: qcom: Enable cache coherency for SA8775P RC Date: Tue, 31 Oct 2023 21:16:24 +0530 Message-Id: <1698767186-5046-2-git-send-email-quic_msarkar@quicinc.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1698767186-5046-1-git-send-email-quic_msarkar@quicinc.com> References: <1698767186-5046-1-git-send-email-quic_msarkar@quicinc.com> X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: AJNGcFJ47bHHblEW8H5fD3inLLx8CUJX X-Proofpoint-GUID: AJNGcFJ47bHHblEW8H5fD3inLLx8CUJX X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-10-31_02,2023-10-31_03,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=715 priorityscore=1501 mlxscore=0 suspectscore=0 clxscore=1015 spamscore=0 phishscore=0 malwarescore=0 adultscore=0 bulkscore=0 impostorscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2310240000 definitions=main-2310310125 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 31 Oct 2023 08:47:00 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781286538979425110 X-GMAIL-MSGID: 1781286538979425110 |
Series |
arm64: qcom: sa8775p: add cache coherency support for SA8775P
|
|
Commit Message
Mrinmay Sarkar
Oct. 31, 2023, 3:46 p.m. UTC
This change will enable cache snooping logic to support
cache coherency for SA8755P RC platform.
Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
1 file changed, 11 insertions(+)
Comments
On 31.10.2023 16:46, Mrinmay Sarkar wrote: > This change will enable cache snooping logic to support > cache coherency for SA8755P RC platform. 8775 > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 6902e97..6f240fc 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -51,6 +51,7 @@ > #define PARF_SID_OFFSET 0x234 > #define PARF_BDF_TRANSLATE_CFG 0x24c > #define PARF_SLV_ADDR_SPACE_SIZE 0x358 > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 > #define PARF_DEVICE_TYPE 0x1000 > #define PARF_BDF_TO_SID_TABLE_N 0x2000 > > @@ -117,6 +118,9 @@ > /* PARF_LTSSM register fields */ > #define LTSSM_EN BIT(8) > > +/* PARF_NO_SNOOP_OVERIDE register value */ override > +#define NO_SNOOP_OVERIDE_EN 0xa is this actually some magic value and not BIT(1) | BIT(3)? > /* PARF_DEVICE_TYPE register fields */ > #define DEVICE_TYPE_RC 0x4 > > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > { > + struct dw_pcie *pci = pcie->pci; > + struct device *dev = pci->dev; > + > + /* Enable cache snooping for SA8775P */ > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); Why only for 8775 and not for other v2.7, or perhaps all other revisions? Konrad
On 10/31/2023 10:20 PM, Konrad Dybcio wrote: > On 31.10.2023 16:46, Mrinmay Sarkar wrote: >> This change will enable cache snooping logic to support >> cache coherency for SA8755P RC platform. > 8775 > >> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 6902e97..6f240fc 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -51,6 +51,7 @@ >> #define PARF_SID_OFFSET 0x234 >> #define PARF_BDF_TRANSLATE_CFG 0x24c >> #define PARF_SLV_ADDR_SPACE_SIZE 0x358 >> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 >> #define PARF_DEVICE_TYPE 0x1000 >> #define PARF_BDF_TO_SID_TABLE_N 0x2000 >> >> @@ -117,6 +118,9 @@ >> /* PARF_LTSSM register fields */ >> #define LTSSM_EN BIT(8) >> >> +/* PARF_NO_SNOOP_OVERIDE register value */ > override >> +#define NO_SNOOP_OVERIDE_EN 0xa > is this actually some magic value and not BIT(1) | BIT(3)? we need to set 1st and 3rd bit. yes, we can use BIT(1) | BIT(3). > >> /* PARF_DEVICE_TYPE register fields */ >> #define DEVICE_TYPE_RC 0x4 >> >> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) >> >> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) >> { >> + struct dw_pcie *pci = pcie->pci; >> + struct device *dev = pci->dev; >> + >> + /* Enable cache snooping for SA8775P */ >> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) >> + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > Why only for 8775 and not for other v2.7, or perhaps all other > revisions? yes this is only required for 8775 due to hw requirement we need to enable cache snooping from the register level for 8775. > Konrad Thanks, Mrinmay
On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > This change will enable cache snooping logic to support > cache coherency for SA8755P RC platform. > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 6902e97..6f240fc 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -51,6 +51,7 @@ > #define PARF_SID_OFFSET 0x234 > #define PARF_BDF_TRANSLATE_CFG 0x24c > #define PARF_SLV_ADDR_SPACE_SIZE 0x358 > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 > #define PARF_DEVICE_TYPE 0x1000 > #define PARF_BDF_TO_SID_TABLE_N 0x2000 > > @@ -117,6 +118,9 @@ > /* PARF_LTSSM register fields */ > #define LTSSM_EN BIT(8) > > +/* PARF_NO_SNOOP_OVERIDE register value */ > +#define NO_SNOOP_OVERIDE_EN 0xa > + > /* PARF_DEVICE_TYPE register fields */ > #define DEVICE_TYPE_RC 0x4 > > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > { > + struct dw_pcie *pci = pcie->pci; > + struct device *dev = pci->dev; > + > + /* Enable cache snooping for SA8775P */ > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) Obviously: please populate a flag in the data structures instead of doing of_device_is_compatible(). Same applies to the patch 2. > + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > + > qcom_pcie_clear_hpc(pcie->pci); > > return 0;
On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote: > On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > > This change will enable cache snooping logic to support > > cache coherency for SA8755P RC platform. > > > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > > --- > > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > index 6902e97..6f240fc 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > @@ -51,6 +51,7 @@ > > #define PARF_SID_OFFSET 0x234 > > #define PARF_BDF_TRANSLATE_CFG 0x24c > > #define PARF_SLV_ADDR_SPACE_SIZE 0x358 > > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 > > #define PARF_DEVICE_TYPE 0x1000 > > #define PARF_BDF_TO_SID_TABLE_N 0x2000 > > > > @@ -117,6 +118,9 @@ > > /* PARF_LTSSM register fields */ > > #define LTSSM_EN BIT(8) > > > > +/* PARF_NO_SNOOP_OVERIDE register value */ > > +#define NO_SNOOP_OVERIDE_EN 0xa > > + > > /* PARF_DEVICE_TYPE register fields */ > > #define DEVICE_TYPE_RC 0x4 > > > > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > > > > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > > { > > + struct dw_pcie *pci = pcie->pci; > > + struct device *dev = pci->dev; > > + > > + /* Enable cache snooping for SA8775P */ > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > > Obviously: please populate a flag in the data structures instead of > doing of_device_is_compatible(). Same applies to the patch 2. > Not necessary at this point. For some unknown reasons, the HW team ended up disabling cache snooping on this specific platform. Whereas on other platforms, it is enabled by default. So I have low expectations that we would need this setting on other platforms in the future. My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg" instance just for this quirk and it looks overkill to me. So if we endup seeing this behavior on other platforms as well (unlikely) then we can switch to the flag approach. - Mani > > + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > > + > > qcom_pcie_clear_hpc(pcie->pci); > > > > return 0; > > > > -- > With best wishes > Dmitry
On 02/11/2023 17:36, Manivannan Sadhasivam wrote: > On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote: >> On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: >>> >>> This change will enable cache snooping logic to support >>> cache coherency for SA8755P RC platform. >>> >>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>> --- >>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >>> index 6902e97..6f240fc 100644 >>> --- a/drivers/pci/controller/dwc/pcie-qcom.c >>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >>> @@ -51,6 +51,7 @@ >>> #define PARF_SID_OFFSET 0x234 >>> #define PARF_BDF_TRANSLATE_CFG 0x24c >>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358 >>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 >>> #define PARF_DEVICE_TYPE 0x1000 >>> #define PARF_BDF_TO_SID_TABLE_N 0x2000 >>> >>> @@ -117,6 +118,9 @@ >>> /* PARF_LTSSM register fields */ >>> #define LTSSM_EN BIT(8) >>> >>> +/* PARF_NO_SNOOP_OVERIDE register value */ >>> +#define NO_SNOOP_OVERIDE_EN 0xa >>> + >>> /* PARF_DEVICE_TYPE register fields */ >>> #define DEVICE_TYPE_RC 0x4 >>> >>> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) >>> >>> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) >>> { >>> + struct dw_pcie *pci = pcie->pci; >>> + struct device *dev = pci->dev; >>> + >>> + /* Enable cache snooping for SA8775P */ >>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) >> >> Obviously: please populate a flag in the data structures instead of >> doing of_device_is_compatible(). Same applies to the patch 2. >> > > Not necessary at this point. For some unknown reasons, the HW team ended up > disabling cache snooping on this specific platform. Whereas on other platforms, > it is enabled by default. So I have low expectations that we would need this > setting on other platforms in the future. > > My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg" > instance just for this quirk and it looks overkill to me. > > So if we endup seeing this behavior on other platforms as well (unlikely) then > we can switch to the flag approach. This register reads zeroes on 8250, can we confirm it works as expected there? I guess some benchmarks with and without 'dma-coherent'? Konrad
On 02/11/2023 11:16, Mrinmay Sarkar wrote: > > On 10/31/2023 10:20 PM, Konrad Dybcio wrote: >> On 31.10.2023 16:46, Mrinmay Sarkar wrote: >>> This change will enable cache snooping logic to support >>> cache coherency for SA8755P RC platform. >> 8775 >> >>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>> --- >>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c >>> b/drivers/pci/controller/dwc/pcie-qcom.c >>> index 6902e97..6f240fc 100644 >>> --- a/drivers/pci/controller/dwc/pcie-qcom.c >>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >>> @@ -51,6 +51,7 @@ >>> #define PARF_SID_OFFSET 0x234 >>> #define PARF_BDF_TRANSLATE_CFG 0x24c >>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358 >>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 >>> #define PARF_DEVICE_TYPE 0x1000 >>> #define PARF_BDF_TO_SID_TABLE_N 0x2000 >>> @@ -117,6 +118,9 @@ >>> /* PARF_LTSSM register fields */ >>> #define LTSSM_EN BIT(8) >>> +/* PARF_NO_SNOOP_OVERIDE register value */ >> override >>> +#define NO_SNOOP_OVERIDE_EN 0xa >> is this actually some magic value and not BIT(1) | BIT(3)? > we need to set 1st and 3rd bit. yes, we can use BIT(1) | BIT(3). It would be great if you could explain what each of these bits means separately, #defining them instead and ORing at usage time. Konrad
On Thu, Nov 02, 2023 at 11:25:36PM +0100, Konrad Dybcio wrote: > > > On 02/11/2023 17:36, Manivannan Sadhasivam wrote: > > On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote: > > > On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > > > > > > This change will enable cache snooping logic to support > > > > cache coherency for SA8755P RC platform. > > > > > > > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > > > > --- > > > > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ > > > > 1 file changed, 11 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > > > index 6902e97..6f240fc 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > > > @@ -51,6 +51,7 @@ > > > > #define PARF_SID_OFFSET 0x234 > > > > #define PARF_BDF_TRANSLATE_CFG 0x24c > > > > #define PARF_SLV_ADDR_SPACE_SIZE 0x358 > > > > +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 > > > > #define PARF_DEVICE_TYPE 0x1000 > > > > #define PARF_BDF_TO_SID_TABLE_N 0x2000 > > > > > > > > @@ -117,6 +118,9 @@ > > > > /* PARF_LTSSM register fields */ > > > > #define LTSSM_EN BIT(8) > > > > > > > > +/* PARF_NO_SNOOP_OVERIDE register value */ > > > > +#define NO_SNOOP_OVERIDE_EN 0xa > > > > + > > > > /* PARF_DEVICE_TYPE register fields */ > > > > #define DEVICE_TYPE_RC 0x4 > > > > > > > > @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > > > > > > > > static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > > > > { > > > > + struct dw_pcie *pci = pcie->pci; > > > > + struct device *dev = pci->dev; > > > > + > > > > + /* Enable cache snooping for SA8775P */ > > > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > > > > > > Obviously: please populate a flag in the data structures instead of > > > doing of_device_is_compatible(). Same applies to the patch 2. > > > > > > > Not necessary at this point. For some unknown reasons, the HW team ended up > > disabling cache snooping on this specific platform. Whereas on other platforms, > > it is enabled by default. So I have low expectations that we would need this > > setting on other platforms in the future. > > > > My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg" > > instance just for this quirk and it looks overkill to me. > > > > So if we endup seeing this behavior on other platforms as well (unlikely) then > > we can switch to the flag approach. > This register reads zeroes on 8250, can we confirm it works as > expected there? I don't know if this register is even implemented in 8250. Mrinmay, can you check? > I guess some benchmarks with and without > 'dma-coherent'? > The performance benefit can be measured by saturating the link. But it is obvious that snooping the cache will give better performance (plus it avoids cache flush in kernel). - Mani
On 11/3/2023 1:28 PM, Manivannan Sadhasivam wrote: > On Thu, Nov 02, 2023 at 11:25:36PM +0100, Konrad Dybcio wrote: >> >> On 02/11/2023 17:36, Manivannan Sadhasivam wrote: >>> On Thu, Nov 02, 2023 at 05:34:24PM +0200, Dmitry Baryshkov wrote: >>>> On Tue, 31 Oct 2023 at 17:46, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: >>>>> This change will enable cache snooping logic to support >>>>> cache coherency for SA8755P RC platform. >>>>> >>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>>>> --- >>>>> drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++ >>>>> 1 file changed, 11 insertions(+) >>>>> >>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >>>>> index 6902e97..6f240fc 100644 >>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c >>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >>>>> @@ -51,6 +51,7 @@ >>>>> #define PARF_SID_OFFSET 0x234 >>>>> #define PARF_BDF_TRANSLATE_CFG 0x24c >>>>> #define PARF_SLV_ADDR_SPACE_SIZE 0x358 >>>>> +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 >>>>> #define PARF_DEVICE_TYPE 0x1000 >>>>> #define PARF_BDF_TO_SID_TABLE_N 0x2000 >>>>> >>>>> @@ -117,6 +118,9 @@ >>>>> /* PARF_LTSSM register fields */ >>>>> #define LTSSM_EN BIT(8) >>>>> >>>>> +/* PARF_NO_SNOOP_OVERIDE register value */ >>>>> +#define NO_SNOOP_OVERIDE_EN 0xa >>>>> + >>>>> /* PARF_DEVICE_TYPE register fields */ >>>>> #define DEVICE_TYPE_RC 0x4 >>>>> >>>>> @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) >>>>> >>>>> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) >>>>> { >>>>> + struct dw_pcie *pci = pcie->pci; >>>>> + struct device *dev = pci->dev; >>>>> + >>>>> + /* Enable cache snooping for SA8775P */ >>>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) >>>> Obviously: please populate a flag in the data structures instead of >>>> doing of_device_is_compatible(). Same applies to the patch 2. >>>> >>> Not necessary at this point. For some unknown reasons, the HW team ended up >>> disabling cache snooping on this specific platform. Whereas on other platforms, >>> it is enabled by default. So I have low expectations that we would need this >>> setting on other platforms in the future. >>> >>> My concern with the usage of flag is that it warrants a new "qcom_pcie_cfg" >>> instance just for this quirk and it looks overkill to me. >>> >>> So if we endup seeing this behavior on other platforms as well (unlikely) then >>> we can switch to the flag approach. >> This register reads zeroes on 8250, can we confirm it works as >> expected there? > I don't know if this register is even implemented in 8250. Mrinmay, can you > check? Yes we have this register in 8250 platform as well and I can see the default value is 0x0. --Mrinmay >> I guess some benchmarks with and without >> 'dma-coherent'? >> > The performance benefit can be measured by saturating the link. But it is > obvious that snooping the cache will give better performance (plus it avoids > cache flush in kernel). > > - Mani >
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 6902e97..6f240fc 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -51,6 +51,7 @@ #define PARF_SID_OFFSET 0x234 #define PARF_BDF_TRANSLATE_CFG 0x24c #define PARF_SLV_ADDR_SPACE_SIZE 0x358 +#define PCIE_PARF_NO_SNOOP_OVERIDE 0x3d4 #define PARF_DEVICE_TYPE 0x1000 #define PARF_BDF_TO_SID_TABLE_N 0x2000 @@ -117,6 +118,9 @@ /* PARF_LTSSM register fields */ #define LTSSM_EN BIT(8) +/* PARF_NO_SNOOP_OVERIDE register value */ +#define NO_SNOOP_OVERIDE_EN 0xa + /* PARF_DEVICE_TYPE register fields */ #define DEVICE_TYPE_RC 0x4 @@ -961,6 +965,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) { + struct dw_pcie *pci = pcie->pci; + struct device *dev = pci->dev; + + /* Enable cache snooping for SA8775P */ + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) + writel(NO_SNOOP_OVERIDE_EN, pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); + qcom_pcie_clear_hpc(pcie->pci); return 0;