Thread (10 messages) 10 messages, 3 authors, 2021-08-06

Re: [PATCH] ARM: dts: bcm283x: increase dwc2's RX FIFO size

From: Stefan Wahren <hidden>
Date: 2021-05-27 13:47:56
Also in: linux-devicetree

Hi Pavel,

fix Nicolas address. Sorry about this.

Am 27.05.21 um 15:17 schrieb Pavel Hofman:
Hi Stefan,

Dne 26. 05. 21 v 19:12 Stefan Wahren napsal(a):
quoted
Am 13.11.20 um 08:18 schrieb Pavel Hofman:
quoted
The previous version of the dwc2 overlay set the RX FIFO size to
256 4-byte words. This is not enough for 1024 bytes of the largest
isochronous high speed packet allowed, because it doesn't take into
account extra space needed by dwc2.

RX FIFO's size is calculated based on the following (in 4byte words):
- 13 locations for SETUP packets
  5*n + 8 for Slave and Buffer DMA mode where n is number of control
  endpoints which is 1 on the bcm283x core

- 1 location for Global OUT NAK

- 2 * 257 locations for status information and the received packet.
  Typically two spaces are recommended so that when the previous packet
  is being transferred to AHB, the USB can receive the subsequent
  packet.

- 10 * 1 location for transfer complete status for last packet of each
  endpoint. The bcm283x core has 5 IN and 5 OUT EPs

- 10 * 1 additional location for EPDisable status for each endpoint

- 5 * 2 additional locations are recommended for each OUT endpoint

Total is 558 locations.

Signed-off-by: Phil Elwell <redacted>
Signed-off-by: Pavel Hofman <redacted>
---
 arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi        | 2 +-
 arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
index e2fd961..20322de 100644
--- a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 &usb {
 	dr_mode = "otg";
-	g-rx-fifo-size = <256>;
+	g-rx-fifo-size = <558>;
 	g-np-tx-fifo-size = <32>;
 	/*
 	 * According to dwc2 the sum of all device EP
diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
index 0ff0e9e..1409d1b 100644
--- a/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 &usb {
 	dr_mode = "peripheral";
-	g-rx-fifo-size = <256>;
+	g-rx-fifo-size = <558>;
sorry for being late at the party, but this change introduce a
regression on Raspberry Pi 4 B:

dwc2 fe980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter
g-tx-fifo-size, setting to default average

I known you didn't change the tx fifo size, but there are complex
constrains regarding the total fifo size.

Are you able to test this with a mainline kernel (not Raspberry Pi
kernel) and send a fix for this?
quoted
 	g-np-tx-fifo-size = <32>;
 	g-tx-fifo-size = <256 256 512 512 512 768 768>;
 };
I think I see the problem.

IIUC the calculations and checks, all g-tx-fifo-size values +
g-rx-fifo-size + g-np-tx-fifo-size must not exceed total_fifo_size. My
RPi4 reports the total_fifo_size as 4080 (in
/sys/kernel/debug/usb/fe980000.usb/hw_params).

Linux mainline
https://github.com/torvalds/linux/search?p=3&q=g-tx-fifo-size :

The increase in value of g-rx-fifo-size exceeds the limit for the DTSI
files we patched:

Both bcm283x-rpi-usb-peripheral.dtsi and bcm283x-rpi-usb-otg.dtsi:
558 + 32 + 256 + 256 + 512 + 512 + 512 + 768 + 768 = 4174 > 4080

while the sum with the previous value of 256 reached just 3872 < 4080.


The raspberrypi repo
https://github.com/raspberrypi/linux/search?q=g-tx-fifo-size :

It has a different mix of the DTSI files
dwc2-overlay.dts
upstream-overlay.dts
upstream-pi4-overlay.dts
yes these overlay files are vendor specific and doesn't exist in
mainline. The upstream*dts were intended to "simulate" mainline
behavior, but unfortunately differ in this case.
all of which define
g-tx-fifo-size = <512 512 512 512 512 256 256>;

Here the calculation holds:
558 + 32 + 512 + 512 + 512 + 512 + 512 + 256 + 256 = 3662 < 4080

My RPi4 uses one of these DTSIs, because my
/sys/kernel/debug/usb/fe980000.usb/params says:

g_rx_fifo_size                : 558
g_np_tx_fifo_size             : 32
g_tx_fifo_size[0]             : 0
g_tx_fifo_size[1]             : 512
g_tx_fifo_size[2]             : 512
g_tx_fifo_size[3]             : 512
g_tx_fifo_size[4]             : 512
g_tx_fifo_size[5]             : 512
g_tx_fifo_size[6]             : 256
g_tx_fifo_size[7]             : 256


IIUC the tx_fifo values in bcm283x-rpi-usb-peripheral.dtsi and
bcm283x-rpi-usb-otg.dtsi files can be lowered to the values used and
tested (at least by me) in the RPi repo. But this is outside of my
knowledge, honestly I do not know what is the most appropriate
distribution of the remaining fifo space among the g_tx_fifo buffers.
Please can the RPi developers (Phil?) suggest a fix?
As author of the mainline bcm283x-rpi-usb-otg.dtsi i was trying to
optimize the fifo sizes for EP 6 and 7. But i don't remember why. So my
suggestion for a fix would be:

g-tx-fifo-size = <256 256 256 512 512 768 768>;

But i'm also unsure about the values.
Thanks a lot to Stefan and to everyone involved.

Pavel.
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help