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 22:22, Felipe Balbi wrote:

Hi,

yfw <nh26223@xxxxxxxxx> 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:

Great! all the 4 patches you sent so far fix the issue on v4.4.21 tag!
Now I see 3 1024-bytes transactions in each SOF.
Great news. But the same changes still can't work in my side. I need to
look at other registers.

Do you mind dumping the dwc3 registers in your side and share to me? Thanks
in advance.

tracepoints contain all that information. We trace every single register
access.
Thanks a lot for the information. I will dig the ftrace sent from Bin and
compared with registers setting my side.

Regards
Yin, Fengwei


Frankly, I think you'd be much better off trying to cherry-pick changes
back to your kernel.

Now I need to find a way to get g_webcam working with today's mainline
so I can test this periodically. Bin, what else did you change in
uvc-gadget.c? Here's what I have:

diff --git a/uvc-gadget.c b/uvc-gadget.c
index 9ef315cf0166..26ce9cdf4ea5 100644
--- a/uvc-gadget.c
+++ b/uvc-gadget.c
@@ -319,7 +319,7 @@ struct uvc_format_info
 };

 static const struct uvc_frame_info uvc_frames_yuyv[] = {
-       {  640, 360, { 666666, 10000000, 50000000, 0 }, },
+       {  640, 480, { 333333, 666666, 1000000, 0 }, },
        { 1280, 720, { 50000000, 0 }, },
        { 0, 0, { 0, }, },
 };
@@ -397,6 +397,8 @@ uvc_events_process_control(struct uvc_device *dev, uint8_t req, uint8_t cs,
        printf("control request (req %02x cs %02x)\n", req, cs);
        (void)dev;
        (void)resp;
+
+       resp->length = 0;
 }

 static void
@@ -732,6 +734,9 @@ int main(int argc, char *argv[])
                fd_set wfds = fds;

                ret = select(dev->fd + 1, NULL, &wfds, &efds, NULL);
+               if (ret < 0)
+                       fprintf(stderr, "Select failed: %s\n", strerror(errno));
+
                if (FD_ISSET(dev->fd, &efds))
                        uvc_events_process(dev);
                if (FD_ISSET(dev->fd, &wfds))


I also have this quick hack in webcam gadget:

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 29b41b5dee04..935fd76ba83e 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -396,6 +399,9 @@ uvc_function_connect(struct uvc_device *uvc)
        struct usb_composite_dev *cdev = uvc->func.config->cdev;
        int ret;

+       if (cdev->deactivations == 0)
+               return;
+
        if ((ret = usb_function_activate(&uvc->func)) < 0)
                INFO(cdev, "UVC connect failed with %d\n", ret);
 }
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index f4ccbd56f4d2..6c0ac0525b80 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -298,7 +300,6 @@ uvc_v4l2_open(struct file *file)
        handle->device = &uvc->video;
        file->private_data = &handle->vfh;

-       uvc_function_connect(uvc);
        return 0;
 }

@@ -340,6 +341,7 @@ uvc_v4l2_poll(struct file *file, poll_table *wait)
        struct video_device *vdev = video_devdata(file);
        struct uvc_device *uvc = video_get_drvdata(vdev);

+       uvc_function_connect(uvc);
        return uvcg_queue_poll(&uvc->video.queue, file, wait);
 }

diff --git a/drivers/usb/gadget/legacy/webcam.c b/drivers/usb/gadget/legacy/webcam.c
index 72c976bf3530..bfbe7f10175a 100644
--- a/drivers/usb/gadget/legacy/webcam.c
+++ b/drivers/usb/gadget/legacy/webcam.c
@@ -191,15 +191,15 @@ static const struct UVC_FRAME_UNCOMPRESSED(3) uvc_frame_yuv_360p = {
        .bFrameIndex            = 1,
        .bmCapabilities         = 0,
        .wWidth                 = cpu_to_le16(640),
-       .wHeight                = cpu_to_le16(360),
+       .wHeight                = cpu_to_le16(480),
        .dwMinBitRate           = cpu_to_le32(18432000),
        .dwMaxBitRate           = cpu_to_le32(55296000),
-       .dwMaxVideoFrameBufferSize      = cpu_to_le32(460800),
-       .dwDefaultFrameInterval = cpu_to_le32(666666),
+       .dwMaxVideoFrameBufferSize      = cpu_to_le32(614400),
+       .dwDefaultFrameInterval = cpu_to_le32(333333),
        .bFrameIntervalType     = 3,
-       .dwFrameInterval[0]     = cpu_to_le32(666666),
-       .dwFrameInterval[1]     = cpu_to_le32(1000000),
-       .dwFrameInterval[2]     = cpu_to_le32(5000000),
+       .dwFrameInterval[0]     = cpu_to_le32(333333),
+       .dwFrameInterval[1]     = cpu_to_le32(666666),
+       .dwFrameInterval[2]     = cpu_to_le32(1000000),
 };

 static const struct UVC_FRAME_UNCOMPRESSED(1) uvc_frame_yuv_720p = {

Still no luck. I'll continue hacking on this tomorrow.

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