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

[Pavel Hofman <pavel.hofman@ivitera.com>]

Dne 28. 05. 21 v 10:59 Pavel Hofman napsal(a):
> Dne 27. 05. 21 v 15:47 Stefan Wahren napsal(a):
> 
>>> 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.
>>
> 
> IIUC this code
> https://github.com/torvalds/linux/blob/master/drivers/usb/dwc2/gadget.c#L4091
> optimizes the FIFO assignment to endpoints. From that I would conclude
> that correct values are specific for each use-case configuration of
> endpoints. Maybe a varied selection (256, 512, 768) is more convenient
> than just 256 and 512. I really do not know what use cases need what TX
> fifo values.
> 

My patch raising  g-rx-fifo-size = 558 has been reverted back to 
g-rx-fifo-size = 256 in upstream. 256 is clearly a wrong value. 558 is 
enough for 2 packets per microframe. How about raising the value in the 
mainline DTS files to 301 instead which will correctly work with 1 
packet per microframe (the most common scenario) and comply with the 
4080 limit of the RX + all TXs sum of the TX configs in the mainline?

Thanks for considering.

Best regards,

Pavel.