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

2016-09-29 Thread yfw 

Hi Bin,

On 2016/9/29 3:48, Bin Liu wrote:

On Wed, Sep 28, 2016 at 09:51:32AM -0500, Bin Liu wrote:

On Wed, Sep 28, 2016 at 05:15:59PM +0300, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

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.


cool, I just sent a huge patch bomb which contains all these tiny fixes
plus a rework of usb_endpoint_maxp(). Care to give a round of review so
we get it into v4.10 (not v4.9, that's too late) merge window?


Sure, will do soon.



thanks for testing. If it's not too much to ask, care to test my
v4.4.21+dwc3 branch? I force-pushed all latest changes to that branch so
it's easy for you to test.


No problem, I will test it in a couple days. I have to jump in another
urgent task which popped up yesterday.



Please make sure to capture dwc3's tracepoints, I need to make sure
we're printing things correctly now ;-)


Yes, I will capture it.


Well, current v4.4.21+dwc3 (HEAD: 60f7f4f) is kinda working, but seems
not stable. Attached are the logs for different cases (console logs +
ftrace).

- sometimes it works, the bus trace shows 3 transactions per SOF!
- but sometimes there is only 1 transactions per SOF, and data packet is
  0 byte from DWC3.
  ftrace: dwc3-g-webcam-zlp-one-per-sof.ftrace

The content of this file confused me:

   irq/441-dwc3-801   [001] d...96.575846: dwc3_prepare_trb: ep2in: 
4/4 trb f20b6000 buf ad889000 size 3072 ctrl 0c69 
(HlcS:SC:isoc-first)


The size field has no PCM1 set to 2. I suppose high bandwidth isoc need
PCM1.

And the registers you dumped:
DCFG = 0x004808ac
show it's super speed mode.

Felipe's patch only set PCM1 for high speed. This could explain why the
size field has no PCM1 set. But confused me more because high bandwidth
isoc work in your side sometime.

Regards
Yin, Fengwei


- sometimes console has ep0 warning. But I forgot how the bus traffic is
  in this case.
  cosnole log: dwc3-g-webcam-ep0-warning-console.log
- sometimes there is no isoch xfter at all on the bus, and console has
  dead lock message as shown below.
  ftrace: dwc3-g-webcam-no-iso-xfter.ftrace
  console log: dwc3-g-webcam-no-iso-xfter.console.log


 42 [   54.119060] =
 43 [   54.124732] [ INFO: possible recursive locking detected ]
 44 [   54.130397] 4.4.21-00210-gabbc900 #51 Not tainted
 45 [   54.135338] -
 46 [   54.141007] uvc-gadget/780 is trying to acquire lock:
 47 [   54.146312]  (&(>irqlock)->rlock){..}, at: [] 
uvcg_queue_cancel+0x2c/0xa4 [usb_f_uvc]
 48 [   54.156507]
 49 [   54.156507] but task is already holding lock:
 50 [   54.162633]  (&(>irqlock)->rlock){..}, at: [] 
uvcg_video_pump+0x5c/0x164 [usb_f_uvc]
 51 [   54.172723]
 52 [   54.172723] other info that might help us debug this:
 53 [   54.179578]  Possible unsafe locking scenario:
 54 [   54.179578]
 55 [   54.185793]CPU0
 56 [   54.188356]
 57 [   54.190919]   lock(&(>irqlock)->rlock);
 58 [   54.195599]   lock(&(>irqlock)->rlock);
 59 [   54.200279]
 60 [   54.200279]  *** DEADLOCK ***
 61 [   54.200279]
 62 [   54.206498]  May be due to missing lock nesting notation
 63 [   54.206498]
 64 [   54.213626] 2 locks held by uvc-gadget/780:
 65 [   54.218017]  #0:  (>video.mutex){+.+.+.}, at: [] 
v4l2_ioctl+0x60/0xf0 [videodev]
 66 [   54.227241]  #1:  (&(>irqlock)->rlock){..}, at: [] 
uvcg_video_pump+0x5c/0x164 [usb_f_uvc]
 67 [   54.237801]
 68 [   54.237801] stack backtrace:
 69 [   54.242381] CPU: 0 PID: 780 Comm: uvc-gadget Not tainted 
4.4.21-00210-gabbc900 #51
 70 [   54.250329] Hardware name: Generic DRA74X (Flattened Device Tree)
 71 [   54.256742] [] (unwind_backtrace) from [] 
(show_stack+0x20/0x24)
 72 [   54.264880] [] (show_stack) from [] 
(dump_stack+0xb4/0xe8)
 73 [   54.272471] [] (dump_stack) from [] 
(__lock_acquire+0x1450/0x1dec)
 74 [   54.280789] [] (__lock_acquire) from [] 
(lock_acquire+0xdc/0x1b8)
 75 [   54.289022] [] (lock_acquire) from [] 
(_raw_spin_lock_irqsave+0x48/0x5c)
 76 [   54.297901] [] (_raw_spin_lock_irqsave) from [] 
(uvcg_queue_cancel+0x2c/0xa4 [usb_f_uvc])
 77 [   54.308343] [] (uvcg_queue_cancel [usb_f_uvc]) from 
[] (uvc_video_complete+0xb0/0x15c [usb_f_uvc])
 78 [   54.319603] [] (uvc_video_complete [usb_f_uvc]) from 
[] (usb_gadget_giveback_request+0x1c/0x20 [udc_core])
 79 [   54.331593] [] (usb_gadget_giveback_request

Re: [PATCH 2/2] usb: dwc3: gadget: set PCM1 field of isochronous-first TRBs

2016-09-29 Thread yfw 

Hi Felipe,

On 2016/9/29 17:48, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> writes:

On 2016/9/26 16:12, Felipe Balbi wrote:

In case of High-Speed, High-Bandwidth endpoints, we
need to tell DWC3 that we have more than one packet
per interval. We do that by setting PCM1 field of
Isochronous-First TRB.

Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
---
 drivers/usb/dwc3/gadget.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 997e82dcc55e..5f7e39407892 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -793,6 +793,9 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
unsigned length, unsigned chain, unsigned node)
 {
struct dwc3_trb *trb;
+   struct dwc3 *dwc = dep->dwc;
+   struct usb_gadget   *gadget = >gadget;
+   enum usb_device_speed   speed = gadget->speed;

dwc3_trace(trace_dwc3_gadget, "%s: req %p dma %08llx length %d%s",
dep->name, req, (unsigned long long) dma,
@@ -819,10 +822,17 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
break;

case USB_ENDPOINT_XFER_ISOC:
-   if (!node)
+   if (!node) {
trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
-   else
+
+   if (speed == USB_SPEED_HIGH) {
+   struct usb_ep *ep = >endpoint;
+   u8 pkts = usb_endpoint_isoc_maxp_mult(ep->desc);
+   trb->size |= DWC3_TRB_SIZE_PCM1(pkts - 1);
+   }
+   } else {
trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS;
+   }

/* always enable Interrupt on Missed ISOC */
trb->ctrl |= DWC3_TRB_CTRL_ISP_IMI;



Looks good to me. You could add
Reviewed-by: Fengwei Yin <nh26...@gmail.com>


looking at the wrong thread, please have a look at this one:

https://marc.info/?l=linux-usb=147506797611366=2

Sorry. I didn't subscribe to usb mail list.  And don't know how to reply
that thread.

Regards
Yin, Fengwei




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 



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


Hi,

yfw <nh26...@gmail.com> writes:

Bin Liu <b-...@ti.com> 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 good to me. You could add
   Reviewed-by: Fengwei Yin <nh26...@gmail.com>


Regards
Yin, Fengwei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Bin,

On 2016/9/28 22:40, Bin Liu wrote:

On Wed, Sep 28, 2016 at 10:13:00PM +0800, yfw wrote:


On 2016/9/28 22:08, Bin Liu wrote:

On Wed, Sep 28, 2016 at 10:03:16PM +0800, yfw wrote:

[snip]



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.


on your 3.18 kernel?

Yes. On my 3.18 kernel. But I am sure the trbs are same for the isoc
transfer. I am suspecting other registers now.





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


in which state you want me to 'cat regdump'? before or after run the
video capture program on the host?

I don't know whether you enabled runtime pm or not. If possible, could you
please help to capture the regdump before and after the video capture app with
runtime pm DISABLED? Thanks a lot.


I just added usbcore.autosuspend=-1 in uboot bootargs. Attached are the
regdumps before and after running video capture.

Thanks a lot for helping on this. Really appreciate it.

Regards
Yin, Fengwei



Regards,
-Bin.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: dwc3: gadget: set PCM1 field of isochronous-first TRBs

2016-09-28 Thread yfw 



On 2016/9/26 16:12, Felipe Balbi wrote:

In case of High-Speed, High-Bandwidth endpoints, we
need to tell DWC3 that we have more than one packet
per interval. We do that by setting PCM1 field of
Isochronous-First TRB.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/gadget.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 997e82dcc55e..5f7e39407892 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -793,6 +793,9 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
unsigned length, unsigned chain, unsigned node)
 {
struct dwc3_trb *trb;
+   struct dwc3 *dwc = dep->dwc;
+   struct usb_gadget   *gadget = >gadget;
+   enum usb_device_speed   speed = gadget->speed;

dwc3_trace(trace_dwc3_gadget, "%s: req %p dma %08llx length %d%s",
dep->name, req, (unsigned long long) dma,
@@ -819,10 +822,17 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
break;

case USB_ENDPOINT_XFER_ISOC:
-   if (!node)
+   if (!node) {
trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
-   else
+
+   if (speed == USB_SPEED_HIGH) {
+   struct usb_ep *ep = >endpoint;
+   u8 pkts = usb_endpoint_isoc_maxp_mult(ep->desc);
+   trb->size |= DWC3_TRB_SIZE_PCM1(pkts - 1);
+   }
+   } else {
trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS;
+   }

/* always enable Interrupt on Missed ISOC */
trb->ctrl |= DWC3_TRB_CTRL_ISP_IMI;



Looks good to me. You could add
   Reviewed-by: Fengwei Yin 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 



On 2016/9/28 22:25, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> writes:

Hi Felipe,

On 2016/9/28 22:15, Felipe Balbi wrote:


Hi,

Bin Liu <b-...@ti.com> writes:

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.


cool, I just sent a huge patch bomb which contains all these tiny fixes
plus a rework of usb_endpoint_maxp(). Care to give a round of review so
we get it into v4.10 (not v4.9, that's too late) merge window?

I have already reviewed all the patches related. You could add my "reviewed-by".

You need to reply on the actual thread, with a real name, sorry. Have a
look at Documentation/SubmittingPatches.

Sure. I will.

Regards
Yin, Fengwei




And thanks a lot for looking at this issue.


yeah, no problem


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Felipe,

On 2016/9/26 16:12, Felipe Balbi wrote:

According to USB Specification 2.0 table 9-4,
wMaxPacketSize is a bitfield. Endpoint's maxpacket
is laid out in bits 10:0. For high-speed,
high-bandwidth isochronous endpoints, bits 12:11
contain a multiplier to tell us how many
transactions we want to try per uframe.

This means that if we want an isochronous endpoint
to issue 3 transfers of 1024 bytes per uframe,
wMaxPacketSize should contain the value:

1024 | (2 << 11)

or 5120 (0x1400). In order to make Host and
Peripheral controller drivers' life easier, we're
adding a helper which returns bits 12:11. Note that
no care is made WRT to checking endpoint type and
gadget's speed. That's left for drivers to handle.

Signed-off-by: Felipe Balbi 
---
 include/uapi/linux/usb/ch9.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
index a8acc24765fe..73bcb24d4077 100644
--- a/include/uapi/linux/usb/ch9.h
+++ b/include/uapi/linux/usb/ch9.h
@@ -423,6 +423,11 @@ struct usb_endpoint_descriptor {
 #define USB_ENDPOINT_XFER_INT  3
 #define USB_ENDPOINT_MAX_ADJUSTABLE0x80

+#define USB_EP_ISOC_MAXP_MULT_SHIFT11
+#define USB_EP_ISOC_MAXP_MULT_MASK (3 << USB_EP_ISOC_MAXP_MULT_SHIFT)
+#define USB_EP_ISOC_MAXP_MULT(m) \
+   (((m) & USB_EP_ISOC_MAXP_MULT_MASK) >> USB_EP_ISOC_MAXP_MULT_SHIFT)
+
 /* The USB 3.0 spec redefines bits 5:4 of bmAttributes as interrupt ep type. */
 #define USB_ENDPOINT_INTRTYPE  0x30
 #define USB_ENDPOINT_INTR_PERIODIC (0 << 4)
@@ -630,6 +635,20 @@ static inline int usb_endpoint_maxp(const struct 
usb_endpoint_descriptor *epd)
return __le16_to_cpu(epd->wMaxPacketSize);
 }

+/**
+ * usb_endpoint_isoc_maxp_mult - get endpoint's transactional opportunities
+ * @epd: endpoint to be checked
+ *
+ * Return @epd's wMaxPacketSize[12:11] + 1
+ */
+static inline int
+usb_endpoint_isoc_maxp_mult(const struct usb_endpoint_descriptor *epd)
+{
+   int maxp = __le16_to_cpu(epd->wMaxPacketSize);
+
+   return USB_EP_ISOC_MAXP_MULT(maxp) + 1;
+}
+
 static inline int usb_endpoint_interrupt_type(
const struct usb_endpoint_descriptor *epd)
 {



Looks good to me. You could add
  Reviewed-by: Fengwei Yin 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Felipe,

On 2016/9/28 22:22, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> 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, { 66, 1000, 5000, 0 }, },
+   {  640, 480, { 33, 66, 100, 0 }, },
{ 1280, 720, { 5000, 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, , , NULL);
+   if (ret < 0)
+   fprintf(stderr, "Select failed: %s\n", strerror(errno));
+
if (FD_ISSET(dev->fd, ))
uvc_events_process(dev);
if (FD_ISSET(dev->fd, ))


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(>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..6

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

2016-09-28 Thread yfw 


On 2016/9/28 22:08, Bin Liu wrote:

On Wed, Sep 28, 2016 at 10:03:16PM +0800, yfw wrote:

[snip]



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.


on your 3.18 kernel?

Yes. On my 3.18 kernel. But I am sure the trbs are same for the isoc
transfer. I am suspecting other registers now.





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


in which state you want me to 'cat regdump'? before or after run the
video capture program on the host?

I don't know whether you enabled runtime pm or not. If possible, could you
please help to capture the regdump before and after the video capture app with
runtime pm DISABLED? Thanks a lot.


Regards
Yin, Fengwei



Thanks,
-Bin.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Felipe,

On 2016/9/28 22:15, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

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.


cool, I just sent a huge patch bomb which contains all these tiny fixes
plus a rework of usb_endpoint_maxp(). Care to give a round of review so
we get it into v4.10 (not v4.9, that's too late) merge window?

I have already reviewed all the patches related. You could add my "reviewed-by".
And thanks a lot for looking at this issue.

Regards
Yin, Fengwei



thanks for testing. If it's not too much to ask, care to test my
v4.4.21+dwc3 branch? I force-pushed all latest changes to that branch so
it's easy for you to test.

Please make sure to capture dwc3's tracepoints, I need to make sure
we're printing things correctly now ;-)

Best


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 



On 2016/9/28 21:35, Bin Liu wrote:

On Wed, Sep 28, 2016 at 11:05:30AM +0300, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> writes:

Bin Liu <b-...@ti.com> 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.

Regards
Yin, Fengwei



Thanks,
-Bin.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Felipe,

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


Hi,

yfw <nh26...@gmail.com> writes:

Bin Liu <b-...@ti.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-28 Thread yfw 

Hi Felipe,

On 2016/9/28 15:37, Felipe Balbi wrote:


Hi,

Felipe Balbi  writes:

Hi,

Bin Liu  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).


Regards
Yin, Fengwei


_ep->desc = chosen_desc;
_ep->comp_desc = NULL;
_ep->maxburst = 0;

We have to fix it like this, at least for now, because changing
usb_endpoint_maxp() involves doing a complete audit of all call sites of
usb_endpoint_maxp(). I'll do that for v4.10 merge window.

Can you check that if this helps in any way?


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-27 Thread yfw 



On 2016/9/27 23:12, Bin Liu wrote:

On Tue, Sep 27, 2016 at 11:02:54PM +0800, yfw wrote:



On 2016/9/27 23:00, Bin Liu wrote:

On Tue, Sep 27, 2016 at 10:53:54PM +0800, yfw wrote:

Hi,

On 2016/9/27 22:28, Bin Liu wrote:

On Tue, Sep 27, 2016 at 09:20:56AM -0500, Bin Liu wrote:

Felipe,

On Tue, Sep 27, 2016 at 10:18:26AM +0300, Felipe Balbi wrote:

[snip]


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.
(dwc3-g_webcam-ftrace.tgz)

Regards,
-Bin.


>From the ftrace log:



irq/441-dwc3-769   [001] d..2   101.207679: dwc3_gadget: ep2in-isoc: req
ed971b00 dma ad9d2000 length 5120

5120 looks wrong to me. 5120 == 0x1400. There is an issue in uvc_video which
set the req_size according maxpacket (which is 0x1400 for high bandwidth isoc).

What about this?


Yeah, seems not right to me too. The UDC would only have 1024 bytes to
transmit.






irq/441-dwc3-769   [001] d..2   101.207682: dwc3_prepare_trb: ep2in-isoc:
trb f2037020 bph  bpl ad9d2000 size 1400 ctrl 0c69


Looks like the "bits 12:11 of wMaxPacketSize" patch from Felipe was not applied.


No, I didn't. I was trying to give a trace for the base line to start
with.

Get it. Waiting for your further testing result.


I am waiting for Felipe's next instruction - going on v4.4.21 tag or his
v4.4.21+dwc3 HEAD.

Is it possible that you try "bits 12:11 of wMaxPacketSize" patches from Felipe
with your current HEAD now?
You may hardcode the req_size (3072) in uvc_video (only for test).

Regards
Yin, Fengwei



Regards,
-Bin.



Regards
Yin, Fengwei



Regards,
-Bin.




irq/441-dwc3-769   [001] d..2   101.208357: dwc3_complete_trb: ep2in-isoc:
trb f2037020 bph  bpl ad9d2000 size 10001400 ctrl 1bfd0c68


MissedIsoc happened.


Regards
Yin, Fengwei


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Strange behavior of CHN bit with dwc3

2016-09-27 Thread yfw 



On 2016/9/27 11:55, mgau...@codeaurora.org wrote:

On 2016-09-22 18:46, yfw wrote:

Hi list,
I tried to enable the high speed, high bandwidth transfer in device
mode for iso type on dwc3 based soc. The platform only supports usb
device 2.0.

I set the MaxPacketSize to 0x1400 so the host could allocate 3072
bytes for uframe. But when I chain three trbs together, the dwc3
behavior is quite weird:



Why are you using three TRBs for one service interval? Is the data
not contiguous?
Since, in your case all 3072 bytes belong to one service interval,
could just one TRB be used with PktCntM1 set to '2' ?


BTW, the configures:
1. 1 trb with one 3072 bytes buffer:
   trb1:  size set to DWC3_TRB_SIZE_PCM1(2) | 0xc00
  DMA = Buffer DMA address
2. 3 trbs with one 3072 bytes buffer:
   trb1: size DWC3_TRB_SIZE_PCM1(2) | 0x400, ctrl with CHN bit set
 DMA = buffer DMA address
   trb2: size: 0x400, ctrl with CHN bit set
 DMA = buffer DMA address + 0x400
   trb3: size: 0x400, ctrl with CHN bit NOT set
 DMA = buffer DMA address + 0x800

should have same behavior. Right?

Regards
Yin, Fengwei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-27 Thread yfw 



On 2016/9/27 23:00, Bin Liu wrote:

On Tue, Sep 27, 2016 at 10:53:54PM +0800, yfw wrote:

Hi,

On 2016/9/27 22:28, Bin Liu wrote:

On Tue, Sep 27, 2016 at 09:20:56AM -0500, Bin Liu wrote:

Felipe,

On Tue, Sep 27, 2016 at 10:18:26AM +0300, Felipe Balbi wrote:

[snip]


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.
(dwc3-g_webcam-ftrace.tgz)

Regards,
-Bin.


From the ftrace log:


irq/441-dwc3-769   [001] d..2   101.207679: dwc3_gadget: ep2in-isoc: req
ed971b00 dma ad9d2000 length 5120

5120 looks wrong to me. 5120 == 0x1400. There is an issue in uvc_video which
set the req_size according maxpacket (which is 0x1400 for high bandwidth isoc).

What about this?




irq/441-dwc3-769   [001] d..2   101.207682: dwc3_prepare_trb: ep2in-isoc:
trb f2037020 bph  bpl ad9d2000 size 1400 ctrl 0c69


Looks like the "bits 12:11 of wMaxPacketSize" patch from Felipe was not applied.


No, I didn't. I was trying to give a trace for the base line to start
with.

Get it. Waiting for your further testing result.

Regards
Yin, Fengwei



Regards,
-Bin.




irq/441-dwc3-769   [001] d..2   101.208357: dwc3_complete_trb: ep2in-isoc:
trb f2037020 bph  bpl ad9d2000 size 10001400 ctrl 1bfd0c68


MissedIsoc happened.


Regards
Yin, Fengwei


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-27 Thread yfw 

Hi,

On 2016/9/27 22:28, Bin Liu wrote:

On Tue, Sep 27, 2016 at 09:20:56AM -0500, Bin Liu wrote:

Felipe,

On Tue, Sep 27, 2016 at 10:18:26AM +0300, Felipe Balbi wrote:

[snip]


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.
(dwc3-g_webcam-ftrace.tgz)

Regards,
-Bin.


From the ftrace log:

> irq/441-dwc3-769   [001] d..2   101.207679: dwc3_gadget: ep2in-isoc: req
> ed971b00 dma ad9d2000 length 5120
5120 looks wrong to me. 5120 == 0x1400. There is an issue in uvc_video which
set the req_size according maxpacket (which is 0x1400 for high bandwidth isoc).

> irq/441-dwc3-769   [001] d..2   101.207682: dwc3_prepare_trb: ep2in-isoc:
> trb f2037020 bph  bpl ad9d2000 size 1400 ctrl 0c69

Looks like the "bits 12:11 of wMaxPacketSize" patch from Felipe was not applied.

> irq/441-dwc3-769   [001] d..2   101.208357: dwc3_complete_trb: ep2in-isoc:
> trb f2037020 bph  bpl ad9d2000 size 10001400 ctrl 1bfd0c68

MissedIsoc happened.


Regards
Yin, Fengwei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-09-26 Thread yfw 

Hi Felipe,

On 2016/9/26 16:12, Felipe Balbi wrote:

According to USB Specification 2.0 table 9-4,
wMaxPacketSize is a bitfield. Endpoint's maxpacket
is laid out in bits 10:0. For high-speed,
high-bandwidth isochronous endpoints, bits 12:11
contain a multiplier to tell us how many
transactions we want to try per uframe.

This means that if we want an isochronous endpoint
to issue 3 transfers of 1024 bytes per uframe,
wMaxPacketSize should contain the value:

1024 | (2 << 11)

or 5120 (0x1400). In order to make Host and
Peripheral controller drivers' life easier, we're
adding a helper which returns bits 12:11. Note that
no care is made WRT to checking endpoint type and
gadget's speed. That's left for drivers to handle.

Signed-off-by: Felipe Balbi 
---
 include/uapi/linux/usb/ch9.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
index a8acc24765fe..73bcb24d4077 100644
--- a/include/uapi/linux/usb/ch9.h
+++ b/include/uapi/linux/usb/ch9.h
@@ -423,6 +423,11 @@ struct usb_endpoint_descriptor {
 #define USB_ENDPOINT_XFER_INT  3
 #define USB_ENDPOINT_MAX_ADJUSTABLE0x80

+#define USB_EP_ISOC_MAXP_MULT_SHIFT11
+#define USB_EP_ISOC_MAXP_MULT_MASK (3 << USB_EP_ISOC_MAXP_MULT_SHIFT)
+#define USB_EP_ISOC_MAXP_MULT(m) \
+   (((m) & USB_EP_ISOC_MAXP_MULT_MASK) >> USB_EP_ISOC_MAXP_MULT_SHIFT)
+
 /* The USB 3.0 spec redefines bits 5:4 of bmAttributes as interrupt ep type. */
 #define USB_ENDPOINT_INTRTYPE  0x30
 #define USB_ENDPOINT_INTR_PERIODIC (0 << 4)
@@ -630,6 +635,20 @@ static inline int usb_endpoint_maxp(const struct 
usb_endpoint_descriptor *epd)
return __le16_to_cpu(epd->wMaxPacketSize);
 }

+/**
+ * usb_endpoint_isoc_maxp_mult - get endpoint's transactional opportunities
+ * @epd: endpoint to be checked
+ *
+ * Return @epd's wMaxPacketSize[12:11] + 1
+ */
+static inline int
+usb_endpoint_isoc_maxp_mult(const struct usb_endpoint_descriptor *epd)
+{
+   int maxp = __le16_to_cpu(epd->wMaxPacketSize);
+
+   return USB_EP_ISOC_MAXP_MULT(maxp) + 1;
+}
+
 static inline int usb_endpoint_interrupt_type(
const struct usb_endpoint_descriptor *epd)
 {


Does this mean the issue of isoc high bandwidth transfer was fixed by
this patchset per your test?

Regards
Yin, Fengwei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: g_webcam Isoch high bandwidth transfer

2016-09-23 Thread yfw 

Hi Felipe,

On 2016/9/23 15:49, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

+Fengwei Yin per his request.

On Thu, Sep 22, 2016 at 10:48:40PM +0300, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

[...]


Here's one that actually compiles, sorry about that.


No worries, I was sleeping ;-)

I will test it out early next week. Thanks.


meanwhile, how about some instructions on how to test this out myself?
How are you using g_webcam and what are you running on host side? Got a
nice list of commands there I can use? I think I can get to bottom of
this much quicker if I can reproduce it locally ;-)


On device side:
- first patch g_webcam as in my first email in this thread to enable
  640x480@30fps;
- # modprobe g_webcam streaming_maxpacket=3072
- then run uvc-gadget to feed the YUV frames;
http://git.ideasonboard.org/uvc-gadget.git


as is, g_webcam never enumerates to the host. It's calls to
usb_function_active() and usb_function_deactivate() are unbalanced. Do
you have any other changes to g_webcam?

With uvc function gadget driver, user daemon uvc-gadget must be started
before connect to host. Not sure whether g_webcam has same requirement.



Also, uvc-gadget.git doesn't compile, had to modify it a bit:

-#include "../drivers/usb/gadget/uvc.h"
+#include "../drivers/usb/gadget/function/uvc.h"

Also fixed a build warning:

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

ret = select(dev->fd + 1, NULL, , , NULL);
+   if (ret < 0)
+   return ret;
if (FD_ISSET(dev->fd, ))
uvc_events_process(dev);
if (FD_ISSET(dev->fd, ))

Laurent, have you tested g_webcam recently? What's the magic to get it
working?

Here's what I get out of dmesg:

[   58.568380] usb 1-9: new high-speed USB device number 5 using xhci_hcd
[   58.738680] usb 1-9: New USB device found, idVendor=1d6b, idProduct=0102
[   58.738683] usb 1-9: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[   58.738685] usb 1-9: Product: Webcam gadget
[   58.738687] usb 1-9: Manufacturer: Linux Foundation
[   58.739133] g_webcam gadget: high-speed config #1: Video
[   58.739138] g_webcam gadget: uvc_function_set_alt(0, 0)
[   58.739139] g_webcam gadget: reset UVC Control
[   58.739149] g_webcam gadget: uvc_function_set_alt(1, 0)
[   58.804369] uvcvideo: Found UVC 1.00 device Webcam gadget (1d6b:0102)
[   58.804479] g_webcam gadget: uvc_function_set_alt(1, 0)
[   64.188459] uvcvideo: UVC non compliance - GET_DEF(PROBE) not supported. 
Enabling workaround.

Looks like you connect your usb device to your usb host port on same
board. Nice.

The GET_DEF is handled by user daemon uvc-gadget. It may be related.

Regards
Yin, Fengwei


[   69.307458] uvcvideo: Failed to query (129) UVC probe control : -110 (exp. 
26).
[   69.307459] uvcvideo: Failed to initialize the device (-5).
[   69.307505] usbcore: registered new interface driver uvcvideo
[   69.307506] USB Video Class driver (1.1.1)
[  146.646012] [ cut here ]
[  146.646023] WARNING: CPU: 0 PID: 2616 at drivers/usb/gadget/composite.c:371 
usb_function_activate+0x77/0x80 [libcomposite]
[  146.646024] Modules linked in: uvcvideo g_webcam usb_f_uvc videobuf2_vmalloc 
videobuf2_memops videobuf2_v4l2 videobuf2_core videodev libcomposite kvm_intel 
kvm psmouse e1000e input_leds hid_generic usbhid atkbd irqbypass evdev
[  146.646054] CPU: 0 PID: 2616 Comm: gst-launch-1.0 Not tainted 
4.8.0-rc7-next-20160922-4-gc71031593917-dirty #20
[  146.646055] Hardware name: Intel Corporation Skylake Client platform/Skylake 
Y LPDDR3 RVP3, BIOS SKLSE2R1.R00.B097.B02.1509020030 09/02/2015
[  146.646058]  c9000769bb70
[  146.646059]  8132d415
[  146.646060]  
[  146.646061]  

[  146.646063]  c9000769bbb0
[  146.646063]  8105ec1b
[  146.646064]  01730769bb90
[  146.646066]  ffea
[  146.646070]  88016c03a150
[  146.646072]  0282 88016d793000 c9000769befc
[  146.646077] Call Trace:
[  146.646086]  [] dump_stack+0x68/0x93
[  146.646090]  [] __warn+0xcb/0xf0
[  146.646095]  [] warn_slowpath_null+0x1d/0x20
[  146.646099]  [] usb_function_activate+0x77/0x80 
[libcomposite]
[  146.646105]  [] uvc_function_connect+0x1e/0x40 [usb_f_uvc]
[  146.646110]  [] uvc_v4l2_open+0x6e/0x80 [usb_f_uvc]
[  146.646116]  [] v4l2_open+0xa0/0x100 [videodev]
[  146.646121]  [] chrdev_open+0xa1/0x1d0
[  146.646125]  [] ? cdev_put+0x30/0x30
[  146.646129]  [] do_dentry_open.isra.17+0x150/0x2e0
[  146.646133]  [] vfs_open+0x45/0x60
[  146.646137]  [] path_openat+0x62d/0x1370
[  146.646141]  [] ? putname+0x54/0x60
[  146.646146]  [] do_filp_open+0x7e/0xe0
[  146.646150]  [] ? preempt_count_sub+0x48/0x70
[  146.646154]  [] ? _raw_spin_unlock+0x16/0x30
[  146.646160]  [] ? __alloc_fd+0xc9/0x180
[  146.646164]  [] do_sys_open+0x123/0x200
[  146.646170]  []

Re: Strange behavior of CHN bit with dwc3

2016-09-23 Thread yfw 

Hi Felipe,

On 2016/9/23 14:17, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> writes:

are you using mainline? If you are, capture dwc3 tracepoints. If you're
not, which kernel are you using?

I am not using the latest kernel. Instead the code base is 3.18 + some usb
patches backported because mainline doesn't support the platform I am using.


v3.18 is really, really old. There isn't much I can do to help
you. Also, claiming that my patch doesn't work wasn't very nice. There
might be a ton of other dependencies that were merged since v3.18.

Yes. This makes a lot sense. And sorry for claiming your patch not
working too early. Let's wait for the test result from Bin (I suppose
his test is using the mainline driver).



You should really be asking for support from whoever gave you this v3.18
kernel. This forum can only help mainline. You're welcome to stick
around and try things on your own, but I'll only make sure that mainline
works and that's about it ;-)

Totally understand. I am also trying to replace the current driver with
the latest one from mainline kernel.

Regards
Yin, Fengwei




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: g_webcam Isoch high bandwidth transfer

2016-09-22 Thread yfw 

Hi Bin,

On 2016/9/23 4:11, Bin Liu wrote:

+Fengwei Yin per his request.

Thanks a lot for adding me to this thread.



On Thu, Sep 22, 2016 at 10:48:40PM +0300, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

[...]


Here's one that actually compiles, sorry about that.


No worries, I was sleeping ;-)

I will test it out early next week. Thanks.


meanwhile, how about some instructions on how to test this out myself?
How are you using g_webcam and what are you running on host side? Got a
nice list of commands there I can use? I think I can get to bottom of
this much quicker if I can reproduce it locally ;-)

I am using similar use case with a different gadget function driver.


On device side:
- first patch g_webcam as in my first email in this thread to enable
  640x480@30fps;
- # modprobe g_webcam streaming_maxpacket=3072
- then run uvc-gadget to feed the YUV frames;
http://git.ideasonboard.org/uvc-gadget.git

- I am using uvc function driver  + configfs.
- maxpacket in configfs was set to 3072.
- uvc-gadget from the same source as Bin.


On host side:
- first check the device ep descriptor, which should be
wMaxPacketSize 0x1400  3x 1024 bytes
- then use luvcview or yavta to capture the video stream

- lsusb give me same wMaxPacketSize.
- I am using example (changed a little bit) from libuvc.

Regards
Yin, Fengwei



Capture the bus trace to check if multiple IN transactions happens in
each SOF.

The data buffer size in the usb_request coming from the uvc driver is
5120 bytes, so there should be 3 IN transactions if the UDC works
correctly.

Regards,
-Bin.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Strange behavior of CHN bit with dwc3

2016-09-22 Thread yfw 

Hi Felipe,

On 2016/9/23 3:47, Felipe Balbi wrote:


Hi,

yfw <nh26...@gmail.com> writes:

Hi Felipe, Bin,



Hi Felipe,
I am very sorry for the multiple sending. The previous were rejected because of
HTML format because I used gmail web interface.

Hi list,
I tried to enable the high speed, high bandwidth transfer in device mode for iso
type on dwc3 based soc. The platform only supports usb device 2.0.

I set the MaxPacketSize to 0x1400 so the host could allocate 3072 bytes for
uframe. But when I chain three trbs together, the dwc3 behavior is quite weird:

If I set the individual trb as following:
[ 32.495556] trb: ff800080f1b0, trb dump: bpl: 0xec803000,bph: 0x0, size:
0x400, ctrl: 0x469
[ 32.495560] trb: ff800080f1c0, trb dump: bpl: 0xec803400,bph: 0x0, size:
0x400, ctrl: 0x469
[ 32.495564] trb: ff800080f1d0, trb dump: bpl: 0xec803800,bph: 0x0, size:
0x400, ctrl: 0xc69

It's actually not high bandwidth. Just normal ISO transfer. Everything is fine.
I got following msg:
[ 32.495909] dwc3_cleanup_done_reqs: trb: ff800080f1b0,trb->size: 0x0,
trb->ctrl: 0x393dc468, trb_num: 3
[ 32.495915] dwc3_cleanup_done_reqs: trb: ff800080f1c0,trb->size: 0x0,
trb->ctrl: 0x393e0478, trb_num: 3
[ 32.495920] dwc3_cleanup_done_reqs: trb: ff800080f1d0,trb->size: 0x0,
trb->ctrl: 0x393e4c78, trb_num: 3

We could see every trb has size to 0 finally.

But if I set the trb chain as:
[ 42.137322] trb: ff800080f000, trb dump: bpl: 0xe2a8c000,bph: 0x0, size:
0x2000400, ctrl: 0x46d
[ 42.137326] trb: ff800080f010, trb dump: bpl: 0xe2a8c400,bph: 0x0, size:
0x400, ctrl: 0x47d
[ 42.137330] trb: ff800080f020, trb dump: bpl: 0xe2a8c800,bph: 0x0, size:
0x400, ctrl: 0xc79

There are three trbs. The first and second has CHN bit set. The first one has
First-ISO type.
And the PCM1 filed is set to 2 for the first one. The last one has IOC bit set.
I tried to let dwc3
send the first one at DATA2 stage, the second one at DATA1 stage and the third
one at DATA0
stage.

But there result is:
[ 42.137872] dwc3_cleanup_done_reqs: trb: ff800080f000,trb->size: 0x0,
trb->ctrl: 0x2cd7c46c, trb_num: 3
[ 42.137877] dwc3_cleanup_done_reqs: trb: ff800080f010,trb->size: 0x400,
trb->ctrl: 0x47d, trb_num: 3
[ 42.137891] dwc3_cleanup_done_reqs: trb: ff800080f020,trb->size:
0x1800, trb->ctrl: 0x2cd7cc78, trb_num: 3

Only the first one is transferred correctly.

I saw the kernel code is using CHN bit. SO I believe it works fine. But it
doesn't work in my environment. Did I miss some obvious registers? Any
comments/hints are appreciated.



Just saw this thread:
http://marc.info/?t=14743911801=1=2

I am fighting with the similar issue now. The patch from Felipe can't work. I


okay, and what was the outcome of your test? Do you have any sniffer
capture you could share? Did you make sure that PCM1 is set correctly?
What is the problem you notice? which gadget driver are you using?

No, I don't have USB tracer. I tried usbmon. But the information from usbmon
is not straight forward enough.

So dumping trb content in device side is what I am using to debug the problem.
On the host side, I am using the libuvc based app to check each iso packet host
get. I also check the throughput of iso transfer to judge whether the iso high
bandwidth is activated or not.




tried that. Here is the my debugging output:


are you using mainline? If you are, capture dwc3 tracepoints. If you're
not, which kernel are you using?

I am not using the latest kernel. Instead the code base is 3.18 + some usb
patches backported because mainline doesn't support the platform I am using.

I will try whether it's possible to apply the latest dwc3 driver from mainline
to the code base I am using. And use the tracepoints with it.




[   33.780999] trb: ff800080f080, trb dump: bpl: 0xdfb23000, bph: 0x0, size:
0x2000400, ctrl: 0x469
[   33.781004] trb: ff800080f090, trb dump: bpl: 0xdfb23400, bph: 0x0, size:
0x400, ctrl: 0x479
[   33.781007] trb: ff800080f0a0, trb dump: bpl: 0xdfb23800, bph: 0x0, size:
0x400, ctrl: 0xc79

This means the PCM1 is correct. Let me manually decode the info (will switch to
trace point then no manually decode needed):

The size of trb has PCM1 field 0x2000400 means PCM1 is 2 (3-1), 0x400 means the
trb has 1024 data.



[   33.782478] dwc3_cleanup_done_reqs: trb: ff800080f080, trb->size: 0x0,
trb->ctrl: 0x13f64468, trb_num: 3, dep->iso_xfer_num: 3993
[   33.782483] dwc3_cleanup_done_reqs: trb: ff800080f090, trb->size: 0x0,
trb->ctrl: 0x13f68478, trb_num: 3, dep->iso_xfer_num: 3994
[   33.782488] dwc3_cleanup_done_reqs: trb: ff800080f0a0, trb->size: 0x0,
trb->ctrl: 0x13f6cc78, trb_num: 3, dep->iso_xfer_num: 3995

The ctrl of trb has uframe number for iso ep if we read it. So
0x13f64468 has uframe number: 0x13f64468 >> 14 = 20441
0x13f68478 has uframe number: 0x13f68478 >> 14 = 20442
0x13f6cc78 has uframe number: 0

Re: Strange behavior of CHN bit with dwc3

2016-09-22 Thread yfw 

Hi Felipe, Bin,



Hi Felipe,
I am very sorry for the multiple sending. The previous were rejected because of
HTML format because I used gmail web interface.

Hi list,
I tried to enable the high speed, high bandwidth transfer in device mode for iso
type on dwc3 based soc. The platform only supports usb device 2.0.

I set the MaxPacketSize to 0x1400 so the host could allocate 3072 bytes for
uframe. But when I chain three trbs together, the dwc3 behavior is quite weird:

If I set the individual trb as following:
[ 32.495556] trb: ff800080f1b0, trb dump: bpl: 0xec803000,bph: 0x0, size:
0x400, ctrl: 0x469
[ 32.495560] trb: ff800080f1c0, trb dump: bpl: 0xec803400,bph: 0x0, size:
0x400, ctrl: 0x469
[ 32.495564] trb: ff800080f1d0, trb dump: bpl: 0xec803800,bph: 0x0, size:
0x400, ctrl: 0xc69

It's actually not high bandwidth. Just normal ISO transfer. Everything is fine.
I got following msg:
[ 32.495909] dwc3_cleanup_done_reqs: trb: ff800080f1b0,trb->size: 0x0,
trb->ctrl: 0x393dc468, trb_num: 3
[ 32.495915] dwc3_cleanup_done_reqs: trb: ff800080f1c0,trb->size: 0x0,
trb->ctrl: 0x393e0478, trb_num: 3
[ 32.495920] dwc3_cleanup_done_reqs: trb: ff800080f1d0,trb->size: 0x0,
trb->ctrl: 0x393e4c78, trb_num: 3

We could see every trb has size to 0 finally.

But if I set the trb chain as:
[ 42.137322] trb: ff800080f000, trb dump: bpl: 0xe2a8c000,bph: 0x0, size:
0x2000400, ctrl: 0x46d
[ 42.137326] trb: ff800080f010, trb dump: bpl: 0xe2a8c400,bph: 0x0, size:
0x400, ctrl: 0x47d
[ 42.137330] trb: ff800080f020, trb dump: bpl: 0xe2a8c800,bph: 0x0, size:
0x400, ctrl: 0xc79

There are three trbs. The first and second has CHN bit set. The first one has
First-ISO type.
And the PCM1 filed is set to 2 for the first one. The last one has IOC bit set.
I tried to let dwc3
send the first one at DATA2 stage, the second one at DATA1 stage and the third
one at DATA0
stage.

But there result is:
[ 42.137872] dwc3_cleanup_done_reqs: trb: ff800080f000,trb->size: 0x0,
trb->ctrl: 0x2cd7c46c, trb_num: 3
[ 42.137877] dwc3_cleanup_done_reqs: trb: ff800080f010,trb->size: 0x400,
trb->ctrl: 0x47d, trb_num: 3
[ 42.137891] dwc3_cleanup_done_reqs: trb: ff800080f020,trb->size:
0x1800, trb->ctrl: 0x2cd7cc78, trb_num: 3

Only the first one is transferred correctly.

I saw the kernel code is using CHN bit. SO I believe it works fine. But it
doesn't work in my environment. Did I miss some obvious registers? Any
comments/hints are appreciated.



Just saw this thread:
http://marc.info/?t=14743911801=1=2

I am fighting with the similar issue now. The patch from Felipe can't work. I
tried that. Here is the my debugging output:
[   33.780999] trb: ff800080f080, trb dump: bpl: 0xdfb23000, bph: 0x0, size: 
0x2000400, ctrl: 0x469
[   33.781004] trb: ff800080f090, trb dump: bpl: 0xdfb23400, bph: 0x0, size: 
0x400, ctrl: 0x479
[   33.781007] trb: ff800080f0a0, trb dump: bpl: 0xdfb23800, bph: 0x0, size: 
0x400, ctrl: 0xc79


[   33.782478] dwc3_cleanup_done_reqs: trb: ff800080f080, trb->size: 0x0, 
trb->ctrl: 0x13f64468, trb_num: 3, dep->iso_xfer_num: 3993
[   33.782483] dwc3_cleanup_done_reqs: trb: ff800080f090, trb->size: 0x0, 
trb->ctrl: 0x13f68478, trb_num: 3, dep->iso_xfer_num: 3994
[   33.782488] dwc3_cleanup_done_reqs: trb: ff800080f0a0, trb->size: 0x0, 
trb->ctrl: 0x13f6cc78, trb_num: 3, dep->iso_xfer_num: 3995


We could see the trb present was transferred correctly. But they are not at
the same uframe stage. The field in trb->ctrl are not same. That means the data
was transferred at different uframe stage. And the video throughput was not
increased as well.


I have no new databook (Only have one for v2.50a). So I tested almost all the
combines I could imagine.

Per my test, the CHN bit matters a lot. If the CHN bit is set, I could get:
[ 42.137872] dwc3_cleanup_done_reqs: trb: ff800080f000,trb->size: 0x0,
trb->ctrl: 0x2cd7c46c, trb_num: 3
[ 42.137877] dwc3_cleanup_done_reqs: trb: ff800080f010,trb->size: 0x400,
trb->ctrl: 0x47d, trb_num: 3
[ 42.137891] dwc3_cleanup_done_reqs: trb: ff800080f020,trb->size:
0x1800, trb->ctrl: 0x2cd7cc78, trb_num: 3

You could see the trb1 and trb3 has same uframe number even the data trb3
present was not transferred.

BTW, I don't subscribe to this mail list. Could you please add me to
http://marc.info/?t=14743911801=1=2
? Thanks a lot.

Regards
Yin, Fengwei



Thanks a lot
Yin, Fengwei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Strange behavior of CHN bit with dwc3

2016-09-22 Thread yfw 

Hi Felipe,
I am very sorry for the multiple sending. The previous were rejected because of 
HTML format because I used gmail web interface.


Hi list,
I tried to enable the high speed, high bandwidth transfer in device mode for iso 
type on dwc3 based soc. The platform only supports usb device 2.0.


I set the MaxPacketSize to 0x1400 so the host could allocate 3072 bytes for 
uframe. But when I chain three trbs together, the dwc3 behavior is quite weird:


If I set the individual trb as following:
[ 32.495556] trb: ff800080f1b0, trb dump: bpl: 0xec803000,bph: 0x0, size: 
0x400, ctrl: 0x469
[ 32.495560] trb: ff800080f1c0, trb dump: bpl: 0xec803400,bph: 0x0, size: 
0x400, ctrl: 0x469
[ 32.495564] trb: ff800080f1d0, trb dump: bpl: 0xec803800,bph: 0x0, size: 
0x400, ctrl: 0xc69


It's actually not high bandwidth. Just normal ISO transfer. Everything is fine. 
I got following msg:
[ 32.495909] dwc3_cleanup_done_reqs: trb: ff800080f1b0,trb->size: 0x0, 
trb->ctrl: 0x393dc468, trb_num: 3
[ 32.495915] dwc3_cleanup_done_reqs: trb: ff800080f1c0,trb->size: 0x0, 
trb->ctrl: 0x393e0478, trb_num: 3
[ 32.495920] dwc3_cleanup_done_reqs: trb: ff800080f1d0,trb->size: 0x0, 
trb->ctrl: 0x393e4c78, trb_num: 3


We could see every trb has size to 0 finally.

But if I set the trb chain as:
[ 42.137322] trb: ff800080f000, trb dump: bpl: 0xe2a8c000,bph: 0x0, size: 
0x2000400, ctrl: 0x46d
[ 42.137326] trb: ff800080f010, trb dump: bpl: 0xe2a8c400,bph: 0x0, size: 
0x400, ctrl: 0x47d
[ 42.137330] trb: ff800080f020, trb dump: bpl: 0xe2a8c800,bph: 0x0, size: 
0x400, ctrl: 0xc79


There are three trbs. The first and second has CHN bit set. The first one has 
First-ISO type.
And the PCM1 filed is set to 2 for the first one. The last one has IOC bit set. 
I tried to let dwc3
send the first one at DATA2 stage, the second one at DATA1 stage and the third 
one at DATA0

stage.

But there result is:
[ 42.137872] dwc3_cleanup_done_reqs: trb: ff800080f000,trb->size: 0x0, 
trb->ctrl: 0x2cd7c46c, trb_num: 3
[ 42.137877] dwc3_cleanup_done_reqs: trb: ff800080f010,trb->size: 0x400, 
trb->ctrl: 0x47d, trb_num: 3
[ 42.137891] dwc3_cleanup_done_reqs: trb: ff800080f020,trb->size: 
0x1800, trb->ctrl: 0x2cd7cc78, trb_num: 3


Only the first one is transferred correctly.

I saw the kernel code is using CHN bit. SO I believe it works fine. But it 
doesn't work in my environment. Did I miss some obvious registers? Any 
comments/hints are appreciated.



Thanks a lot
Yin, Fengwei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html