mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
@ 2024-08-27  5:49 Akash Kumar
  2024-08-27 23:15 ` Thinh Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: Akash Kumar @ 2024-08-27  5:49 UTC (permalink / raw)
  To: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
	Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally
  Cc: Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel, Akash Kumar

Use 2K TX FIFO size for low-resolution UVC cameras to support the
maximum possible UVC instances. Restrict 2K TX FIFO size based on
the minimum maxburst required to run low-resolution UVC cameras.

Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
 drivers/usb/dwc3/gadget.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 89fc690fdf34..f342ccda6705 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
 	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
 		num_fifos = dwc->tx_fifo_resize_max_num;
 
+	if (dep->endpoint.maxburst <= 1 &&
+	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
+		num_fifos = 2;
+
 	/* FIFO size for a single buffer */
 	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-27  5:49 [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints Akash Kumar
@ 2024-08-27 23:15 ` Thinh Nguyen
  2024-08-28  2:06   ` Thinh Nguyen
  2024-08-28  4:11   ` AKASH KUMAR
  0 siblings, 2 replies; 7+ messages in thread
From: Thinh Nguyen @ 2024-08-27 23:15 UTC (permalink / raw)
  To: Akash Kumar
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
	Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

On Tue, Aug 27, 2024, Akash Kumar wrote:
> Use 2K TX FIFO size for low-resolution UVC cameras to support the
> maximum possible UVC instances. Restrict 2K TX FIFO size based on
> the minimum maxburst required to run low-resolution UVC cameras.
> 
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> ---
>  drivers/usb/dwc3/gadget.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 89fc690fdf34..f342ccda6705 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>  	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>  		num_fifos = dwc->tx_fifo_resize_max_num;
>  
> +	if (dep->endpoint.maxburst <= 1 &&
> +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> +		num_fifos = 2;
> +
>  	/* FIFO size for a single buffer */
>  	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>  
> -- 
> 2.17.1
> 

These settings are starting to get too specific for each application.
Can we find a better calculation?

Perhaps something like this? (code not tested)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 9a18973ebc05..d54b08f92aea 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -908,15 +908,10 @@ 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)) ||
+	if (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;
+		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
+				  dwc->tx_fifo_resize_max_num);
 
 	/* FIFO size for a single buffer */
 	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);


Thanks,
Thinh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-27 23:15 ` Thinh Nguyen
@ 2024-08-28  2:06   ` Thinh Nguyen
  2024-08-28  4:11   ` AKASH KUMAR
  1 sibling, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2024-08-28  2:06 UTC (permalink / raw)
  To: Akash Kumar
  Cc: Greg Kroah-Hartman, Jing Leng, Felipe Balbi, Jack Pham, kernel,
	Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

On Tue, Aug 27, 2024, Thinh Nguyen wrote:
> On Tue, Aug 27, 2024, Akash Kumar wrote:
> > Use 2K TX FIFO size for low-resolution UVC cameras to support the
> > maximum possible UVC instances. Restrict 2K TX FIFO size based on
> > the minimum maxburst required to run low-resolution UVC cameras.
> > 
> > Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> > ---
> >  drivers/usb/dwc3/gadget.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 89fc690fdf34..f342ccda6705 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> >  	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> >  		num_fifos = dwc->tx_fifo_resize_max_num;
> >  
> > +	if (dep->endpoint.maxburst <= 1 &&
> > +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> > +		num_fifos = 2;
> > +
> >  	/* FIFO size for a single buffer */
> >  	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> >  
> > -- 
> > 2.17.1
> > 
> 
> These settings are starting to get too specific for each application.
> Can we find a better calculation?
> 
> Perhaps something like this? (code not tested)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 9a18973ebc05..d54b08f92aea 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -908,15 +908,10 @@ 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)) ||
> +	if (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;
> +		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
> +				  dwc->tx_fifo_resize_max_num);
>  
>  	/* FIFO size for a single buffer */
>  	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> 
> 

Also, for highspeed, we don't check maxburst. Check for additional
packets properly.

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-27 23:15 ` Thinh Nguyen
  2024-08-28  2:06   ` Thinh Nguyen
@ 2024-08-28  4:11   ` AKASH KUMAR
  2024-08-28 23:45     ` Thinh Nguyen
  1 sibling, 1 reply; 7+ messages in thread
From: AKASH KUMAR @ 2024-08-28  4:11 UTC (permalink / raw)
  To: Thinh Nguyen
  Cc: Greg Kroah-Hartman, Jing Leng, Felipe Balbi, Jack Pham, kernel,
	Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

Hi Thinh,

On 8/28/2024 4:45 AM, Thinh Nguyen wrote:
> On Tue, Aug 27, 2024, Akash Kumar wrote:
>> Use 2K TX FIFO size for low-resolution UVC cameras to support the
>> maximum possible UVC instances. Restrict 2K TX FIFO size based on
>> the minimum maxburst required to run low-resolution UVC cameras.
>>
>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 89fc690fdf34..f342ccda6705 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>>   	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>>   		num_fifos = dwc->tx_fifo_resize_max_num;
>>   
>> +	if (dep->endpoint.maxburst <= 1 &&
>> +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
>> +		num_fifos = 2;
>> +
>>   	/* FIFO size for a single buffer */
>>   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>>   
>> -- 
>> 2.17.1
>>
> These settings are starting to get too specific for each application.
> Can we find a better calculation?
>
> Perhaps something like this? (code not tested)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 9a18973ebc05..d54b08f92aea 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -908,15 +908,10 @@ 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)) ||
> +	if (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;
> +		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
> +				  dwc->tx_fifo_resize_max_num);
>   
>   	/* FIFO size for a single buffer */
>   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>
should be fine for me, as earlier there was no case handling for 
maxburst <= 1, by allocating fifo based on maxburst itself

should be a good solution and will work for all as they customize based 
on maxburst through init scripts, let me test and update.

Thanks,
Akash

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-28  4:11   ` AKASH KUMAR
@ 2024-08-28 23:45     ` Thinh Nguyen
  2024-08-29  7:37       ` AKASH KUMAR
  0 siblings, 1 reply; 7+ messages in thread
