From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 04E09C369CA for ; Thu, 17 Apr 2025 06:56:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=rbiGEnPXfTJBFF8/Jzz38hsNkOYWx3lXorEkxn6cETU=; b=d3XffjTmTP53WoRRc1uFIeazmZ De0Hps7thVQUWE8vt7ydM/ZmKky0dXH5o5tunmeO/IZj1jpA65D+5UJTS2Um+8PVoHDUp5XPi2q8q ZtjpWk/5EqUfKT4/76veXVUlqn9RMN3Yju899+J4XBZONBBK2QUc2UDSH1IiOAvB5YgchH7uo69XZ oGAtidzH6LJFRAy+NWWqQGcOJU0QxeJGXVXx+2BU/Fet/K25odHYZNJO5Zic9vnxMWSIzeGFpECIq F7VCb5t1XPkG0KXrJT9M1gOkP6Aiw/7ocwYrbIauJCdPQJsaIJqdv8U1eXAPTj56ySBcyrQtErI+V A5MEk+1w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1u5JAU-0000000Byd5-02ii; Thu, 17 Apr 2025 06:56:14 +0000 Received: from m16.mail.163.com ([220.197.31.4]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1u5J2W-0000000Bwwo-1v6d; Thu, 17 Apr 2025 06:48:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:From: Content-Type; bh=rbiGEnPXfTJBFF8/Jzz38hsNkOYWx3lXorEkxn6cETU=; b=kK5v809ETPJ3ci6A09qlTPH3adIYbW9/Wauc6MkwQWvQRd9/IEBmozRHpc54gC beR3IZ7QNyYS2A6bsWB9lC2X4ufe5Z1KXmlpjYrAVWcIYRMQBuayV0qNfqbvNePj fFU4aG/eE57jAb6wKfZwxdKctJIffsLPv2dTN1P9fNeWM= Received: from [192.168.142.52] (unknown []) by gzga-smtp-mtada-g0-2 (Coremail) with SMTP id _____wBn37D7owBo5BmAAg--.29857S2; Thu, 17 Apr 2025 14:47:26 +0800 (CST) Message-ID: Date: Thu, 17 Apr 2025 14:47:23 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] PCI: dw-rockchip: Configure max payload size on host init To: Niklas Cassel Cc: Bjorn Helgaas , lpieralisi@kernel.org, kw@linux.com, bhelgaas@google.com, heiko@sntech.de, manivannan.sadhasivam@linaro.org, robh@kernel.org, jingoohan1@gmail.com, thomas.richard@bootlin.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, Shawn Lin References: <20250416204051.GA78956@bhelgaas> Content-Language: en-US From: Hans Zhang <18255117159@163.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID: _____wBn37D7owBo5BmAAg--.29857S2 X-Coremail-Antispam: 1Uf129KBjvJXoW3Ww15ZrW5WFW5urWkAr4fZrb_yoW3ZF15p3 4ayF13KrWkJrW7K3Z5tF1DGr1xtr1qyF4UGF45G34rtF1a9r1Dtry29r1Sqa47Wry5JFya gw4UJ3yIvw45J3DanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07UmiihUUUUU= X-Originating-IP: [222.71.101.198] X-CM-SenderInfo: rpryjkyvrrlimvzbiqqrwthudrp/1tbiWxMyo2gAnhDdWQAAs7 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250416_234800_912234_583E3499 X-CRM114-Status: GOOD ( 22.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2025/4/17 14:01, Niklas Cassel wrote: > On Thu, Apr 17, 2025 at 10:19:10AM +0800, Hans Zhang wrote: >> On 2025/4/17 04:40, Bjorn Helgaas wrote: >>> On Wed, Apr 16, 2025 at 11:19:26PM +0800, Hans Zhang wrote: >>>> The RK3588's PCIe controller defaults to a 128-byte max payload size, >>>> but its hardware capability actually supports 256 bytes. This results >>>> in suboptimal performance with devices that support larger payloads. >>>> >>>> Signed-off-by: Hans Zhang <18255117159@163.com> >>>> --- >>>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 18 ++++++++++++++++++ >>>> 1 file changed, 18 insertions(+) >>>> >>>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c >>>> index c624b7ebd118..5bbb536a2576 100644 >>>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c >>>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c >>>> @@ -477,6 +477,22 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg) >>>> return IRQ_HANDLED; >>>> } >>>> +static void rockchip_pcie_set_max_payload(struct rockchip_pcie *rockchip) >>>> +{ >>>> + struct dw_pcie *pci = &rockchip->pci; >>>> + u32 dev_cap, dev_ctrl; >>>> + u16 offset; >>>> + >>>> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); >>>> + dev_cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCAP); >>>> + dev_cap &= PCI_EXP_DEVCAP_PAYLOAD; >>>> + >>>> + dev_ctrl = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL); >>>> + dev_ctrl &= ~PCI_EXP_DEVCTL_PAYLOAD; >>>> + dev_ctrl |= dev_cap << 5; >>>> + dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, dev_ctrl); >>>> +} >>> >>> I can't really complain too much about this since meson does basically >>> the same thing, but there are some things I don't like about this: >>> >>> - I don't think it's safe to set MPS higher in all cases. If we set >>> the Root Port MPS=256, and an Endpoint only supports MPS=128, the >>> Endpoint may do a 256-byte DMA read (assuming its MRRS>=256). In >>> that case the RP may respond with a 256-byte payload the Endpoint >>> can't handle. The generic code in pci_configure_mps() might be >>> smart enough to avoid that situation, but I'm not confident about >>> it. Maybe I could be convinced. >>> >> >> Dear Bjorn, >> >> Thank you very much for your reply. If we set the Root Port MPS=256, and an >> Endpoint only supports MPS=128. Finally, Root Port is also set to MPS=128 in >> pci_configure_mps. > > In you example below, the Endpoint has: > DevCap: MaxPayload 512 bytes > > So at least your example can't be used to prove this specific point. > But perhaps you just wanted to show that your Max Payload Size increase > actually works? > Dear Niklas, Do you have an Endpoint with MPS=128? If so, you can also help verify the logic of the pci_configure_mps function. I don't have an Endpoint with MPS=128 here. The processing logic of the pci_configure_mps function has been verified on our own SOC platform. Please refer to the following log. Our Root Port will set MPS=512. 0002:30:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [80] Power Management version 3 Flags: PMEClk- DSI- D1+ D2- AuxCurrent=0mA PME(D0+,D1+,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [90] MSI: Enable+ Count=1/32 Maskable+ 64bit+ Address: 000000000e060040 Data: 0000 Masking: fffffffe Pending: 00000000 Capabilities: [b0] MSI-X: Enable- Count=2 Masked- Vector table: BAR=0 offset=00000040 PBA: BAR=0 offset=00000040 Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00 DevCap: MaxPayload 512 bytes, PhantFunc 0 ExtTag- RBE+ DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+ RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 256 bytes, MaxReadReq 1024 bytes 0002:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8125 2.5GbE Controller (rev 05) Subsystem: Realtek Semiconductor Co., Ltd. RTL8125 2.5GbE Controller Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR-