[RFC PATCH 0/1] DWC2 DMA issue

[RFC PATCH 0/1] DWC2 DMA issue

[Alexandre Bailon]

For context, I am trying to reduce latency of UAC2 gadget
and one way to achieve it is to reduce the number of USB requests.
The SoC on the board I use (vim3l) use a DWC2 IP.
If I enable the UAC2 gadget with 3 requests, every ms I get data from isocronous
in endpoint and everything works fine (I can listen what come from the interface)
But if use only two requests, then, what I listen is hugly!
What I noticed is that after two sucessful USB transfer, I get a ZLP.

I suspect that happens because in dwc2_gadget_fill_isoc_desc, we may update
the DMA descriptor whereas it is already owned by the DMA.
On completion of a request, the gadget queue the request that has been completed.
The DWC2 driver clear DEV_DMA_L bit of last descriptor and add a new descriptor to DMA.
In that way, we never stop DMA and keep the endpoint enabled while we are
feeding it with data.

If we use three or more request when we start the gadget, the DMA has three descriptors
and when queue a new one, DMA is processing descriptor L-1, the driver update descriptor L
and the descriptor L+1.

But when we only have two requests, the DMA has only two requests and when we queue
the request on completion, DMA is processing descriptor L, the driver update it and add
the descriptor L+1. Instead of using the new descriptor, the DMA raise a BNA interrupt,
the controller send a ZLP to host and the driver re-initialize the endpoint to prepare
the next transfers.

The patch try to fix it. I am not conviced so far this is the best fix so
I am open to any suggestion.

Alexandre Bailon (1):
  usb: dwc2: Don't set DEV_DMA_L by default for isoc transfer

 drivers/usb/dwc2/gadget.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

-- 
2.41.0

[RFC PATCH 1/1] usb: dwc2: Don't set DEV_DMA_L by default for isoc transfer

[Alexandre Bailon]

Currently, by default, we set DEV_DMA_L to notify DMA that the current
descriptor is the last one.
But, to get better performances, the driver doesn't stop the DMA and add
a new descriptor at the end of the list and clear DEV_DMA_L bit from
previous descriptor. This works well except if the DMA has already fetched
the descriptor. Updating the descriptor owned by DMA can cause some issues.

This updates the driver to no update descriptor while it is in use.
Instead, this assumes that we are always going to queue a request
and feed the DMA with new descriptors.
In case where we don't feed the DMA, we stop the transfer using a BNA.

NOTE:
To be tested!!!
Currently, only tested and validated using UAC2.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/usb/dwc2/gadget.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 8b15742d9e8a..c542811468ce 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -928,10 +928,6 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 		return 1;
 	}
 
-	/* Clear L bit of previous desc if more than one entries in the chain */
-	if (hs_ep->next_desc)
-		hs_ep->desc_list[index - 1].status &= ~DEV_DMA_L;
-
 	dev_dbg(hsotg->dev, "%s: Filling ep %d, dir %s isoc desc # %d\n",
 		__func__, hs_ep->index, hs_ep->dir_in ? "in" : "out", index);
 
@@ -939,8 +935,9 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
 
 	desc->buf = dma_buff;
-	desc->status |= (DEV_DMA_L | DEV_DMA_IOC |
-			 ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	desc->status |= (DEV_DMA_IOC | ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	if ((hs_ep->next_desc+1) >= MAX_DMA_DESC_NUM_HS_ISOC)
+		desc->status |= DEV_DMA_L;
 
 	if (hs_ep->dir_in) {
 		if (len)
@@ -968,6 +965,15 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	if (hs_ep->next_desc >= MAX_DMA_DESC_NUM_HS_ISOC)
 		hs_ep->next_desc = 0;
 
+
+	/*
+	 * Safeguard: Make sure we stop transfer if we don't have a valid descriptor.
+	 */
+	index = hs_ep->next_desc;
+	desc = &hs_ep->desc_list[index];
+	desc->status = 0;
+	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
+
 	return 0;
 }
 
-- 
2.41.0