Messages in this thread | | | Date | Fri, 20 Apr 2012 08:41:00 +1000 | From | Ryan Mallon <> | Subject | Re: [PATCH v2] dmaengine/ep93xx_dma: Implement double buffering for M2M DMA channels |
| |
On 19/04/12 19:19, Rafal Prylowski wrote:
> Add double buffering support for M2M DMA channels. Implement this by using EP93xx > M2M DMA Buffer and Control Finite State Machines to be sure that we are not > disabling the channel when it's actually operating. > > Signed-off-by: Rafal Prylowski <prylowski@metasoft.pl> > Tested-by: H Hartley Sweeten <hsweeten@visionengravers.com> > Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Applied to ep93xx-fixes.
Thanks, ~Ryan
> --- > This version fixes issues with interrupt storm observed when dma is > used with spi-ep93xx.c and mmc_spi.c drivers. > Patch applies to v3.4rc3. > > drivers/dma/ep93xx_dma.c | 117 +++++++++++++++++++++++++++++-------- > 1 file changed, 93 insertions(+), 24 deletions(-) > > Index: linux-2.6/drivers/dma/ep93xx_dma.c > =================================================================== > --- linux-2.6.orig/drivers/dma/ep93xx_dma.c > +++ linux-2.6/drivers/dma/ep93xx_dma.c > @@ -71,6 +71,7 @@ > #define M2M_CONTROL_TM_SHIFT 13 > #define M2M_CONTROL_TM_TX (1 << M2M_CONTROL_TM_SHIFT) > #define M2M_CONTROL_TM_RX (2 << M2M_CONTROL_TM_SHIFT) > +#define M2M_CONTROL_NFBINT BIT(21) > #define M2M_CONTROL_RSS_SHIFT 22 > #define M2M_CONTROL_RSS_SSPRX (1 << M2M_CONTROL_RSS_SHIFT) > #define M2M_CONTROL_RSS_SSPTX (2 << M2M_CONTROL_RSS_SHIFT) > @@ -79,7 +80,22 @@ > #define M2M_CONTROL_PWSC_SHIFT 25 > > #define M2M_INTERRUPT 0x0004 > -#define M2M_INTERRUPT_DONEINT BIT(1) > +#define M2M_INTERRUPT_MASK 6 > + > +#define M2M_STATUS 0x000c > +#define M2M_STATUS_CTL_SHIFT 1 > +#define M2M_STATUS_CTL_IDLE (0 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_CTL_STALL (1 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_CTL_MEMRD (2 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_CTL_MEMWR (3 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_CTL_BWCWAIT (4 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_CTL_MASK (7 << M2M_STATUS_CTL_SHIFT) > +#define M2M_STATUS_BUF_SHIFT 4 > +#define M2M_STATUS_BUF_NO (0 << M2M_STATUS_BUF_SHIFT) > +#define M2M_STATUS_BUF_ON (1 << M2M_STATUS_BUF_SHIFT) > +#define M2M_STATUS_BUF_NEXT (2 << M2M_STATUS_BUF_SHIFT) > +#define M2M_STATUS_BUF_MASK (3 << M2M_STATUS_BUF_SHIFT) > +#define M2M_STATUS_DONE BIT(6) > > #define M2M_BCR0 0x0010 > #define M2M_BCR1 0x0014 > @@ -426,15 +442,6 @@ static int m2p_hw_interrupt(struct ep93x > > /* > * M2M DMA implementation > - * > - * For the M2M transfers we don't use NFB at all. This is because it simply > - * doesn't work well with memcpy transfers. When you submit both buffers it is > - * extremely unlikely that you get an NFB interrupt, but it instead reports > - * DONE interrupt and both buffers are already transferred which means that we > - * weren't able to update the next buffer. > - * > - * So for now we "simulate" NFB by just submitting buffer after buffer > - * without double buffering. > */ > > static int m2m_hw_setup(struct ep93xx_dma_chan *edmac) > @@ -543,6 +550,11 @@ static void m2m_hw_submit(struct ep93xx_ > m2m_fill_desc(edmac); > control |= M2M_CONTROL_DONEINT; > > + if (ep93xx_dma_advance_active(edmac)) { > + m2m_fill_desc(edmac); > + control |= M2M_CONTROL_NFBINT; > + } > + > /* > * Now we can finally enable the channel. For M2M channel this must be > * done _after_ the BCRx registers are programmed. > @@ -560,32 +572,89 @@ static void m2m_hw_submit(struct ep93xx_ > } > } > > +/* > + * According to EP93xx User's Guide, we should receive DONE interrupt when all > + * M2M DMA controller transactions complete normally. This is not always the > + * case - sometimes EP93xx M2M DMA asserts DONE interrupt when the DMA channel > + * is still running (channel Buffer FSM in DMA_BUF_ON state, and channel > + * Control FSM in DMA_MEM_RD state, observed at least in IDE-DMA operation). > + * In effect, disabling the channel when only DONE bit is set could stop > + * currently running DMA transfer. To avoid this, we use Buffer FSM and > + * Control FSM to check current state of DMA channel. > + */ > static int m2m_hw_interrupt(struct ep93xx_dma_chan *edmac) > { > + u32 status = readl(edmac->regs + M2M_STATUS); > + u32 ctl_fsm = status & M2M_STATUS_CTL_MASK; > + u32 buf_fsm = status & M2M_STATUS_BUF_MASK; > + bool done = status & M2M_STATUS_DONE; > + bool last_done; > u32 control; > + struct ep93xx_dma_desc *desc; > > - if (!(readl(edmac->regs + M2M_INTERRUPT) & M2M_INTERRUPT_DONEINT)) > + /* Accept only DONE and NFB interrupts */ > + if (!(readl(edmac->regs + M2M_INTERRUPT) & M2M_INTERRUPT_MASK)) > return INTERRUPT_UNKNOWN; > > - /* Clear the DONE bit */ > - writel(0, edmac->regs + M2M_INTERRUPT); > + if (done) { > + /* Clear the DONE bit */ > + writel(0, edmac->regs + M2M_INTERRUPT); > + } > > - /* Disable interrupts and the channel */ > - control = readl(edmac->regs + M2M_CONTROL); > - control &= ~(M2M_CONTROL_DONEINT | M2M_CONTROL_ENABLE); > - writel(control, edmac->regs + M2M_CONTROL); > + /* > + * Check whether we are done with descriptors or not. This, together > + * with DMA channel state, determines action to take in interrupt. > + */ > + desc = ep93xx_dma_get_active(edmac); > + last_done = !desc || desc->txd.cookie; > > /* > - * Since we only get DONE interrupt we have to find out ourselves > - * whether there still is something to process. So we try to advance > - * the chain an see whether it succeeds. > + * Use M2M DMA Buffer FSM and Control FSM to check current state of > + * DMA channel. Using DONE and NFB bits from channel status register > + * or bits from channel interrupt register is not reliable. > */ > - if (ep93xx_dma_advance_active(edmac)) { > - edmac->edma->hw_submit(edmac); > - return INTERRUPT_NEXT_BUFFER; > + if (!last_done && > + (buf_fsm == M2M_STATUS_BUF_NO || > + buf_fsm == M2M_STATUS_BUF_ON)) { > + /* > + * Two buffers are ready for update when Buffer FSM is in > + * DMA_NO_BUF state. Only one buffer can be prepared without > + * disabling the channel or polling the DONE bit. > + * To simplify things, always prepare only one buffer. > + */ > + if (ep93xx_dma_advance_active(edmac)) { > + m2m_fill_desc(edmac); > + if (done && !edmac->chan.private) { > + /* Software trigger for memcpy channel */ > + control = readl(edmac->regs + M2M_CONTROL); > + control |= M2M_CONTROL_START; > + writel(control, edmac->regs + M2M_CONTROL); > + } > + return INTERRUPT_NEXT_BUFFER; > + } else { > + last_done = true; > + } > } > > - return INTERRUPT_DONE; > + /* > + * Disable the channel only when Buffer FSM is in DMA_NO_BUF state > + * and Control FSM is in DMA_STALL state. > + */ > + if (last_done && > + buf_fsm == M2M_STATUS_BUF_NO && > + ctl_fsm == M2M_STATUS_CTL_STALL) { > + /* Disable interrupts and the channel */ > + control = readl(edmac->regs + M2M_CONTROL); > + control &= ~(M2M_CONTROL_DONEINT | M2M_CONTROL_NFBINT > + | M2M_CONTROL_ENABLE); > + writel(control, edmac->regs + M2M_CONTROL); > + return INTERRUPT_DONE; > + } > + > + /* > + * Nothing to do this time. > + */ > + return INTERRUPT_NEXT_BUFFER; > } > > /*
| |