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@xxxxxxxxxxx> > > > > > --- > > > > > 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