| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4F6A78ED.3050403@gmail.com>
Date: Thu, 22 Mar 2012 11:57:17 +1100
From: Ryan Mallon <rmallon@...il.com>
To: Rafal Prylowski <prylowski@...asoft.pl>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"vinod.koul@...el.com" <vinod.koul@...el.com>,
Mika Westerberg <mika.westerberg@....fi>,
H Hartley Sweeten <hartleys@...ionengravers.com>
Subject: Re: [PATCH] ep93xx: Implement double buffering for M2M DMA channels
On 20/03/12 19:09, Rafal Prylowski wrote:
> Implement double buffering for M2M DMA channels.
>
I haven't looked through the patch yet, since I'm waiting on more
information from Mika and Hartley's testing.
However, the commit log doesn't tell me why we want this change. Is it a
performance improvement? If so, do you have some numbers that we can
paste into the commit log?
Thanks,
~Ryan
> Signed-off-by: Rafal Prylowski <prylowski@...asoft.pl>
>
> Index: slave-dma/drivers/dma/ep93xx_dma.c
> ===================================================================
> --- slave-dma.orig/drivers/dma/ep93xx_dma.c
> +++ slave-dma/drivers/dma/ep93xx_dma.c
> @@ -69,6 +69,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)
> @@ -77,7 +78,23 @@
> #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 +443,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 +551,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 +573,85 @@ 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;
> u32 control;
>
> - 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.
> + */
> + last = list_first_entry(edmac->active.next,
> + struct ep93xx_dma_desc, node)->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 was proven not to be
> + * reliable.
> */
> - if (ep93xx_dma_advance_active(edmac)) {
> - edmac->edma->hw_submit(edmac);
> + if (!last &&
> + (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.
> + */
> + 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;
> }
>
> - 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 &&
> + 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;
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists