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.

Bulk Transfers: Ensures that num_fifos is optimized by considering both
the maxburst and DT property "tx-fifo-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 below and maxburst
for Super-speed and above eps, along with a constraint with the DT
property "tx-fifo-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. This structured approach ensures that the appropriate number
of FIFOs is allocated based on the endpoint type and USB speed.

Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
Changes for v7:
fixed indentations for if checks.

Changes for v6:
The code has been refactored to replace multiple if checks with a
switch-case structure based on the USB speed. This change improves
readability and maintainability by clearly defining behavior for
different USB speeds. This structured approach ensures that the
appropriate number of FIFOs is allocated based on the endpoint
type and USB speed.

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 | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 10178e5eda5a..af3d5b2f7b67 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -771,15 +771,30 @@ 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 (dep->endpoint.maxburst > 6 &&
-	    (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;
+	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;
+	}
 
 	/* FIFO size for a single buffer */
 	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
-- 
2.17.1

On 10/17/2024 12:14 PM, 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.
>
> Bulk Transfers: Ensures that num_fifos is optimized by considering both
> the maxburst and DT property "tx-fifo-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 below and maxburst
> for Super-speed and above eps, along with a constraint with the DT
> property "tx-fifo-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. This structured approach ensures that the appropriate number
> of FIFOs is allocated based on the endpoint type and USB speed.
>
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> ---
> Changes for v7:
> fixed indentations for if checks.
>
> Changes for v6:
> The code has been refactored to replace multiple if checks with a
> switch-case structure based on the USB speed. This change improves
> readability and maintainability by clearly defining behavior for
> different USB speeds. This structured approach ensures that the
> appropriate number of FIFOs is allocated based on the endpoint
> type and USB speed.
>
> 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 | 33 ++++++++++++++++++++++++---------
>   1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 10178e5eda5a..af3d5b2f7b67 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -771,15 +771,30 @@ 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 (dep->endpoint.maxburst > 6 &&
> -	    (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;
> +	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,

Hi Akash,

We are currently working on enabling UVC functionality on supports USB 
HS and below speeds with using a single port RAM configuration.

In order to better understand the logic behind resizing HS ISO transfers 
when using a streaming packet size of 3072 in the UVC use case, we would 
like to know if there is a specific reason for using 
(usb_endpoint_maxp_mult(dep->endpoint.desc) + 1) to determine the number 
of FIFOs. This calculation could result in having 4 FIFOs instead of 3, 
even though 3 FIFOs should be sufficient for handling the 3072 streaming 
packet size. In our specific use case, this could potentially lead to an 
insufficient RAM issue, as we are limited by the amount of RAM available 
in the single port RAM configuration and supports upto HS-speed.

> +					  dwc->tx_fifo_resize_max_num);
> +			break;
> +		}
> +		fallthrough;
> +	case USB_SPEED_FULL:
> +		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
> +			num_fifos = 2;


And we need to understand on above logic as well. Why using num_fifos =2 
for bulk transfer for HS and FULL speed?. It can ending with 2k (4 * 
Maxpacktsize).
As per our understanding (2 x Maxpacketsize) is sufficient for bulk. Its 
means that num_fifos = 1 is sufficient for bulk. As i mentioned above 
this could potentially lead to an insufficient RAM issue in our platform.
> +		break;
> +	default:
> +		break;
> +	}
>   
>   	/* FIFO size for a single buffer */
>   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);

Hi Selvarasu,

