Skip to contentSkip to conversation

usb: dwc3: gadget: Reduce register read/write operations

PATCH New Thinh Nguyen 1 participant 4 messages Nov 30, 2021

Subsystems (2)
USB DesignWare USB3 DRD IP driver
lore
Share & tools
lore

Thinh Nguyen

author maintainer
#0 lore raw

cover usb: dwc3: gadget: Reduce register read/write operations

Depends on the device platform, a register read/write may take a few microseconds to complete. A register read on a HAPS platform takes ~2us where as register write takes less than 1us. These delays impact performance of small transfers or interface(s) with many endpoints such as UASP interface. For large transfer size and low rate of events generated, this series does not make much of a difference.

This series avoids register read/write operations where they are not needed. On the HAPS platform with UASP in SSP Gen 2x2 speed, we observed almost double in performance speed when testing 4KB transfers reading and writing.

Thinh Nguyen (3):
usb: dwc3: gadget: Skip checking Update Transfer status
usb: dwc3: gadget: Ignore Update Transfer cmd params
usb: dwc3: gadget: Skip reading GEVNTSIZn

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

base-commit: 4d012040161cba054208555300d9fdf2b8925c34

Signature · 1 line
2.28.0
author maintainer
#1 lore raw

PATCH 1/3 usb: dwc3: gadget: Skip checking Update Transfer status

If we're not setting CMDACT (from "No Response" Update Transfer command), then there's no point in checking for the command status. So skip it. This can reduce a register read delay and improve performance.

Signed-off-by Thinh Nguyen <[email protected]>
---
 drivers/usb/dwc3/gadget.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/usb/dwc3/gadget.c +7 -0
index 7e3db00e9759..00b3f19c4d3e 100644
@@ -357,6 +357,12 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
 		cmd |= DWC3_DEPCMD_CMDACT;
 
 	dwc3_writel(dep->regs, DWC3_DEPCMD, cmd);
+
+	if (!(cmd & DWC3_DEPCMD_CMDACT)) {
+		ret = 0;
+		goto skip_status;
+	}
+
 	do {
 		reg = dwc3_readl(dep->regs, DWC3_DEPCMD);
 		if (!(reg & DWC3_DEPCMD_CMDACT)) {

@@ -398,6 +404,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
 		cmd_status = -ETIMEDOUT;
 	}
 
+skip_status:
 	trace_dwc3_gadget_ep_cmd(dep, cmd, params, cmd_status);
 
 	if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
Signature · 1 line
2.28.0
author maintainer
#2 lore raw

PATCH 2/3 usb: dwc3: gadget: Ignore Update Transfer cmd params

In reply to Thinh Nguyen

The controller doesn't check for Update Transfer command parameters DEPCMDPAR{0,1,2}. Writing to these registers is unnecessary. Ignoring this improves performance slightly by removing the register write delay.

Signed-off-by Thinh Nguyen <[email protected]>
---
 drivers/usb/dwc3/gadget.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/usb/dwc3/gadget.c +11 -3
index 00b3f19c4d3e..f66baaef8a40 100644
@@ -331,9 +331,17 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
 		}
 	}
 
-	dwc3_writel(dep->regs, DWC3_DEPCMDPAR0, params->param0);
-	dwc3_writel(dep->regs, DWC3_DEPCMDPAR1, params->param1);
-	dwc3_writel(dep->regs, DWC3_DEPCMDPAR2, params->param2);
+	/*
+	 * For some commands such as Update Transfer command, DEPCMDPARn
+	 * registers are reserved. Since the driver often sends Update Transfer
+	 * command, don't write to DEPCMDPARn to avoid register write delays and
+	 * improve performance.
+	 */
+	if (DWC3_DEPCMD_CMD(cmd) != DWC3_DEPCMD_UPDATETRANSFER) {
+		dwc3_writel(dep->regs, DWC3_DEPCMDPAR0, params->param0);
+		dwc3_writel(dep->regs, DWC3_DEPCMDPAR1, params->param1);
+		dwc3_writel(dep->regs, DWC3_DEPCMDPAR2, params->param2);
+	}
 
 	/*
 	 * Synopsys Databook 2.60a states in section 6.3.2.5.6 of that if we're
Signature · 1 line
2.28.0
author maintainer
#3 lore raw

PATCH 3/3 usb: dwc3: gadget: Skip reading GEVNTSIZn

In reply to Thinh Nguyen

The driver knows what it needs to set for GEVNTSIZn, and the controller doesn't modify this register unless there's a hard reset. To save a few microseconds of register read in read-modify-write operation, simply do register write with the expected values. This can improve performance when there are many interrupts generated, which the driver needs to check and handle.

Signed-off-by Thinh Nguyen <[email protected]>
---
 drivers/usb/dwc3/gadget.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/usb/dwc3/gadget.c +4 -8
index f66baaef8a40..c0c89374716b 100644
@@ -4082,7 +4082,6 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt)
 	struct dwc3 *dwc = evt->dwc;
 	irqreturn_t ret = IRQ_NONE;
 	int left;
-	u32 reg;
 
 	left = evt->count;
 

@@ -4114,9 +4113,8 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt)
 	ret = IRQ_HANDLED;
 
 	/* Unmask interrupt */
-	reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
-	reg &= ~DWC3_GEVNTSIZ_INTMASK;
-	dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg);
+	dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
+		    DWC3_GEVNTSIZ_SIZE(evt->length));
 
 	if (dwc->imod_interval) {
 		dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), DWC3_GEVNTCOUNT_EHB);

@@ -4145,7 +4143,6 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
 	struct dwc3 *dwc = evt->dwc;
 	u32 amount;
 	u32 count;
-	u32 reg;
 
 	if (pm_runtime_suspended(dwc->dev)) {
 		pm_runtime_get(dwc->dev);

@@ -4172,9 +4169,8 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
 	evt->flags |= DWC3_EVENT_PENDING;
 
 	/* Mask interrupt */
-	reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
-	reg |= DWC3_GEVNTSIZ_INTMASK;
-	dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg);
+	dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
+		    DWC3_GEVNTSIZ_INTMASK | DWC3_GEVNTSIZ_SIZE(evt->length));
 
 	amount = min(count, evt->length - evt->lpos);
 	memcpy(evt->cache + evt->lpos, evt->buf + evt->lpos, amount);
Signature · 1 line
2.28.0