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

2020-05-22 Thread Wesley Cheng



On 5/22/2020 2:54 AM, Felipe Balbi wrote:
> Wesley Cheng  writes:
> 
>> 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 
> 
> I don't remember giving you a Reviewed-by, did I?
> 

Hi Felipe,

Sorry, I put the Reviewed-by tag by mistake, I sent a follow up email to
disregard the tags.  If you need me to resubmit the patch series
version, please let me know.  Thanks!

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


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

2020-05-22 Thread Felipe Balbi
Wesley Cheng  writes:

> 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 

I don't remember giving you a Reviewed-by, did I?

-- 
balbi


signature.asc
Description: PGP signature


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

2020-05-21 Thread 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 
> Reviewed-by: Rob Herring 
> 

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