On 11/6/2024 12:29 PM, Selvarasu Ganesan wrote:
> On 10/17/2024 12:14 PM, 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.
>>
>> Bulk Transfers: Ensures that num_fifos is optimized by considering both
>> the maxburst and DT property "tx-fifo-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 below and maxburst
>> for Super-speed and above eps, along with a constraint with the DT
>> property "tx-fifo-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. This structured approach ensures that the appropriate number
>> of FIFOs is allocated based on the endpoint type and USB speed.
>>
>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>> ---
>> Changes for v7:
>> fixed indentations for if checks.
>>
>> Changes for v6:
>> The code has been refactored to replace multiple if checks with a
>> switch-case structure based on the USB speed. This change improves
>> readability and maintainability by clearly defining behavior for
>> different USB speeds. This structured approach ensures that the
>> appropriate number of FIFOs is allocated based on the endpoint
>> type and USB speed.
>>
>> 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 | 33 ++++++++++++++++++++++++---------
>>    1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 10178e5eda5a..af3d5b2f7b67 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -771,15 +771,30 @@ 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 (dep->endpoint.maxburst > 6 &&
>> -	    (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;
>> +	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,
> Hi Akash,
>
> We are currently working on enabling UVC functionality on supports USB
> HS and below speeds with using a single port RAM configuration.
>
> In order to better understand the logic behind resizing HS ISO transfers
> when using a streaming packet size of 3072 in the UVC use case, we would
> like to know if there is a specific reason for using
> (usb_endpoint_maxp_mult(dep->endpoint.desc) + 1) to determine the number
> of FIFOs. This calculation could result in having 4 FIFOs instead of 3,
> even though 3 FIFOs should be sufficient for handling the 3072 streaming
> packet size. In our specific use case, this could potentially lead to an
> insufficient RAM issue, as we are limited by the amount of RAM available
> in the single port RAM configuration and supports upto HS-speed.
the reason for using maxp +1 is to use higher fifo size to support 1080 
mjpeg uvc streaming in HS or any uvc instances using less maxpacket like 
512 for UVC usecase,
can use 2k fifo. For any fifo constraint environment we have suggested 
to use dt property "tx-fifo-max-num" which can set accordingly, for your 
case you can set to
2 or 3.
>
>> +					  dwc->tx_fifo_resize_max_num);
>> +			break;
>> +		}
>> +		fallthrough;
>> +	case USB_SPEED_FULL:
>> +		if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
>> +			num_fifos = 2;
>
> And we need to understand on above logic as well. Why using num_fifos =2
> for bulk transfer for HS and FULL speed?. It can ending with 2k (4 *
> Maxpacktsize).
> As per our understanding (2 x Maxpacketsize) is sufficient for bulk. Its
> means that num_fifos = 1 is sufficient for bulk. As i mentioned above
> this could potentially lead to an insufficient RAM issue in our platform.
2k Fifo is useful for bulk eps incase of multiple bulk operations 
simultaneously to avoid buffer underrrun issues,
Like i shared above DT property can be used to limit max fifo per ep to 
support platforms with less RAM size.
>> +		break;
>> +	default:
>> +		break;
>> +	}
>>    
>>    	/* FIFO size for a single buffer */
>>    	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
Thanks,
Akash