From: Thinh Nguyen @ 2024-08-28 23:45 UTC (permalink / raw)
  To: AKASH KUMAR
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
	Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

On Wed, Aug 28, 2024, AKASH KUMAR wrote:
> Hi Thinh,
> 
> On 8/28/2024 4:45 AM, Thinh Nguyen wrote:
> > On Tue, Aug 27, 2024, Akash Kumar wrote:
> > > Use 2K TX FIFO size for low-resolution UVC cameras to support the
> > > maximum possible UVC instances. Restrict 2K TX FIFO size based on
> > > the minimum maxburst required to run low-resolution UVC cameras.
> > > 
> > > Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> > > ---
> > >   drivers/usb/dwc3/gadget.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 89fc690fdf34..f342ccda6705 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> > >   	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> > >   		num_fifos = dwc->tx_fifo_resize_max_num;
> > > +	if (dep->endpoint.maxburst <= 1 &&
> > > +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> > > +		num_fifos = 2;
> > > +
> > >   	/* FIFO size for a single buffer */
> > >   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> > > -- 
> > > 2.17.1
> > > 
> > These settings are starting to get too specific for each application.
> > Can we find a better calculation?
> > 
> > Perhaps something like this? (code not tested)
> > 
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 9a18973ebc05..d54b08f92aea 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -908,15 +908,10 @@ 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)) ||
> > +	if (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;
> > +		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
> > +				  dwc->tx_fifo_resize_max_num);
> >   	/* FIFO size for a single buffer */
> >   	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> > 
> should be fine for me, as earlier there was no case handling for maxburst <=
> 1, by allocating fifo based on maxburst itself
> 
> should be a good solution and will work for all as they customize based on
> maxburst through init scripts, let me test and update.
> 

As I noted in the follow up response, you need to also account for
additional transaction opportunities for isoc if operating in highspeed.
(ie. use usb_endpoint_maxp_mult()).

BR,
Thinh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-28 23:45     ` Thinh Nguyen
@ 2024-08-29  7:37       ` AKASH KUMAR
  2024-08-29 23:08         ` Thinh Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: AKASH KUMAR @ 2024-08-29  7:37 UTC (permalink / raw)
  To: Thinh Nguyen
  Cc: Greg Kroah-Hartman, Jing Leng, Felipe Balbi, Jack Pham, kernel,
	Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

Hi Thinh,

On 8/29/2024 5:15 AM, Thinh Nguyen wrote:
> On Wed, Aug 28, 2024, AKASH KUMAR wrote:
>> Hi Thinh,
>>
>> On 8/28/2024 4:45 AM, Thinh Nguyen wrote:
>>> On Tue, Aug 27, 2024, Akash Kumar wrote:
>>>> Use 2K TX FIFO size for low-resolution UVC cameras to support the
>>>> maximum possible UVC instances. Restrict 2K TX FIFO size based on
>>>> the minimum maxburst required to run low-resolution UVC cameras.
>>>>
>>>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>>>> ---
>>>>    drivers/usb/dwc3/gadget.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index 89fc690fdf34..f342ccda6705 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>>>>    	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>>>>    		num_fifos = dwc->tx_fifo_resize_max_num;
>>>> +	if (dep->endpoint.maxburst <= 1 &&
>>>> +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
>>>> +		num_fifos = 2;
>>>> +
>>>>    	/* FIFO size for a single buffer */
>>>>    	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>>>> -- 
>>>> 2.17.1
>>>>
>>> These settings are starting to get too specific for each application.
>>> Can we find a better calculation?
>>>
>>> Perhaps something like this? (code not tested)
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 9a18973ebc05..d54b08f92aea 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -908,15 +908,10 @@ 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)) ||
>>> +	if (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;
>>> +		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
>>> +				  dwc->tx_fifo_resize_max_num);
>>>    	/* FIFO size for a single buffer */
>>>    	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>>>
>> should be fine for me, as earlier there was no case handling for maxburst <=
>> 1, by allocating fifo based on maxburst itself
>>
>> should be a good solution and will work for all as they customize based on
>> maxburst through init scripts, let me test and update.
>>
> As I noted in the follow up response, you need to also account for
> additional transaction opportunities for isoc if operating in highspeed.
> (ie. use usb_endpoint_maxp_mult()).
yeah i think that we can control with maxpacket size for high speed cams 
and we can
work with maxburst requested only to allocate fifo as both are 
configurable entity and one
should be free to adjust fifo as per his environment and not add any 
additional limitation.
Let me know if we aligned and go ahead with above suggestion only.


Thanks,
Akash

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints
  2024-08-29  7:37       ` AKASH KUMAR
