media: uvcvideo: Support large SuperSpeedPlus isochronous endpoints
Subsystems (3)
PATCH 1/1 media: uvcvideo: Support large SuperSpeedPlus isochronous endpoints
USB 3.1 increased maximum isochronous bandwidth to 96KB per interval, too much for 16 bits and the SuperSpeed Endpoint Companion descriptor. A new SuperSpeedPlus Isochronous Endpoint Companion descriptor was introduced to encode such bandwidths, see spec sections 9.6.7, 9.6.8.
Support the descriptor with code based on xhci_get_max_esit_payload() and widen all 'psize' variables to 32 bits. Subsequent calculations are 32 bit already and not expected to overflow, so this change ought to suffice for proper alt setting selection on USB 3.x Gen 2 devices.
---
This change appears to be a strict necessity for supporting USB3 Gen2 isochronous devices meaningfully. Whether it's sufficient I don't know, I don't have such HW. No regression seen on High Speed and SuperSpeed.
drivers/media/usb/uvc/uvc_
drivers/media/usb/uvc/uvc_driver.c | 2 +- drivers/media/usb/uvc/uvc_video.c | 13 +++++++++---- drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-)
drivers/media/usb/uvc/uvc_driver.c +1 -1
@@ -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
drivers/media/usb/uvc/uvc_video.c +9 -4
@@ -1898,13 +1898,18 @@ static void uvc_video_stop_transfer(struct uvc_streaming *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) +u32 uvc_endpoint_max_bpi (struct usb_device *dev,struct usb_host_endpoint *ep) { -u16 psize; +u32 psize;switch (dev->speed) { -case USB_SPEED_SUPER :case USB_SPEED_SUPER_PLUS : +/* check for the SS+ Isoc EP Companion descriptor */ +if (USB_SS_SSP_ISOC_COMP (ep->ss_ep_comp.bmAttributes)) +return le32_to_cpu (ep->ssp_isoc_ep_comp.dwBytesPerInterval); + fallthrough; +case USB_SPEED_SUPER : +/* use the SS EP Companion descriptor */ return le16_to_cpu (ep->ss_ep_comp.wBytesPerInterval);default : psize =usb_endpoint_maxp (&ep->desc);
@@ -1923,7 +1928,7 @@ 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);
drivers/media/usb/uvc/uvcvideo.h +2 -2
@@ -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;
@@ -813,7 +813,7 @@ 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); +u32 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,
Signature · 1 line
2.48.1
Greg KH
maintainerQuoted 10 lines · Michal Pecio
USB 3.1 increased maximum isochronous bandwidth to 96KB per interval, too much for 16 bits and the SuperSpeed Endpoint Companion descriptor. A new SuperSpeedPlus Isochronous Endpoint Companion descriptor was introduced to encode such bandwidths, see spec sections 9.6.7, 9.6.8.
Support the descriptor with code based on
xhci_get_max_esit_payload()and widen all 'psize' variables to 32 bits. Subsequent calculations are 32 bit already and not expected to overflow, so this change ought to suffice for proper alt setting selection on USB 3.x Gen 2 devices.
Quoted 6 lines
Signed-off-by Michal Pecio <[email protected]>---
This change appears to be a strict necessity for supporting USB3 Gen2 isochronous devices meaningfully. Whether it's sufficient I don't know, I don't have such HW. No regression seen on High Speed and SuperSpeed.
If you don't have the hardware, why make this change?
Quoted 1 line
drivers/media/usb/uvc/uvc_
This line looks odd, because:
Quoted 4 lines
drivers/media/usb/uvc/uvc_driver.c | 2 +- drivers/media/usb/uvc/uvc_video.c | 13 +++++++++---- drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-)
Only 3 files were changed. What went wrong?
thanks,
greg k-h
Quoted 16 lines · Greg KH
Quoted 15 lines · Michal Pecio
USB 3.1 increased maximum isochronous bandwidth to 96KB per interval, too much for 16 bits and the SuperSpeed Endpoint Companion descriptor. A new SuperSpeedPlus Isochronous Endpoint Companion descriptor was introduced to encode such bandwidths, see spec sections 9.6.7, 9.6.8.
Support the descriptor with code based on
xhci_get_max_esit_payload()and widen all 'psize' variables to 32 bits. Subsequent calculations are 32 bit already and not expected to overflow, so this change ought to suffice for proper alt setting selection on USB 3.x Gen 2 devices.Signed-off-by Michal Pecio <[email protected]>---
Quoted 6 lines
Quoted 4 lines
This change appears to be a strict necessity for supporting USB3 Gen2 isochronous devices meaningfully. Whether it's sufficient I don't know, I don't have such HW. No regression seen on High Speed and SuperSpeed.
If you don't have the hardware, why make this change?
Because it's an obvious and easily removed limitation, and may be the only thing missing in uvcvideo to support such hardware. I saw it when investigating a related SuperSpeed problem earlier.
If media believes that more is needed, or if there is objection to such changes based on spec alone then fine, at least the patch is out there for anyone wondering why things aren't working.
Quoted 10 lines
Quoted 1 line
drivers/media/usb/uvc/uvc_This line looks odd, because:
Quoted 4 lines
drivers/media/usb/uvc/uvc_driver.c | 2 +- drivers/media/usb/uvc/uvc_video.c | 13 +++++++++---- drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-)Only 3 files were changed. What went wrong?
Sorry, some editing mistake.