On 11/6/2024 2:34 PM, AKASH KUMAR wrote:
> Hi Selvarasu,
>
> On 11/6/2024 12:29 PM, Selvarasu Ganesan wrote:
>> On 10/17/2024 12:14 PM, 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.
>>>
>>> Bulk Transfers: Ensures that num_fifos is optimized by considering both
>>> the maxburst and DT property "tx-fifo-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 below and maxburst
>>> for Super-speed and above eps, along with a constraint with the DT
>>> property "tx-fifo-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. This structured approach ensures that the appropriate number
>>> of FIFOs is allocated based on the endpoint type and USB speed.
>>>
>>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>>> ---
>>> Changes for v7:
>>> fixed indentations for if checks.
>>>
>>> Changes for v6:
>>> The code has been refactored to replace multiple if checks with a
>>> switch-case structure based on the USB speed. This change improves
>>> readability and maintainability by clearly defining behavior for
>>> different USB speeds. This structured approach ensures that the
>>> appropriate number of FIFOs is allocated based on the endpoint
>>> type and USB speed.
>>>
>>> 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 | 33 ++++++++++++++++++++++++---------
>>>    1 file changed, 24 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 10178e5eda5a..af3d5b2f7b67 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -771,15 +771,30 @@ 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 (dep->endpoint.maxburst > 6 &&
>>> -        (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;
>>> +    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,
>> Hi Akash,
>>
>> We are currently working on enabling UVC functionality on supports USB
>> HS and below speeds with using a single port RAM configuration.
>>
>> In order to better understand the logic behind resizing HS ISO transfers
>> when using a streaming packet size of 3072 in the UVC use case, we would
>> like to know if there is a specific reason for using
>> (usb_endpoint_maxp_mult(dep->endpoint.desc) + 1) to determine the number
>> of FIFOs. This calculation could result in having 4 FIFOs instead of 3,
>> even though 3 FIFOs should be sufficient for handling the 3072 streaming
>> packet size. In our specific use case, this could potentially lead to an
>> insufficient RAM issue, as we are limited by the amount of RAM available
>> in the single port RAM configuration and supports upto HS-speed.
> the reason for using maxp +1 is to use higher fifo size to support 
> 1080 mjpeg uvc streaming in HS or any uvc instances using less 
> maxpacket like 512 for UVC usecase,
> can use 2k fifo. For any fifo constraint environment we have suggested 
> to use dt property "tx-fifo-max-num" which can set accordingly, for 
> your case you can set to
> 2 or 3.

Hi Akash,

Thank you for the update.

We can utilize the tx-fifo-max-num dt property to manage the FIFO size.
However, we are unclear how the streaming image frame size or formats 
are connected to the FIFO size. It seems more likely that it should be 
related to the maximum packet size.
For example, in  HS mode, it is possible to support 1080p MJPEG UVC 
streaming if we set the maximum streaming packet size to 1024 bytes with 
only one FIFO. In a typical UVC scenario, the maximum packet size is 
3072 bytes. According to calculations, using three FIFOs (3 * 1024) 
should be sufficient to handle this maximum packet size.


>>
>>> + dwc->tx_fifo_resize_max_num);
>>> +            break;
>>> +        }
>>> +        fallthrough;
>>> +    case USB_SPEED_FULL:
>>> +        if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
>>> +            num_fifos = 2;
>>
>> And we need to understand on above logic as well. Why using num_fifos =2
>> for bulk transfer for HS and FULL speed?. It can ending with 2k (4 *
>> Maxpacktsize).
>> As per our understanding (2 x Maxpacketsize) is sufficient for bulk. Its
>> means that num_fifos = 1 is sufficient for bulk. As i mentioned above
>> this could potentially lead to an insufficient RAM issue in our 
>> platform.
> 2k Fifo is useful for bulk eps incase of multiple bulk operations 
> simultaneously to avoid buffer underrrun issues,
> Like i shared above DT property can be used to limit max fifo per ep 
> to support platforms with less RAM size.


Please correct me if we wrong.
Each IN EPs has its own TxFIFOs. So, We are unclear how a buffer 
under-run or overrun issue could occur if we only use HS mode, as it 
does not support burst transfers.
We cannot use the same tx-fifo-max-num property where require max 1 fifo 
for bulk and max 3 fifo for ISO.


>>> +        break;
>>> +    default:
>>> +        break;
>>> +    }
>>>           /* FIFO size for a single buffer */
>>>        fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> Thanks,
> Akash
>

On Thu, Oct 17, 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.
> 
> Bulk Transfers: Ensures that num_fifos is optimized by considering both
> the maxburst and DT property "tx-fifo-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 below and maxburst
> for Super-speed and above eps, along with a constraint with the DT
> property "tx-fifo-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. This structured approach ensures that the appropriate number
> of FIFOs is allocated based on the endpoint type and USB speed.
> 
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> ---
> Changes for v7:
> fixed indentations for if checks.
> 
> Changes for v6:
> The code has been refactored to replace multiple if checks with a
> switch-case structure based on the USB speed. This change improves
> readability and maintainability by clearly defining behavior for
> different USB speeds. This structured approach ensures that the
> appropriate number of FIFOs is allocated based on the endpoint
> type and USB speed.
> 
> 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 | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 10178e5eda5a..af3d5b2f7b67 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -771,15 +771,30 @@ 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 (dep->endpoint.maxburst > 6 &&
> -	    (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;
> +	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;
> +	}
>  
>  	/* FIFO size for a single buffer */
>  	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> -- 
> 2.17.1

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh