| Message ID | 1400500288-10263-2-git-send-email-josh.wu@atmel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Andreas Bießmann |
| Headers | show |
Hi Josh, On 05/19/2014 07:51 PM, Josh Wu wrote: > Add to code to flush the dcache after we writing in DMA buffer. > Also we need invalidate the dcache before we check the status in the > DMA buffer. > > Tested in SAMA5D3x-EK with gmac0. Tftp download speed shows in below: > Disable DCache: 1.1 MiB/s > Enable DCache: 1.6 MiB/s > Increase speed with about 40%. > > The code should have no impact with the boards which are not > enable_dcache(). > Tested in AT91SAM9M10G45EK. > > Signed-off-by: Josh Wu <josh.wu@atmel.com> For this patch set, tested ok on sama5d3xek board. Tested-by: Bo Shen <voice.shen@atmel.com> Acked-by: Bo Shen <voice.shen@atmel.com> Best Regards, Bo Shen > --- > v1 -> v2: > no change. > > drivers/net/macb.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 781a272..b18f07b 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -194,6 +194,39 @@ int macb_miiphy_write(const char *devname, u8 phy_adr, u8 reg, u16 value) > } > #endif > > +#define IS_RX 1 > +#define IS_TX 0 > +static inline void macb_invalidate_ring_desc(struct macb_device *macb, bool is_rx) > +{ > + if (is_rx) > + invalidate_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + > + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); > + else > + invalidate_dcache_range(macb->tx_ring_dma, macb->tx_ring_dma + > + CONFIG_SYS_MACB_TX_RING_SIZE * sizeof(struct macb_dma_desc)); > +} > + > +static inline void macb_flush_ring_desc(struct macb_device *macb, bool is_rx) > +{ > + if (is_rx) > + flush_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + > + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); > + else > + flush_dcache_range(macb->tx_ring_dma, macb->tx_ring_dma + > + CONFIG_SYS_MACB_TX_RING_SIZE * sizeof(struct macb_dma_desc)); > +} > + > +static inline void macb_flush_rx_buffer(struct macb_device *macb) > +{ > + flush_dcache_range(macb->rx_buffer_dma, > + macb->rx_buffer_dma + CONFIG_SYS_MACB_RX_BUFFER_SIZE); > +} > + > +static inline void macb_invalidate_rx_buffer(struct macb_device *macb) > +{ > + invalidate_dcache_range(macb->rx_buffer_dma, > + macb->rx_buffer_dma + CONFIG_SYS_MACB_RX_BUFFER_SIZE); > +} > > #if defined(CONFIG_CMD_NET) > > @@ -217,6 +250,9 @@ static int macb_send(struct eth_device *netdev, void *packet, int length) > macb->tx_ring[tx_head].ctrl = ctrl; > macb->tx_ring[tx_head].addr = paddr; > barrier(); > + macb_flush_ring_desc(macb, IS_TX); > + /* Do we need check paddr and length is dcache line aligned? */ > + flush_dcache_range(paddr, paddr + length); > macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE) | MACB_BIT(TSTART)); > > /* > @@ -225,6 +261,7 @@ static int macb_send(struct eth_device *netdev, void *packet, int length) > */ > for (i = 0; i <= CONFIG_SYS_MACB_TX_TIMEOUT; i++) { > barrier(); > + macb_invalidate_ring_desc(macb, IS_TX); > ctrl = macb->tx_ring[tx_head].ctrl; > if (ctrl & TXBUF_USED) > break; > @@ -253,6 +290,8 @@ static void reclaim_rx_buffers(struct macb_device *macb, > unsigned int i; > > i = macb->rx_tail; > + > + macb_invalidate_ring_desc(macb, IS_RX); > while (i > new_tail) { > macb->rx_ring[i].addr &= ~RXADDR_USED; > i++; > @@ -266,6 +305,7 @@ static void reclaim_rx_buffers(struct macb_device *macb, > } > > barrier(); > + macb_flush_ring_desc(macb, IS_RX); > macb->rx_tail = new_tail; > } > > @@ -279,6 +319,8 @@ static int macb_recv(struct eth_device *netdev) > u32 status; > > for (;;) { > + macb_invalidate_ring_desc(macb, IS_RX); > + > if (!(macb->rx_ring[rx_tail].addr & RXADDR_USED)) > return -1; > > @@ -292,6 +334,8 @@ static int macb_recv(struct eth_device *netdev) > if (status & RXBUF_FRAME_END) { > buffer = macb->rx_buffer + 128 * macb->rx_tail; > length = status & RXBUF_FRMLEN_MASK; > + > + macb_invalidate_rx_buffer(macb); > if (wrapped) { > unsigned int headlen, taillen; > > @@ -506,6 +550,9 @@ static int macb_init(struct eth_device *netdev, bd_t *bd) > macb->rx_ring[i].ctrl = 0; > paddr += 128; > } > + macb_flush_ring_desc(macb, IS_RX); > + macb_flush_rx_buffer(macb); > + > for (i = 0; i < CONFIG_SYS_MACB_TX_RING_SIZE; i++) { > macb->tx_ring[i].addr = 0; > if (i == (CONFIG_SYS_MACB_TX_RING_SIZE - 1)) > @@ -513,6 +560,8 @@ static int macb_init(struct eth_device *netdev, bd_t *bd) > else > macb->tx_ring[i].ctrl = TXBUF_USED; > } > + macb_flush_ring_desc(macb, IS_TX); > + > macb->rx_tail = macb->tx_head = macb->tx_tail = 0; > > macb_writel(macb, RBQP, macb->rx_ring_dma); > @@ -663,6 +712,8 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr) > * sizeof(struct macb_dma_desc), > &macb->tx_ring_dma); > > + /* TODO: we need check the rx/tx_ring_dma is dcache line aligned */ > + > macb->regs = regs; > macb->phy_addr = phy_addr; > >
Hi Josh, On 19.05.14 13:51, Josh Wu wrote: > Add to code to flush the dcache after we writing in DMA buffer. > Also we need invalidate the dcache before we check the status in the > DMA buffer. > > Tested in SAMA5D3x-EK with gmac0. Tftp download speed shows in below: > Disable DCache: 1.1 MiB/s > Enable DCache: 1.6 MiB/s > Increase speed with about 40%. > > The code should have no impact with the boards which are not > enable_dcache(). > Tested in AT91SAM9M10G45EK. > > Signed-off-by: Josh Wu <josh.wu@atmel.com> > --- > v1 -> v2: > no change. > > drivers/net/macb.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 781a272..b18f07b 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -194,6 +194,39 @@ int macb_miiphy_write(const char *devname, u8 phy_adr, u8 reg, u16 value) > } > #endif > > +#define IS_RX 1 > +#define IS_TX 0 > +static inline void macb_invalidate_ring_desc(struct macb_device *macb, bool is_rx) > +{ > + if (is_rx) > + invalidate_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + > + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); these lines produce checkpatch 'line over 80 chars' warnings. Could you please check my macb cleanup patch [1] and adopt yours to that one? Could you please also do a formal review of that patch? Also it could make sens to introduce some MACB_RX_RING_BYTE_SIZE or something like this to prevent writing always the multiply by sizeof(macb_dma_desc). If you could a new version of this patch it could go in as the first version was in merge window phase AFAIR. The other two patches in this series are ok. Best regards Andreas Bießmann [1] http://patchwork.ozlabs.org/patch/352624/
Hi, Andreas On 5/27/2014 5:06 AM, Andreas Bießmann wrote: > Hi Josh, > > On 19.05.14 13:51, Josh Wu wrote: >> Add to code to flush the dcache after we writing in DMA buffer. >> Also we need invalidate the dcache before we check the status in the >> DMA buffer. >> >> Tested in SAMA5D3x-EK with gmac0. Tftp download speed shows in below: >> Disable DCache: 1.1 MiB/s >> Enable DCache: 1.6 MiB/s >> Increase speed with about 40%. >> >> The code should have no impact with the boards which are not >> enable_dcache(). >> Tested in AT91SAM9M10G45EK. >> >> Signed-off-by: Josh Wu <josh.wu@atmel.com> >> --- >> v1 -> v2: >> no change. >> >> drivers/net/macb.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >> index 781a272..b18f07b 100644 >> --- a/drivers/net/macb.c >> +++ b/drivers/net/macb.c >> @@ -194,6 +194,39 @@ int macb_miiphy_write(const char *devname, u8 phy_adr, u8 reg, u16 value) >> } >> #endif >> >> +#define IS_RX 1 >> +#define IS_TX 0 >> +static inline void macb_invalidate_ring_desc(struct macb_device *macb, bool is_rx) >> +{ >> + if (is_rx) >> + invalidate_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + >> + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); > these lines produce checkpatch 'line over 80 chars' warnings. Could you > please check my macb cleanup patch [1] and adopt yours to that one? No problem. > Could you please also do a formal review of that patch? Thanks for the clean up patch. I already add my Reviewed-by in that patch. > > Also it could make sens to introduce some MACB_RX_RING_BYTE_SIZE or > something like this to prevent writing always the multiply by > sizeof(macb_dma_desc). > If you could a new version of this patch it could go in as the first > version was in merge window phase AFAIR. The other two patches in this > series are ok. So I will resend a new version of this patch which will rebase on your clean up macb patch. Thanks. Best Regards, Josh Wu > > Best regards > > Andreas Bießmann > > [1] http://patchwork.ozlabs.org/patch/352624/
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 781a272..b18f07b 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -194,6 +194,39 @@ int macb_miiphy_write(const char *devname, u8 phy_adr, u8 reg, u16 value) } #endif +#define IS_RX 1 +#define IS_TX 0 +static inline void macb_invalidate_ring_desc(struct macb_device *macb, bool is_rx) +{ + if (is_rx) + invalidate_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); + else + invalidate_dcache_range(macb->tx_ring_dma, macb->tx_ring_dma + + CONFIG_SYS_MACB_TX_RING_SIZE * sizeof(struct macb_dma_desc)); +} + +static inline void macb_flush_ring_desc(struct macb_device *macb, bool is_rx) +{ + if (is_rx) + flush_dcache_range(macb->rx_ring_dma, macb->rx_ring_dma + + CONFIG_SYS_MACB_RX_RING_SIZE * sizeof(struct macb_dma_desc)); + else + flush_dcache_range(macb->tx_ring_dma, macb->tx_ring_dma + + CONFIG_SYS_MACB_TX_RING_SIZE * sizeof(struct macb_dma_desc)); +} + +static inline void macb_flush_rx_buffer(struct macb_device *macb) +{ + flush_dcache_range(macb->rx_buffer_dma, + macb->rx_buffer_dma + CONFIG_SYS_MACB_RX_BUFFER_SIZE); +} + +static inline void macb_invalidate_rx_buffer(struct macb_device *macb) +{ + invalidate_dcache_range(macb->rx_buffer_dma, + macb->rx_buffer_dma + CONFIG_SYS_MACB_RX_BUFFER_SIZE); +} #if defined(CONFIG_CMD_NET) @@ -217,6 +250,9 @@ static int macb_send(struct eth_device *netdev, void *packet, int length) macb->tx_ring[tx_head].ctrl = ctrl; macb->tx_ring[tx_head].addr = paddr; barrier(); + macb_flush_ring_desc(macb, IS_TX); + /* Do we need check paddr and length is dcache line aligned? */ + flush_dcache_range(paddr, paddr + length); macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE) | MACB_BIT(TSTART)); /* @@ -225,6 +261,7 @@ static int macb_send(struct eth_device *netdev, void *packet, int length) */ for (i = 0; i <= CONFIG_SYS_MACB_TX_TIMEOUT; i++) { barrier(); + macb_invalidate_ring_desc(macb, IS_TX); ctrl = macb->tx_ring[tx_head].ctrl; if (ctrl & TXBUF_USED) break; @@ -253,6 +290,8 @@ static void reclaim_rx_buffers(struct macb_device *macb, unsigned int i; i = macb->rx_tail; + + macb_invalidate_ring_desc(macb, IS_RX); while (i > new_tail) { macb->rx_ring[i].addr &= ~RXADDR_USED; i++; @@ -266,6 +305,7 @@ static void reclaim_rx_buffers(struct macb_device *macb, } barrier(); + macb_flush_ring_desc(macb, IS_RX); macb->rx_tail = new_tail; } @@ -279,6 +319,8 @@ static int macb_recv(struct eth_device *netdev) u32 status; for (;;) { + macb_invalidate_ring_desc(macb, IS_RX); + if (!(macb->rx_ring[rx_tail].addr & RXADDR_USED)) return -1; @@ -292,6 +334,8 @@ static int macb_recv(struct eth_device *netdev) if (status & RXBUF_FRAME_END) { buffer = macb->rx_buffer + 128 * macb->rx_tail; length = status & RXBUF_FRMLEN_MASK; + + macb_invalidate_rx_buffer(macb); if (wrapped) { unsigned int headlen, taillen; @@ -506,6 +550,9 @@ static int macb_init(struct eth_device *netdev, bd_t *bd) macb->rx_ring[i].ctrl = 0; paddr += 128; } + macb_flush_ring_desc(macb, IS_RX); + macb_flush_rx_buffer(macb); + for (i = 0; i < CONFIG_SYS_MACB_TX_RING_SIZE; i++) { macb->tx_ring[i].addr = 0; if (i == (CONFIG_SYS_MACB_TX_RING_SIZE - 1)) @@ -513,6 +560,8 @@ static int macb_init(struct eth_device *netdev, bd_t *bd) else macb->tx_ring[i].ctrl = TXBUF_USED; } + macb_flush_ring_desc(macb, IS_TX); + macb->rx_tail = macb->tx_head = macb->tx_tail = 0; macb_writel(macb, RBQP, macb->rx_ring_dma); @@ -663,6 +712,8 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr) * sizeof(struct macb_dma_desc), &macb->tx_ring_dma); + /* TODO: we need check the rx/tx_ring_dma is dcache line aligned */ + macb->regs = regs; macb->phy_addr = phy_addr;
Add to code to flush the dcache after we writing in DMA buffer. Also we need invalidate the dcache before we check the status in the DMA buffer. Tested in SAMA5D3x-EK with gmac0. Tftp download speed shows in below: Disable DCache: 1.1 MiB/s Enable DCache: 1.6 MiB/s Increase speed with about 40%. The code should have no impact with the boards which are not enable_dcache(). Tested in AT91SAM9M10G45EK. Signed-off-by: Josh Wu <josh.wu@atmel.com> --- v1 -> v2: no change. drivers/net/macb.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)