Hi Bhupesh,
On Monday 11 February 2013 21:27:50 Bhupesh SHARMA wrote:
> On Friday, February 08, 2013 4:22 AM Laurent Pinchart wrote:
> > On Thursday 17 January 2013 16:23:50 Bhupesh Sharma wrote:
> > > As per the USB3.0 specs, the bandwidth requirements of a UVC's video
> > > streaming endpoint will change to support super-speed. These changes
> > > will be dependent on whether the UVC video streaming endpoint is Bulk
> > > orIsochronous:
> > >
> > > - If video streaming endpoint is Isochronous:
> > > As per Section 4.4.8.2 (Isochronous Transfer Bandwidth Requirements)
> > > of the USB3.0 specs:
> > > A SuperSpeed isochronous endpoint can move up to three burst
> > > transactions of up to 16 maximum sized packets (3 * 16 * 1024 bytes)
> > > per service interval.
> > >
> > > - If video streaming endpoint is Bulk:
> > > As per 4.4.6.1 (Bulk Transfer Data Packet Size) of the USB3.0 specs:
> > > An endpoint for bulk transfers shall set the maximum data packet
> > > payload size in its endpoint descriptor to 1024 bytes.
> > > It also specifies the burst size that the endpoint can accept from or
> > > transmit on the SuperSpeed bus. The allowable burst size for a
> > > bulk endpoint shall be in the range of 1 to 16.
> > >
> > > So, in the Isochronous case, we can define the USB request's buffer to
> > > be equal to = (Maximum packet size) * (bMaxBurst + 1) * (Mult + 1), so
> > > that the UDC driver can try to send out this buffer in one Isochronous
> > > service interval.
> > >
> > > The same computation will hold good for the Bulk case as the Mult
> > > value is 0 here and we can have a USB request buffer of maximum
> > > 16 * 1024 bytes size, which can be sent out by the UDC driver as per
> > > the Bulk bandwidth allocation on the USB3 bus.
> > >
> > > This patch adds the above-mentioned support and is also USB2.0
> > > backward compliant.
> >
> > This looks good to me, except...
> >
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx>
> > > ---
> > >
> > > drivers/usb/gadget/uvc_video.c | 9 +++++++--
> > > 1 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/uvc_video.c
> > > b/drivers/usb/gadget/uvc_video.c index b0e53a8..f7d1913 100644
> > > --- a/drivers/usb/gadget/uvc_video.c
> > > +++ b/drivers/usb/gadget/uvc_video.c
> > > @@ -235,7 +235,10 @@ uvc_video_alloc_requests(struct uvc_video
> > > *video)
> > > BUG_ON(video->req_size);
> > >
> > > for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
> > > - video->req_buffer[i] = kmalloc(video->ep->maxpacket,
> > > GFP_KERNEL);
> > > + video->req_buffer[i] = kmalloc(video->ep->maxpacket *
> > > + video->ep->maxburst *
> >
> > If I'm not mistaken maxburst is 0 for FS and HS endpoints (from
> > config_ep_by_speed in drivers/usb/gadget/composite.c). The buffer size
> > will then be pretty small :-)
>
> Actually my initial patch had this approach:
> video->req_buffer[i] = kmalloc(video->ep->maxpacket *
> (video->ep->maxburst + 1) *
> (video->ep->mult + 1))
>
> but due to a patch from Felipe:
>
> usb: gadget: composite: fix ep->maxburst initialization
>
> bMaxBurst field on endpoint companion descriptor
> is supposed to contain the number of burst minus
> 1. When passing that to controller drivers, we
> should be passing the real number instead (by
> incrementing 1).
>
> While doing that, also fix the assumption on
> dwc3 that value comes decremented by one.
>
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
>
> as we have now:
> _ep->maxburst = comp_desc->bMaxBurst + 1;
>
> So, even if comp_desc->bMaxBurst is 0, _ep->maxburst will be 1.
> So, this patch seems correct to me.
For SS, sure, but if you connect the gadget to a USB 2.0 host maxburst will be
set to 0, so buffer allocation will break.
Please make sure you test all your patches with both USB 2.0 and USB 3.0
hosts.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
