ppc4xx DMA fixes for simultaneous sg transfers
Peter Fercher
peter.fercher at scs.ch
Tue Jan 17 03:33:52 EST 2006
can somebody provide an example usage of the ppc4xx_ scatter/gather dma
library by mvista ?
-----Original Message-----
From: linuxppc-embedded-bounces at ozlabs.org
[mailto:linuxppc-embedded-bounces at ozlabs.org] On Behalf Of ductusrhe
Sent: Mittwoch, 7. Dezember 2005 22:33
To: linuxppc-embedded at ozlabs.org
Subject: ppc4xx DMA fixes for simultaneous sg transfers
Linux 2.4.20 (probably 2.6 as well?)
Conclusion:
Scatter/gather DMA is not thread safe.
Background:
1. We run all four dma channels simultaneously in SG mode on the PPC440EP,
starting and stopping them in different threads.
2. Also we need to change channel configs between different transfers, i.e.
run ppc4xx_init_dma_channel() to set read or write mode.
Problem and cause:
1. All is well when running one channel at a time, but when
starting/stopping a new channel during the time another is active can - if
you are unlucky - cause problems.
Because all channels SG start/stop bits are in the same register (ASGC),
it must not be read then changed and written to the way it was done.
That can cause a channel that is just done, to start again, or a newly
started channel to stop.
The register includes a feature that can be used as a semaphore - the
read enable (mask) bits, but it was not used correctly in the code, it was
enabled for all channels in the start-up.
2. It is said in the comment of ppc4xx_init_dma_channel() that it should
only be used once in the start-up. If you comply, you will not have any
problem with it.
However, when running this for a channel when other channels are active,
can cause channels to stop or maybe never give an interrupt or false
interrupts. The method clears the entire status register, not only the bits
for the given channel.
Solution:
1. - In ppc4xx_alloc_dma_handle(), do not touch the ASGC register!
- In ppc4xx_enable_dma_sgl() and ppc4xx_disable_dma_sgl(), when setting
the ASGC register, only change the given channel.
That's done without reading the register at all, just set/clear the
enable bit of the channel to change, then set the MASK_ENABLE for the same
channel.
Probably this is the way the register was intended to be handled?
2. - In ppc4xx_init_dma_channel(), only clear the correct bits of the DMASR
register, not the whole register.
(The polarity was also not set as it's supposed to in this function,
but there exists a patch for that)
I can send a patch with our changes, that works very well and stable, but
there are many changes and not all of them in line with the current official
version of the files (we handle the sg descriptor list differently). Maybe
someone feeling for it will take a look at the changes and make them into
the real code, since the above fixes does not interfere with the usage, only
improves thread safety.
I have no idea if this has been fixed in some patch already... but I have
not seen it on this list anyway.
I'm not a regular poster, just want to help others avoid some of the
struggle when running many channels.
/Ronnie Hedlund
Our changed code:
In "ppc4xx_dma.c"
-----------------------
/*
* The comment states that this function should only be run at start-up,
and never more.
* That is unacceptable, with the fix it can be run anywhere as long as the
given channel is not running.
*/
int ppc4xx_init_dma_channel(unsigned int dmanr, ppc_dma_ch_t * p_init)
{
unsigned int status_bits[] = { DMA_CS0 | DMA_TS0 | DMA_CH0_ERR,
DMA_CS1 | DMA_TS1 | DMA_CH1_ERR,
DMA_CS2 | DMA_TS2 | DMA_CH2_ERR,
DMA_CS3 | DMA_TS3 | DMA_CH3_ERR};
unsigned int polarity;
uint32_t control = 0;
ppc_dma_ch_t *p_dma_ch = &dma_channels[dmanr];
DMA_MODE_READ = (unsigned long) DMA_TD; /* Peripheral to Memory */
DMA_MODE_WRITE = 0; /* Memory to Peripheral */
if (!p_init) {
printk("ppc4xx_init_dma_channel: NULL p_init\n");
return DMA_STATUS_NULL_POINTER;
}
if (dmanr >= MAX_PPC4xx_DMA_CHANNELS) {
printk("ppc4xx_init_dma_channel: bad channel %d\n", dmanr);
return DMA_STATUS_BAD_CHANNEL;
}
#if DCRN_POL > 0
polarity = mfdcr(DCRN_POL);
#else
polarity = 0;
#endif
/* Setup the control register based on the values passed to
* us in p_init. Then, over-write the control register with this
* new value.
*/
control |= SET_DMA_CONTROL;
switch (dmanr) {
case 0:
/* clear all polarity signals and then "or" in new signal
levels */
polarity &= ~GET_DMA_POLARITY(0);
polarity |= p_init->polarity;
#if DCRN_POL > 0
mtdcr(DCRN_POL, polarity);
#endif
mtdcr(DCRN_DMACR0, control);
break;
case 1:
polarity &= ~GET_DMA_POLARITY(1);
polarity |= p_init->polarity;
#if DCRN_POL > 0
mtdcr(DCRN_POL, polarity);
#endif
mtdcr(DCRN_DMACR1, control);
break;
case 2:
polarity &= ~GET_DMA_POLARITY(2);
polarity |= p_init->polarity;
#if DCRN_POL > 0
mtdcr(DCRN_POL, polarity);
#endif
mtdcr(DCRN_DMACR2, control);
break;
case 3:
polarity &= ~GET_DMA_POLARITY(3);
polarity |= p_init->polarity;
#if DCRN_POL > 0
mtdcr(DCRN_POL, polarity);
#endif
mtdcr(DCRN_DMACR3, control);
break;
default:
return DMA_STATUS_BAD_CHANNEL;
}
/* save these values in our dma channel structure */
memcpy(p_dma_ch, p_init, sizeof (ppc_dma_ch_t));
/*
* The peripheral width values written in the control register are:
* PW_8 0
* PW_16 1
* PW_32 2
* PW_64 3
*
* Since the DMA count register takes the number of "transfers",
* we need to divide the count sent to us in certain
* functions by the appropriate number. It so happens that our
* right shift value is equal to the peripheral width value.
*/
p_dma_ch->shift = p_init->pwidth;
/*
* Save the control word for easy access.
*/
p_dma_ch->control = control;
/*
* clear status register for the channel
* only TS, CS and RI needs to be cleared.
*/
mtdcr(DCRN_DMASR, status_bits[dmanr]);
return DMA_STATUS_GOOD;
}
In "ppc4xx_sgdma.c"
-----------------------
int
ppc4xx_alloc_dma_handle(sgl_handle_t * phandle, unsigned int mode, unsigned
int dmanr)
{
sgl_list_info_t *psgl = NULL;
ppc_dma_ch_t *p_dma_ch = &dma_channels[dmanr];
//uint32_t sg_command;
if (dmanr >= MAX_PPC4xx_DMA_CHANNELS) {
printk("ppc4xx_alloc_dma_handle: invalid channel 0x%x\n",
dmanr);
return DMA_STATUS_BAD_CHANNEL;
}
if (!phandle) {
printk("ppc4xx_alloc_dma_handle: null handle pointer\n");
return DMA_STATUS_NULL_POINTER;
}
/* Get memory for the listinfo struct */
psgl = kmalloc(sizeof(sgl_list_info_t), GFP_KERNEL);
if (psgl == NULL) {
*phandle = (sgl_handle_t) NULL;
return DMA_STATUS_OUT_OF_MEMORY;
}
memset(psgl, 0, sizeof(sgl_list_info_t));
/* dma_addr is unused now */
psgl->dmanr = dmanr;
/*
* Modify and save the control word. These words will be
* written to each sgl descriptor. The DMA engine then
* loads this control word into the control register
* every time it reads a new descriptor.
*/
psgl->control = p_dma_ch->control;
/* Clear all mode bits */
psgl->control &= ~(DMA_TM_MASK | DMA_TD);
/* Save control word and mode */
psgl->control |= (mode | DMA_CE_ENABLE);
/* PPC Errata? DMA else ignore count on first in list */
psgl->control |= SET_DMA_TCE(1);
/* In MM mode, we must set ETD/TCE */
if (mode == DMA_MODE_MM)
psgl->control |= DMA_ETD_OUTPUT | DMA_TCE_ENABLE;
if (p_dma_ch->int_enable) {
/* Enable channel interrupt */
psgl->control |= DMA_CIE_ENABLE;
} else {
psgl->control &= ~DMA_CIE_ENABLE;
}
/* we must not touch the SGC, it can cause problems to other
channels! */
psgl->sgl_control = SG_LINK;
if (p_dma_ch->int_enable) {
if (p_dma_ch->tce_enable)
{
/* reuse as Terminal Count Interrupt Enable on all
descr. */
psgl->sgl_control |= SG_TCI_ENABLE;
}
psgl->sgl_control |= SG_ERI_ENABLE | SG_ETI_ENABLE;
}
*phandle = (sgl_handle_t) psgl;
return DMA_STATUS_GOOD;
}
void
ppc4xx_enable_dma_sgl(sgl_handle_t handle)
{
sgl_list_info_t *psgl = (sgl_list_info_t *) handle;
ppc_dma_ch_t *p_dma_ch;
uint32_t sg_command;
if (!handle) {
printk("ppc4xx_enable_dma_sgl: null handle\n");
return;
} else if (psgl->dmanr > (MAX_PPC4xx_DMA_CHANNELS - 1)) {
printk("ppc4xx_enable_dma_sgl: bad channel in handle %d\n",
psgl->dmanr);
return;
} else if (!psgl->phead) {
printk("ppc4xx_enable_dma_sgl: sg list empty\n");
return;
}
p_dma_ch = &dma_channels[psgl->dmanr];
psgl->ptail->control_count &= ~SG_LINK; /* make this the last
dscrptr */
if (p_dma_ch->int_enable)
{
/* Require Terminal Count interrupt on last */
psgl->ptail->control_count |= SG_TCI_ENABLE;
}
/* No more changes to tail object allowed */
//dma_cache_wback((unsigned long)psgl->ptail, sizeof(ppc_sgl_t));
dma_cache_wback_inv((unsigned long)psgl->ptail, sizeof(ppc_sgl_t));
ppc4xx_set_sg_addr(psgl->dmanr, virt_to_phys(psgl->phead));
sg_command = 0;
switch (psgl->dmanr) {
case 0:
sg_command = SSG0_ENABLE | SSG0_MASK_ENABLE;
break;
case 1:
sg_command = SSG1_ENABLE | SSG1_MASK_ENABLE;
break;
case 2:
sg_command = SSG2_ENABLE | SSG2_MASK_ENABLE;
break;
case 3:
sg_command = SSG3_ENABLE | SSG3_MASK_ENABLE;
break;
default:
printk("ppc4xx_enable_dma_sgl: bad channel: %d\n",
psgl->dmanr);
}
mtdcr(DCRN_ASGC, sg_command); /* start transfer */
}
void
ppc4xx_disable_dma_sgl(sgl_handle_t handle)
{
sgl_list_info_t *psgl = (sgl_list_info_t *) handle;
uint32_t sg_command;
if (!handle) {
printk("ppc4xx_disable_dma_sgl: null handle\n");
return;
} else if (psgl->dmanr > (MAX_PPC4xx_DMA_CHANNELS - 1)) {
printk("ppc4xx_disable_dma_sgl: bad channel in handle %d\n",
psgl->dmanr);
return;
}
sg_command = 0; //disable dma
switch (psgl->dmanr) {
case 0:
sg_command = SSG0_MASK_ENABLE;
break;
case 1:
sg_command = SSG1_MASK_ENABLE;
break;
case 2:
sg_command = SSG2_MASK_ENABLE;
break;
case 3:
sg_command = SSG3_MASK_ENABLE;
break;
default:
printk("ppc4xx_disable_dma_sgl: bad channel: %d\n",
psgl->dmanr);
}
mtdcr(DCRN_ASGC, sg_command); /* stop transfer */
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://ozlabs.org/pipermail/linuxppc-embedded/attachments/20060116/7ec47150/attachment.htm
More information about the Linuxppc-embedded
mailing list