1. Patchew
  2. linux
  3. View series

[PATCH v5] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs

Akash Kumar posted 1 patch 2 months ago
There is a newer version of this series
drivers/usb/dwc3/gadget.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
[PATCH v5] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
Posted by Akash Kumar 2 months ago 
The current logic is rigid, setting num_fifos to fixed values.
3 for any maxburst greater than 1.
tx_fifo_resize_max_num for maxburst greater than 6.
Additionally, it did not differentiate much between bulk and
isochronous transfers, applying similar logic to both.

The updated logic is more flexible and specifically designed to meet
the unique requirements of both bulk and isochronous transfers. We
have made every effort to satisfy all needs and requirements,
verified on our specific platform and application.

The updated logic is more flexible and specifically designed to meet
the unique requirements of both bulk and isochronous transfers. We
have made every effort to satisfy all needs and requirements, verified
on our specific platform and application.

Bulk Transfers: Ensures that num_fifos is optimized by considering both
the maxburst and DT property tx_fifo_resize_max_num for super speed and
above. For high-speed and below bulk endpoints, a 2K TxFIFO allocation
is used to meet efficient data transfer needs, considering
FIFO-constrained platforms.

Isochronous Transfers: Ensures that num_fifos is sufficient by considering
the maximum packet multiplier for HS and maxburst for SS, along with a
constraint with the DT property tx_fifo_resize_max_num.

This change aims to optimize the allocation of Tx FIFOs for both bulk
and isochronous endpoints, potentially improving data transfer efficiency
and overall performance. It also enhances support for all use cases,
which can be tweaked with DT parameters and the endpoint’s maxburst and
maxpacket.

Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
Changes for v5:
Update Calculation for HS and below bulk and isoc eps based on
suggestion and fixed at 2k for bulk eps considering fifo
constrained platforms.

Changes for v4:
Updated commit message as per review comments to clarify that it has
been tested on specific platforms only and tried best to match all
expectations.

Changes for v3:
Redefine logic for resizing tx fifos,added check based on  operating
speed and used maxp for HS and maxburst for SS  and defined max
allocation based on dt property.

Changes for v2:
Redefine logic for resizing tx fifos, handled fifo based on  minimum
of maxp and maxburts.

Changes for v1:
Added additional condition to allocate tx fifo for hs isoc  eps,
keeping the other resize logic same.
---
 drivers/usb/dwc3/gadget.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 291bc549935b..035d49155d57 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -771,15 +771,21 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
 
 	ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
 
