Message ID | 20230718025221.4001329-1-vidyas@nvidia.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp1480773vqt; Mon, 17 Jul 2023 20:23:48 -0700 (PDT) X-Google-Smtp-Source: APBJJlFCAjMiW+qnhcYDgK/PtlfxQVev4VZKHPkMw6OO8mrybuEO8FUPxXisICv8LEhcRcERroWb X-Received: by 2002:a17:902:a401:b0:1b9:d310:e85b with SMTP id p1-20020a170902a40100b001b9d310e85bmr11271756plq.68.1689650628112; Mon, 17 Jul 2023 20:23:48 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1689650628; cv=pass; d=google.com; s=arc-20160816; b=inhooVfy5i6Wof0PvMzf05SnCBWUEpYXzN6BzBJA6QHvsiN1QFyf27QGO9M49NzWdf VgU/gY8ztW8e+2oJll9mHfApCRjtKa7A963xxMuqpeOOqJrn3+XRLPM6apuXuIPwSuMB rW2NKztevl+P/IpBLT36o6yRxn3UDnq03XnkyoPf6NgI/+Lcv/rf7IzphphbEl9h62PA Lp1IfeVtYhsKJ6iFsQ0+Pu1B4dtnT4V8fRmkEHvwJVSumStZj1Bwi7R0Z7bB+L9ktI0+ 99K1icNZuun2y8aM31k63srVEOaV9Wcz9DDKrml5ehXcAZPMXh/eRXk37cckapj/HDT1 9Kbg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=P723caHDol4blPPBY4pGCuAxWiX6ebABAOFoIPIHrbA=; fh=0k+UsfDnS6vWpA9pJh19GZjPHqqsejDIncErNi9Avj0=; b=RKnWlGYtCCJi6wm2w/hJIxZhig0XgQokOOvfsU8S1Pxf3OKfQn3VzgbeKcI6LC083v Yo6WKS7/wEmJh4MYseU08KiZTsCdYjTdBMTTZVk3XneJiBIdj5gL0gTT/CE3QCp2i0MX XE9sVKO3qBeUCaHWvx3O33ePFykWZhXu+1Sv8cJE5kgWL35LwMAjRYzddIjvTZrKn1dC pu4c1WlsqUPpBWrIRUN4w5FLbzKhur25bvCYVhRvfvaDSvBz8MLx8UWSrhATSzl0+EpT 1wTcPgOlA6XZjC2vCOipQ11fiBid+J6xH7SSNJqfOY92dKTwWv1zd0z/MSlDH1sMb1+p 5H9g== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@Nvidia.com header.s=selector2 header.b=smGa2TVJ; arc=pass (i=1 spf=pass spfdomain=nvidia.com dmarc=pass fromdomain=nvidia.com); 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=REJECT sp=REJECT dis=NONE) header.from=nvidia.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i17-20020a17090332d100b001b8a4954be1si951233plr.595.2023.07.17.20.23.34; Mon, 17 Jul 2023 20:23:48 -0700 (PDT) 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=@Nvidia.com header.s=selector2 header.b=smGa2TVJ; arc=pass (i=1 spf=pass spfdomain=nvidia.com dmarc=pass fromdomain=nvidia.com); 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=REJECT sp=REJECT dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230451AbjGRCwr (ORCPT <rfc822;daweilics@gmail.com> + 99 others); Mon, 17 Jul 2023 22:52:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229736AbjGRCwp (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 17 Jul 2023 22:52:45 -0400 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2064.outbound.protection.outlook.com [40.107.223.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0CB57100; Mon, 17 Jul 2023 19:52:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QQ1nGNChL9rW6+s+QobA7VtdmcYihqtnIiwOoV9QnTDjGOoROasIKNAh69ogtmYF70HhBHxIUBWSt6D+sQuDQAckOl12x2uykDF95+GLv3Au9DrGpAWlDyM4hRuj0H2zq0g+nmLv3uKIMzTUT0ezNf8ckS1iBV23hKUW5L1hqhSkle8LsJC4MSvPL/mNbOfc/TUt6wtTpc6fEN08TxFYnvJ5EJLuL+WcB4qWasJbmTI4iQQ3bC6Tk6Ldj/jtXv1jDmML/3KiEUR2HyzcVgOHKYKtDTqnMpPvWgfUP5iy/QZeCbk+QFHHiIuM4ErkaiuW3N3MKy3tC6fC2sbWGMHbIg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=P723caHDol4blPPBY4pGCuAxWiX6ebABAOFoIPIHrbA=; b=KQejInKtTbQv7ZTaSyNBZpqaXppQZz69CBNcnDs1VjkBgsldZt1lWJK3yyGPqWDpLkX1aXEZvs/kurc0VgFcfsZ5TUnvhJIFNpLfvB00RVmss8nk4lwoidJF1Yvsea7R5Uy+KJnNKsmSyWWTCQP1IZIoe+2pAMhzlysxPE5/5gdleKzUdSpA6Zo6FYC5AB3fq3wvirgw0nHfcjkDJmQc+YQlD6SnPjUN3NFRoofEbAnjHxAcEo9Vs9HSBk7oglVCRS5Dwjruw95LwVGaQnZFzGeVgUalK3JMWL9vYyzWdadfdoehnW0nO0EWsEXgWF4+mvYeY5LL9drWGITIcVxgWA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 216.228.117.160) smtp.rcpttodomain=kernel.org smtp.mailfrom=nvidia.com; dmarc=pass (p=reject sp=reject pct=100) action=none header.from=nvidia.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=P723caHDol4blPPBY4pGCuAxWiX6ebABAOFoIPIHrbA=; b=smGa2TVJ1izKO6up3E2qhirGKx4jTgQKK2yOz2d/k3BLXzbK7pu5+ZElQMpw4lqPzBroR4402XhsCMz/QBm3Pcx4c1YKIycYeFuqyfNWHQXto9wpWAJb8X0gpuveYeVe7xdOLHiPWnpP4IgXIaDujxLFQ5sUONdAXmSf7xx0xmEHBkFWA13KJXlcF8ptwL01/S94lXyS1aYI5r8i8/+XpnICRUWjtVTyNGsvG4FSANXAEwkQ/6FRN/DWDiBChzn8y9kyvyCu8qJxc6Qmxi76ESkrLvKSWR3TvD5eNIfn/oy9NJ+Ta1EtD7c2tshZp3XHlfn+9Lrwq1gHNlmNpwKKLg== Received: from DS7PR03CA0349.namprd03.prod.outlook.com (2603:10b6:8:55::24) by SJ0PR12MB5661.namprd12.prod.outlook.com (2603:10b6:a03:422::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6588.28; Tue, 18 Jul 2023 02:52:40 +0000 Received: from DM6NAM11FT033.eop-nam11.prod.protection.outlook.com (2603:10b6:8:55:cafe::df) by DS7PR03CA0349.outlook.office365.com (2603:10b6:8:55::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6588.33 via Frontend Transport; Tue, 18 Jul 2023 02:52:40 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 216.228.117.160) smtp.mailfrom=nvidia.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=nvidia.com; Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates 216.228.117.160 as permitted sender) receiver=protection.outlook.com; client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C Received: from mail.nvidia.com (216.228.117.160) by DM6NAM11FT033.mail.protection.outlook.com (10.13.172.221) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6588.33 via Frontend Transport; Tue, 18 Jul 2023 02:52:40 +0000 Received: from rnnvmail205.nvidia.com (10.129.68.10) by mail.nvidia.com (10.129.200.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.5; Mon, 17 Jul 2023 19:52:27 -0700 Received: from rnnvmail205.nvidia.com (10.129.68.10) by rnnvmail205.nvidia.com (10.129.68.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.37; Mon, 17 Jul 2023 19:52:27 -0700 Received: from vidyas-desktop.nvidia.com (10.127.8.9) by mail.nvidia.com (10.129.68.10) with Microsoft SMTP Server id 15.2.986.37 via Frontend Transport; Mon, 17 Jul 2023 19:52:23 -0700 From: Vidya Sagar <vidyas@nvidia.com> To: <lpieralisi@kernel.org>, <kw@linux.com>, <robh@kernel.org>, <bhelgaas@google.com>, <thierry.reding@gmail.com>, <jonathanh@nvidia.com>, <Sergey.Semin@baikalelectronics.ru> CC: <linux-pci@vger.kernel.org>, <linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <kthota@nvidia.com>, <mmaddireddy@nvidia.com>, <vidyas@nvidia.com>, <sagar.tv@gmail.com> Subject: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" Date: Tue, 18 Jul 2023 08:22:21 +0530 Message-ID: <20230718025221.4001329-1-vidyas@nvidia.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230619102604.3735001-1-vidyas@nvidia.com> References: <20230619102604.3735001-1-vidyas@nvidia.com> MIME-Version: 1.0 X-NVConfidentiality: public Content-Transfer-Encoding: 8bit Content-Type: text/plain X-NV-OnPremToCloud: ExternallySecured X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM6NAM11FT033:EE_|SJ0PR12MB5661:EE_ X-MS-Office365-Filtering-Correlation-Id: 7e6fa4ac-40ac-44ba-d17b-08db873a0d44 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Fhtol6cov1EfwWk00rG/ae1dvyibAOmP4k27gZ9wF0xGLtn8UAUiMx1xM028/1Tu+A4VxBxrMbhX75yBVEy2YrZdRH8iQKo8LMb8BK2m8HVEgEJIF5+3t4IWl6CBCz8RQIvFrp/U+fDw2d+Wq47F2fKkEl+eZm6v5m4wPa27Pn2PpHhtQFN6zlWvp5wGCqMicWNmwLmzk8b3wCE9F6w7fWMxl/C1s1GochRV4k8UL/xzpe0m42Oq5JPrX7AtqO8ZPec7p2QHflXeCP5BBBsUD/LLn8OlM4RtZiLD4tGi+qGivP4BB7/SB0k+Qq9gyuHiCfOREfbDp1Ek1NdGBjMNQ1h3kLPTn4791/8HmLUEgpZB8HO5Nsv1zlNAmdP8+0wHn1h5aBjRfnxGnimqdnRJ/sRrNxvQS7JQZOL0SxlD8u+2bPum598RZZ5ZWZPZBMnSYGqdRzqx6kQosL3vXLECYSBrLjnoHEatBekYI5UNMb0YpEolnNNqY+E5NkSenYuXo8iCkGQmp74ViAocis/+YdRm4u/EwbRcTUmPb30PKdGbRdgZKSaOeDiOeaVSKdOGj335/VxAJ+HZ+lSS4NNzk0kbNPWj769j45nC93Z1xKUUUYQiI2y40sx3U0jeamG1OMcO9KoEqrup0qEDQ6k0AJnH0qYscrsB0kQBzzDeCXdiHF2viUkOXXzvjWsHfPvtoXjPYWqGt6qineLGLvtHIJSAYKv/CG31XibHSK9ZF6DmA2EDXTunNq/6BT1x9E7X X-Forefront-Antispam-Report: CIP:216.228.117.160;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:mail.nvidia.com;PTR:dc6edge1.nvidia.com;CAT:NONE;SFS:(13230028)(4636009)(396003)(346002)(39860400002)(136003)(376002)(82310400008)(451199021)(36840700001)(46966006)(40470700004)(47076005)(70586007)(2906002)(36860700001)(7416002)(7696005)(83380400001)(336012)(2616005)(186003)(1076003)(426003)(36756003)(5660300002)(26005)(356005)(110136005)(40460700003)(86362001)(82740400003)(7636003)(40480700001)(8676002)(478600001)(8936002)(54906003)(316002)(4326008)(41300700001)(70206006);DIR:OUT;SFP:1101; X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jul 2023 02:52:40.1961 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 7e6fa4ac-40ac-44ba-d17b-08db873a0d44 X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com] X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT033.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB5661 X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1768128097118912418 X-GMAIL-MSGID: 1771727097531027876 |
Series |
[V4] Revert "PCI: tegra194: Enable support for 256 Byte payload"
|
|
Commit Message
Vidya Sagar
July 18, 2023, 2:52 a.m. UTC
After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte
payload"), we set MPS=256 for tegra194 Root Ports.
By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*"
parameter), Linux configures the MPS of every device to match the
upstream bridge, which is impossible if the Root Port has MPS=256
and a device only supports MPS=128.
This scenario results in uncorrectable Malformed TLP errors if the
Root Port sends TLPs with payloads larger than 128 bytes. These
errors can be avoided by using the "pci=pcie_bus_safe" parameter,
but it doesn't seem to be a good idea to always have this parameter
even for basic functionality to work.
Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte
payload") so the Root Ports default to MPS=128, which all devices
support.
If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf"
to get the benefit of larger MPS settings.
[ rewrote commit message based on Bjorn's suggestion ]
Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload")
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V4:
* Rewrote commit message based on Bjorn's suggestion
V3:
* Fixed a build issue
V2:
* Addressed review comments from Bjorn
drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
Comments
Hi Krzysztof, Bjorn, On 18/07/2023 03:52, Vidya Sagar wrote: > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we set MPS=256 for tegra194 Root Ports. > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > parameter), Linux configures the MPS of every device to match the > upstream bridge, which is impossible if the Root Port has MPS=256 > and a device only supports MPS=128. > > This scenario results in uncorrectable Malformed TLP errors if the > Root Port sends TLPs with payloads larger than 128 bytes. These > errors can be avoided by using the "pci=pcie_bus_safe" parameter, > but it doesn't seem to be a good idea to always have this parameter > even for basic functionality to work. > > Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload") so the Root Ports default to MPS=128, which all devices > support. > > If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" > to get the benefit of larger MPS settings. > > [ rewrote commit message based on Bjorn's suggestion ] > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Rewrote commit message based on Bjorn's suggestion > > V3: > * Fixed a build issue > > V2: > * Addressed review comments from Bjorn > > drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..a772faff14b5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > + u16 val_16; > + > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > PCI_EXP_LNKSTA); > val_16 &= ~PCI_EXP_LNKSTA_SLC; > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); I see a version of this patch here ... https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 However, I don't see this in -next yet. If you are happy with this latest version, could we get this into -next? FWIW ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Thanks! Jon
On Fri, Jul 21, 2023 at 09:23:01AM +0100, Jon Hunter wrote: > ... > I see a version of this patch here ... > > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 > > However, I don't see this in -next yet. If you are happy with this latest > version, could we get this into -next? I'm on vacation until Tuesday; will build a new -next branch Tuesday or Wednesday.
Hi Bjorn, On 21/07/2023 11:35, Bjorn Helgaas wrote: > On Fri, Jul 21, 2023 at 09:23:01AM +0100, Jon Hunter wrote: >> ... > >> I see a version of this patch here ... >> >> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 >> >> However, I don't see this in -next yet. If you are happy with this latest >> version, could we get this into -next? > > I'm on vacation until Tuesday; will build a new -next branch Tuesday > or Wednesday. A friendly reminder on this. Can we queue this up for -next? I know that there is further discussion on-going about if the core could handle this, but right now PCIe is broken on our NVIDIA IGX Orin board and we would like to merge this now in the short-term at least. Thanks! Jon
On Tue, Jul 18, 2023 at 08:22:21AM +0530, Vidya Sagar wrote: > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we set MPS=256 for tegra194 Root Ports. > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > parameter), Linux configures the MPS of every device to match the > upstream bridge, which is impossible if the Root Port has MPS=256 > and a device only supports MPS=128. Thanks for pointing out that I broke this log by omitting the mention of a switch. Is the rewording below better? If so, Krzysztof can amend the commit. After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"), we initialize MPS=256 for tegra194 Root Ports before enumerating the hierarchy. Consider an Endpoint that supports only MPS=128. In the default situation (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux tries to configure the MPS of every device to match the upstream bridge. If the Endpoint is directly below the Root Port, Linux can reduce the Root Port MPS to 128 to match the Endpoint. But if there's a switch in the middle, Linux doesn't reduce the Root Port MPS because other devices below the switch may already be configured with MPS larger than 128. > This scenario results in uncorrectable Malformed TLP errors if the > Root Port sends TLPs with payloads larger than 128 bytes. These > errors can be avoided by using the "pci=pcie_bus_safe" parameter, > but it doesn't seem to be a good idea to always have this parameter > even for basic functionality to work. > > Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload") so the Root Ports default to MPS=128, which all devices > support. > > If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" > to get the benefit of larger MPS settings. > > [ rewrote commit message based on Bjorn's suggestion ] > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") 4fb8e46c1bc4 appeared in v6.0-rc1, so this wouldn't be a candidate for v6.5, but it does sound like it should be tagged for stable? If so, Krzysztof can probably add that as well. > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Rewrote commit message based on Bjorn's suggestion > > V3: > * Fixed a build issue > > V2: > * Addressed review comments from Bjorn > > drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..a772faff14b5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > + u16 val_16; > + > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > PCI_EXP_LNKSTA); > val_16 &= ~PCI_EXP_LNKSTA_SLC; > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > -- > 2.25.1 >
On 8/2/2023 2:10 AM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Tue, Jul 18, 2023 at 08:22:21AM +0530, Vidya Sagar wrote: >> After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte >> payload"), we set MPS=256 for tegra194 Root Ports. >> >> By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" >> parameter), Linux configures the MPS of every device to match the >> upstream bridge, which is impossible if the Root Port has MPS=256 >> and a device only supports MPS=128. > > Thanks for pointing out that I broke this log by omitting the mention > of a switch. Is the rewording below better? If so, Krzysztof can > amend the commit. Yes. The below rewording looks good. Thanks, Vidya Sagar > > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we initialize MPS=256 for tegra194 Root Ports before enumerating > the hierarchy. > > Consider an Endpoint that supports only MPS=128. In the default situation > (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux > tries to configure the MPS of every device to match the upstream bridge. > If the Endpoint is directly below the Root Port, Linux can reduce the Root > Port MPS to 128 to match the Endpoint. But if there's a switch in the > middle, Linux doesn't reduce the Root Port MPS because other devices below > the switch may already be configured with MPS larger than 128. > >> This scenario results in uncorrectable Malformed TLP errors if the >> Root Port sends TLPs with payloads larger than 128 bytes. These >> errors can be avoided by using the "pci=pcie_bus_safe" parameter, >> but it doesn't seem to be a good idea to always have this parameter >> even for basic functionality to work. >> >> Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte >> payload") so the Root Ports default to MPS=128, which all devices >> support. >> >> If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" >> to get the benefit of larger MPS settings. >> >> [ rewrote commit message based on Bjorn's suggestion ] >> >> Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > > 4fb8e46c1bc4 appeared in v6.0-rc1, so this wouldn't be a candidate for > v6.5, but it does sound like it should be tagged for stable? If so, > Krzysztof can probably add that as well. > >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> V4: >> * Rewrote commit message based on Bjorn's suggestion >> >> V3: >> * Fixed a build issue >> >> V2: >> * Addressed review comments from Bjorn >> >> drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 4fdadc7b045f..a772faff14b5 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); >> val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); >> dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); >> @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> struct device *dev = pcie->dev; >> u32 val; >> int ret; >> - u16 val_16; >> >> if (pcie->ep_state == EP_STATE_ENABLED) >> return; >> @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> /* Clear Slot Clock Configuration bit if SRNS configuration */ >> if (pcie->enable_srns) { >> + u16 val_16; >> + >> val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + >> PCI_EXP_LNKSTA); >> val_16 &= ~PCI_EXP_LNKSTA_SLC; >> dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, >> val_16); >> } >> - >> clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); >> >> val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); >> -- >> 2.25.1 >>
Hello! [...] > > > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > > > payload"), we set MPS=256 for tegra194 Root Ports. > > > > > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > > > parameter), Linux configures the MPS of every device to match the > > > upstream bridge, which is impossible if the Root Port has MPS=256 > > > and a device only supports MPS=128. > > > > Thanks for pointing out that I broke this log by omitting the mention > > of a switch. Is the rewording below better? If so, Krzysztof can > > amend the commit. > Yes. The below rewording looks good. Updated commit at: https://git.kernel.org/pci/pci/c/ebfde1584d9f Thank you everyone! Krzysztof
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 4fdadc7b045f..a772faff14b5 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) struct device *dev = pcie->dev; u32 val; int ret; - u16 val_16; if (pcie->ep_state == EP_STATE_ENABLED) return; @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - /* Clear Slot Clock Configuration bit if SRNS configuration */ if (pcie->enable_srns) { + u16 val_16; + val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA); val_16 &= ~PCI_EXP_LNKSTA_SLC; dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, val_16); } - clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK);