Re: [PATCH v2 0/3] Re-introduce TX FIFO resize for larger EP bursting

[Wesley Cheng]

On 5/21/2020 1:36 AM, Wesley Cheng wrote:
> Changes in V2:
>  - Modified TXFIFO resizing logic to ensure that each EP is reserved a
>    FIFO.
>  - Removed dev_dbg() prints and fixed typos from patches
>  - Added some more description on the dt-bindings commit message
>


> Reviewed-by: Felipe Balbi <balbi@xxxxxxxxxx>
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> 

Sorry, please disregard the Reviewed-by tags in the patches.  I added
those mistakenly.

> Currently, there is no functionality to allow for resizing the TXFIFOs, and
> relying on the HW default setting for the TXFIFO depth.  In most cases, the
> HW default is probably sufficient, but for USB compositions that contain
> multiple functions that require EP bursting, the default settings
> might not be enough.  Also to note, the current SW will assign an EP to a
> function driver w/o checking to see if the TXFIFO size for that particular
> EP is large enough. (this is a problem if there are multiple HW defined
> values for the TXFIFO size)
> 
> It is mentioned in the SNPS databook that a minimum of TX FIFO depth = 3
> is required for an EP that supports bursting.  Otherwise, there may be
> frequent occurences of bursts ending.  For high bandwidth functions,
> such as data tethering (protocols that support data aggregation), mass
> storage, and media transfer protocol (over FFS), the bMaxBurst value can be
> large, and a bigger TXFIFO depth may prove to be beneficial in terms of USB
> throughput. (which can be associated to system access latency, etc...)  It
> allows for a more consistent burst of traffic, w/o any interruptions, as
> data is readily available in the FIFO.
> 
> With testing done using the mass storage function driver, the results show
> that with a larger TXFIFO depth, the bandwidth increased significantly.
> 
> Test Parameters:
>  - Platform: Qualcomm SM8150
>  - bMaxBurst = 6
>  - USB req size = 256kB
>  - Num of USB reqs = 16
>  - USB Speed = Super-Speed
>  - Function Driver: Mass Storage (w/ ramdisk)
>  - Test Application: CrystalDiskMark
> 
> Results:
> 
> TXFIFO Depth = 3 max packets
> 
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x     | 
> Read      |9 loops    | 193.60
> 	  |           | 195.86
>           |           | 184.77
>           |           | 193.60
> -------------------------------------------
> 
> TXFIFO Depth = 6 max packets
> 
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x     | 
> Read      |9 loops    | 287.35
> 	  |           | 304.94
>           |           | 289.64
>           |           | 293.61
> -------------------------------------------
> 
> Wesley Cheng (3):
>   usb: dwc3: Resize TX FIFOs to meet EP bursting requirements
>   arm64: boot: dts: qcom: sm8150: Enable dynamic TX FIFO resize logic
>   dt-bindings: usb: dwc3: Add entry for tx-fifo-resize
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt |   2 +-
>  arch/arm64/boot/dts/qcom/sm8150.dtsi           |   1 +
>  drivers/usb/dwc3/core.c                        |   2 +
>  drivers/usb/dwc3/core.h                        |   8 ++
>  drivers/usb/dwc3/ep0.c                         |  37 ++++++++-
>  drivers/usb/dwc3/gadget.c                      | 111 +++++++++++++++++++++++++
>  6 files changed, 159 insertions(+), 2 deletions(-)
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project