@ 2024-08-29 23:08         ` Thinh Nguyen
  0 siblings, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2024-08-29 23:08 UTC (permalink / raw)
  To: AKASH KUMAR
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
	Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally,
	Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
	linux-kernel

On Thu, Aug 29, 2024, AKASH KUMAR wrote:
> Hi Thinh,
> 
> On 8/29/2024 5:15 AM, Thinh Nguyen wrote:
> > On Wed, Aug 28, 2024, AKASH KUMAR wrote:
> > > Hi Thinh,
> > > 
> > > On 8/28/2024 4:45 AM, Thinh Nguyen wrote:
> > > > On Tue, Aug 27, 2024, Akash Kumar wrote:
> > > > > Use 2K TX FIFO size for low-resolution UVC cameras to support the
> > > > > maximum possible UVC instances. Restrict 2K TX FIFO size based on
> > > > > the minimum maxburst required to run low-resolution UVC cameras.
> > > > > 
> > > > > Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> > > > > ---
> > > > >    drivers/usb/dwc3/gadget.c | 4 ++++
> > > > >    1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > > index 89fc690fdf34..f342ccda6705 100644
> > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > @@ -788,6 +788,10 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> > > > >    	     usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> > > > >    		num_fifos = dwc->tx_fifo_resize_max_num;
> > > > > +	if (dep->endpoint.maxburst <= 1 &&
> > > > > +	    usb_endpoint_xfer_isoc(dep->endpoint.desc))
> > > > > +		num_fifos = 2;
> > > > > +
> > > > >    	/* FIFO size for a single buffer */
> > > > >    	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> > > > > -- 
> > > > > 2.17.1
> > > > > 
> > > > These settings are starting to get too specific for each application.
> > > > Can we find a better calculation?
> > > > 
> > > > Perhaps something like this? (code not tested)
> > > > 
> > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > index 9a18973ebc05..d54b08f92aea 100644
> > > > --- a/drivers/usb/dwc3/gadget.c
> > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > @@ -908,15 +908,10 @@ 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)) ||
> > > > +	if (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;
> > > > +		num_fifos = min_t(unsigned int, dep->endpoint.maxburst + 1,
> > > > +				  dwc->tx_fifo_resize_max_num);
> > > >    	/* FIFO size for a single buffer */
> > > >    	fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> > > > 
> > > should be fine for me, as earlier there was no case handling for maxburst <=
> > > 1, by allocating fifo based on maxburst itself
> > > 
> > > should be a good solution and will work for all as they customize based on
> > > maxburst through init scripts, let me test and update.
> > > 
> > As I noted in the follow up response, you need to also account for
> > additional transaction opportunities for isoc if operating in highspeed.
> > (ie. use usb_endpoint_maxp_mult()).
> yeah i think that we can control with maxpacket size for high speed cams and
> we can
> work with maxburst requested only to allocate fifo as both are configurable
> entity and one
> should be free to adjust fifo as per his environment and not add any
> additional limitation.
> Let me know if we aligned and go ahead with above suggestion only.
> 

I'm not quite clear here. Perhaps you can provide the change so we can
review.

BR,
Thinh

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-08-29 23:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-27  5:49 [PATCH] usb: dwc3: gadget: Fix TX FIFO size for HS ISOC endpoints Akash Kumar
2024-08-27 23:15 ` Thinh Nguyen
2024-08-28  2:06   ` Thinh Nguyen
2024-08-28  4:11   ` AKASH KUMAR
2024-08-28 23:45     ` Thinh Nguyen
2024-08-29  7:37       ` AKASH KUMAR
2024-08-29 23:08         ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®