Re: [PATCH 1/2] usb: add helper to extract bits 12:11 of wMaxPacketSize

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Felipe,

On 2016/9/28 16:05, Felipe Balbi wrote:

Hi,

yfw <nh26223@xxxxxxxxx> writes:
Bin Liu <b-liu@xxxxxx> writes:
Does this mean the issue of isoc high bandwidth transfer was fixed by
this patchset per your test?

No, I couldn't get g_webcam to work yet.

In mainline, g_webcam is broken with DWC3. Also these two patches don't
fix the issue on v4.4.21.

care to send tracepoint output? Best if you could cherry pick my latest
tracepoint changes so we have the best output possible.

I have also built a branch with v4.4.21 + all dwc3 patches (except for
device properties and PCI stuff) which you could use for testing. If you
run with that, then I can get proper trace output and I can try to
figure out what's missing.

Branch is here:

  git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git usb/v4.4.21+dwc3

This branch does not generate the isoch traffic (on ep2in). ftrace
attached (dwc3-g_webcam-v4.4.21+tp.ftrace).

I also attached the ftrace log for v4.4.21 tag
(dwc3-g_webcam-v4.4.21.ftrace) for comparison, which has ep2in isoch
traffic, but only one transaction per SOF.

Sorry, I didn't realize the ftrace file size is huge. Attached the
tarball here.

looking at webcam.c, it'll set wMaxPacket correctly (to 0x1400) if you
pass streaming_maxpacket=3072. So that's one thing.

I'll look at this log with more detail tomorrow, though.

here's a bug in composite.c because of a bug in usb_endpoint_maxp().

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 32176f779861..f6a7583ab6d1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -197,7 +197,7 @@ int config_ep_by_speed(struct usb_gadget *g,

 ep_found:
        /* commit results */
-       _ep->maxpacket = usb_endpoint_maxp(chosen_desc);
+       _ep->maxpacket = usb_endpoint_maxp(chosen_desc) & 0x7ff;

uvc_video set the each request size (you could check function
uvc_video_alloc_requests()) for uvc:
         req_size = video->ep->maxpacket
                  * max_t(unsigned int, video->ep->maxburst, 1)
                  * (video->ep->mult + 1);


If we change the ep->maxpacket like this, uvc layer will only set the
req size to 1024 (while it should be 3072 if we want to use high
bindwidth isoc transfer).

it'll be 1024 * (mult + 1). Hmm, mult isn't set for highspeed isoc
endpoints. So here you go:

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index f6a7583ab6d1..03d9a7886922 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -201,7 +201,11 @@ int config_ep_by_speed(struct usb_gadget *g,
        _ep->desc = chosen_desc;
        _ep->comp_desc = NULL;
        _ep->maxburst = 0;
-       _ep->mult = 0;
+       _ep->mult = 1;
+
+       if (g->speed == USB_SPEED_HIGH && usb_endpoint_xfer_isoc(_ep->desc))
+               _ep->mult = usb_endpoint_isoc_maxp_mult(_ep->desc);
+
        if (!want_comp_desc)
                return 0;

@@ -218,7 +222,7 @@ int config_ep_by_speed(struct usb_gadget *g,
                switch (usb_endpoint_type(_ep->desc)) {
                case USB_ENDPOINT_XFER_ISOC:
                        /* mult: bits 1:0 of bmAttributes */
-                       _ep->mult = comp_desc->bmAttributes & 0x3;
+                       _ep->mult = (comp_desc->bmAttributes & 0x3) + 1;
                case USB_ENDPOINT_XFER_BULK:
                case USB_ENDPOINT_XFER_INT:
                        _ep->maxburst = comp_desc->bMaxBurst + 1;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 3d0d5d94a62f..0f01c04d7cbd 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -243,7 +243,7 @@ uvc_video_alloc_requests(struct uvc_video *video)

        req_size = video->ep->maxpacket
                 * max_t(unsigned int, video->ep->maxburst, 1)
-                * (video->ep->mult + 1);
+                * (video->ep->mult);

        for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
                video->req_buffer[i] = kmalloc(req_size, GFP_KERNEL);


Looks fine to me.

Regards
Yin, Fengwei
--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux