On Fri, Jun 08, 2018 at 09:44:45PM +0800, Robin Gong wrote:
> The legacy sdma driver has below limitations or drawbacks:
> 1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
> one page size for one channel regardless of only few BDs needed
> most time. But in few cases, the max PAGE_SIZE maybe not enough.
> 2. One SDMA channel can't stop immediatley once channel disabled which
> means SDMA interrupt may come in after this channel terminated.There
> are some patches for this corner case such as commit "2746e2c389f9",
> but not cover non-cyclic.
>
> The common virt-dma overcomes the above limitations. It can alloc bd
> dynamically and free bd once this tx transfer done. No memory wasted or
> maximum limititation here, only depends on how many memory can be requested
> from kernel. For No.2, such issue can be workaround by checking if there
> is available descript("sdmac->desc") now once the unwanted interrupt
> coming. At last the common virt-dma is easier for sdma driver maintain.
>
> Change from v1:
> 1. split v1 patch into 5 patches.
> 2. remove some unnecessary condition check.
> 3. remove unneccessary 'pending' list.
>
> Robin Gong (5):
> dmaengine: imx-sdma: add virt-dma support
> Revert "dmaengine: imx-sdma: fix pagefault when channel is disabled
> during interrupt"
> dmaengine: imx-sdma: remove usless lock
> dmaengine: imx-sdma: remove the maximum limation for bd numbers
> dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
>
> drivers/dma/Kconfig | 1 +
> drivers/dma/imx-sdma.c | 392 ++++++++++++++++++++++++++++---------------------
> 2 files changed, 227 insertions(+), 166 deletions(-)
Please put the attached patch in front of your series. It makes the
virt-dma support patch smaller and thus easier to review.
Sascha
--------------------------------8<----------------------------------
>From a70ccdf780cc6fcddd2d06c4a3eb0123d4aba443 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
Date: Fri, 8 Jun 2018 10:20:18 +0200
Subject: [PATCH 1/2] dmaengine: imx-sdma: factor out a struct sdma_desc from
struct sdma_channel
This is a preparation step to make the adding of virt-dma easier.
We create a struct sdma_desc, move some fields from struct sdma_channel
there and add a pointer from the former to the latter. For now we
allocate the data statically in struct sdma_channel, but with
virt-dma support it will be dynamically allocated.
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
drivers/dma/imx-sdma.c | 137 +++++++++++++++++++++++++----------------
1 file changed, 83 insertions(+), 54 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ccd03c3cedfe..556d08712f4a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -295,6 +295,30 @@ struct sdma_context_data {
struct sdma_engine;
+/**
+ * struct sdma_desc - descriptor structor for one transfer
+ * @vd descriptor for virt dma
+ * @num_bd max NUM_BD. number of descriptors currently handling
+ * @buf_tail ID of the buffer that was processed
+ * @buf_ptail ID of the previous buffer that was processed
+ * @period_len period length, used in cyclic.
+ * @chn_real_count the real count updated from bd->mode.count
+ * @chn_count the transfer count setuped
+ * @sdmac sdma_channel pointer
+ * @bd pointer of alloced bd
+ */
+struct sdma_desc {
+ unsigned int num_bd;
+ dma_addr_t bd_phys;
+ unsigned int buf_tail;
+ unsigned int buf_ptail;
+ unsigned int period_len;
+ unsigned int chn_real_count;
+ unsigned int chn_count;
+ struct sdma_channel *sdmac;
+ struct sdma_buffer_descriptor *bd;
+};
+
/**
* struct sdma_channel - housekeeping for a SDMA channel
*
@@ -305,11 +329,10 @@ struct sdma_engine;
* @event_id0 aka dma request line
* @event_id1 for channels that use 2 events
* @word_size peripheral access size
- * @buf_tail ID of the buffer that was processed
- * @buf_ptail ID of the previous buffer that was processed
- * @num_bd max NUM_BD. number of descriptors currently handling
*/
struct sdma_channel {
+ struct sdma_desc *desc;
+ struct sdma_desc _desc;
struct sdma_engine *sdma;
unsigned int channel;
enum dma_transfer_direction direction;
@@ -317,12 +340,6 @@ struct sdma_channel {
unsigned int event_id0;
unsigned int event_id1;
enum dma_slave_buswidth word_size;
- unsigned int buf_tail;
- unsigned int buf_ptail;
- unsigned int num_bd;
- unsigned int period_len;
- struct sdma_buffer_descriptor *bd;
- dma_addr_t bd_phys;
unsigned int pc_from_device, pc_to_device;
unsigned int device_to_device;
unsigned long flags;
@@ -332,10 +349,8 @@ struct sdma_channel {
u32 shp_addr, per_addr;
struct dma_chan chan;
spinlock_t lock;
- struct dma_async_tx_descriptor desc;
+ struct dma_async_tx_descriptor txdesc;
enum dma_status status;
- unsigned int chn_count;
- unsigned int chn_real_count;
struct tasklet_struct tasklet;
struct imx_dma_data data;
bool enabled;
@@ -398,6 +413,8 @@ struct sdma_engine {
u32 spba_start_addr;
u32 spba_end_addr;
unsigned int irq;
+ dma_addr_t bd0_phys;
+ struct sdma_buffer_descriptor *bd0;
};
static struct sdma_driver_data sdma_imx31 = {
@@ -632,7 +649,7 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
u32 address)
{
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
void *buf_virt;
dma_addr_t buf_phys;
int ret;
@@ -707,7 +724,9 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* call callback function.
*/
while (1) {
- bd = &sdmac->bd[sdmac->buf_tail];
+ struct sdma_desc *desc = sdmac->desc;
+
+ bd = &desc->bd[desc->buf_tail];
if (bd->mode.status & BD_DONE)
break;
@@ -723,11 +742,11 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* the number of bytes present in the current buffer descriptor.
*/
- sdmac->chn_real_count = bd->mode.count;
+ desc->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
- bd->mode.count = sdmac->period_len;
- sdmac->buf_ptail = sdmac->buf_tail;
- sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
+ bd->mode.count = desc->period_len;
+ desc->buf_ptail = desc->buf_tail;
+ desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
/*
* The callback is called from the interrupt context in order
@@ -736,7 +755,7 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* executed.
*/
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
+ dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
if (error)
sdmac->status = old_status;
@@ -749,17 +768,17 @@ static void mxc_sdma_handle_channel_normal(unsigned long data)
struct sdma_buffer_descriptor *bd;
int i, error = 0;
- sdmac->chn_real_count = 0;
+ sdmac->desc->chn_real_count = 0;
/*
* non loop mode. Iterate over all descriptors, collect
* errors and call callback function
*/
- for (i = 0; i < sdmac->num_bd; i++) {
- bd = &sdmac->bd[i];
+ for (i = 0; i < sdmac->desc->num_bd; i++) {
+ bd = &sdmac->desc->bd[i];
if (bd->mode.status & (BD_DONE | BD_RROR))
error = -EIO;
- sdmac->chn_real_count += bd->mode.count;
+ sdmac->desc->chn_real_count += bd->mode.count;
}
if (error)
@@ -767,9 +786,9 @@ static void mxc_sdma_handle_channel_normal(unsigned long data)
else
sdmac->status = DMA_COMPLETE;
- dma_cookie_complete(&sdmac->desc);
+ dma_cookie_complete(&sdmac->txdesc);
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
+ dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
}
static irqreturn_t sdma_int_handler(int irq, void *dev_id)
@@ -897,7 +916,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
int channel = sdmac->channel;
int load_address;
struct sdma_context_data *context = sdma->context;
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
int ret;
unsigned long flags;
@@ -1100,18 +1119,22 @@ static int sdma_set_channel_priority(struct sdma_channel *sdmac,
static int sdma_request_channel(struct sdma_channel *sdmac)
{
struct sdma_engine *sdma = sdmac->sdma;
+ struct sdma_desc *desc;
int channel = sdmac->channel;
int ret = -EBUSY;
- sdmac->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &sdmac->bd_phys,
+ sdmac->desc = &sdmac->_desc;
+ desc = sdmac->desc;
+
+ desc->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &desc->bd_phys,
GFP_KERNEL);
- if (!sdmac->bd) {
+ if (!desc->bd) {
ret = -ENOMEM;
goto out;
}
- sdma->channel_control[channel].base_bd_ptr = sdmac->bd_phys;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
sdma_set_channel_priority(sdmac, MXC_SDMA_DEFAULT_PRIORITY);
return 0;
@@ -1176,10 +1199,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ahb;
- dma_async_tx_descriptor_init(&sdmac->desc, chan);
- sdmac->desc.tx_submit = sdma_tx_submit;
+ dma_async_tx_descriptor_init(&sdmac->txdesc, chan);
+ sdmac->txdesc.tx_submit = sdma_tx_submit;
/* txd.flags will be overwritten in prep funcs */
- sdmac->desc.flags = DMA_CTRL_ACK;
+ sdmac->txdesc.flags = DMA_CTRL_ACK;
return 0;
@@ -1194,6 +1217,7 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
+ struct sdma_desc *desc = sdmac->desc;
sdma_disable_channel(chan);
@@ -1207,7 +1231,7 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
sdma_set_channel_priority(sdmac, 0);
- dma_free_coherent(NULL, PAGE_SIZE, sdmac->bd, sdmac->bd_phys);
+ dma_free_coherent(NULL, PAGE_SIZE, desc->bd, desc->bd_phys);
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
@@ -1223,6 +1247,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
int ret, i, count;
int channel = sdmac->channel;
struct scatterlist *sg;
+ struct sdma_desc *desc = sdmac->desc;
if (sdmac->status == DMA_IN_PROGRESS)
return NULL;
@@ -1230,9 +1255,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->chn_real_count = 0;
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
@@ -1249,9 +1274,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
goto err_out;
}
- sdmac->chn_count = 0;
+ desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = sg->dma_address;
@@ -1266,7 +1291,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
}
bd->mode.count = count;
- sdmac->chn_count += count;
+ desc->chn_count += count;
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES) {
ret = -EINVAL;
@@ -1307,10 +1332,10 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
bd->mode.status = param;
}
- sdmac->num_bd = sg_len;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ desc->num_bd = sg_len;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
- return &sdmac->desc;
+ return &sdmac->txdesc;
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1326,6 +1351,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
int ret, i = 0, buf = 0;
+ struct sdma_desc *desc = sdmac->desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1334,10 +1360,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
- sdmac->period_len = period_len;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->chn_real_count = 0;
+ desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
sdmac->direction = direction;
@@ -1358,7 +1384,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
}
while (buf < buf_len) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = dma_addr;
@@ -1389,10 +1415,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
i++;
}
- sdmac->num_bd = num_periods;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ desc->num_bd = num_periods;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
- return &sdmac->desc;
+ return &sdmac->txdesc;
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1431,13 +1457,14 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
struct dma_tx_state *txstate)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_desc *desc = sdmac->desc;
u32 residue;
if (sdmac->flags & IMX_DMA_SG_LOOP)
- residue = (sdmac->num_bd - sdmac->buf_ptail) *
- sdmac->period_len - sdmac->chn_real_count;
+ residue = (desc->num_bd - desc->buf_ptail) *
+ desc->period_len - desc->chn_real_count;
else
- residue = sdmac->chn_count - sdmac->chn_real_count;
+ residue = desc->chn_count - desc->chn_real_count;
dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
residue);
@@ -1661,6 +1688,8 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto err_dma_alloc;
+ sdma->bd0 = sdma->channel[0].desc->bd;
+
sdma_config_ownership(&sdma->channel[0], false, true, false);
/* Set Command Channel (Channel Zero) */
--
2.17.1
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