-	if ((dep->endpoint.maxburst > 1 &&
-	     usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
-	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
-		num_fifos = 3;
+	if (dwc->gadget->speed <= USB_SPEED_HIGH) {
+		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
+			num_fifos = 2;
+		if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
+			num_fifos = min_t(unsigned int,
+					  usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
+					  dwc->tx_fifo_resize_max_num);
+	}
 
-	if (dep->endpoint.maxburst > 6 &&
+	if (dwc->gadget->speed > USB_SPEED_HIGH &&
 	    (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
-	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
-		num_fifos = dwc->tx_fifo_resize_max_num;
+	     usb_endpoint_xfer_isoc(dep->endpoint.desc)))
+		num_fifos = min_t(unsigned int,
+				  dep->endpoint.maxburst,
+				  dwc->tx_fifo_resize_max_num);
 
 	/* FIFO size for a single buffer */
 	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
-- 
2.17.1
Re: [PATCH v5] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
Posted by Thinh Nguyen 1 month, 4 weeks ago 
On Sat, Sep 28, 2024, Akash Kumar wrote:
> The current logic is rigid, setting num_fifos to fixed values.
> 3 for any maxburst greater than 1.
> tx_fifo_resize_max_num for maxburst greater than 6.
> Additionally, it did not differentiate much between bulk and
> isochronous transfers, applying similar logic to both.
> 
> The updated logic is more flexible and specifically designed to meet
> the unique requirements of both bulk and isochronous transfers. We
> have made every effort to satisfy all needs and requirements,
> verified on our specific platform and application.
> 
> The updated logic is more flexible and specifically designed to meet
> the unique requirements of both bulk and isochronous transfers. We
> have made every effort to satisfy all needs and requirements, verified
> on our specific platform and application.

Was there a copy-and-paste issue with the above paragraph?

> 
> Bulk Transfers: Ensures that num_fifos is optimized by considering both
> the maxburst and DT property tx_fifo_resize_max_num for super speed and
> above. For high-speed and below bulk endpoints, a 2K TxFIFO allocation
> is used to meet efficient data transfer needs, considering
> FIFO-constrained platforms.
> 
> Isochronous Transfers: Ensures that num_fifos is sufficient by considering
> the maximum packet multiplier for HS and maxburst for SS, along with a
> constraint with the DT property tx_fifo_resize_max_num.
> 
> This change aims to optimize the allocation of Tx FIFOs for both bulk
> and isochronous endpoints, potentially improving data transfer efficiency
> and overall performance. It also enhances support for all use cases,
> which can be tweaked with DT parameters and the endpoint’s maxburst and
> maxpacket.
> 
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> ---
> Changes for v5:
> Update Calculation for HS and below bulk and isoc eps based on
> suggestion and fixed at 2k for bulk eps considering fifo
> constrained platforms.
> 
> Changes for v4:
> Updated commit message as per review comments to clarify that it has
> been tested on specific platforms only and tried best to match all
> expectations.
> 
> Changes for v3:
> Redefine logic for resizing tx fifos,added check based on  operating
> speed and used maxp for HS and maxburst for SS  and defined max
> allocation based on dt property.
> 
> Changes for v2:
> Redefine logic for resizing tx fifos, handled fifo based on  minimum
> of maxp and maxburts.
> 
> Changes for v1:
> Added additional condition to allocate tx fifo for hs isoc  eps,
> keeping the other resize logic same.
> ---
>  drivers/usb/dwc3/gadget.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 291bc549935b..035d49155d57 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -771,15 +771,21 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>  
>  	ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>  
> -	if ((dep->endpoint.maxburst > 1 &&
> -	     usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
> -	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> -		num_fifos = 3;
> +	if (dwc->gadget->speed <= USB_SPEED_HIGH) {
> +		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
> +			num_fifos = 2;
> +		if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
> +			num_fifos = min_t(unsigned int,
> +					  usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
> +					  dwc->tx_fifo_resize_max_num);
> +	}
>  
> -	if (dep->endpoint.maxburst > 6 &&
> +	if (dwc->gadget->speed > USB_SPEED_HIGH &&
>  	    (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> -	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> -		num_fifos = dwc->tx_fifo_resize_max_num;
> +	     usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> +		num_fifos = min_t(unsigned int,
> +				  dep->endpoint.maxburst,
> +				  dwc->tx_fifo_resize_max_num);
>  

Do you have a default value for other endpoints?

Perhaps we can do something like this? What do you think?

	num_fifos = 1;

	switch (dwc->gadget->speed) {
	case USB_SPEED_SUPER_PLUS:
	case USB_SPEED_SUPER:
		if (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
		    usb_endpoint_xfer_isoc(dep->endpoint.desc))
			num_fifos = min_t(unsigned int,
					  dep->endpoint.maxburst,
					  dwc->tx_fifo_resize_max_num);
		break;
	case USB_SPEED_HIGH:
		if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
			num_fifos = min_t(unsigned int,
					  usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
					  dwc->tx_fifo_resize_max_num);
			break;
		}
		fallthrough;
	case USB_SPEED_FULL:
		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
			num_fifos = 2;
		break;
	default:
		break;
	}

Thanks,
Thinh

>  	/* FIFO size for a single buffer */
>  	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> -- 
> 2.17.1
>
Re: [PATCH v5] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
Posted by AKASH KUMAR 1 month, 4 weeks ago 
Hi,

On 10/1/2024 6:50 AM, Thinh Nguyen wrote:
> On Sat, Sep 28, 2024, Akash Kumar wrote:
>> The current logic is rigid, setting num_fifos to fixed values.
>> 3 for any maxburst greater than 1.
>> tx_fifo_resize_max_num for maxburst greater than 6.
>> Additionally, it did not differentiate much between bulk and
>> isochronous transfers, applying similar logic to both.
>>
>> The updated logic is more flexible and specifically designed to meet
>> the unique requirements of both bulk and isochronous transfers. We
>> have made every effort to satisfy all needs and requirements,
>> verified on our specific platform and application.
>>
>> The updated logic is more flexible and specifically designed to meet
>> the unique requirements of both bulk and isochronous transfers. We
>> have made every effort to satisfy all needs and requirements, verified
>> on our specific platform and application.
> Was there a copy-and-paste issue with the above paragraph?
yeah my mistake, will fix.
>> Bulk Transfers: Ensures that num_fifos is optimized by considering both
>> the maxburst and DT property tx_fifo_resize_max_num for super speed and
>> above. For high-speed and below bulk endpoints, a 2K TxFIFO allocation
>> is used to meet efficient data transfer needs, considering
>> FIFO-constrained platforms.
>>
>> Isochronous Transfers: Ensures that num_fifos is sufficient by considering
>> the maximum packet multiplier for HS and maxburst for SS, along with a
>> constraint with the DT property tx_fifo_resize_max_num.
>>
>> This change aims to optimize the allocation of Tx FIFOs for both bulk
>> and isochronous endpoints, potentially improving data transfer efficiency
>> and overall performance. It also enhances support for all use cases,
>> which can be tweaked with DT parameters and the endpoint’s maxburst and
>> maxpacket.
>>
>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>> ---
>> Changes for v5:
>> Update Calculation for HS and below bulk and isoc eps based on
>> suggestion and fixed at 2k for bulk eps considering fifo
>> constrained platforms.
>>
>> Changes for v4:
>> Updated commit message as per review comments to clarify that it has
>> been tested on specific platforms only and tried best to match all
>> expectations.
>>
>> Changes for v3:
>> Redefine logic for resizing tx fifos,added check based on  operating
>> speed and used maxp for HS and maxburst for SS  and defined max
>> allocation based on dt property.
>>
>> Changes for v2:
>> Redefine logic for resizing tx fifos, handled fifo based on  minimum
>> of maxp and maxburts.
>>
>> Changes for v1:
>> Added additional condition to allocate tx fifo for hs isoc  eps,
>> keeping the other resize logic same.
>> ---
>>   drivers/usb/dwc3/gadget.c | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 291bc549935b..035d49155d57 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -771,15 +771,21 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>>   
>>   	ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>>   
>> -	if ((dep->endpoint.maxburst > 1 &&
>> -	     usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
>> -	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
>> -		num_fifos = 3;
>> +	if (dwc->gadget->speed <= USB_SPEED_HIGH) {
>> +		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
>> +			num_fifos = 2;
>> +		if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
>> +			num_fifos = min_t(unsigned int,
>> +					  usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
>> +					  dwc->tx_fifo_resize_max_num);
>> +	}
>>   
>> -	if (dep->endpoint.maxburst > 6 &&
>> +	if (dwc->gadget->speed > USB_SPEED_HIGH &&
>>   	    (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
>> -	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>> -		num_fifos = dwc->tx_fifo_resize_max_num;
>> +	     usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>> +		num_fifos = min_t(unsigned int,
>> +				  dep->endpoint.maxburst,
>> +				  dwc->tx_fifo_resize_max_num);
>>   
> Do you have a default value for other endpoints?
num_fifos is initialized to 1 in beginning for other eps.
>
> Perhaps we can do something like this? What do you think?
yeah can used since we are using gadget speed only to decide and it will 
be faster as well.
>
> 	num_fifos = 1;
>
> 	switch (dwc->gadget->speed) {
> 	case USB_SPEED_SUPER_PLUS:
> 	case USB_SPEED_SUPER:
> 		if (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> 		    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> 			num_fifos = min_t(unsigned int,
> 					  dep->endpoint.maxburst,
> 					  dwc->tx_fifo_resize_max_num);
> 		break;
> 	case USB_SPEED_HIGH:
> 		if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
> 			num_fifos = min_t(unsigned int,
> 					  usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
> 					  dwc->tx_fifo_resize_max_num);
> 			break;
> 		}
> 		fallthrough;
> 	case USB_SPEED_FULL:
> 		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
> 			num_fifos = 2;
> 		break;
> 	default:
> 		break;
> 	}
>
> Thanks,
> Thinh
>
>>   	/* FIFO size for a single buffer */
>>   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>> -- 
>> 2.17.1
Thanks,
Akash

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.