Hi,
On Wed, Jan 25, 2012 at 09:57:34AM +0200, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jan 24, 2012 at 01:55:15PM -0800, Paul Zimmerman wrote:
> > Is it possible to make this optional? As I read the patch, right now the FIFO
> > resizing will be done unconditionally after every SetConfig, is that correct?
> >
> > The reason I am concerned, is that some users may have pre-configured their RTL
> > with the exact FIFO sizes they want to use for their application, but your code
> > will blow that away unconditionally. I don't think that is very nice.
>
> Hmm, true that _does_ make sense. OTOH, that particular could be loosing
> some oportunities to increase performance by not using this (maybe not
> at this current form, because it's the simplest).
>
> e.g.:
>
> Let's say a certain user configured FIFO sizes thinking about his own
> composite gadget which has (IN eps only): 1 control IN endpoints (1 *
> 512), 2 bulk endpoints (2 * 1024) and 1 interrupt endpoint (1 * 64).
>
> If I don't resize fifos and this device is used as a mass storage
> device, we will be loosing an oportunity to better use the fifo. We
> could allocate ram1_fifo_size - ep0_fifo_size to the bulk TX endpoint,
> right ?
>
> I _do_ understand that the current form is just the simplest possible
> solution to the bad default/reset values we had on Silicon, but I have
> been experimenting with better allocating fifo depending on how many
> endpoints are enabled, their wMaxPacketSize and the size of RAM1.
>
> > You probably don't want a module parameter. Maybe we could enable this depending
> > on the PCI ID, or from platform data?
>
> Platform data will be vanishing soon, but we can have a devicetree
> attribute to select that, for sure ;-)
>
> Will hack that up quickly and send a revised patch.
here's the differential diff to add a flag for fifo resizing:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 455bb1e..d6b44a5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -48,6 +48,7 @@
#include <linux/list.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
+#include <linux/of.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
@@ -404,6 +405,7 @@ static void dwc3_core_exit(struct dwc3 *dwc)
static int __devinit dwc3_probe(struct platform_device *pdev)
{
+ struct device_node *node = pdev->dev.of_node;
struct resource *res;
struct dwc3 *dwc;
@@ -469,6 +471,9 @@ static int __devinit dwc3_probe(struct platform_device *pdev)
else
dwc->maximum_speed = DWC3_DCFG_SUPERSPEED;
+ if (of_get_property(node, "tx-fifo-resize", NULL))
+ dwc->needs_fifo_resize = true;
+
pm_runtime_enable(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);
pm_runtime_forbid(&pdev->dev);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e58aedf..123c3aa 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -603,6 +603,7 @@ struct dwc3_request {
* @ep0_expect_in: true when we expect a DATA IN transfer
* @start_config_issued: true when StartConfig command has been issued
* @setup_packet_pending: true when there's a Setup Packet in FIFO. Workaround
+ * @needs_fifo_resize: not all users might want fifo resizing, flag it
* @resize_fifos: tells us it's ok to reconfigure our TxFIFO sizes.
* @ep0_next_event: hold the next expected event
* @ep0state: state of endpoint zero
@@ -661,6 +662,7 @@ struct dwc3 {
unsigned start_config_issued:1;
unsigned setup_packet_pending:1;
unsigned delayed_status:1;
+ unsigned needs_fifo_resize:1;
unsigned resize_fifos:1;
enum dwc3_ep0_next ep0_next_event;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5d07244..4747351 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -155,6 +155,9 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc)
int mdwidth;
int num;
+ if (!dwc->needs_fifo_resize)
+ return 0;
+
ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
attached you can find final patch.
--
balbi
From a04548247f395ef71f3e39161477469c97a9c038 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Wed, 18 Jan 2012 18:04:09 +0200
Subject: [PATCH] usb: dwc3: gadget: dynamically re-size TxFifos
Organization: Texas Instruments\n
We need to dynamically re-size TxFifos for the
cases where default values will not do.
While at that, we create a simple function which,
for now, will just allocate one full packet fifo
space for each of the enabled endpoints.
This can be improved later in order to allow for
better throughput by allocating more space for
endpoints which could make good use of that like
isochronous and bulk.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/dwc3/core.c | 5 +++
drivers/usb/dwc3/core.h | 16 +++++++++-
drivers/usb/dwc3/ep0.c | 11 ++++++-
drivers/usb/dwc3/gadget.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 455bb1e..d6b44a5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -48,6 +48,7 @@
#include <linux/list.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
+#include <linux/of.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
@@ -404,6 +405,7 @@ static void dwc3_core_exit(struct dwc3 *dwc)
static int __devinit dwc3_probe(struct platform_device *pdev)
{
+ struct device_node *node = pdev->dev.of_node;
struct resource *res;
struct dwc3 *dwc;
@@ -469,6 +471,9 @@ static int __devinit dwc3_probe(struct platform_device *pdev)
else
dwc->maximum_speed = DWC3_DCFG_SUPERSPEED;
+ if (of_get_property(node, "tx-fifo-resize", NULL))
+ dwc->needs_fifo_resize = true;
+
pm_runtime_enable(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);
pm_runtime_forbid(&pdev->dev);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f4878c4..123c3aa 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -172,6 +172,10 @@
#define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31)
#define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17)
+/* Global TX Fifo Size Register */
+#define DWC3_GTXFIFOSIZ_TXFDEF(n) ((n) & 0xffff)
+#define DWC3_GTXFIFOSIZ_TXFSTADDR(n) ((n) & 0xffff0000)
+
/* Global HWPARAMS1 Register */
#define DWC3_GHWPARAMS1_EN_PWROPT(n) ((n & (3 << 24)) >> 24)
#define DWC3_GHWPARAMS1_EN_PWROPT_NO 0
@@ -546,8 +550,13 @@ struct dwc3_hwparams {
#define DWC3_MODE_DRD 2
#define DWC3_MODE_HUB 3
+#define DWC3_MDWIDTH(n) (((n) & 0xff00) >> 8)
+
/* HWPARAMS1 */
-#define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
+#define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
+
+/* HWPARAMS7 */
+#define DWC3_RAM1_DEPTH(n) ((n) & 0xffff)
struct dwc3_request {
struct usb_request request;
@@ -594,6 +603,8 @@ struct dwc3_request {
* @ep0_expect_in: true when we expect a DATA IN transfer
* @start_config_issued: true when StartConfig command has been issued
* @setup_packet_pending: true when there's a Setup Packet in FIFO. Workaround
+ * @needs_fifo_resize: not all users might want fifo resizing, flag it
+ * @resize_fifos: tells us it's ok to reconfigure our TxFIFO sizes.
* @ep0_next_event: hold the next expected event
* @ep0state: state of endpoint zero
* @link_state: link state
@@ -651,6 +662,8 @@ struct dwc3 {
unsigned start_config_issued:1;
unsigned setup_packet_pending:1;
unsigned delayed_status:1;
+ unsigned needs_fifo_resize:1;
+ unsigned resize_fifos:1;
enum dwc3_ep0_next ep0_next_event;
enum dwc3_ep0_state ep0state;
@@ -812,6 +825,7 @@ union dwc3_event {
/* prototypes */
void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
+int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
int dwc3_host_init(struct dwc3 *dwc);
void dwc3_host_exit(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 0bdfb33..a4b0c01 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -464,8 +464,11 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
case DWC3_ADDRESS_STATE:
ret = dwc3_ep0_delegate_req(dwc, ctrl);
/* if the cfg matches and the cfg is non zero */
- if (cfg && (!ret || (ret == USB_GADGET_DELAYED_STATUS)))
+ if (cfg && (!ret || (ret == USB_GADGET_DELAYED_STATUS))) {
dwc->dev_state = DWC3_CONFIGURED_STATE;
+ dwc->resize_fifos = true;
+ dev_dbg(dwc->dev, "resize fifos flag SET\n");
+ }
break;
case DWC3_CONFIGURED_STATE:
@@ -714,6 +717,12 @@ static void dwc3_ep0_do_control_status(struct dwc3 *dwc, u32 epnum)
{
struct dwc3_ep *dep = dwc->eps[epnum];
+ if (dwc->resize_fifos) {
+ dev_dbg(dwc->dev, "starting to resize fifos\n");
+ dwc3_gadget_resize_tx_fifos(dwc);
+ dwc->resize_fifos = 0;
+ }
+
WARN_ON(dwc3_ep0_start_control_status(dep));
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 18a9c20..4747351 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -125,6 +125,81 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state)
return -ETIMEDOUT;
}
+/**
+ * dwc3_gadget_resize_tx_fifos - reallocate fifo spaces for current use-case
+ * @dwc: pointer to our context structure
+ *
+ * This function will a best effort FIFO allocation in order
+ * to improve FIFO usage and throughput, while still allowing
+ * us to enable as many endpoints as possible.
+ *
+ * Keep in mind that this operation will be highly dependent
+ * on the configured size for RAM1 - which contains TxFifo -,
+ * the amount of endpoints enabled on coreConsultant tool, and
+ * the width of the Master Bus.
+ *
+ * In the ideal world, we would always be able to satisfy the
+ * following equation:
+ *
+ * ((512 + 2 * MDWIDTH-Bytes) + (Number of IN Endpoints - 1) * \
+ * (3 * (1024 + MDWIDTH-Bytes) + MDWIDTH-Bytes)) / MDWIDTH-Bytes
+ *
+ * Unfortunately, due to many variables (Silicon die size, cost,
+ * targeted use-cases, etc) that's not always the case.
+ */
+int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc)
+{
+ int last_fifo_depth = 0;
+ int ram1_depth;
+ int fifo_size;
+ int mdwidth;
+ int num;
+
+ if (!dwc->needs_fifo_resize)
+ return 0;
+
+ ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
+ mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
+
+ /* MDWIDTH is represented in bits, we need it in bytes */
+ mdwidth >>= 3;
+
+ /*
+ * FIXME For now we will only allocate 1 wMaxPacketSize space
+ * for each enabled endpoint, later patches will come to
+ * improve this algorithm so that we better use the internal
+ * FIFO space
+ */
+ for (num = 0; num < DWC3_ENDPOINTS_NUM; num++) {
+ struct dwc3_ep *dep = dwc->eps[num];
+ int fifo_number = dep->number >> 1;
+ int tmp;
+
+ if (!(dep->number & 1))
+ continue;
+
+ if (!(dep->flags & DWC3_EP_ENABLED))
+ continue;
+
+ tmp = dep->endpoint.maxpacket;
+ tmp += mdwidth;
+ tmp += mdwidth;
+
+ fifo_size = DIV_ROUND_UP(tmp, mdwidth);
+ fifo_size |= (last_fifo_depth << 16);
+
+ dev_vdbg(dwc->dev, "%s: Fifo Addr %04x Size %d\n",
+ dep->name, last_fifo_depth, fifo_size & 0xffff);
+
+ dwc3_writel(dwc->regs, DWC3_GTXFIFOSIZ(fifo_number),
+ fifo_size);
+
+ last_fifo_depth += (fifo_size & 0xffff);
+ }
+
+ return 0;
+}
+
void dwc3_map_buffer_to_dma(struct dwc3_request *req)
{
struct dwc3 *dwc = req->dep->dwc;
--
1.7.8.2
Attachment:
signature.asc
Description: Digital signature
