Re: [PATCH v3 4/4] media: uvcvideo: eUSB2 double isochronous bandwidth support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Aug 07, 2025 at 08:38:55AM +0200, Ricardo Ribalda wrote:
> On Thu, 7 Aug 2025 at 07:54, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > From: Tao Q Tao <tao.q.tao@intel.com>
> >
> > Use usb_endpoint_max_isoc_bpi() from the USB framework to find the maximum
> > bytes per interval for the endpoint. Consequently this adds eUSB2
> > isochronous mode and SuperSpeedPlus Isochronous Endpoint Compaion support
> > where larger bpi values are possible.
> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>
> > Co-developed-by: Amardeep Rai <amardeep.rai@intel.com>
> > Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
> > Signed-off-by: Tao Q Tao <tao.q.tao@intel.com>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> nit: I believe the last tag should be: Signed-off-by:

Related discussion on the workflow mailing list:
https://lore.kernel.org/workflows/bbe1ea08-23bf-43dc-960d-bb8a214b65fa@hammernet.be/T/#t

To answer a question Sakari asked off-list, I'm fine with this patch
being merged through the linux-usb tree.

> > ---
> >  drivers/media/usb/uvc/uvc_driver.c |  4 ++--
> >  drivers/media/usb/uvc/uvc_video.c  | 24 +++---------------------
> >  drivers/media/usb/uvc/uvcvideo.h   |  4 +---
> >  3 files changed, 6 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index da24a655ab68..fde0bc95622c 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -536,7 +536,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> >         unsigned int nformats = 0, nframes = 0, nintervals = 0;
> >         unsigned int size, i, n, p;
> >         u32 *interval;
> > -       u16 psize;
> > +       u32 psize;
> >         int ret = -EINVAL;
> >
> >         if (intf->cur_altsetting->desc.bInterfaceSubClass
> > @@ -772,7 +772,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> >                                 streaming->header.bEndpointAddress);
> >                 if (ep == NULL)
> >                         continue;
> > -               psize = uvc_endpoint_max_bpi(dev->udev, ep);
> > +               psize = usb_endpoint_max_isoc_bpi(dev->udev, ep);
> >                 if (psize > streaming->maxpsize)
> >                         streaming->maxpsize = psize;
> >         }
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index a75af314e46b..335b1c4eff9b 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1860,24 +1860,6 @@ static void uvc_video_stop_transfer(struct uvc_streaming *stream,
> >                 uvc_free_urb_buffers(stream);
> >  }
> >
> > -/*
> > - * Compute the maximum number of bytes per interval for an endpoint.
> > - */
> > -u16 uvc_endpoint_max_bpi(struct usb_device *dev, struct usb_host_endpoint *ep)
> > -{
> > -       u16 psize;
> > -
> > -       switch (dev->speed) {
> > -       case USB_SPEED_SUPER:
> > -       case USB_SPEED_SUPER_PLUS:
> > -               return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
> > -       default:
> > -               psize = usb_endpoint_maxp(&ep->desc);
> > -               psize *= usb_endpoint_maxp_mult(&ep->desc);
> > -               return psize;
> > -       }
> > -}
> > -
> >  /*
> >   * Initialize isochronous URBs and allocate transfer buffers. The packet size
> >   * is given by the endpoint.
> > @@ -1888,10 +1870,10 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
> >         struct urb *urb;
> >         struct uvc_urb *uvc_urb;
> >         unsigned int npackets, i;
> > -       u16 psize;
> > +       u32 psize;
> >         u32 size;
> >
> > -       psize = uvc_endpoint_max_bpi(stream->dev->udev, ep);
> > +       psize = usb_endpoint_max_isoc_bpi(stream->dev->udev, ep);
> >         size = stream->ctrl.dwMaxVideoFrameSize;
> >
> >         npackets = uvc_alloc_urb_buffers(stream, size, psize, gfp_flags);
> > @@ -2034,7 +2016,7 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream,
> >                                 continue;
> >
> >                         /* Check if the bandwidth is high enough. */
> > -                       psize = uvc_endpoint_max_bpi(stream->dev->udev, ep);
> > +                       psize = usb_endpoint_max_isoc_bpi(stream->dev->udev, ep);
> >                         if (psize >= bandwidth && psize < best_psize) {
> >                                 altsetting = alts->desc.bAlternateSetting;
> >                                 best_psize = psize;
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index b9f8eb62ba1d..a77ba76e033a 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -450,7 +450,7 @@ struct uvc_streaming {
> >
> >         struct usb_interface *intf;
> >         int intfnum;
> > -       u16 maxpsize;
> > +       u32 maxpsize;
> >
> >         struct uvc_streaming_header header;
> >         enum v4l2_buf_type type;
> > @@ -818,8 +818,6 @@ void uvc_ctrl_cleanup_fh(struct uvc_fh *handle);
> >  /* Utility functions */
> >  struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
> >                                             u8 epaddr);
> > -u16 uvc_endpoint_max_bpi(struct usb_device *dev, struct usb_host_endpoint *ep);
> > -
> >  /* Quirks support */
> >  void uvc_video_decode_isight(struct uvc_urb *uvc_urb,
> >                              struct uvc_buffer *buf,

-- 
Regards,

Laurent Pinchart