Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 0/3] usb: dwc3: gadget: improve isoc handling
@ 2020-07-01  9:31 Michael Grzeschik
  2020-07-01  9:31 ` [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask Michael Grzeschik
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Michael Grzeschik @ 2020-07-01  9:31 UTC (permalink / raw)
  To: linux-usb; +Cc: Thinh.Nguyen, gregkh, kernel, balbi

These three patches improve the isoc handling and make the dwc3 gadget
driver somewhat usable with the UVC gadget for isochronous endpoints.

The first patch makes starting isochronous transfers more reliable. I
think it's more less, what Thinh suggested some time ago[1]. It's still
not perfect because the first request must still be queued within 2
seconds but it's a lot better than the current situation.

The second patch makes it possible to have gaps in the data stream. The
UVC gadget relies on such behaviour. Without this, using the UVC gadget
with a live stream stops after the first frame that needs more time to
be scheduled.

[1] https://marc.info/?l=linux-usb&m=156088170723824&w=4

Michael Grzeschik (2):
  usb: dwc3: gadget: add frame number mask
  usb: dwc3: gadget: when the started list is empty stop the active xfer

Michael Olbrich (1):
  usb: dwc3: gadget: make starting isoc transfers more robust

 drivers/usb/dwc3/gadget.c | 30 +++++++++++++++++++++++++++---
 drivers/usb/dwc3/gadget.h |  2 ++
 2 files changed, 29 insertions(+), 3 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask
  2020-07-01  9:31 [PATCH v4 0/3] usb: dwc3: gadget: improve isoc handling Michael Grzeschik
@ 2020-07-01  9:31 ` Michael Grzeschik
  2020-07-01  9:52   ` Sergei Shtylyov
  2020-07-01  9:31 ` [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust Michael Grzeschik
  2020-07-01  9:31 ` [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer Michael Grzeschik
  2 siblings, 1 reply; 7+ messages in thread
From: Michael Grzeschik @ 2020-07-01  9:31 UTC (permalink / raw)
  To: linux-usb; +Cc: Thinh.Nguyen, gregkh, kernel, balbi

This patch adds an define DWC3_FRNUMBER_MASK for the common used
0x3fff mask and uses it.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v4: - first version

 drivers/usb/dwc3/gadget.c | 4 ++--
 drivers/usb/dwc3/gadget.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8de6f10d335e1c0..5fb78535efa9bdc 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1403,7 +1403,7 @@ static int dwc3_gadget_start_isoc_quirk(struct dwc3_ep *dep)
 		 * Check if we can start isoc transfer on the next interval or
 		 * 4 uframes in the future with BIT[15:14] as dep->combo_num
 		 */
-		test_frame_number = dep->frame_number & 0x3fff;
+		test_frame_number = dep->frame_number & DWC3_FRNUMBER_MASK;
 		test_frame_number |= dep->combo_num << 14;
 		test_frame_number += max_t(u32, 4, dep->interval);
 
@@ -1450,7 +1450,7 @@ static int dwc3_gadget_start_isoc_quirk(struct dwc3_ep *dep)
 	else if (test0 && test1)
 		dep->combo_num = 0;
 
-	dep->frame_number &= 0x3fff;
+	dep->frame_number &= DWC3_FRNUMBER_MASK;
 	dep->frame_number |= dep->combo_num << 14;
 	dep->frame_number += max_t(u32, 4, dep->interval);
 
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index 24dca38720225dd..a0fc46f72b9e30a 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -54,6 +54,8 @@ struct dwc3;
 /* U2 Device exit Latency */
 #define DWC3_DEFAULT_U2_DEV_EXIT_LAT	0x1FF	/* Less then 511 microsec */
 
+/* Frame/Microframe Number Mask */
+#define DWC3_FRNUMBER_MASK		0x3fff
 /* -------------------------------------------------------------------------- */
 
 #define to_dwc3_request(r)	(container_of(r, struct dwc3_request, request))
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust
  2020-07-01  9:31 [PATCH v4 0/3] usb: dwc3: gadget: improve isoc handling Michael Grzeschik
  2020-07-01  9:31 ` [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask Michael Grzeschik
@ 2020-07-01  9:31 ` Michael Grzeschik
  2020-07-01 18:06   ` Thinh Nguyen
  2020-07-01  9:31 ` [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer Michael Grzeschik
  2 siblings, 1 reply; 7+ messages in thread
From: Michael Grzeschik @ 2020-07-01  9:31 UTC (permalink / raw)
  To: linux-usb
  Cc: Thinh.Nguyen, gregkh, kernel, balbi, Michael Olbrich, Thinh Nguyen

From: Michael Olbrich <m.olbrich@pengutronix.de>

Currently __dwc3_gadget_start_isoc must be called very shortly after
XferNotReady. Otherwise the frame number is outdated and start transfer
will fail, even with several retries.

DSTS provides the lower 14 bit of the frame number. Use it in combination
with the frame number provided by XferNotReady to guess the current frame
number. This will succeed unless more than one 14 rollover has happened
since XferNotReady.

Start transfer might still fail if the frame number is increased
immediately after DSTS is read. So retries are still needed.
Don't drop the current request if this happens. This way it is not lost and
can be used immediately to try again with the next frame number.

With this change, __dwc3_gadget_start_isoc is still not successfully in all
cases bit it increases the acceptable delay after XferNotReady
significantly.

Reviewed-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: - removed last_frame_number from struct
          - included rollover variable
v2 -> v3: - moved the calculation before the retry loop
          - skipping the calculation if bInterval > 14
v3 -> v4: - used define for 0x3fff framenumber mask
          - added limitation for this to usb > fullspeed

 drivers/usb/dwc3/gadget.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5fb78535efa9bdc..a2145f905d67067 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1463,6 +1463,7 @@ static int dwc3_gadget_start_isoc_quirk(struct dwc3_ep *dep)
 
 static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
 {
+	const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
 	struct dwc3 *dwc = dep->dwc;
 	int ret;
 	int i;
@@ -1480,6 +1481,27 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
 			return dwc3_gadget_start_isoc_quirk(dep);
 	}
 
+	if (desc->bInterval <= 14 &&
+	    dwc->gadget.speed >= USB_SPEED_SUPER) {
+		u32 frame = __dwc3_gadget_get_frame(dwc);
+		bool rollover = frame <
+				(dep->frame_number & DWC3_FRNUMBER_MASK);
+
+		/*
+		 * frame_number is set from XferNotReady and may be already
+		 * out of date. DSTS only provides the lower 14 bit of the
+		 * current frame number. So add the upper two bits of
+		 * frame_number and handle a possible rollover.
+		 * This will provide the correct frame_number unless more than
+		 * rollover has happened since XferNotReady.
+		 */
+
+		dep->frame_number = (dep->frame_number & ~DWC3_FRNUMBER_MASK) |
+				     frame;
+		if (rollover)
+			dep->frame_number += BIT(14);
+	}
+
 	for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) {
 		dep->frame_number = DWC3_ALIGN_FRAME(dep, i + 1);
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer
  2020-07-01  9:31 [PATCH v4 0/3] usb: dwc3: gadget: improve isoc handling Michael Grzeschik
  2020-07-01  9:31 ` [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask Michael Grzeschik
  2020-07-01  9:31 ` [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust Michael Grzeschik
@ 2020-07-01  9:31 ` Michael Grzeschik
  2020-07-01  9:53   ` Sergei Shtylyov
  2 siblings, 1 reply; 7+ messages in thread
From: Michael Grzeschik @ 2020-07-01  9:31 UTC (permalink / raw)
  To: linux-usb; +Cc: Thinh.Nguyen, gregkh, kernel, balbi, Thinh Nguyen

When we have nothing left to be queued after handling the last trb
we have to stop the current transfer. This way we can ensure that
the next request will be queued with an new and valid timestamp
and will not directly run into an missed xfer.

Reviewed-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: - This Patch replaces the following patch by Michael Olbrich:
            usb: dwc3: gadget: restart the transfer if a isoc request is queued too late
v2 -> v3: - rearranged condition for easier read (suggested by Tinh Nguyen)
v3 -> v4: - no changes

 drivers/usb/dwc3/gadget.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a2145f905d67067..24aafacc2b5b344 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2738,7 +2738,9 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
 	if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
 		goto out;
 
-	if (status == -EXDEV && list_empty(&dep->started_list))
+	if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+		list_empty(&dep->started_list) &&
+		(list_empty(&dep->pending_list) || status == -EXDEV))
 		dwc3_stop_active_transfer(dep, true, true);
 	else if (dwc3_gadget_ep_should_continue(dep))
 		if (__dwc3_gadget_kick_transfer(dep) == 0)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask
  2020-07-01  9:31 ` [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask Michael Grzeschik
@ 2020-07-01  9:52   ` Sergei Shtylyov
  0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2020-07-01  9:52 UTC (permalink / raw)
  To: Michael Grzeschik, linux-usb; +Cc: Thinh.Nguyen, gregkh, kernel, balbi

On 01.07.2020 12:31, Michael Grzeschik wrote:

> This patch adds an define DWC3_FRNUMBER_MASK for the common used

   s/an/a/. Commonly.

> 0x3fff mask and uses it.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

MBR, Sergei

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer
  2020-07-01  9:31 ` [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer Michael Grzeschik
@ 2020-07-01  9:53   ` Sergei Shtylyov
  0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2020-07-01  9:53 UTC (permalink / raw)
  To: Michael Grzeschik, linux-usb
  Cc: Thinh.Nguyen, gregkh, kernel, balbi, Thinh Nguyen

On 01.07.2020 12:31, Michael Grzeschik wrote:

> When we have nothing left to be queued after handling the last trb
> we have to stop the current transfer. This way we can ensure that
> the next request will be queued with an new and valid timestamp

    s/an/a/.

> and will not directly run into an missed xfer.
> 
> Reviewed-by: Thinh Nguyen <thinhn@synopsys.com>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust
  2020-07-01  9:31 ` [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust Michael Grzeschik
@ 2020-07-01 18:06   ` Thinh Nguyen
  0 siblings, 0 replies; 7+ messages in thread
From: Thinh Nguyen @ 2020-07-01 18:06 UTC (permalink / raw)
  To: Michael Grzeschik, linux-usb
  Cc: Thinh Nguyen, gregkh, kernel, balbi, Michael Olbrich

Michael Grzeschik wrote:
> From: Michael Olbrich <m.olbrich@pengutronix.de>
>
> Currently __dwc3_gadget_start_isoc must be called very shortly after
> XferNotReady. Otherwise the frame number is outdated and start transfer
> will fail, even with several retries.
>
> DSTS provides the lower 14 bit of the frame number. Use it in combination
> with the frame number provided by XferNotReady to guess the current frame
> number. This will succeed unless more than one 14 rollover has happened
> since XferNotReady.
>
> Start transfer might still fail if the frame number is increased
> immediately after DSTS is read. So retries are still needed.
> Don't drop the current request if this happens. This way it is not lost and
> can be used immediately to try again with the next frame number.
>
> With this change, __dwc3_gadget_start_isoc is still not successfully in all
> cases bit it increases the acceptable delay after XferNotReady
> significantly.
>
> Reviewed-by: Thinh Nguyen <thinhn@synopsys.com>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>
> ---
> v1 -> v2: - removed last_frame_number from struct
>            - included rollover variable
> v2 -> v3: - moved the calculation before the retry loop
>            - skipping the calculation if bInterval > 14
> v3 -> v4: - used define for 0x3fff framenumber mask
>            - added limitation for this to usb > fullspeed
>
>   drivers/usb/dwc3/gadget.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 5fb78535efa9bdc..a2145f905d67067 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1463,6 +1463,7 @@ static int dwc3_gadget_start_isoc_quirk(struct dwc3_ep *dep)
>   
>   static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>   {
> +	const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
>   	struct dwc3 *dwc = dep->dwc;
>   	int ret;
>   	int i;
> @@ -1480,6 +1481,27 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>   			return dwc3_gadget_start_isoc_quirk(dep);
>   	}
>   
> +	if (desc->bInterval <= 14 &&
> +	    dwc->gadget.speed >= USB_SPEED_SUPER) {

Highspeed is still applicable. Just ignore fullspeed.

> +		u32 frame = __dwc3_gadget_get_frame(dwc);
> +		bool rollover = frame <
> +				(dep->frame_number & DWC3_FRNUMBER_MASK);
> +
> +		/*
> +		 * frame_number is set from XferNotReady and may be already
> +		 * out of date. DSTS only provides the lower 14 bit of the
> +		 * current frame number. So add the upper two bits of
> +		 * frame_number and handle a possible rollover.
> +		 * This will provide the correct frame_number unless more than
> +		 * rollover has happened since XferNotReady.
> +		 */
> +
> +		dep->frame_number = (dep->frame_number & ~DWC3_FRNUMBER_MASK) |
> +				     frame;
> +		if (rollover)
> +			dep->frame_number += BIT(14);
> +	}
> +
>   	for (i = 0; i < DWC3_ISOC_MAX_RETRIES; i++) {
>   		dep->frame_number = DWC3_ALIGN_FRAME(dep, i + 1);
>   

BR,
Thinh

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01  9:31 [PATCH v4 0/3] usb: dwc3: gadget: improve isoc handling Michael Grzeschik
2020-07-01  9:31 ` [PATCH v4 1/3] usb: dwc3: gadget: add frame number mask Michael Grzeschik
2020-07-01  9:52   ` Sergei Shtylyov
2020-07-01  9:31 ` [PATCH v4 2/3] usb: dwc3: gadget: make starting isoc transfers more robust Michael Grzeschik
2020-07-01 18:06   ` Thinh Nguyen
2020-07-01  9:31 ` [PATCH v4 3/3] usb: dwc3: gadget: when the started list is empty stop the active xfer Michael Grzeschik
2020-07-01  9:53   ` Sergei Shtylyov

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git