On Fri, Jan 2, 2015 at 11:03 AM, Felipe Balbi <balbi@xxxxxx> wrote: > Hi, > > (please use reply-all to keep mailing lists in Cc, also avoid > top-posting) I am trying not to read netdev right now... and failing, obviously. > > On Fri, Jan 02, 2015 at 10:58:29AM -0800, Dave Taht wrote: >> The beaglebone only has a 100mbit phy, so you aren't going to get more >> than that. > > very true :-) Still, with AM437x SK which is definitely GigE, I'm > getting 201Mbits/sec. > >> (so do a lot of IoT devices). >> >> So you have the two patches that went by on BQL and on NAPI for the beagle? > > no, got any pointers ? the relevant thread was "am335x: cpsw: phy ignores max-speed setting" and the initial very small BQL enablement patch was here: https://patchwork.ozlabs.org/patch/407640/ (it needed a saner treatment of a failure to dma something in cpsw_tx_packet_submit - the patch as is has also been part of nelsons trees for the beaglebone for a while) But it was rightly pointed out later in the thread that this change +#define CPSW_POLL_WEIGHT 16 made for the biggest part of the improvement, and someone else on the thread proposed handling that more dynamically for 100mbit phys with another patch (that I can't find at the moment) ... but the root cause of the excessive latency in this driver was the single tx/rx dma queue, which you are addressing in your patch set. So if you glop on more of the above, mo better, perhaps you will win bigger. I will try to slice out some time to boot up a beagle on net-next next week. >> On Fri, Jan 2, 2015 at 10:55 AM, Felipe Balbi <balbi@xxxxxx> wrote: >> > Hi, >> > >> > On Fri, Jan 02, 2015 at 10:49:49AM -0800, Dave Taht wrote: >> >> +1. >> >> >> >> We'd had a thread on netdev (can't find it now) where we discussed >> >> adding BQL support and also something saner for the NAPI handling to >> >> this driver. >> > >> > yeah, currently is completely borked. I'm on a gigabit network and I'm >> > getting 94Mbits/sec, total crap. >> > >> >> Initial results for the beaglebone black were pretty spectacular, and >> >> it does look like this is way cleaner infrastructure underneat th deal >> >> with. Are you testing >> > >> > cool, if I new more about networking I'd certainly help, but I can help >> > testing for sure, just keep me in Cc ;-) >> > >> >> on the beaglebone black.? do you remember that convo? >> > >> > yeah, testing on beagleboneblack and AM437x SK. >> > >> > cheers >> > >> >> On Fri, Jan 2, 2015 at 10:10 AM, Felipe Balbi <balbi@xxxxxx> wrote: >> >> > Now we can introduce dedicated IRQ handlers >> >> > for each of the IRQ events. This helps with >> >> > cleaning up a little bit of the clutter in >> >> > cpsw_interrupt() while also making sure that >> >> > TX IRQs will try to handle TX buffers while >> >> > RX IRQs will try to handle RX buffers. >> >> > >> >> > Signed-off-by: Felipe Balbi <balbi@xxxxxx> >> >> > --- >> >> > drivers/net/ethernet/ti/cpsw.c | 41 ++++++++++++++++++++++++++++++----------- >> >> > 1 file changed, 30 insertions(+), 11 deletions(-) >> >> > >> >> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c >> >> > index 6e04128..c9081bd 100644 >> >> > --- a/drivers/net/ethernet/ti/cpsw.c >> >> > +++ b/drivers/net/ethernet/ti/cpsw.c >> >> > @@ -754,18 +754,36 @@ requeue: >> >> > dev_kfree_skb_any(new_skb); >> >> > } >> >> > >> >> > -static irqreturn_t cpsw_interrupt(int irq, void *dev_id) >> >> > +static irqreturn_t cpsw_dummy_interrupt(int irq, void *dev_id) >> >> > { >> >> > struct cpsw_priv *priv = dev_id; >> >> > int value = irq - priv->irqs_table[0]; >> >> > >> >> > - /* NOTICE: Ending IRQ here. The trick with the 'value' variable above >> >> > - * is to make sure we will always write the correct value to the EOI >> >> > - * register. Namely 0 for RX_THRESH Interrupt, 1 for RX Interrupt, 2 >> >> > - * for TX Interrupt and 3 for MISC Interrupt. >> >> > - */ >> >> > cpdma_ctlr_eoi(priv->dma, value); >> >> > >> >> > + return IRQ_HANDLED; >> >> > +} >> >> > + >> >> > +static irqreturn_t cpsw_tx_interrupt(int irq, void *dev_id) >> >> > +{ >> >> > + struct cpsw_priv *priv = dev_id; >> >> > + >> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_TX); >> >> > + cpdma_chan_process(priv->txch, 128); >> >> > + >> >> > + priv = cpsw_get_slave_priv(priv, 1); >> >> > + if (priv) >> >> > + cpdma_chan_process(priv->txch, 128); >> >> > + >> >> > + return IRQ_HANDLED; >> >> > +} >> >> > + >> >> > +static irqreturn_t cpsw_rx_interrupt(int irq, void *dev_id) >> >> > +{ >> >> > + struct cpsw_priv *priv = dev_id; >> >> > + >> >> > + cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX); >> >> > + >> >> > cpsw_intr_disable(priv); >> >> > if (priv->irq_enabled == true) { >> >> > cpsw_disable_irq(priv); >> >> > @@ -1617,7 +1635,8 @@ static void cpsw_ndo_poll_controller(struct net_device *ndev) >> >> > >> >> > cpsw_intr_disable(priv); >> >> > cpdma_ctlr_int_ctrl(priv->dma, false); >> >> > - cpsw_interrupt(ndev->irq, priv); >> >> > + cpsw_rx_interrupt(priv->irq[1], priv); >> >> > + cpsw_tx_interrupt(priv->irq[2], priv); >> >> > cpdma_ctlr_int_ctrl(priv->dma, true); >> >> > cpsw_intr_enable(priv); >> >> > } >> >> > @@ -2351,7 +2370,7 @@ static int cpsw_probe(struct platform_device *pdev) >> >> > goto clean_ale_ret; >> >> > >> >> > priv->irqs_table[0] = irq; >> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt, >> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt, >> >> > 0, dev_name(&pdev->dev), priv); >> >> > if (ret < 0) { >> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret); >> >> > @@ -2363,7 +2382,7 @@ static int cpsw_probe(struct platform_device *pdev) >> >> > goto clean_ale_ret; >> >> > >> >> > priv->irqs_table[1] = irq; >> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt, >> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_rx_interrupt, >> >> > 0, dev_name(&pdev->dev), priv); >> >> > if (ret < 0) { >> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret); >> >> > @@ -2375,7 +2394,7 @@ static int cpsw_probe(struct platform_device *pdev) >> >> > goto clean_ale_ret; >> >> > >> >> > priv->irqs_table[2] = irq; >> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt, >> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_tx_interrupt, >> >> > 0, dev_name(&pdev->dev), priv); >> >> > if (ret < 0) { >> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret); >> >> > @@ -2387,7 +2406,7 @@ static int cpsw_probe(struct platform_device *pdev) >> >> > goto clean_ale_ret; >> >> > >> >> > priv->irqs_table[3] = irq; >> >> > - ret = devm_request_irq(&pdev->dev, irq, cpsw_interrupt, >> >> > + ret = devm_request_irq(&pdev->dev, irq, cpsw_dummy_interrupt, >> >> > 0, dev_name(&pdev->dev), priv); >> >> > if (ret < 0) { >> >> > dev_err(priv->dev, "error attaching irq (%d)\n", ret); >> >> > -- >> >> > 2.2.0 >> >> > >> >> > -- >> >> > To unsubscribe from this list: send the line "unsubscribe netdev" in >> >> > the body of a message to majordomo@xxxxxxxxxxxxxxx >> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> >> >> >> >> -- >> >> Dave Täht >> >> >> >> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks >> > >> > -- >> > balbi >> >> >> >> -- >> Dave Täht >> >> thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks > > -- > balbi -- Dave Täht thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
