* [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements
@ 2019-10-25 1:03 Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case Joel Stanley
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
Here are some lightly tested improvements to the FSI driver that should
improve throughput.
They apply on top of the previous FSI series I sent.
Joel Stanley (7):
fsi: aspeed: busy loop in the write case
fsi: master: Change default divisor to 14
fsi: aspeed: Enable relative addressing
fsi: aspeed: Only select OPB0 once
fsi: aspeed: Avoid copying read data twice
fsi: aspeed: Pass fsi_master_aspeed insead of base
fsi: aspeed: Special case repeated full word reads
drivers/fsi/fsi-master-aspeed.c | 131 +++++++++++++++++++++++---------
1 file changed, 94 insertions(+), 37 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 1:52 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14 Joel Stanley
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
We busy loop in the read case, make the write case the same. Note that
this may cause issues when doing a break, which takes a long time.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 3524b3dfe549..59537cab4f68 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -204,7 +204,7 @@ static u32 opb_write(void __iomem *base, uint32_t addr, uint32_t val,
ret = readl_poll_timeout(base + OPB_IRQ_STATUS, reg,
(reg & OPB0_XFER_ACK_EN) != 0,
- 1, 10000);
+ 0, 10000);
status = readl(base + OPB0_STATUS);
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 1:56 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing Joel Stanley
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
We were running at 127, which equates to a bus clock speed of approx
1.4MHz. This changes that to approx 14MHz, which works on the EVB and is
reliable on the Tacoma systems.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 59537cab4f68..bef7690a0ddf 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -76,6 +76,8 @@
#define FSI_NUM_DEBUGFS_ENTRIES 17
+#define DEFAULT_DIVISOR 14
+
struct fsi_master_aspeed;
struct fsi_master_aspeed_debugfs_entry {
@@ -441,7 +443,8 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
opb_write(aspeed->base, ctrl_base + FSI_MECTRL, reg, 4);
reg = cpu_to_be32(FSI_MMODE_ECRC | FSI_MMODE_EPC
- | fsi_mmode_crs0(0x7f) | fsi_mmode_crs1(0x7f)
+ | fsi_mmode_crs0(DEFAULT_DIVISOR)
+ | fsi_mmode_crs1(DEFAULT_DIVISOR)
| FSI_MMODE_P8_TO_LSB);
opb_write(aspeed->base, ctrl_base + FSI_MMODE, reg, 4);
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14 Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 1:57 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once Joel Stanley
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index bef7690a0ddf..d2c01956663f 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -42,6 +42,7 @@
/* MMODE: Mode control */
#define FSI_MMODE_EIP 0x80000000 /* Enable interrupt polling */
#define FSI_MMODE_ECRC 0x40000000 /* Enable error recovery */
+#define FSI_MMODE_RELA 0x20000000 /* Enable relative address commands */
#define FSI_MMODE_EPC 0x10000000 /* Enable parity checking */
#define FSI_MMODE_P8_TO_LSB 0x00000010 /* Timeout value LSB */
/* MSB=1, LSB=0 is 0.8 ms */
@@ -442,7 +443,7 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
reg = cpu_to_be32(FSI_MECTRL_EOAE | FSI_MECTRL_P8_AUTO_TERM);
opb_write(aspeed->base, ctrl_base + FSI_MECTRL, reg, 4);
- reg = cpu_to_be32(FSI_MMODE_ECRC | FSI_MMODE_EPC
+ reg = cpu_to_be32(FSI_MMODE_ECRC | FSI_MMODE_EPC | FSI_MMODE_RELA
| fsi_mmode_crs0(DEFAULT_DIVISOR)
| fsi_mmode_crs1(DEFAULT_DIVISOR)
| FSI_MMODE_P8_TO_LSB);
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
` (2 preceding siblings ...)
2019-10-25 1:03 ` [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 1:59 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice Joel Stanley
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
The driver can leve OPB0 selected to save a AHB write per OPB operation.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index d2c01956663f..58b090e2cf46 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -197,7 +197,6 @@ static u32 opb_write(void __iomem *base, uint32_t addr, uint32_t val,
if (xfer_size < 0)
return xfer_size;
- writel(0x1, base + OPB0_SELECT);
writel(CMD_WRITE, base + OPB0_RW);
writel(xfer_size, base + OPB0_XFER_SIZE);
writel(addr, base + OPB0_FSI_ADDR);
@@ -233,7 +232,6 @@ static int opb_read(void __iomem *base, uint32_t addr, size_t size, u32 *out)
if (xfer_size < 0)
return xfer_size;
- writel(0x1, base + OPB0_SELECT);
writel(CMD_READ, base + OPB0_RW);
writel(xfer_size, base + OPB0_XFER_SIZE);
writel(addr, base + OPB0_FSI_ADDR);
@@ -596,6 +594,13 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
writel(0x0011bb1b, aspeed->base + OPB0_W_ENDIAN);
writel(0xffaa5500, aspeed->base + 0x50);
+ /*
+ * Select OPB0 for all operations.
+ * Will need to be reworked when enabling DMA or anything that uses
+ * OPB1.
+ */
+ writel(0x1, aspeed->base + OPB0_SELECT);
+
rc = opb_read(aspeed->base, ctrl_base + FSI_MVER, 4, &raw);
if (rc) {
dev_err(&pdev->dev, "failed to read hub version\n");
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
` (3 preceding siblings ...)
2019-10-25 1:03 ` [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 2:09 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 6/7] fsi: aspeed: Pass fsi_master_aspeed insead of base Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads Joel Stanley
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 58b090e2cf46..c98b1bf179a8 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -303,20 +303,17 @@ static int aspeed_master_read(struct fsi_master *master, int link,
{
struct fsi_master_aspeed *aspeed = to_fsi_master_aspeed(master);
int ret;
- u32 data;
if (id != 0)
return -EINVAL;
addr += link * FSI_HUB_LINK_SIZE;
- ret = opb_read(aspeed->base, fsi_base + addr, size, &data);
+ ret = opb_read(aspeed->base, fsi_base + addr, size, val);
ret = check_errors(aspeed, ret);
if (ret)
return ret;
- memcpy(val, &data, size);
-
return 0;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 6/7] fsi: aspeed: Pass fsi_master_aspeed insead of base
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
` (4 preceding siblings ...)
2019-10-25 1:03 ` [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads Joel Stanley
6 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
In preparation for a future change.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 61 +++++++++++++++++----------------
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index c98b1bf179a8..7e515b43b7a6 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -188,9 +188,10 @@ static u32 get_xfer_size(size_t size) {
}
}
-static u32 opb_write(void __iomem *base, uint32_t addr, uint32_t val,
- size_t size)
+static u32 opb_write(struct fsi_master_aspeed *aspeed, uint32_t addr,
+ uint32_t val, size_t size)
{
+ void __iomem *base = aspeed->base;
u32 reg, ret, status, xfer_size;
xfer_size = get_xfer_size(size);
@@ -223,8 +224,10 @@ static u32 opb_write(void __iomem *base, uint32_t addr, uint32_t val,
return 0;
}
-static int opb_read(void __iomem *base, uint32_t addr, size_t size, u32 *out)
+static int opb_read(struct fsi_master_aspeed *aspeed, uint32_t addr,
+ size_t size, u32 *out)
{
+ void __iomem *base = aspeed->base;
u32 result, reg, xfer_size;
int status, ret;
@@ -271,9 +274,9 @@ static int check_errors(struct fsi_master_aspeed *aspeed, int err)
if (trace_fsi_master_aspeed_opb_error_enabled()) {
__be32 mresp0, mstap0, mesrb0;
- opb_read(aspeed->base, ctrl_base + FSI_MRESP0, 4, &mresp0);
- opb_read(aspeed->base, ctrl_base + FSI_MSTAP0, 4, &mstap0);
- opb_read(aspeed->base, ctrl_base + FSI_MESRB0, 4, &mesrb0);
+ opb_read(aspeed, ctrl_base + FSI_MRESP0, 4, &mresp0);
+ opb_read(aspeed, ctrl_base + FSI_MSTAP0, 4, &mstap0);
+ opb_read(aspeed, ctrl_base + FSI_MESRB0, 4, &mesrb0);
trace_fsi_master_aspeed_opb_error(
be32_to_cpu(mresp0),
@@ -285,7 +288,7 @@ static int check_errors(struct fsi_master_aspeed *aspeed, int err)
/* Check MAEB (0x70) ? */
/* Then clear errors in master */
- ret = opb_write(aspeed->base, ctrl_base + 0xd0,
+ ret = opb_write(aspeed, ctrl_base + 0xd0,
cpu_to_be32(0x20000000), 4);
if (ret) {
/* TODO: log? return different code? */
@@ -308,7 +311,7 @@ static int aspeed_master_read(struct fsi_master *master, int link,
return -EINVAL;
addr += link * FSI_HUB_LINK_SIZE;
- ret = opb_read(aspeed->base, fsi_base + addr, size, val);
+ ret = opb_read(aspeed, fsi_base + addr, size, val);
ret = check_errors(aspeed, ret);
if (ret)
@@ -327,7 +330,7 @@ static int aspeed_master_write(struct fsi_master *master, int link,
return -EINVAL;
addr += link * FSI_HUB_LINK_SIZE;
- ret = opb_write(aspeed->base, fsi_base + addr, *(uint32_t *)val, size);
+ ret = opb_write(aspeed, fsi_base + addr, *(uint32_t *)val, size);
ret = check_errors(aspeed, ret);
if (ret)
@@ -347,12 +350,12 @@ static int aspeed_master_link_enable(struct fsi_master *master, int link)
reg = cpu_to_be32(0x80000000 >> bit);
- result = opb_write(aspeed->base, ctrl_base + FSI_MSENP0 + (4 * idx),
+ result = opb_write(aspeed, ctrl_base + FSI_MSENP0 + (4 * idx),
reg, 4);
mdelay(FSI_LINK_ENABLE_SETUP_TIME);
- ret = opb_read(aspeed->base, ctrl_base + FSI_MENP0 + (4 * idx),
+ ret = opb_read(aspeed, ctrl_base + FSI_MENP0 + (4 * idx),
4, &result);
if (ret)
return ret;
@@ -428,46 +431,46 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
reg = cpu_to_be32(FSI_MRESP_RST_ALL_MASTER | FSI_MRESP_RST_ALL_LINK
| FSI_MRESP_RST_MCR | FSI_MRESP_RST_PYE);
- opb_write(aspeed->base, ctrl_base + FSI_MRESP0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MRESP0, reg, 4);
/* Initialize the MFSI (hub master) engine */
reg = cpu_to_be32(FSI_MRESP_RST_ALL_MASTER | FSI_MRESP_RST_ALL_LINK
| FSI_MRESP_RST_MCR | FSI_MRESP_RST_PYE);
- opb_write(aspeed->base, ctrl_base + FSI_MRESP0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MRESP0, reg, 4);
reg = cpu_to_be32(FSI_MECTRL_EOAE | FSI_MECTRL_P8_AUTO_TERM);
- opb_write(aspeed->base, ctrl_base + FSI_MECTRL, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MECTRL, reg, 4);
reg = cpu_to_be32(FSI_MMODE_ECRC | FSI_MMODE_EPC | FSI_MMODE_RELA
| fsi_mmode_crs0(DEFAULT_DIVISOR)
| fsi_mmode_crs1(DEFAULT_DIVISOR)
| FSI_MMODE_P8_TO_LSB);
- opb_write(aspeed->base, ctrl_base + FSI_MMODE, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MMODE, reg, 4);
reg = cpu_to_be32(0xffff0000);
- opb_write(aspeed->base, ctrl_base + FSI_MDLYR, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MDLYR, reg, 4);
reg = cpu_to_be32(~0);
- opb_write(aspeed->base, ctrl_base + FSI_MSENP0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MSENP0, reg, 4);
/* Leave enabled long enough for master logic to set up */
mdelay(FSI_LINK_ENABLE_SETUP_TIME);
- opb_write(aspeed->base, ctrl_base + FSI_MCENP0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MCENP0, reg, 4);
- opb_read(aspeed->base, ctrl_base + FSI_MAEB, 4, NULL);
+ opb_read(aspeed, ctrl_base + FSI_MAEB, 4, NULL);
reg = cpu_to_be32(FSI_MRESP_RST_ALL_MASTER | FSI_MRESP_RST_ALL_LINK);
- opb_write(aspeed->base, ctrl_base + FSI_MRESP0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MRESP0, reg, 4);
- opb_read(aspeed->base, ctrl_base + FSI_MLEVP0, 4, NULL);
+ opb_read(aspeed, ctrl_base + FSI_MLEVP0, 4, NULL);
/* Reset the master bridge */
reg = cpu_to_be32(FSI_MRESB_RST_GEN);
- opb_write(aspeed->base, ctrl_base + FSI_MRESB0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MRESB0, reg, 4);
reg = cpu_to_be32(FSI_MRESB_RST_ERR);
- opb_write(aspeed->base, ctrl_base + FSI_MRESB0, reg, 4);
+ opb_write(aspeed, ctrl_base + FSI_MRESB0, reg, 4);
return 0;
}
@@ -478,7 +481,7 @@ static int fsi_master_aspeed_debugfs_get(void *data, u64 *val)
u32 out;
struct fsi_master_aspeed_debugfs_entry *entry = data;
- rc = opb_read(entry->aspeed->base, ctrl_base + entry->addr, 4, &out);
+ rc = opb_read(entry->aspeed, ctrl_base + entry->addr, 4, &out);
if (rc)
return rc;
@@ -491,7 +494,7 @@ static int fsi_master_aspeed_debugfs_set(void *data, u64 val)
u32 in = cpu_to_be32((u32)(val & 0xFFFFFFFFULL));
struct fsi_master_aspeed_debugfs_entry *entry = data;
- rc = opb_write(entry->aspeed->base, ctrl_base + entry->addr, in, 4);
+ rc = opb_write(entry->aspeed, ctrl_base + entry->addr, in, 4);
if (rc)
return rc;
@@ -507,7 +510,7 @@ static int fsi_master_aspeed_clock_debugfs_get(void *data, u64 *val)
u32 out;
int rc;
- rc = opb_read(aspeed->base, ctrl_base, 4, &out);
+ rc = opb_read(aspeed, ctrl_base, 4, &out);
if (rc)
return rc;
@@ -525,7 +528,7 @@ static int fsi_master_aspeed_clock_debugfs_set(void *data, u64 val)
if (val > 0x3ff)
return -EINVAL;
- rc = opb_read(aspeed->base, ctrl_base, 4, &raw);
+ rc = opb_read(aspeed, ctrl_base, 4, &raw);
if (rc)
return rc;
@@ -534,7 +537,7 @@ static int fsi_master_aspeed_clock_debugfs_set(void *data, u64 val)
reg &= ~(0x3ff << 18);
reg |= (val & 0x3ff) << 18;
- rc = opb_write(aspeed->base, ctrl_base, cpu_to_be32(reg), 4);
+ rc = opb_write(aspeed, ctrl_base, cpu_to_be32(reg), 4);
if (rc)
return rc;
@@ -598,7 +601,7 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
*/
writel(0x1, aspeed->base + OPB0_SELECT);
- rc = opb_read(aspeed->base, ctrl_base + FSI_MVER, 4, &raw);
+ rc = opb_read(aspeed, ctrl_base + FSI_MVER, 4, &raw);
if (rc) {
dev_err(&pdev->dev, "failed to read hub version\n");
return rc;
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
` (5 preceding siblings ...)
2019-10-25 1:03 ` [PATCH linux dev-5.3 6/7] fsi: aspeed: Pass fsi_master_aspeed insead of base Joel Stanley
@ 2019-10-25 1:03 ` Joel Stanley
2019-10-29 3:16 ` Jeremy Kerr
6 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2019-10-25 1:03 UTC (permalink / raw)
To: Jeremy Kerr, Andrew Jeffery; +Cc: openbmc
The driver can skip doing some of the AHB2OPB setup if the operation is
of the same type. Experiment with this for full word reads, which could
be extended to writes if it shows an improvement.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-master-aspeed.c | 50 ++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 7e515b43b7a6..30e818728402 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -92,6 +92,8 @@ struct fsi_master_aspeed {
void __iomem *base;
struct clk *clk;
+ bool last_was_fw_read;
+
struct dentry *debugfs_dir;
struct fsi_master_aspeed_debugfs_entry debugfs[FSI_NUM_DEBUGFS_ENTRIES];
};
@@ -205,6 +207,8 @@ static u32 opb_write(struct fsi_master_aspeed *aspeed, uint32_t addr,
writel(0x1, base + OPB_IRQ_CLEAR);
writel(0x1, base + OPB_TRIGGER);
+ aspeed->last_was_fw_read = false;
+
ret = readl_poll_timeout(base + OPB_IRQ_STATUS, reg,
(reg & OPB0_XFER_ACK_EN) != 0,
0, 10000);
@@ -224,6 +228,43 @@ static u32 opb_write(struct fsi_master_aspeed *aspeed, uint32_t addr,
return 0;
}
+static int opb_read_repeat_fullword(struct fsi_master_aspeed *aspeed,
+ uint32_t addr, u32 *out)
+{
+ void __iomem *base = aspeed->base;
+ u32 result, reg;
+ int status, ret;
+
+ writel(addr, base + OPB0_FSI_ADDR);
+ writel(0x1, base + OPB_IRQ_CLEAR);
+ writel(0x1, base + OPB_TRIGGER);
+
+ ret = readl_poll_timeout(base + OPB_IRQ_STATUS, reg,
+ (reg & OPB0_XFER_ACK_EN) != 0,
+ 0, 10000);
+
+ status = readl(base + OPB0_STATUS);
+
+ result = readl(base + OPB0_FSI_DATA_R);
+
+ trace_fsi_master_aspeed_opb_read(addr, 4, result,
+ readl(base + OPB0_STATUS),
+ reg);
+
+ /* Return error when poll timed out */
+ if (ret)
+ return ret;
+
+ /* Command failed, master will reset */
+ if (status & STATUS_ERR_ACK)
+ return -EIO;
+
+ if (out)
+ memcpy(out, &result, 4);
+
+ return 0;
+}
+
static int opb_read(struct fsi_master_aspeed *aspeed, uint32_t addr,
size_t size, u32 *out)
{
@@ -241,6 +282,9 @@ static int opb_read(struct fsi_master_aspeed *aspeed, uint32_t addr,
writel(0x1, base + OPB_IRQ_CLEAR);
writel(0x1, base + OPB_TRIGGER);
+ if (size == 4)
+ aspeed->last_was_fw_read = true;
+
ret = readl_poll_timeout(base + OPB_IRQ_STATUS, reg,
(reg & OPB0_XFER_ACK_EN) != 0,
0, 10000);
@@ -311,7 +355,11 @@ static int aspeed_master_read(struct fsi_master *master, int link,
return -EINVAL;
addr += link * FSI_HUB_LINK_SIZE;
- ret = opb_read(aspeed, fsi_base + addr, size, val);
+
+ if (aspeed->last_was_fw_read)
+ ret = opb_read_repeat_fullword(aspeed, fsi_base + addr, val);
+ else
+ ret = opb_read(aspeed, fsi_base + addr, size, val);
ret = check_errors(aspeed, ret);
if (ret)
--
2.23.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case
2019-10-25 1:03 ` [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case Joel Stanley
@ 2019-10-25 1:52 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2019-10-25 1:52 UTC (permalink / raw)
To: Joel Stanley, Jeremy Kerr; +Cc: openbmc
On Fri, 25 Oct 2019, at 12:03, Joel Stanley wrote:
> We busy loop in the read case, make the write case the same. Note that
> this may cause issues when doing a break, which takes a long time.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14
2019-10-25 1:03 ` [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14 Joel Stanley
@ 2019-10-25 1:56 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2019-10-25 1:56 UTC (permalink / raw)
To: Joel Stanley, Jeremy Kerr; +Cc: openbmc
On Fri, 25 Oct 2019, at 12:03, Joel Stanley wrote:
> We were running at 127, which equates to a bus clock speed of approx
> 1.4MHz. This changes that to approx 14MHz, which works on the EVB and is
> reliable on the Tacoma systems.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing
2019-10-25 1:03 ` [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing Joel Stanley
@ 2019-10-25 1:57 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2019-10-25 1:57 UTC (permalink / raw)
To: Joel Stanley, Jeremy Kerr; +Cc: openbmc
On Fri, 25 Oct 2019, at 12:03, Joel Stanley wrote:
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once
2019-10-25 1:03 ` [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once Joel Stanley
@ 2019-10-25 1:59 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2019-10-25 1:59 UTC (permalink / raw)
To: Joel Stanley, Jeremy Kerr; +Cc: openbmc
On Fri, 25 Oct 2019, at 12:03, Joel Stanley wrote:
> The driver can leve
s/leve/leave/
> OPB0 selected to save a AHB write per OPB operation.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice
2019-10-25 1:03 ` [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice Joel Stanley
@ 2019-10-25 2:09 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2019-10-25 2:09 UTC (permalink / raw)
To: Joel Stanley, Jeremy Kerr; +Cc: openbmc
On Fri, 25 Oct 2019, at 12:03, Joel Stanley wrote:
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads
2019-10-25 1:03 ` [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads Joel Stanley
@ 2019-10-29 3:16 ` Jeremy Kerr
2019-10-29 3:53 ` Joel Stanley
0 siblings, 1 reply; 15+ messages in thread
From: Jeremy Kerr @ 2019-10-29 3:16 UTC (permalink / raw)
To: Joel Stanley, Andrew Jeffery; +Cc: openbmc
Hi Joel,
> The driver can skip doing some of the AHB2OPB setup if the operation is
> of the same type. Experiment with this for full word reads, which could
> be extended to writes if it shows an improvement.
I would have taken a slightly different approach here - to do the "last
value" caching at the OPB register set level, rather than working at the
FSI-operation level. ie, keep a copy of the AHB registers that control
OPB transactions, then only set them when they differ from the intended
writes.
That would give us something like this for opb_read():
if (aspeed->opb0_select != 0x1) {
aspeed->opb0_select = 0x1;
writel(aspeed->opb0_select, base + OPB0_SELECT);
}
if (aspeed->opb0_rw != CMD_READ) {
aspeed->opb0_rw = CMD_READ;
writel(aspeed->opb0_rw, base + OPB0_RW);
}
if (aspeed->opb0_xfer_size != XFER_WORD) {
aspeed->opb0_xfer_size = XFER_WORD;
writel(aspeed->opb0_xfer_size, base + OPB0_XFER_SIZE);
}
if (aspeed->opb0_addr != addr) {
aspeed->opb0_addr = addr;
writel(aspeed->opb0_addr, base + OPB0_FSI_ADDR
}
writel(0x1, base + OPB_IRQ_CLEAR);
writel(0x1, base + OPB_TRIGGER);
[which we could simplify with a helper function for the caching...]
However, if the aim is to just stage this to just the fullword reads for
now, this looks fine for me.
Cheers,
Jeremy
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads
2019-10-29 3:16 ` Jeremy Kerr
@ 2019-10-29 3:53 ` Joel Stanley
0 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2019-10-29 3:53 UTC (permalink / raw)
To: Jeremy Kerr; +Cc: Andrew Jeffery, OpenBMC Maillist
On Tue, 29 Oct 2019 at 03:17, Jeremy Kerr <jk@ozlabs.org> wrote:
>
> Hi Joel,
>
> > The driver can skip doing some of the AHB2OPB setup if the operation is
> > of the same type. Experiment with this for full word reads, which could
> > be extended to writes if it shows an improvement.
>
> I would have taken a slightly different approach here - to do the "last
> value" caching at the OPB register set level, rather than working at the
> FSI-operation level. ie, keep a copy of the AHB registers that control
> OPB transactions, then only set them when they differ from the intended
> writes.
>
> That would give us something like this for opb_read():
>
> if (aspeed->opb0_select != 0x1) {
> aspeed->opb0_select = 0x1;
> writel(aspeed->opb0_select, base + OPB0_SELECT);
> }
>
> if (aspeed->opb0_rw != CMD_READ) {
> aspeed->opb0_rw = CMD_READ;
> writel(aspeed->opb0_rw, base + OPB0_RW);
> }
>
> if (aspeed->opb0_xfer_size != XFER_WORD) {
> aspeed->opb0_xfer_size = XFER_WORD;
> writel(aspeed->opb0_xfer_size, base + OPB0_XFER_SIZE);
> }
>
> if (aspeed->opb0_addr != addr) {
> aspeed->opb0_addr = addr;
> writel(aspeed->opb0_addr, base + OPB0_FSI_ADDR
> }
>
> writel(0x1, base + OPB_IRQ_CLEAR);
> writel(0x1, base + OPB_TRIGGER);
>
> [which we could simplify with a helper function for the caching...]
I am not convinced that this would be faster. In the best case (same
operation) it's N branches. In the worst case, it's N branches and N
mmio operations. I don't have any data for how often the values
change, so that's something I could collect.
I'll defer merging this change until we have more data.
Cheers,
Joel
>
> However, if the aim is to just stage this to just the fullword reads for
> now, this looks fine for me.
>
> Cheers,
>
>
> Jeremy
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-10-29 3:53 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25 1:03 [PATCH linux dev-5.3 0/7] AST2600 FSI speed improvements Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 1/7] fsi: aspeed: busy loop in the write case Joel Stanley
2019-10-25 1:52 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 2/7] fsi: master: Change default divisor to 14 Joel Stanley
2019-10-25 1:56 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 3/7] fsi: aspeed: Enable relative addressing Joel Stanley
2019-10-25 1:57 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 4/7] fsi: aspeed: Only select OPB0 once Joel Stanley
2019-10-25 1:59 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 5/7] fsi: aspeed: Avoid copying read data twice Joel Stanley
2019-10-25 2:09 ` Andrew Jeffery
2019-10-25 1:03 ` [PATCH linux dev-5.3 6/7] fsi: aspeed: Pass fsi_master_aspeed insead of base Joel Stanley
2019-10-25 1:03 ` [PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads Joel Stanley
2019-10-29 3:16 ` Jeremy Kerr
2019-10-29 3:53 ` Joel Stanley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.