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.
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.
--
balbi
Attachment:
signature.asc
Description: PGP signature
