[PATCH RFC 0/2] dmaengine: Introduce Scheduled DMA sub-framework

[PATCH RFC 0/2] dmaengine: Introduce Scheduled DMA sub-framework

[Maxime Ripard]

Hi everyone,

This patch set aims at introducing a new sub-framework to DMAengine to
ease the dmaengine development, while factorising most of the
"administrative" code each drivers have to duplicate.

Most of the same issues are always seen for new drivers, and are
pointed out at each review. Among the major point of errors, we can
find how to dispatch new descriptors to a new idle channel
efficiently, how to schedule requests when you have more requests than
channels, how to allocate new descriptors, etc.

All of this code is currently duplicated by each and every drivers,
with some variations. Of course, this code is rather error prone, and
most of the drivers will face the same issues.

It's exactly the point of the Scheduled DMA framework to abstract this
away from the drivers, only requiring from the drivers the
hardware-specific behaviour (how to start, stop, pause a channel, how
to start the transfer of a descriptor on a given channel, etc.), LLI
operations (iterators, queuing, accessors, etc.) and an optionnal
scheduling hints function to prevent the core to schedule the transfer
of a descriptor on a channel that wouldn't be able to transfer it.

Please not that this is still at an RFC stage, and that there's still
a few bugs hanging around (resources leaked, the memcpy of the A31 can
only work once, etc.) but this RFC is more headed toward wether we are
on the right path or not, and reviews of the general approach.

Thanks!
Maxime

Maxime Ripard (2):
  dmaengine: Introduce scheduled DMA framework
  dmaengine: sun6i: Convert to scheduled DMA

 drivers/dma/Kconfig         |   4 +
 drivers/dma/Makefile        |   1 +
 drivers/dma/scheduled-dma.c | 571 +++++++++++++++++++++++++++++
 drivers/dma/scheduled-dma.h | 140 ++++++++
 drivers/dma/sun6i-dma.c     | 853 ++++++++++----------------------------------
 5 files changed, 897 insertions(+), 672 deletions(-)
 create mode 100644 drivers/dma/scheduled-dma.c
 create mode 100644 drivers/dma/scheduled-dma.h

-- 
2.3.3

[PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Maxime Ripard]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 19951 bytes --]

This framework aims at easing the development of dmaengine drivers by providing
generic implementations of the functions usually required by dmaengine, while
abstracting away most of the logic required.

It is very relevant for controllers that have more requests than channels,
where you need to have some scheduling that is usually very bug prone, and
shouldn't be implemented in each and every driver.

This introduces a new set of callbacks for drivers to implement the device
specific behaviour. These new sets of callbacks aims at providing both how to
handle channel related operations (start the transfer of a new descriptor,
pause, resume, etc.) and the LLI related operations (iterator and various
accessors).

So far, the only transfer types supported are memcpy and slave transfers, but
eventually should support new transfer types as new drivers get converted.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/Kconfig         |   4 +
 drivers/dma/Makefile        |   1 +
 drivers/dma/scheduled-dma.c | 571 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/dma/scheduled-dma.h | 140 +++++++++++
 4 files changed, 716 insertions(+)
 create mode 100644 drivers/dma/scheduled-dma.c
 create mode 100644 drivers/dma/scheduled-dma.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index a874b6ec6650..032bf5fcd58b 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -406,6 +406,7 @@ config DMA_SUN6I
 	depends on RESET_CONTROLLER
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
+	select DMA_SCHEDULED
 	help
 	  Support for the DMA engine first found in Allwinner A31 SoCs.
 
@@ -431,6 +432,9 @@ config DMA_ENGINE
 config DMA_VIRTUAL_CHANNELS
 	tristate
 
+config DMA_SCHEDULED
+	bool
+
 config DMA_ACPI
 	def_bool y
 	depends on ACPI
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index f915f61ec574..1db31814c749 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
 obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
 obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
 obj-$(CONFIG_DMA_OF) += of-dma.o
+obj-$(CONFIG_DMA_SCHEDULED) += scheduled-dma.o
 
 obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
 obj-$(CONFIG_DMATEST) += dmatest.o
diff --git a/drivers/dma/scheduled-dma.c b/drivers/dma/scheduled-dma.c
new file mode 100644
index 000000000000..482d04f2ccbc
--- /dev/null
+++ b/drivers/dma/scheduled-dma.c
@@ -0,0 +1,571 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/dmaengine.h>
+#include <linux/dmapool.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include "scheduled-dma.h"
+#include "virt-dma.h"
+
+static struct sdma_request *sdma_pop_queued_transfer(struct sdma *sdma,
+						     struct sdma_channel *schan)
+{
+	struct sdma_request *sreq = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+
+	/* No requests are awaiting an available channel */
+	if (list_empty(&sdma->pend_reqs))
+		goto out;
+
+	/*
+	 * If we don't have any validate_request callback, any request
+	 * can be dispatched to any channel.
+	 *
+	 * Remove the first entry and return it.
+	 */
+	if (!sdma->ops->validate_request) {
+		sreq = list_first_entry(&sdma->pend_reqs,
+					struct sdma_request, node);
+		list_del_init(&sreq->node);
+		goto out;
+	}
+
+	list_for_each_entry(sreq, &sdma->pend_reqs, node) {
+		/*
+		 * Ask the driver to validate that the request can
+		 * happen on the channel.
+		 */
+		if (sdma->ops->validate_request(schan, sreq)) {
+			list_del_init(&sreq->node);
+			goto out;
+		}
+
+		/* Otherwise, just keep looping */
+	}
+
+out:
+	spin_unlock_irqrestore(&sdma->lock, flags);
+
+	return sreq;
+}
+
+/*
+ * Besoin d'une fonction pour pusher un descriptor vers un pchan
+ *
+ * Flow normal:
+ *   - Election d'un pchan (Framework)
+ *   - Push d'un descripteur vers le pchan (Driver)
+ *   - idle....
+ *   - interrupt (driver)
+ *   - Transfert terminé, notification vers le framework (driver)
+ *   - Nouveau transfert dans la queue?
+ *     + Si oui, on est reparti
+ *     + Si non, on sort de l'interrupt, le pchan est libéré
+ */
+
+struct sdma_desc *sdma_report(struct sdma *sdma,
+			      struct sdma_channel *schan,
+			      enum sdma_report_status status)
+{
+	struct sdma_desc *sdesc = NULL;
+	struct virt_dma_desc *vdesc;
+	struct sdma_request *sreq;
+
+	switch (status) {
+	case SDMA_REPORT_TRANSFER:
+		/*
+		 * We're done with the current transfer that was in this
+		 * physical channel.
+		 */
+		vchan_cookie_complete(&schan->desc->vdesc);
+
+		/*
+		 * Now, try to see if there's any queued transfer
+		 * awaiting an available channel.
+		 *
+		 * If not, just bail out, and mark the pchan as
+		 * available.
+		 *
+		 * If there's such a transfer, validate that the
+		 * driver can handle it, and ask it to do the
+		 * transfer.
+		 */
+		sreq = sdma_pop_queued_transfer(sdma, schan);
+		if (!sreq) {
+			list_add_tail(&schan->node, &sdma->avail_chans);
+			return NULL;
+		}
+
+		/* Mark the request as assigned to a particular channel */
+		sreq->chan = schan;
+
+		/* Retrieve the next transfer descriptor */
+		vdesc = vchan_next_desc(&sreq->vchan);
+		schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
+
+		break;
+
+	default:
+		break;
+	}
+
+	return sdesc;
+}
+EXPORT_SYMBOL_GPL(sdma_report);
+
+static enum dma_status sdma_tx_status(struct dma_chan *chan,
+				      dma_cookie_t cookie,
+				      struct dma_tx_state *state)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	struct sdma *sdma = to_sdma(chan->device);
+	struct sdma_channel *schan = sreq->chan;
+	struct virt_dma_desc *vd;
+	struct sdma_desc *desc;
+	enum dma_status ret;
+	unsigned long flags;
+	size_t bytes = 0;
+	void *lli;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+
+	ret = dma_cookie_status(chan, cookie, state);
+	if (ret == DMA_COMPLETE)
+		goto out;
+
+	vd = vchan_find_desc(&sreq->vchan, cookie);
+	desc = to_sdma_desc(&vd->tx);
+
+	if (vd) {
+		lli = desc->v_lli;
+		while (true) {
+			bytes += sdma->ops->lli_size(lli);
+
+			if (!sdma->ops->lli_has_next(lli))
+				break;
+
+			lli = sdma->ops->lli_next(lli);
+		}
+	} else if (chan) {
+		bytes = sdma->ops->channel_residue(schan);
+	}
+
+	dma_set_residue(state, bytes);
+
+out:
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	return ret;
+};
+
+static int sdma_config(struct dma_chan *chan,
+		       struct dma_slave_config *config)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+	memcpy(&sreq->cfg, config, sizeof(*config));
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	return 0;
+}
+
+static int sdma_pause(struct dma_chan *chan)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	struct sdma_channel *schan = sreq->chan;
+	struct sdma *sdma = to_sdma(chan->device);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+
+	/*
+	 * If the request is currently scheduled on a channel, just
+	 * pause the channel.
+	 *
+	 * If not, remove the request from the pending list.
+	 */
+	if (schan) {
+		sdma->ops->channel_pause(schan);
+	} else {
+		spin_lock(&sdma->lock);
+		list_del_init(&sreq->node);
+		spin_unlock(&sdma->lock);
+	}
+
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	return 0;
+}
+
+static int sdma_resume(struct dma_chan *chan)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	struct sdma_channel *schan = sreq->chan;
+	struct sdma *sdma = to_sdma(chan->device);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+
+	/*
+	 * If the request is currently scheduled on a channel, just
+	 * resume the channel.
+	 *
+	 * If not, add the request from the pending list.
+	 */
+	if (schan) {
+		sdma->ops->channel_resume(schan);
+	} else {
+		spin_lock(&sdma->lock);
+		list_add_tail(&sreq->node, &sdma->pend_reqs);
+		spin_unlock(&sdma->lock);
+	}
+
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	return 0;
+}
+
+static int sdma_terminate(struct dma_chan *chan)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	struct sdma_channel *schan = sreq->chan;
+	struct sdma *sdma = to_sdma(chan->device);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+
+	/*
+	 * If the request is currently scheduled on a channel,
+	 * terminate the channel.
+	 *
+	 * If not, prevent the request from being scheduled.
+	 */
+	if (schan) {
+		sdma->ops->channel_terminate(schan);
+	} else {
+		spin_lock(&sdma->lock);
+		list_del_init(&sreq->node);
+		spin_unlock(&sdma->lock);
+	}
+
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	/*
+	 * Flush all the pending descriptors from our vchan
+	 */
+	vchan_free_chan_resources(&sreq->vchan);
+
+	return 0;
+}
+
+
+static struct dma_async_tx_descriptor *sdma_prep_memcpy(struct dma_chan *chan,
+							dma_addr_t dest, dma_addr_t src,
+							size_t len, unsigned long flags)
+{
+	struct sdma_request *req = to_sdma_request(chan);
+	struct sdma *sdma = to_sdma(chan->device);
+	struct sdma_desc *desc;
+	dma_addr_t p_lli;
+	void *v_lli;
+
+	if (!len)
+		return NULL;
+
+	/* Allocate our representation of a descriptor */
+	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
+	if (!desc)
+		return NULL;
+
+	v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
+	if (!v_lli)
+		goto err_desc_free;
+
+	/* Ask the driver to initialise its hardware descriptor */
+	if (sdma->ops->lli_init(v_lli, req->private,
+				SDMA_TRANSFER_MEMCPY,
+				DMA_MEM_TO_MEM, src, dest, len,
+				NULL))
+		goto err_lli_free;
+
+	/* Create our single item LLI */
+	sdma->ops->lli_queue(NULL, v_lli, p_lli);
+	desc->p_lli = p_lli;
+	desc->v_lli = v_lli;
+
+	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
+
+err_lli_free:
+	dma_pool_free(sdma->pool, v_lli, p_lli);
+err_desc_free:
+	kfree(desc);
+
+	return NULL;
+}
+
+static struct dma_async_tx_descriptor *sdma_prep_slave_sg(struct dma_chan *chan,
+							  struct scatterlist *sgl,
+							  unsigned int sg_len,
+							  enum dma_transfer_direction dir,
+							  unsigned long flags, void *context)
+{
+	struct sdma_request *req = to_sdma_request(chan);
+	struct dma_slave_config *config = &req->cfg;
+	struct sdma *sdma = to_sdma(chan->device);
+	void *v_lli, *prev_v_lli = NULL;
+	struct scatterlist *sg;
+	struct sdma_desc *desc;
+	dma_addr_t p_lli;
+	int i;
+
+	if (!sgl || !sg_len)
+		return NULL;
+
+	/* Allocate our representation of a descriptor */
+	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
+	if (!desc)
+		return NULL;
+
+	/*
+	 * For each scatter list entry, build up our representation of
+	 * the LLI, and ask the driver to create its hardware
+	 * descriptor.
+	 */
+	for_each_sg(sgl, sg, sg_len, i) {
+		v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
+		if (!v_lli)
+			goto err_lli_free;
+
+		/* Ask the driver to initialise its hardware descriptor */
+		if (sdma->ops->lli_init(v_lli, req->private,
+					SDMA_TRANSFER_SLAVE,
+					dir, sg_dma_address(sg),
+					config->dst_addr, sg_dma_len(sg),
+					config))
+			goto err_lli_free;
+
+		/*
+		 * If it's our first item, initialise our descriptor
+		 * pointers to the lli.
+		 *
+		 * Otherwise, queue it to the end of the LLI.
+		 */
+		if (!prev_v_lli) {
+			desc->p_lli = p_lli;
+			desc->v_lli = v_lli;
+			prev_v_lli = v_lli;
+		} else {
+			/* And to queue it at the end of its hardware LLI */
+			prev_v_lli = sdma->ops->lli_queue(prev_v_lli, v_lli, p_lli);
+		}
+	}
+
+	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
+
+err_lli_free:
+#warning "Free the LLI"
+
+	kfree(desc);
+	return NULL;
+}
+
+static void sdma_issue_pending(struct dma_chan *chan)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	struct sdma *sdma = to_sdma(chan->device);
+	struct virt_dma_desc *vdesc;
+	struct sdma_channel *schan;
+	struct sdma_desc *sdesc;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+
+	/* See if we have anything to do */
+	if (!vchan_issue_pending(&sreq->vchan))
+		goto out_chan_unlock;
+
+	/* Is some work in progress already? */
+	if (sreq->chan)
+		goto out_chan_unlock;
+
+	spin_lock(&sdma->lock);
+
+	/* Is there an available channel */
+	if (list_empty(&sdma->avail_chans))
+		goto out_main_unlock;
+
+	/*
+	 * If there's no validate_request callback, it means that all
+	 * channels can transfer any request. Pick the first available
+	 * channel.
+	 *
+	 * Otherwise, iterate over all the pending channels and call
+	 * validate_request.
+	 */
+	if (!sdma->ops->validate_request) {
+		schan = list_first_entry(&sdma->avail_chans,
+					 struct sdma_channel, node);
+	} else {
+		list_for_each_entry(schan, &sdma->avail_chans, node) {
+			if (sdma->ops->validate_request(schan, sreq)) {
+				list_del_init(&schan->node);
+				break;
+			}
+		}
+	}
+
+	if (!schan)
+		goto out_main_unlock;
+
+	sreq->chan = schan;
+
+	/* Retrieve the next transfer descriptor */
+	vdesc = vchan_next_desc(&sreq->vchan);
+	schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
+
+	sdma->ops->channel_start(schan, sdesc);
+
+out_main_unlock:
+	spin_unlock(&sdma->lock);
+out_chan_unlock:
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+}
+
+static void sdma_free_chan_resources(struct dma_chan *chan)
+{
+	struct sdma_request *sreq = to_sdma_request(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sreq->vchan.lock, flags);
+	list_del_init(&sreq->node);
+	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
+
+	vchan_free_chan_resources(&sreq->vchan);
+}
+
+static void sdma_free_desc(struct virt_dma_desc *vdesc)
+{
+#warning "Free the descriptors"
+}
+
+struct sdma *sdma_alloc(struct device *dev,
+			unsigned int channels,
+			unsigned int requests,
+			ssize_t lli_size,
+			ssize_t priv_size)
+{
+	struct sdma *sdma;
+	int ret, i;
+
+	sdma = devm_kzalloc(dev, sizeof(*sdma) + priv_size, GFP_KERNEL);
+	if (!sdma) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	INIT_LIST_HEAD(&sdma->pend_reqs);
+	INIT_LIST_HEAD(&sdma->avail_chans);
+	spin_lock_init(&sdma->lock);
+
+	sdma->pool = dmam_pool_create(dev_name(dev), dev, lli_size, 4, 0);
+	if (!sdma->pool) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	sdma->channels_nr = channels;
+	sdma->channels = devm_kcalloc(dev, channels, sizeof(*sdma->channels), GFP_KERNEL);
+	if (!sdma->channels) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	for (i = 0; i < channels; i++) {
+		struct sdma_channel *schan = &sdma->channels[i];
+
+		list_add_tail(&schan->node, &sdma->avail_chans);
+		schan->index = i;
+	}
+
+	sdma->requests_nr = requests;
+	sdma->requests = devm_kcalloc(dev, requests, sizeof(*sdma->requests), GFP_KERNEL);
+	if (!sdma->channels) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	INIT_LIST_HEAD(&sdma->ddev.channels);
+
+	for (i = 0; i < requests; i++) {
+		struct sdma_request *sreq = &sdma->requests[i];
+
+		INIT_LIST_HEAD(&sreq->node);
+		sreq->vchan.desc_free = sdma_free_desc;
+		vchan_init(&sreq->vchan, &sdma->ddev);
+	}
+
+	return sdma;
+
+out:
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(sdma_alloc);
+
+void sdma_free(struct sdma *sdma)
+{
+	return;
+}
+EXPORT_SYMBOL(sdma_free);
+
+int sdma_register(struct sdma *sdma,
+		  struct sdma_ops *ops)
+{
+	struct dma_device *ddev = &sdma->ddev;
+
+	sdma->ops = ops;
+
+	ddev->device_config = sdma_config;
+	ddev->device_tx_status = sdma_tx_status;
+	ddev->device_issue_pending = sdma_issue_pending;
+	ddev->device_free_chan_resources = sdma_free_chan_resources;
+
+	if (ops->channel_pause)
+		ddev->device_pause = sdma_pause;
+
+	if (ops->channel_resume)
+		ddev->device_resume = sdma_resume;
+
+	if (ops->channel_terminate)
+		ddev->device_terminate_all = sdma_terminate;
+
+	if (dma_has_cap(DMA_SLAVE, ddev->cap_mask))
+		ddev->device_prep_slave_sg = sdma_prep_slave_sg;
+
+	if (dma_has_cap(DMA_MEMCPY, ddev->cap_mask))
+		ddev->device_prep_dma_memcpy = sdma_prep_memcpy;
+
+	dma_async_device_register(ddev);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdma_register);
+
+int sdma_unregister(struct sdma *sdma)
+{
+	dma_async_device_unregister(&sdma->ddev);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdma_unregister);
diff --git a/drivers/dma/scheduled-dma.h b/drivers/dma/scheduled-dma.h
new file mode 100644
index 000000000000..d24c8143b2b6
--- /dev/null
+++ b/drivers/dma/scheduled-dma.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "virt-dma.h"
+
+#ifndef _SCHEDULED_DMA_H_
+#define _SCHEDULED_DMA_H_
+
+enum sdma_transfer_type {
+	SDMA_TRANSFER_MEMCPY,
+	SDMA_TRANSFER_SLAVE,
+};
+
+enum sdma_report_status {
+	SDMA_REPORT_CHUNK,
+	SDMA_REPORT_TRANSFER,
+};
+
+struct sdma_desc {
+	struct virt_dma_desc	vdesc;
+
+	/* Entry point to our LLI */
+	dma_addr_t		p_lli;
+	void			*v_lli;
+};
+
+struct sdma_channel {
+	struct sdma_desc	*desc;
+	unsigned int		index;
+	struct list_head	node;
+	void			*private;
+};
+
+struct sdma_request {
+	struct dma_slave_config	cfg;
+	struct list_head	node;
+	struct virt_dma_chan	vchan;
+
+	struct sdma_channel	*chan;
+	void			*private;
+};
+
+struct sdma_ops {
+	/* LLI management operations */
+	bool			(*lli_has_next)(void *v_lli);
+	void			*(*lli_next)(void *v_lli);
+	int			(*lli_init)(void *v_lli, void *sreq_priv,
+					    enum sdma_transfer_type type,
+					    enum dma_transfer_direction dir,
+					    dma_addr_t src,
+					    dma_addr_t dst, u32 len,
+					    struct dma_slave_config *config);
+	void			*(*lli_queue)(void *prev_v_lli, void *v_lli, dma_addr_t p_lli);
+	size_t			(*lli_size)(void *v_lli);
+
+	/* Scheduler helper */
+	struct sdma_request	*(*validate_request)(struct sdma_channel *chan,
+						     struct sdma_request *req);
+
+	/* Transfer Management Functions */
+	int			(*channel_pause)(struct sdma_channel *chan);
+	int			(*channel_resume)(struct sdma_channel *chan);
+	int			(*channel_start)(struct sdma_channel *chan, struct sdma_desc *sdesc);
+	int			(*channel_terminate)(struct sdma_channel *chan);
+	size_t			(*channel_residue)(struct sdma_channel *chan);
+};
+
+struct sdma {
+	struct dma_device	ddev;
+	struct sdma_ops		*ops;
+
+	struct dma_pool		*pool;
+
+	struct sdma_channel	*channels;
+	int			channels_nr;
+	struct sdma_request	*requests;
+	int			requests_nr;
+
+	struct list_head	avail_chans;
+	struct list_head	pend_reqs;
+
+	spinlock_t		lock;
+
+	unsigned long		private[];
+};
+
+static inline struct sdma *to_sdma(struct dma_device *d)
+{
+	return container_of(d, struct sdma, ddev);
+}
+
+static inline struct sdma_request *to_sdma_request(struct dma_chan *chan)
+{
+	return container_of(chan, struct sdma_request, vchan.chan);
+}
+
+static inline struct sdma_desc *to_sdma_desc(struct dma_async_tx_descriptor *tx)
+{
+	return container_of(tx, struct sdma_desc, vdesc.tx);
+}
+
+static inline void *sdma_priv(struct sdma *sdma)
+{
+	return (void*)sdma->private;
+}
+
+static inline void sdma_set_chan_private(struct sdma *sdma, void *ptr)
+{
+	int i;
+
+	for (i = 0; i < sdma->channels_nr; i++) {
+		struct sdma_channel *schan = &sdma->channels[i];
+
+		schan->private = ptr;
+	}
+}
+
+struct sdma_desc *sdma_report(struct sdma *sdma,
+			      struct sdma_channel *chan,
+			      enum sdma_report_status status);
+
+struct sdma *sdma_alloc(struct device *dev,
+			unsigned int channels,
+			unsigned int requests,
+			ssize_t lli_size,
+			ssize_t priv_size);
+void sdma_free(struct sdma *sdma);
+
+int sdma_register(struct sdma *sdma,
+		  struct sdma_ops *ops);
+int sdma_unregister(struct sdma *sdma);
+
+#endif /* _SCHEDULED_DMA_H_ */
-- 
2.3.3

[PATCH RFC 2/2] dmaengine: sun6i: Convert to scheduled DMA

[Maxime Ripard]

The sun6i driver is a good candidate for the new scheduled DMA driver. Convert
it to it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 853 ++++++++++--------------------------------------
 1 file changed, 181 insertions(+), 672 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 7ebcf9bec698..9133593fe7f5 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include "scheduled-dma.h"
 #include "virt-dma.h"
 
 /*
@@ -51,17 +52,19 @@
 /*
  * Channels specific registers
  */
-#define DMA_CHAN_ENABLE		0x00
+#define DMA_CHAN_BASE(chan)	(0x100 + (chan) * 0x40)
+
+#define DMA_CHAN_ENABLE(chan)	(DMA_CHAN_BASE(chan) + 0x00)
 #define DMA_CHAN_ENABLE_START		BIT(0)
 #define DMA_CHAN_ENABLE_STOP		0
 
-#define DMA_CHAN_PAUSE		0x04
+#define DMA_CHAN_PAUSE(chan)	(DMA_CHAN_BASE(chan) + 0x04)
 #define DMA_CHAN_PAUSE_PAUSE		BIT(1)
 #define DMA_CHAN_PAUSE_RESUME		0
 
-#define DMA_CHAN_LLI_ADDR	0x08
+#define DMA_CHAN_LLI_ADDR(chan)	(DMA_CHAN_BASE(chan) + 0x08)
 
-#define DMA_CHAN_CUR_CFG	0x0c
+#define DMA_CHAN_CUR_CFG(chan)	(DMA_CHAN_BASE(chan) + 0x0c)
 #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
@@ -74,13 +77,13 @@
 #define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
 #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
 
-#define DMA_CHAN_CUR_SRC	0x10
+#define DMA_CHAN_CUR_SRC(chan)	(DMA_CHAN_BASE(chan) + 0x10)
 
-#define DMA_CHAN_CUR_DST	0x14
+#define DMA_CHAN_CUR_DST(chan)	(DMA_CHAN_BASE(chan) + 0x14)
 
-#define DMA_CHAN_CUR_CNT	0x18
+#define DMA_CHAN_CUR_CNT(chan)	(DMA_CHAN_BASE(chan) + 0x18)
 
-#define DMA_CHAN_CUR_PARA	0x1c
+#define DMA_CHAN_CUR_PARA(chan)	(DMA_CHAN_BASE(chan) + 0x1c)
 
 
 /*
@@ -125,114 +128,14 @@ struct sun6i_dma_lli {
 	struct sun6i_dma_lli	*v_lli_next;
 };
 
-
-struct sun6i_desc {
-	struct virt_dma_desc	vd;
-	dma_addr_t		p_lli;
-	struct sun6i_dma_lli	*v_lli;
-};
-
-struct sun6i_pchan {
-	u32			idx;
-	void __iomem		*base;
-	struct sun6i_vchan	*vchan;
-	struct sun6i_desc	*desc;
-	struct sun6i_desc	*done;
-};
-
-struct sun6i_vchan {
-	struct virt_dma_chan	vc;
-	struct list_head	node;
-	struct dma_slave_config	cfg;
-	struct sun6i_pchan	*phy;
-	u8			port;
-};
-
 struct sun6i_dma_dev {
-	struct dma_device	slave;
 	void __iomem		*base;
 	struct clk		*clk;
 	int			irq;
-	spinlock_t		lock;
 	struct reset_control	*rstc;
-	struct tasklet_struct	task;
-	atomic_t		tasklet_shutdown;
-	struct list_head	pending;
-	struct dma_pool		*pool;
-	struct sun6i_pchan	*pchans;
-	struct sun6i_vchan	*vchans;
 	const struct sun6i_dma_config *cfg;
 };
 
-static struct device *chan2dev(struct dma_chan *chan)
-{
-	return &chan->dev->device;
-}
-
-static inline struct sun6i_dma_dev *to_sun6i_dma_dev(struct dma_device *d)
-{
-	return container_of(d, struct sun6i_dma_dev, slave);
-}
-
-static inline struct sun6i_vchan *to_sun6i_vchan(struct dma_chan *chan)
-{
-	return container_of(chan, struct sun6i_vchan, vc.chan);
-}
-
-static inline struct sun6i_desc *
-to_sun6i_desc(struct dma_async_tx_descriptor *tx)
-{
-	return container_of(tx, struct sun6i_desc, vd.tx);
-}
-
-static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
-{
-	dev_dbg(sdev->slave.dev, "Common register:\n"
-		"\tmask0(%04x): 0x%08x\n"
-		"\tmask1(%04x): 0x%08x\n"
-		"\tpend0(%04x): 0x%08x\n"
-		"\tpend1(%04x): 0x%08x\n"
-		"\tstats(%04x): 0x%08x\n",
-		DMA_IRQ_EN(0), readl(sdev->base + DMA_IRQ_EN(0)),
-		DMA_IRQ_EN(1), readl(sdev->base + DMA_IRQ_EN(1)),
-		DMA_IRQ_STAT(0), readl(sdev->base + DMA_IRQ_STAT(0)),
-		DMA_IRQ_STAT(1), readl(sdev->base + DMA_IRQ_STAT(1)),
-		DMA_STAT, readl(sdev->base + DMA_STAT));
-}
-
-static inline void sun6i_dma_dump_chan_regs(struct sun6i_dma_dev *sdev,
-					    struct sun6i_pchan *pchan)
-{
-	phys_addr_t reg = virt_to_phys(pchan->base);
-
-	dev_dbg(sdev->slave.dev, "Chan %d reg: %pa\n"
-		"\t___en(%04x): \t0x%08x\n"
-		"\tpause(%04x): \t0x%08x\n"
-		"\tstart(%04x): \t0x%08x\n"
-		"\t__cfg(%04x): \t0x%08x\n"
-		"\t__src(%04x): \t0x%08x\n"
-		"\t__dst(%04x): \t0x%08x\n"
-		"\tcount(%04x): \t0x%08x\n"
-		"\t_para(%04x): \t0x%08x\n\n",
-		pchan->idx, &reg,
-		DMA_CHAN_ENABLE,
-		readl(pchan->base + DMA_CHAN_ENABLE),
-		DMA_CHAN_PAUSE,
-		readl(pchan->base + DMA_CHAN_PAUSE),
-		DMA_CHAN_LLI_ADDR,
-		readl(pchan->base + DMA_CHAN_LLI_ADDR),
-		DMA_CHAN_CUR_CFG,
-		readl(pchan->base + DMA_CHAN_CUR_CFG),
-		DMA_CHAN_CUR_SRC,
-		readl(pchan->base + DMA_CHAN_CUR_SRC),
-		DMA_CHAN_CUR_DST,
-		readl(pchan->base + DMA_CHAN_CUR_DST),
-		DMA_CHAN_CUR_CNT,
-		readl(pchan->base + DMA_CHAN_CUR_CNT),
-		DMA_CHAN_CUR_PARA,
-		readl(pchan->base + DMA_CHAN_CUR_PARA));
-}
-
 static inline s8 convert_burst(u32 maxburst)
 {
 	switch (maxburst) {
@@ -254,20 +157,16 @@ static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 	return addr_width >> 1;
 }
 
-static void *sun6i_dma_lli_add(struct sun6i_dma_lli *prev,
-			       struct sun6i_dma_lli *next,
-			       dma_addr_t next_phy,
-			       struct sun6i_desc *txd)
+static void *sun6i_dma_lli_queue(void *prev_v_lli,
+				 void *v_lli,
+				 dma_addr_t p_lli)
 {
-	if ((!prev && !txd) || !next)
-		return NULL;
+	struct sun6i_dma_lli *prev = (struct sun6i_dma_lli*)prev_v_lli;
+	struct sun6i_dma_lli *next = (struct sun6i_dma_lli*)v_lli;
 
-	if (!prev) {
-		txd->p_lli = next_phy;
-		txd->v_lli = next;
-	} else {
-		prev->p_lli_next = next_phy;
-		prev->v_lli_next = next;
+	if (prev) {
+		prev->p_lli_next = p_lli;
+		prev->v_lli_next = v_lli;
 	}
 
 	next->p_lli_next = LLI_LAST_ITEM;
@@ -283,29 +182,28 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 {
 	u8 src_width, dst_width, src_burst, dst_burst;
 
-	if (!config)
-		return -EINVAL;
-
-	src_burst = convert_burst(config->src_maxburst);
-	if (src_burst)
-		return src_burst;
+	if (config) {
+		src_burst = convert_burst(config->src_maxburst);
+		if (src_burst)
+			return src_burst;
 
-	dst_burst = convert_burst(config->dst_maxburst);
-	if (dst_burst)
-		return dst_burst;
+		dst_burst = convert_burst(config->dst_maxburst);
+		if (dst_burst)
+			return dst_burst;
 
-	src_width = convert_buswidth(config->src_addr_width);
-	if (src_width)
-		return src_width;
+		src_width = convert_buswidth(config->src_addr_width);
+		if (src_width)
+			return src_width;
 
-	dst_width = convert_buswidth(config->dst_addr_width);
-	if (dst_width)
-		return dst_width;
+		dst_width = convert_buswidth(config->dst_addr_width);
+		if (dst_width)
+			return dst_width;
 
-	lli->cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
-		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
-		DMA_CHAN_CFG_DST_BURST(dst_burst) |
-		DMA_CHAN_CFG_DST_WIDTH(dst_width);
+		lli->cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
+			DMA_CHAN_CFG_SRC_WIDTH(src_width) |
+			DMA_CHAN_CFG_DST_BURST(dst_burst) |
+			DMA_CHAN_CFG_DST_WIDTH(dst_width);
+	}
 
 	lli->src = src;
 	lli->dst = dst;
@@ -315,156 +213,29 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 	return 0;
 }
 
-static inline void sun6i_dma_dump_lli(struct sun6i_vchan *vchan,
-				      struct sun6i_dma_lli *lli)
-{
-	phys_addr_t p_lli = virt_to_phys(lli);
-
-	dev_dbg(chan2dev(&vchan->vc.chan),
-		"\n\tdesc:   p - %pa v - 0x%p\n"
-		"\t\tc - 0x%08x s - 0x%08x d - 0x%08x\n"
-		"\t\tl - 0x%08x p - 0x%08x n - 0x%08x\n",
-		&p_lli, lli,
-		lli->cfg, lli->src, lli->dst,
-		lli->len, lli->para, lli->p_lli_next);
-}
-
-static void sun6i_dma_free_desc(struct virt_dma_desc *vd)
-{
-	struct sun6i_desc *txd = to_sun6i_desc(&vd->tx);
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vd->tx.chan->device);
-	struct sun6i_dma_lli *v_lli, *v_next;
-	dma_addr_t p_lli, p_next;
-
-	if (unlikely(!txd))
-		return;
-
-	p_lli = txd->p_lli;
-	v_lli = txd->v_lli;
-
-	while (v_lli) {
-		v_next = v_lli->v_lli_next;
-		p_next = v_lli->p_lli_next;
-
-		dma_pool_free(sdev->pool, v_lli, p_lli);
-
-		v_lli = v_next;
-		p_lli = p_next;
-	}
-
-	kfree(txd);
-}
-
-static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
+static int sun6i_dma_channel_start(struct sdma_channel *schan,
+				   struct sdma_desc *sdesc)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vchan->vc.chan.device);
-	struct virt_dma_desc *desc = vchan_next_desc(&vchan->vc);
-	struct sun6i_pchan *pchan = vchan->phy;
+	struct sun6i_dma_dev *sdc = schan->private;
 	u32 irq_val, irq_reg, irq_offset;
 
-	if (!pchan)
-		return -EAGAIN;
-
-	if (!desc) {
-		pchan->desc = NULL;
-		pchan->done = NULL;
-		return -EAGAIN;
-	}
-
-	list_del(&desc->node);
-
-	pchan->desc = to_sun6i_desc(&desc->tx);
-	pchan->done = NULL;
-
-	sun6i_dma_dump_lli(vchan, pchan->desc->v_lli);
-
-	irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
-	irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
+	irq_reg = schan->index / DMA_IRQ_CHAN_NR;
+	irq_offset = schan->index % DMA_IRQ_CHAN_NR;
 
-	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_offset));
+	irq_val = readl(sdc->base + DMA_IRQ_EN(irq_offset));
 	irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
-	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_offset));
+	writel(irq_val, sdc->base + DMA_IRQ_EN(irq_offset));
 
-	writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
-	writel(DMA_CHAN_ENABLE_START, pchan->base + DMA_CHAN_ENABLE);
-
-	sun6i_dma_dump_com_regs(sdev);
-	sun6i_dma_dump_chan_regs(sdev, pchan);
+	writel(sdesc->p_lli, sdc->base + DMA_CHAN_LLI_ADDR(schan->index));
+	writel(DMA_CHAN_ENABLE_START, sdc->base + DMA_CHAN_ENABLE(schan->index));
 
 	return 0;
 }
 
-static void sun6i_dma_tasklet(unsigned long data)
-{
-	struct sun6i_dma_dev *sdev = (struct sun6i_dma_dev *)data;
-	const struct sun6i_dma_config *cfg = sdev->cfg;
-	struct sun6i_vchan *vchan;
-	struct sun6i_pchan *pchan;
-	unsigned int pchan_alloc = 0;
-	unsigned int pchan_idx;
-
-	list_for_each_entry(vchan, &sdev->slave.channels, vc.chan.device_node) {
-		spin_lock_irq(&vchan->vc.lock);
-
-		pchan = vchan->phy;
-
-		if (pchan && pchan->done) {
-			if (sun6i_dma_start_desc(vchan)) {
-				/*
-				 * No current txd associated with this channel
-				 */
-				dev_dbg(sdev->slave.dev, "pchan %u: free\n",
-					pchan->idx);
-
-				/* Mark this channel free */
-				vchan->phy = NULL;
-				pchan->vchan = NULL;
-			}
-		}
-		spin_unlock_irq(&vchan->vc.lock);
-	}
-
-	spin_lock_irq(&sdev->lock);
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
-		pchan = &sdev->pchans[pchan_idx];
-
-		if (pchan->vchan || list_empty(&sdev->pending))
-			continue;
-
-		vchan = list_first_entry(&sdev->pending,
-					 struct sun6i_vchan, node);
-
-		/* Remove from pending channels */
-		list_del_init(&vchan->node);
-		pchan_alloc |= BIT(pchan_idx);
-
-		/* Mark this channel allocated */
-		pchan->vchan = vchan;
-		vchan->phy = pchan;
-		dev_dbg(sdev->slave.dev, "pchan %u: alloc vchan %p\n",
-			pchan->idx, &vchan->vc);
-	}
-	spin_unlock_irq(&sdev->lock);
-
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
-		if (!(pchan_alloc & BIT(pchan_idx)))
-			continue;
-
-		pchan = sdev->pchans + pchan_idx;
-		vchan = pchan->vchan;
-		if (vchan) {
-			spin_lock_irq(&vchan->vc.lock);
-			sun6i_dma_start_desc(vchan);
-			spin_unlock_irq(&vchan->vc.lock);
-		}
-	}
-}
-
 static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 {
-	struct sun6i_dma_dev *sdev = dev_id;
-	struct sun6i_vchan *vchan;
-	struct sun6i_pchan *pchan;
+	struct sdma *sdma = dev_id;
+	struct sun6i_dma_dev *sdev = sdma_priv(sdma);
 	int i, j, ret = IRQ_NONE;
 	u32 status;
 
@@ -473,395 +244,174 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 		if (!status)
 			continue;
 
-		dev_dbg(sdev->slave.dev, "DMA irq status %s: 0x%x\n",
+		dev_dbg(sdma->ddev.dev, "DMA irq status %s: 0x%x\n",
 			i ? "high" : "low", status);
 
 		writel(status, sdev->base + DMA_IRQ_STAT(i));
 
 		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
 			if (status & DMA_IRQ_QUEUE) {
-				pchan = sdev->pchans + j;
-				vchan = pchan->vchan;
-
-				if (vchan) {
-					spin_lock(&vchan->vc.lock);
-					vchan_cookie_complete(&pchan->desc->vd);
-					pchan->done = pchan->desc;
-					spin_unlock(&vchan->vc.lock);
-				}
+				struct sdma_channel *schan = sdma->channels + j;
+				struct sdma_desc *sdesc;
+
+				sdesc = sdma_report(sdma, schan, SDMA_REPORT_TRANSFER);
+				if (sdesc)
+					sun6i_dma_channel_start(schan, sdesc);
 			}
 
 			status = status >> DMA_IRQ_CHAN_WIDTH;
 		}
 
-		if (!atomic_read(&sdev->tasklet_shutdown))
-			tasklet_schedule(&sdev->task);
 		ret = IRQ_HANDLED;
 	}
 
 	return ret;
 }
 
-static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
-		struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
-		size_t len, unsigned long flags)
+static int sun6i_dma_lli_init(void *v_lli, void *sreq_priv,
+			      enum sdma_transfer_type type,
+			      enum dma_transfer_direction dir,
+			      dma_addr_t src,
+			      dma_addr_t dst, u32 len,
+			      struct dma_slave_config *config)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct sun6i_dma_lli *v_lli;
-	struct sun6i_desc *txd;
-	dma_addr_t p_lli;
+	struct sun6i_dma_lli *lli = v_lli;
 	s8 burst, width;
+	int ret;
 
-	dev_dbg(chan2dev(chan),
-		"%s; chan: %d, dest: %pad, src: %pad, len: %zu. flags: 0x%08lx\n",
-		__func__, vchan->vc.chan.chan_id, &dest, &src, len, flags);
-
-	if (!len)
-		return NULL;
-
-	txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
-	if (!txd)
-		return NULL;
-
-	v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
-	if (!v_lli) {
-		dev_err(sdev->slave.dev, "Failed to alloc lli memory\n");
-		goto err_txd_free;
-	}
-
-	v_lli->src = src;
-	v_lli->dst = dest;
-	v_lli->len = len;
-	v_lli->para = NORMAL_WAIT;
-
-	burst = convert_burst(8);
-	width = convert_buswidth(DMA_SLAVE_BUSWIDTH_4_BYTES);
-	v_lli->cfg |= DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM) |
-		DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
-		DMA_CHAN_CFG_DST_LINEAR_MODE |
-		DMA_CHAN_CFG_SRC_LINEAR_MODE |
-		DMA_CHAN_CFG_SRC_BURST(burst) |
-		DMA_CHAN_CFG_SRC_WIDTH(width) |
-		DMA_CHAN_CFG_DST_BURST(burst) |
-		DMA_CHAN_CFG_DST_WIDTH(width);
-
-	sun6i_dma_lli_add(NULL, v_lli, p_lli, txd);
-
-	sun6i_dma_dump_lli(vchan, v_lli);
-
-	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
-
-err_txd_free:
-	kfree(txd);
-	return NULL;
-}
-
-static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
-		struct dma_chan *chan, struct scatterlist *sgl,
-		unsigned int sg_len, enum dma_transfer_direction dir,
-		unsigned long flags, void *context)
-{
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct dma_slave_config *sconfig = &vchan->cfg;
-	struct sun6i_dma_lli *v_lli, *prev = NULL;
-	struct sun6i_desc *txd;
-	struct scatterlist *sg;
-	dma_addr_t p_lli;
-	int i, ret;
-
-	if (!sgl)
-		return NULL;
-
-	if (!is_slave_direction(dir)) {
-		dev_err(chan2dev(chan), "Invalid DMA direction\n");
-		return NULL;
-	}
-
-	txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
-	if (!txd)
-		return NULL;
-
-	for_each_sg(sgl, sg, sg_len, i) {
-		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
-		if (!v_lli)
-			goto err_lli_free;
+	ret = sun6i_dma_cfg_lli(lli, src, dst, len,
+				config);
+	if (ret)
+		return ret;
 
+	switch (type) {
+	case SDMA_TRANSFER_MEMCPY:
+		burst = convert_burst(8);
+		width = convert_buswidth(DMA_SLAVE_BUSWIDTH_4_BYTES);
+
+		lli->cfg |= DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM) |
+			DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
+			DMA_CHAN_CFG_DST_LINEAR_MODE |
+			DMA_CHAN_CFG_SRC_LINEAR_MODE |
+			DMA_CHAN_CFG_SRC_BURST(burst) |
+			DMA_CHAN_CFG_DST_BURST(burst) |
+			DMA_CHAN_CFG_SRC_WIDTH(width) |
+			DMA_CHAN_CFG_DST_WIDTH(width);
+		break;
+
+	case SDMA_TRANSFER_SLAVE:
 		if (dir == DMA_MEM_TO_DEV) {
-			ret = sun6i_dma_cfg_lli(v_lli, sg_dma_address(sg),
-						sconfig->dst_addr, sg_dma_len(sg),
-						sconfig);
-			if (ret)
-				goto err_cur_lli_free;
-
-			v_lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
+			lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
 				DMA_CHAN_CFG_SRC_LINEAR_MODE |
-				DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM) |
-				DMA_CHAN_CFG_DST_DRQ(vchan->port);
-
-			dev_dbg(chan2dev(chan),
-				"%s; chan: %d, dest: %pad, src: %pad, len: %u. flags: 0x%08lx\n",
-				__func__, vchan->vc.chan.chan_id,
-				&sconfig->dst_addr, &sg_dma_address(sg),
-				sg_dma_len(sg), flags);
-
+				DMA_CHAN_CFG_DST_DRQ((u32)sreq_priv) |
+				DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM);
 		} else {
-			ret = sun6i_dma_cfg_lli(v_lli, sconfig->src_addr,
-						sg_dma_address(sg), sg_dma_len(sg),
-						sconfig);
-			if (ret)
-				goto err_cur_lli_free;
-
-			v_lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
+			lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
 				DMA_CHAN_CFG_SRC_IO_MODE |
 				DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
-				DMA_CHAN_CFG_SRC_DRQ(vchan->port);
-
-			dev_dbg(chan2dev(chan),
-				"%s; chan: %d, dest: %pad, src: %pad, len: %u. flags: 0x%08lx\n",
-				__func__, vchan->vc.chan.chan_id,
-				&sg_dma_address(sg), &sconfig->src_addr,
-				sg_dma_len(sg), flags);
+				DMA_CHAN_CFG_SRC_DRQ((u32)sreq_priv);
 		}
 
-		prev = sun6i_dma_lli_add(prev, v_lli, p_lli, txd);
-	}
-
-	dev_dbg(chan2dev(chan), "First: %pad\n", &txd->p_lli);
-	for (prev = txd->v_lli; prev; prev = prev->v_lli_next)
-		sun6i_dma_dump_lli(vchan, prev);
+		break;
 
-	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
+	default:
+		break;
+	}
 
-err_cur_lli_free:
-	dma_pool_free(sdev->pool, v_lli, p_lli);
-err_lli_free:
-	for (prev = txd->v_lli; prev; prev = prev->v_lli_next)
-		dma_pool_free(sdev->pool, prev, virt_to_phys(prev));
-	kfree(txd);
-	return NULL;
+	return 0;
 }
 
-static int sun6i_dma_config(struct dma_chan *chan,
-			    struct dma_slave_config *config)
+static bool sun6i_dma_lli_has_next(void *v_lli)
 {
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
+	struct sun6i_dma_lli *lli = v_lli;
 
-	memcpy(&vchan->cfg, config, sizeof(*config));
-
-	return 0;
-}
+	return lli->v_lli_next != NULL;
+}	
 
-static int sun6i_dma_pause(struct dma_chan *chan)
+static void *sun6i_dma_lli_next(void *v_lli)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct sun6i_pchan *pchan = vchan->phy;
-
-	dev_dbg(chan2dev(chan), "vchan %p: pause\n", &vchan->vc);
-
-	if (pchan) {
-		writel(DMA_CHAN_PAUSE_PAUSE,
-		       pchan->base + DMA_CHAN_PAUSE);
-	} else {
-		spin_lock(&sdev->lock);
-		list_del_init(&vchan->node);
-		spin_unlock(&sdev->lock);
-	}
+	struct sun6i_dma_lli *lli = v_lli;
 
-	return 0;
+	return lli->v_lli_next;
 }
 
-static int sun6i_dma_resume(struct dma_chan *chan)
+static size_t sun6i_dma_lli_size(void *v_lli)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct sun6i_pchan *pchan = vchan->phy;
-	unsigned long flags;
-
-	dev_dbg(chan2dev(chan), "vchan %p: resume\n", &vchan->vc);
-
-	spin_lock_irqsave(&vchan->vc.lock, flags);
-
-	if (pchan) {
-		writel(DMA_CHAN_PAUSE_RESUME,
-		       pchan->base + DMA_CHAN_PAUSE);
-	} else if (!list_empty(&vchan->vc.desc_issued)) {
-		spin_lock(&sdev->lock);
-		list_add_tail(&vchan->node, &sdev->pending);
-		spin_unlock(&sdev->lock);
-	}
-
-	spin_unlock_irqrestore(&vchan->vc.lock, flags);
+	struct sun6i_dma_lli *lli = v_lli;
 
-	return 0;
+	return lli->len;
 }
 
-static int sun6i_dma_terminate_all(struct dma_chan *chan)
+static int sun6i_dma_channel_pause(struct sdma_channel *schan)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct sun6i_pchan *pchan = vchan->phy;
-	unsigned long flags;
-	LIST_HEAD(head);
-
-	spin_lock(&sdev->lock);
-	list_del_init(&vchan->node);
-	spin_unlock(&sdev->lock);
-
-	spin_lock_irqsave(&vchan->vc.lock, flags);
-
-	vchan_get_all_descriptors(&vchan->vc, &head);
-
-	if (pchan) {
-		writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
-		writel(DMA_CHAN_PAUSE_RESUME, pchan->base + DMA_CHAN_PAUSE);
+	struct sun6i_dma_dev *sdc = schan->private;
 
-		vchan->phy = NULL;
-		pchan->vchan = NULL;
-		pchan->desc = NULL;
-		pchan->done = NULL;
-	}
-
-	spin_unlock_irqrestore(&vchan->vc.lock, flags);
-
-	vchan_dma_desc_free_list(&vchan->vc, &head);
+	writel(DMA_CHAN_PAUSE_PAUSE, sdc->base + DMA_CHAN_PAUSE(schan->index));
 
 	return 0;
 }
 
-static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
-					   dma_cookie_t cookie,
-					   struct dma_tx_state *state)
+static int sun6i_dma_channel_resume(struct sdma_channel *schan)
 {
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	struct sun6i_pchan *pchan = vchan->phy;
-	struct sun6i_dma_lli *lli;
-	struct virt_dma_desc *vd;
-	struct sun6i_desc *txd;
-	enum dma_status ret;
-	unsigned long flags;
-	size_t bytes = 0;
-
-	ret = dma_cookie_status(chan, cookie, state);
-	if (ret == DMA_COMPLETE)
-		return ret;
-
-	spin_lock_irqsave(&vchan->vc.lock, flags);
+	struct sun6i_dma_dev *sdc = schan->private;
 
-	vd = vchan_find_desc(&vchan->vc, cookie);
-	txd = to_sun6i_desc(&vd->tx);
+	writel(DMA_CHAN_PAUSE_RESUME, sdc->base + DMA_CHAN_PAUSE(schan->index));
 
-	if (vd) {
-		for (lli = txd->v_lli; lli != NULL; lli = lli->v_lli_next)
-			bytes += lli->len;
-	} else if (!pchan || !pchan->desc) {
-		bytes = 0;
-	} else {
-		bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
-	}
-
-	spin_unlock_irqrestore(&vchan->vc.lock, flags);
-
-	dma_set_residue(state, bytes);
-
-	return ret;
+	return 0;
 }
 
-static void sun6i_dma_issue_pending(struct dma_chan *chan)
+static int sun6i_dma_channel_terminate(struct sdma_channel *schan)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	unsigned long flags;
-
-	spin_lock_irqsave(&vchan->vc.lock, flags);
-
-	if (vchan_issue_pending(&vchan->vc)) {
-		spin_lock(&sdev->lock);
-
-		if (!vchan->phy && list_empty(&vchan->node)) {
-			list_add_tail(&vchan->node, &sdev->pending);
-			tasklet_schedule(&sdev->task);
-			dev_dbg(chan2dev(chan), "vchan %p: issued\n",
-				&vchan->vc);
-		}
+	struct sun6i_dma_dev *sdc = schan->private;
 
-		spin_unlock(&sdev->lock);
-	} else {
-		dev_dbg(chan2dev(chan), "vchan %p: nothing to issue\n",
-			&vchan->vc);
-	}
-
-	spin_unlock_irqrestore(&vchan->vc.lock, flags);
-}
+	writel(DMA_CHAN_ENABLE_STOP, sdc->base + DMA_CHAN_ENABLE(schan->index));
+	writel(DMA_CHAN_PAUSE_RESUME, sdc->base + DMA_CHAN_PAUSE(schan->index));
 
-static int sun6i_dma_alloc_chan_resources(struct dma_chan *chan)
-{
 	return 0;
 }
 
-static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
+static size_t sun6i_dma_channel_residue(struct sdma_channel *schan)
 {
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
-	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
-	unsigned long flags;
-
-	spin_lock_irqsave(&sdev->lock, flags);
-	list_del_init(&vchan->node);
-	spin_unlock_irqrestore(&sdev->lock, flags);
+	struct sun6i_dma_dev *sdc = schan->private;
 
-	vchan_free_chan_resources(&vchan->vc);
+	return readl(sdc->base + DMA_CHAN_CUR_CNT(schan->index));
 }
 
 static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
 					   struct of_dma *ofdma)
 {
-	struct sun6i_dma_dev *sdev = ofdma->of_dma_data;
-	struct sun6i_vchan *vchan;
+	struct sdma *sdma = ofdma->of_dma_data;
+	struct sun6i_dma_dev *sdev = sdma_priv(sdma);
+	struct sdma_request *sreq;
 	struct dma_chan *chan;
-	u8 port = dma_spec->args[0];
+	u32 port = dma_spec->args[0];
 
 	if (port > sdev->cfg->nr_max_requests)
 		return NULL;
 
-	chan = dma_get_any_slave_channel(&sdev->slave);
+	chan = dma_get_any_slave_channel(&sdma->ddev);
 	if (!chan)
 		return NULL;
 
-	vchan = to_sun6i_vchan(chan);
-	vchan->port = port;
+	sreq = to_sdma_request(chan);
+	sreq->private = (void *)port;
 
 	return chan;
 }
 
-static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
-{
-	/* Disable all interrupts from DMA */
-	writel(0, sdev->base + DMA_IRQ_EN(0));
-	writel(0, sdev->base + DMA_IRQ_EN(1));
-
-	/* Prevent spurious interrupts from scheduling the tasklet */
-	atomic_inc(&sdev->tasklet_shutdown);
-
-	/* Make sure we won't have any further interrupts */
-	devm_free_irq(sdev->slave.dev, sdev->irq, sdev);
-
-	/* Actually prevent the tasklet from being scheduled */
-	tasklet_kill(&sdev->task);
-}
-
-static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
-{
-	int i;
-
-	for (i = 0; i < sdev->cfg->nr_max_vchans; i++) {
-		struct sun6i_vchan *vchan = &sdev->vchans[i];
-
-		list_del(&vchan->vc.chan.device_node);
-		tasklet_kill(&vchan->vc.task);
-	}
-}
+static struct sdma_ops sun6i_dma_ops = {
+	.channel_pause		= sun6i_dma_channel_pause,
+	.channel_residue	= sun6i_dma_channel_residue,
+	.channel_resume		= sun6i_dma_channel_resume,
+	.channel_start		= sun6i_dma_channel_start,
+	.channel_terminate	= sun6i_dma_channel_terminate,
+
+	.lli_has_next		= sun6i_dma_lli_has_next,
+	.lli_init		= sun6i_dma_lli_init,
+	.lli_next		= sun6i_dma_lli_next,
+	.lli_queue		= sun6i_dma_lli_queue,
+	.lli_size		= sun6i_dma_lli_size,
+};
 
 /*
  * For A31:
@@ -904,19 +454,29 @@ static struct of_device_id sun6i_dma_match[] = {
 
 static int sun6i_dma_probe(struct platform_device *pdev)
 {
+	const struct sun6i_dma_config *cfg;
 	const struct of_device_id *device;
 	struct sun6i_dma_dev *sdc;
 	struct resource *res;
-	int ret, i;
-
-	sdc = devm_kzalloc(&pdev->dev, sizeof(*sdc), GFP_KERNEL);
-	if (!sdc)
-		return -ENOMEM;
+	struct sdma *sdma;
+	int ret;
 
 	device = of_match_device(sun6i_dma_match, &pdev->dev);
 	if (!device)
 		return -ENODEV;
-	sdc->cfg = device->data;
+	cfg = device->data;
+
+	sdma = sdma_alloc(&pdev->dev,
+			  cfg->nr_max_channels,
+			  cfg->nr_max_vchans,
+			  sizeof(struct sun6i_dma_lli),
+			  sizeof(*sdc));
+	if (IS_ERR(sdma))
+		return PTR_ERR(sdma);
+
+	sdc = sdma_priv(sdma);
+	sdma_set_chan_private(sdma, sdc);
+	sdc->cfg = cfg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	sdc->base = devm_ioremap_resource(&pdev->dev, res);
@@ -941,75 +501,28 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 		return PTR_ERR(sdc->rstc);
 	}
 
-	sdc->pool = dmam_pool_create(dev_name(&pdev->dev), &pdev->dev,
-				     sizeof(struct sun6i_dma_lli), 4, 0);
-	if (!sdc->pool) {
-		dev_err(&pdev->dev, "No memory for descriptors dma pool\n");
-		return -ENOMEM;
-	}
+	platform_set_drvdata(pdev, sdma);
 
-	platform_set_drvdata(pdev, sdc);
-	INIT_LIST_HEAD(&sdc->pending);
-	spin_lock_init(&sdc->lock);
-
-	dma_cap_set(DMA_PRIVATE, sdc->slave.cap_mask);
-	dma_cap_set(DMA_MEMCPY, sdc->slave.cap_mask);
-	dma_cap_set(DMA_SLAVE, sdc->slave.cap_mask);
-
-	INIT_LIST_HEAD(&sdc->slave.channels);
-	sdc->slave.device_alloc_chan_resources	= sun6i_dma_alloc_chan_resources;
-	sdc->slave.device_free_chan_resources	= sun6i_dma_free_chan_resources;
-	sdc->slave.device_tx_status		= sun6i_dma_tx_status;
-	sdc->slave.device_issue_pending		= sun6i_dma_issue_pending;
-	sdc->slave.device_prep_slave_sg		= sun6i_dma_prep_slave_sg;
-	sdc->slave.device_prep_dma_memcpy	= sun6i_dma_prep_dma_memcpy;
-	sdc->slave.copy_align			= 4;
-	sdc->slave.device_config		= sun6i_dma_config;
-	sdc->slave.device_pause			= sun6i_dma_pause;
-	sdc->slave.device_resume		= sun6i_dma_resume;
-	sdc->slave.device_terminate_all		= sun6i_dma_terminate_all;
-	sdc->slave.src_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
-						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
-						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
-	sdc->slave.dst_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
-						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
-						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
-	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
-						  BIT(DMA_MEM_TO_DEV);
-	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-	sdc->slave.dev = &pdev->dev;
-
-	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels,
-				   sizeof(struct sun6i_pchan), GFP_KERNEL);
-	if (!sdc->pchans)
-		return -ENOMEM;
-
-	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_vchans,
-				   sizeof(struct sun6i_vchan), GFP_KERNEL);
-	if (!sdc->vchans)
-		return -ENOMEM;
-
-	tasklet_init(&sdc->task, sun6i_dma_tasklet, (unsigned long)sdc);
-
-	for (i = 0; i < sdc->cfg->nr_max_channels; i++) {
-		struct sun6i_pchan *pchan = &sdc->pchans[i];
-
-		pchan->idx = i;
-		pchan->base = sdc->base + 0x100 + i * 0x40;
-	}
+	dma_cap_set(DMA_PRIVATE, sdma->ddev.cap_mask);
+	dma_cap_set(DMA_MEMCPY, sdma->ddev.cap_mask);
+	dma_cap_set(DMA_SLAVE, sdma->ddev.cap_mask);
 
-	for (i = 0; i < sdc->cfg->nr_max_vchans; i++) {
-		struct sun6i_vchan *vchan = &sdc->vchans[i];
-
-		INIT_LIST_HEAD(&vchan->node);
-		vchan->vc.desc_free = sun6i_dma_free_desc;
-		vchan_init(&vchan->vc, &sdc->slave);
-	}
+	sdma->ddev.copy_align		= 4;
+	sdma->ddev.src_addr_widths	= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+					  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+					  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdma->ddev.dst_addr_widths	= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+					  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+					  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdma->ddev.directions		= BIT(DMA_DEV_TO_MEM) |
+					  BIT(DMA_MEM_TO_DEV);
+	sdma->ddev.residue_granularity	= DMA_RESIDUE_GRANULARITY_BURST;
+	sdma->ddev.dev = &pdev->dev;
 
 	ret = reset_control_deassert(sdc->rstc);
 	if (ret) {
 		dev_err(&pdev->dev, "Couldn't deassert the device from reset\n");
-		goto err_chan_free;
+		goto err_free;
 	}
 
 	ret = clk_prepare_enable(sdc->clk);
@@ -1019,20 +532,20 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
-			       dev_name(&pdev->dev), sdc);
+			       dev_name(&pdev->dev), sdma);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot request IRQ\n");
 		goto err_clk_disable;
 	}
 
-	ret = dma_async_device_register(&sdc->slave);
+	ret = sdma_register(sdma, &sun6i_dma_ops);
 	if (ret) {
 		dev_warn(&pdev->dev, "Failed to register DMA engine device\n");
-		goto err_irq_disable;
+		goto err_clk_disable;
 	}
 
 	ret = of_dma_controller_register(pdev->dev.of_node, sun6i_dma_of_xlate,
-					 sdc);
+					 sdma);
 	if (ret) {
 		dev_err(&pdev->dev, "of_dma_controller_register failed\n");
 		goto err_dma_unregister;
@@ -1050,31 +563,27 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	return 0;
 
 err_dma_unregister:
-	dma_async_device_unregister(&sdc->slave);
-err_irq_disable:
-	sun6i_kill_tasklet(sdc);
+	sdma_unregister(sdma);
 err_clk_disable:
 	clk_disable_unprepare(sdc->clk);
 err_reset_assert:
 	reset_control_assert(sdc->rstc);
-err_chan_free:
-	sun6i_dma_free(sdc);
+err_free:
+	sdma_free(sdma);
+
 	return ret;
 }
 
 static int sun6i_dma_remove(struct platform_device *pdev)
 {
-	struct sun6i_dma_dev *sdc = platform_get_drvdata(pdev);
+	struct sdma *sdma = platform_get_drvdata(pdev);
+	struct sun6i_dma_dev *sdc = sdma_priv(sdma);
 
 	of_dma_controller_free(pdev->dev.of_node);
-	dma_async_device_unregister(&sdc->slave);
-
-	sun6i_kill_tasklet(sdc);
-
+	sdma_unregister(sdma);
 	clk_disable_unprepare(sdc->clk);
 	reset_control_assert(sdc->rstc);
-
-	sun6i_dma_free(sdc);
+	sdma_free(sdma);
 
 	return 0;
 }
-- 
2.3.3

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Ludovic Desroches]

On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:

[...]

> +/*
> + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> + *
> + * Flow normal:
> + *   - Election d'un pchan (Framework)
> + *   - Push d'un descripteur vers le pchan (Driver)
> + *   - idle....
> + *   - interrupt (driver)
> + *   - Transfert terminé, notification vers le framework (driver)
> + *   - Nouveau transfert dans la queue?
> + *     + Si oui, on est reparti
> + *     + Si non, on sort de l'interrupt, le pchan est libéré
> + */

As I am French too, I totally agree to translate comments in French but
I am not sure it will satisfy everyone :)

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Ludovic Desroches]

On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:
> This framework aims at easing the development of dmaengine drivers by providing
> generic implementations of the functions usually required by dmaengine, while
> abstracting away most of the logic required.
> 

For sure it will ease writing new dmaengine drivers! Moreover, adopting
this framework will convert the driver to use virtual channels isn't it?

> It is very relevant for controllers that have more requests than channels,
> where you need to have some scheduling that is usually very bug prone, and
> shouldn't be implemented in each and every driver.
> 
> This introduces a new set of callbacks for drivers to implement the device
> specific behaviour. These new sets of callbacks aims at providing both how to
> handle channel related operations (start the transfer of a new descriptor,
> pause, resume, etc.) and the LLI related operations (iterator and various
> accessors).
> 
> So far, the only transfer types supported are memcpy and slave transfers, but
> eventually should support new transfer types as new drivers get converted.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/dma/Kconfig         |   4 +
>  drivers/dma/Makefile        |   1 +
>  drivers/dma/scheduled-dma.c | 571 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/dma/scheduled-dma.h | 140 +++++++++++
>  4 files changed, 716 insertions(+)
>  create mode 100644 drivers/dma/scheduled-dma.c
>  create mode 100644 drivers/dma/scheduled-dma.h
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index a874b6ec6650..032bf5fcd58b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -406,6 +406,7 @@ config DMA_SUN6I
>  	depends on RESET_CONTROLLER
>  	select DMA_ENGINE
>  	select DMA_VIRTUAL_CHANNELS
> +	select DMA_SCHEDULED
>  	help
>  	  Support for the DMA engine first found in Allwinner A31 SoCs.
>  
> @@ -431,6 +432,9 @@ config DMA_ENGINE
>  config DMA_VIRTUAL_CHANNELS
>  	tristate
>  
> +config DMA_SCHEDULED
> +	bool
> +

I think, it should select DMA_VIRTUAL_CHANNELS

>  config DMA_ACPI
>  	def_bool y
>  	depends on ACPI
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index f915f61ec574..1db31814c749 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
>  obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
>  obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
>  obj-$(CONFIG_DMA_OF) += of-dma.o
> +obj-$(CONFIG_DMA_SCHEDULED) += scheduled-dma.o
>  
>  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
>  obj-$(CONFIG_DMATEST) += dmatest.o
> diff --git a/drivers/dma/scheduled-dma.c b/drivers/dma/scheduled-dma.c
> new file mode 100644
> index 000000000000..482d04f2ccbc
> --- /dev/null
> +++ b/drivers/dma/scheduled-dma.c
> @@ -0,0 +1,571 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/dmaengine.h>
> +#include <linux/dmapool.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +#include "scheduled-dma.h"
> +#include "virt-dma.h"
> +
> +static struct sdma_request *sdma_pop_queued_transfer(struct sdma *sdma,
> +						     struct sdma_channel *schan)
> +{
> +	struct sdma_request *sreq = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +
> +	/* No requests are awaiting an available channel */
> +	if (list_empty(&sdma->pend_reqs))
> +		goto out;
> +
> +	/*
> +	 * If we don't have any validate_request callback, any request
> +	 * can be dispatched to any channel.
> +	 *
> +	 * Remove the first entry and return it.
> +	 */
> +	if (!sdma->ops->validate_request) {
> +		sreq = list_first_entry(&sdma->pend_reqs,
> +					struct sdma_request, node);
> +		list_del_init(&sreq->node);
> +		goto out;
> +	}
> +
> +	list_for_each_entry(sreq, &sdma->pend_reqs, node) {
> +		/*
> +		 * Ask the driver to validate that the request can
> +		 * happen on the channel.
> +		 */
> +		if (sdma->ops->validate_request(schan, sreq)) {
> +			list_del_init(&sreq->node);
> +			goto out;
> +		}
> +
> +		/* Otherwise, just keep looping */
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +
> +	return sreq;
> +}
> +
> +/*
> + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> + *
> + * Flow normal:
> + *   - Election d'un pchan (Framework)
> + *   - Push d'un descripteur vers le pchan (Driver)
> + *   - idle....
> + *   - interrupt (driver)
> + *   - Transfert terminé, notification vers le framework (driver)
> + *   - Nouveau transfert dans la queue?
> + *     + Si oui, on est reparti
> + *     + Si non, on sort de l'interrupt, le pchan est libéré
> + */
> +
> +struct sdma_desc *sdma_report(struct sdma *sdma,
> +			      struct sdma_channel *schan,
> +			      enum sdma_report_status status)
> +{
> +	struct sdma_desc *sdesc = NULL;
> +	struct virt_dma_desc *vdesc;
> +	struct sdma_request *sreq;
> +
> +	switch (status) {
> +	case SDMA_REPORT_TRANSFER:
> +		/*
> +		 * We're done with the current transfer that was in this
> +		 * physical channel.
> +		 */
> +		vchan_cookie_complete(&schan->desc->vdesc);
> +
> +		/*
> +		 * Now, try to see if there's any queued transfer
> +		 * awaiting an available channel.
> +		 *
> +		 * If not, just bail out, and mark the pchan as
> +		 * available.
> +		 *
> +		 * If there's such a transfer, validate that the
> +		 * driver can handle it, and ask it to do the
> +		 * transfer.
> +		 */
> +		sreq = sdma_pop_queued_transfer(sdma, schan);
> +		if (!sreq) {
> +			list_add_tail(&schan->node, &sdma->avail_chans);
> +			return NULL;
> +		}
> +
> +		/* Mark the request as assigned to a particular channel */
> +		sreq->chan = schan;
> +
> +		/* Retrieve the next transfer descriptor */
> +		vdesc = vchan_next_desc(&sreq->vchan);
> +		schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> +
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return sdesc;
> +}
> +EXPORT_SYMBOL_GPL(sdma_report);
> +
> +static enum dma_status sdma_tx_status(struct dma_chan *chan,
> +				      dma_cookie_t cookie,
> +				      struct dma_tx_state *state)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct virt_dma_desc *vd;
> +	struct sdma_desc *desc;
> +	enum dma_status ret;
> +	unsigned long flags;
> +	size_t bytes = 0;
> +	void *lli;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	ret = dma_cookie_status(chan, cookie, state);
> +	if (ret == DMA_COMPLETE)
> +		goto out;
> +
> +	vd = vchan_find_desc(&sreq->vchan, cookie);
> +	desc = to_sdma_desc(&vd->tx);
> +
> +	if (vd) {
> +		lli = desc->v_lli;
> +		while (true) {
> +			bytes += sdma->ops->lli_size(lli);
> +
> +			if (!sdma->ops->lli_has_next(lli))
> +				break;
> +
> +			lli = sdma->ops->lli_next(lli);
> +		}
> +	} else if (chan) {
> +		bytes = sdma->ops->channel_residue(schan);
> +	}
> +
> +	dma_set_residue(state, bytes);
> +
> +out:
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return ret;
> +};
> +
> +static int sdma_config(struct dma_chan *chan,
> +		       struct dma_slave_config *config)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +	memcpy(&sreq->cfg, config, sizeof(*config));
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_pause(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel, just
> +	 * pause the channel.
> +	 *
> +	 * If not, remove the request from the pending list.
> +	 */
> +	if (schan) {
> +		sdma->ops->channel_pause(schan);
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_del_init(&sreq->node);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_resume(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel, just
> +	 * resume the channel.
> +	 *
> +	 * If not, add the request from the pending list.
> +	 */

Is such a case possible? The request should be already in the pending
list, isn't it? By the way, I may have missed something but where do
you add a request to the pending list? I have seen it only in the resume
case.

> +	if (schan) {
> +		sdma->ops->channel_resume(schan);
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_add_tail(&sreq->node, &sdma->pend_reqs);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_terminate(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel,
> +	 * terminate the channel.
> +	 *
> +	 * If not, prevent the request from being scheduled.
> +	 */
> +	if (schan) {
> +		sdma->ops->channel_terminate(schan);
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_del_init(&sreq->node);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * Flush all the pending descriptors from our vchan
> +	 */
> +	vchan_free_chan_resources(&sreq->vchan);
> +
> +	return 0;
> +}
> +
> +
> +static struct dma_async_tx_descriptor *sdma_prep_memcpy(struct dma_chan *chan,
> +							dma_addr_t dest, dma_addr_t src,
> +							size_t len, unsigned long flags)
> +{
> +	struct sdma_request *req = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct sdma_desc *desc;
> +	dma_addr_t p_lli;
> +	void *v_lli;
> +
> +	if (!len)
> +		return NULL;
> +
> +	/* Allocate our representation of a descriptor */
> +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> +	if (!desc)
> +		return NULL;
> +
> +	v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> +	if (!v_lli)
> +		goto err_desc_free;
> +
> +	/* Ask the driver to initialise its hardware descriptor */
> +	if (sdma->ops->lli_init(v_lli, req->private,
> +				SDMA_TRANSFER_MEMCPY,
> +				DMA_MEM_TO_MEM, src, dest, len,
> +				NULL))
> +		goto err_lli_free;
> +
> +	/* Create our single item LLI */
> +	sdma->ops->lli_queue(NULL, v_lli, p_lli);
> +	desc->p_lli = p_lli;
> +	desc->v_lli = v_lli;
> +
> +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
> +
> +err_lli_free:
> +	dma_pool_free(sdma->pool, v_lli, p_lli);
> +err_desc_free:
> +	kfree(desc);
> +
> +	return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *sdma_prep_slave_sg(struct dma_chan *chan,
> +							  struct scatterlist *sgl,
> +							  unsigned int sg_len,
> +							  enum dma_transfer_direction dir,
> +							  unsigned long flags, void *context)
> +{
> +	struct sdma_request *req = to_sdma_request(chan);
> +	struct dma_slave_config *config = &req->cfg;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	void *v_lli, *prev_v_lli = NULL;
> +	struct scatterlist *sg;
> +	struct sdma_desc *desc;
> +	dma_addr_t p_lli;
> +	int i;
> +
> +	if (!sgl || !sg_len)
> +		return NULL;
> +
> +	/* Allocate our representation of a descriptor */
> +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> +	if (!desc)
> +		return NULL;
> +
> +	/*
> +	 * For each scatter list entry, build up our representation of
> +	 * the LLI, and ask the driver to create its hardware
> +	 * descriptor.
> +	 */
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> +		if (!v_lli)
> +			goto err_lli_free;
> +
> +		/* Ask the driver to initialise its hardware descriptor */
> +		if (sdma->ops->lli_init(v_lli, req->private,
> +					SDMA_TRANSFER_SLAVE,
> +					dir, sg_dma_address(sg),
> +					config->dst_addr, sg_dma_len(sg),
> +					config))
> +			goto err_lli_free;
> +
> +		/*
> +		 * If it's our first item, initialise our descriptor
> +		 * pointers to the lli.
> +		 *
> +		 * Otherwise, queue it to the end of the LLI.
> +		 */
> +		if (!prev_v_lli) {
> +			desc->p_lli = p_lli;
> +			desc->v_lli = v_lli;
> +			prev_v_lli = v_lli;
> +		} else {
> +			/* And to queue it at the end of its hardware LLI */
> +			prev_v_lli = sdma->ops->lli_queue(prev_v_lli, v_lli, p_lli);
> +		}
> +	}
> +
> +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
> +
> +err_lli_free:
> +#warning "Free the LLI"
> +
> +	kfree(desc);
> +	return NULL;
> +}
> +
> +static void sdma_issue_pending(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct virt_dma_desc *vdesc;
> +	struct sdma_channel *schan;
> +	struct sdma_desc *sdesc;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/* See if we have anything to do */
> +	if (!vchan_issue_pending(&sreq->vchan))
> +		goto out_chan_unlock;
> +
> +	/* Is some work in progress already? */
> +	if (sreq->chan)
> +		goto out_chan_unlock;
> +
> +	spin_lock(&sdma->lock);
> +
> +	/* Is there an available channel */
> +	if (list_empty(&sdma->avail_chans))
> +		goto out_main_unlock;
> +
> +	/*
> +	 * If there's no validate_request callback, it means that all
> +	 * channels can transfer any request. Pick the first available
> +	 * channel.
> +	 *
> +	 * Otherwise, iterate over all the pending channels and call
> +	 * validate_request.
> +	 */
> +	if (!sdma->ops->validate_request) {
> +		schan = list_first_entry(&sdma->avail_chans,
> +					 struct sdma_channel, node);
> +	} else {
> +		list_for_each_entry(schan, &sdma->avail_chans, node) {
> +			if (sdma->ops->validate_request(schan, sreq)) {
> +				list_del_init(&schan->node);
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (!schan)
> +		goto out_main_unlock;
> +
> +	sreq->chan = schan;
> +
> +	/* Retrieve the next transfer descriptor */
> +	vdesc = vchan_next_desc(&sreq->vchan);
> +	schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> +
> +	sdma->ops->channel_start(schan, sdesc);
> +
> +out_main_unlock:
> +	spin_unlock(&sdma->lock);
> +out_chan_unlock:
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +}
> +
> +static void sdma_free_chan_resources(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +	list_del_init(&sreq->node);
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	vchan_free_chan_resources(&sreq->vchan);
> +}
> +
> +static void sdma_free_desc(struct virt_dma_desc *vdesc)
> +{
> +#warning "Free the descriptors"
> +}
> +
> +struct sdma *sdma_alloc(struct device *dev,
> +			unsigned int channels,
> +			unsigned int requests,
> +			ssize_t lli_size,
> +			ssize_t priv_size)
> +{
> +	struct sdma *sdma;
> +	int ret, i;
> +
> +	sdma = devm_kzalloc(dev, sizeof(*sdma) + priv_size, GFP_KERNEL);
> +	if (!sdma) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	INIT_LIST_HEAD(&sdma->pend_reqs);
> +	INIT_LIST_HEAD(&sdma->avail_chans);
> +	spin_lock_init(&sdma->lock);
> +
> +	sdma->pool = dmam_pool_create(dev_name(dev), dev, lli_size, 4, 0);
> +	if (!sdma->pool) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	sdma->channels_nr = channels;
> +	sdma->channels = devm_kcalloc(dev, channels, sizeof(*sdma->channels), GFP_KERNEL);
> +	if (!sdma->channels) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < channels; i++) {
> +		struct sdma_channel *schan = &sdma->channels[i];
> +
> +		list_add_tail(&schan->node, &sdma->avail_chans);
> +		schan->index = i;
> +	}
> +
> +	sdma->requests_nr = requests;
> +	sdma->requests = devm_kcalloc(dev, requests, sizeof(*sdma->requests), GFP_KERNEL);
> +	if (!sdma->channels) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	INIT_LIST_HEAD(&sdma->ddev.channels);
> +
> +	for (i = 0; i < requests; i++) {
> +		struct sdma_request *sreq = &sdma->requests[i];
> +
> +		INIT_LIST_HEAD(&sreq->node);
> +		sreq->vchan.desc_free = sdma_free_desc;
> +		vchan_init(&sreq->vchan, &sdma->ddev);
> +	}
> +
> +	return sdma;
> +
> +out:
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(sdma_alloc);
> +
> +void sdma_free(struct sdma *sdma)
> +{
> +	return;
> +}
> +EXPORT_SYMBOL(sdma_free);
> +
> +int sdma_register(struct sdma *sdma,
> +		  struct sdma_ops *ops)
> +{
> +	struct dma_device *ddev = &sdma->ddev;
> +
> +	sdma->ops = ops;
> +
> +	ddev->device_config = sdma_config;
> +	ddev->device_tx_status = sdma_tx_status;
> +	ddev->device_issue_pending = sdma_issue_pending;
> +	ddev->device_free_chan_resources = sdma_free_chan_resources;
> +
> +	if (ops->channel_pause)
> +		ddev->device_pause = sdma_pause;
> +
> +	if (ops->channel_resume)
> +		ddev->device_resume = sdma_resume;
> +
> +	if (ops->channel_terminate)
> +		ddev->device_terminate_all = sdma_terminate;
> +
> +	if (dma_has_cap(DMA_SLAVE, ddev->cap_mask))
> +		ddev->device_prep_slave_sg = sdma_prep_slave_sg;
> +
> +	if (dma_has_cap(DMA_MEMCPY, ddev->cap_mask))
> +		ddev->device_prep_dma_memcpy = sdma_prep_memcpy;
> +
> +	dma_async_device_register(ddev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdma_register);
> +
> +int sdma_unregister(struct sdma *sdma)
> +{
> +	dma_async_device_unregister(&sdma->ddev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdma_unregister);
> diff --git a/drivers/dma/scheduled-dma.h b/drivers/dma/scheduled-dma.h
> new file mode 100644
> index 000000000000..d24c8143b2b6
> --- /dev/null
> +++ b/drivers/dma/scheduled-dma.h
> @@ -0,0 +1,140 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include "virt-dma.h"
> +
> +#ifndef _SCHEDULED_DMA_H_
> +#define _SCHEDULED_DMA_H_
> +
> +enum sdma_transfer_type {
> +	SDMA_TRANSFER_MEMCPY,
> +	SDMA_TRANSFER_SLAVE,
> +};
> +
> +enum sdma_report_status {
> +	SDMA_REPORT_CHUNK,
> +	SDMA_REPORT_TRANSFER,
> +};
> +
> +struct sdma_desc {
> +	struct virt_dma_desc	vdesc;
> +
> +	/* Entry point to our LLI */
> +	dma_addr_t		p_lli;
> +	void			*v_lli;
> +};
> +
> +struct sdma_channel {
> +	struct sdma_desc	*desc;
> +	unsigned int		index;
> +	struct list_head	node;
> +	void			*private;
> +};
> +
> +struct sdma_request {
> +	struct dma_slave_config	cfg;
> +	struct list_head	node;
> +	struct virt_dma_chan	vchan;
> +
> +	struct sdma_channel	*chan;
> +	void			*private;
> +};
> +
> +struct sdma_ops {
> +	/* LLI management operations */
> +	bool			(*lli_has_next)(void *v_lli);
> +	void			*(*lli_next)(void *v_lli);
> +	int			(*lli_init)(void *v_lli, void *sreq_priv,
> +					    enum sdma_transfer_type type,
> +					    enum dma_transfer_direction dir,
> +					    dma_addr_t src,
> +					    dma_addr_t dst, u32 len,
> +					    struct dma_slave_config *config);
> +	void			*(*lli_queue)(void *prev_v_lli, void *v_lli, dma_addr_t p_lli);
> +	size_t			(*lli_size)(void *v_lli);
> +
> +	/* Scheduler helper */
> +	struct sdma_request	*(*validate_request)(struct sdma_channel *chan,
> +						     struct sdma_request *req);
> +
> +	/* Transfer Management Functions */
> +	int			(*channel_pause)(struct sdma_channel *chan);
> +	int			(*channel_resume)(struct sdma_channel *chan);
> +	int			(*channel_start)(struct sdma_channel *chan, struct sdma_desc *sdesc);
> +	int			(*channel_terminate)(struct sdma_channel *chan);
> +	size_t			(*channel_residue)(struct sdma_channel *chan);
> +};
> +
> +struct sdma {
> +	struct dma_device	ddev;
> +	struct sdma_ops		*ops;
> +
> +	struct dma_pool		*pool;
> +
> +	struct sdma_channel	*channels;
> +	int			channels_nr;
> +	struct sdma_request	*requests;
> +	int			requests_nr;
> +
> +	struct list_head	avail_chans;
> +	struct list_head	pend_reqs;
> +
> +	spinlock_t		lock;
> +
> +	unsigned long		private[];
> +};
> +
> +static inline struct sdma *to_sdma(struct dma_device *d)
> +{
> +	return container_of(d, struct sdma, ddev);
> +}
> +
> +static inline struct sdma_request *to_sdma_request(struct dma_chan *chan)
> +{
> +	return container_of(chan, struct sdma_request, vchan.chan);
> +}
> +
> +static inline struct sdma_desc *to_sdma_desc(struct dma_async_tx_descriptor *tx)
> +{
> +	return container_of(tx, struct sdma_desc, vdesc.tx);
> +}
> +
> +static inline void *sdma_priv(struct sdma *sdma)
> +{
> +	return (void*)sdma->private;
> +}
> +
> +static inline void sdma_set_chan_private(struct sdma *sdma, void *ptr)
> +{
> +	int i;
> +
> +	for (i = 0; i < sdma->channels_nr; i++) {
> +		struct sdma_channel *schan = &sdma->channels[i];
> +
> +		schan->private = ptr;
> +	}
> +}
> +
> +struct sdma_desc *sdma_report(struct sdma *sdma,
> +			      struct sdma_channel *chan,
> +			      enum sdma_report_status status);
> +
> +struct sdma *sdma_alloc(struct device *dev,
> +			unsigned int channels,
> +			unsigned int requests,
> +			ssize_t lli_size,
> +			ssize_t priv_size);
> +void sdma_free(struct sdma *sdma);
> +
> +int sdma_register(struct sdma *sdma,
> +		  struct sdma_ops *ops);
> +int sdma_unregister(struct sdma *sdma);
> +
> +#endif /* _SCHEDULED_DMA_H_ */
> -- 
> 2.3.3
> 

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Vinod Koul]

On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:
> This framework aims at easing the development of dmaengine drivers by providing
> generic implementations of the functions usually required by dmaengine, while
> abstracting away most of the logic required.
> 
> It is very relevant for controllers that have more requests than channels,
> where you need to have some scheduling that is usually very bug prone, and
> shouldn't be implemented in each and every driver.
> 
> This introduces a new set of callbacks for drivers to implement the device
> specific behaviour. These new sets of callbacks aims at providing both how to
> handle channel related operations (start the transfer of a new descriptor,
> pause, resume, etc.) and the LLI related operations (iterator and various
> accessors).
> 
> So far, the only transfer types supported are memcpy and slave transfers, but
> eventually should support new transfer types as new drivers get converted.
A good direction indeed, few comments below...

> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/dma/Kconfig         |   4 +
>  drivers/dma/Makefile        |   1 +
>  drivers/dma/scheduled-dma.c | 571 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/dma/scheduled-dma.h | 140 +++++++++++
>  4 files changed, 716 insertions(+)
>  create mode 100644 drivers/dma/scheduled-dma.c
>  create mode 100644 drivers/dma/scheduled-dma.h
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index a874b6ec6650..032bf5fcd58b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -406,6 +406,7 @@ config DMA_SUN6I
>  	depends on RESET_CONTROLLER
>  	select DMA_ENGINE
>  	select DMA_VIRTUAL_CHANNELS
> +	select DMA_SCHEDULED
ummm, selecting without converting driver?
>  	help
>  	  Support for the DMA engine first found in Allwinner A31 SoCs.
>  
> @@ -431,6 +432,9 @@ config DMA_ENGINE
>  config DMA_VIRTUAL_CHANNELS
>  	tristate
>  
> +config DMA_SCHEDULED
> +	bool
it might make sense to do select DMA_VIRTUAL_CHANNELS but this is fine too.
> +
>  config DMA_ACPI
>  	def_bool y
>  	depends on ACPI
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index f915f61ec574..1db31814c749 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
>  obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
>  obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
>  obj-$(CONFIG_DMA_OF) += of-dma.o
> +obj-$(CONFIG_DMA_SCHEDULED) += scheduled-dma.o
>  
>  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
>  obj-$(CONFIG_DMATEST) += dmatest.o
> diff --git a/drivers/dma/scheduled-dma.c b/drivers/dma/scheduled-dma.c
> new file mode 100644
> index 000000000000..482d04f2ccbc
> --- /dev/null
> +++ b/drivers/dma/scheduled-dma.c
> @@ -0,0 +1,571 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/dmaengine.h>
> +#include <linux/dmapool.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +#include "scheduled-dma.h"
> +#include "virt-dma.h"
> +
> +static struct sdma_request *sdma_pop_queued_transfer(struct sdma *sdma,
> +						     struct sdma_channel *schan)
> +{
> +	struct sdma_request *sreq = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +
> +	/* No requests are awaiting an available channel */
> +	if (list_empty(&sdma->pend_reqs))
> +		goto out;
> +
> +	/*
> +	 * If we don't have any validate_request callback, any request
> +	 * can be dispatched to any channel.
> +	 *
> +	 * Remove the first entry and return it.
> +	 */
> +	if (!sdma->ops->validate_request) {
> +		sreq = list_first_entry(&sdma->pend_reqs,
> +					struct sdma_request, node);
> +		list_del_init(&sreq->node);
> +		goto out;
> +	}
> +
> +	list_for_each_entry(sreq, &sdma->pend_reqs, node) {
> +		/*
> +		 * Ask the driver to validate that the request can
> +		 * happen on the channel.
> +		 */
> +		if (sdma->ops->validate_request(schan, sreq)) {
> +			list_del_init(&sreq->node);
> +			goto out;
> +		}
> +
> +		/* Otherwise, just keep looping */
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +
> +	return sreq;
> +}
> +
> +/*
> + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> + *
> + * Flow normal:
> + *   - Election d'un pchan (Framework)
> + *   - Push d'un descripteur vers le pchan (Driver)
> + *   - idle....
> + *   - interrupt (driver)
> + *   - Transfert terminé, notification vers le framework (driver)
> + *   - Nouveau transfert dans la queue?
> + *     + Si oui, on est reparti
> + *     + Si non, on sort de l'interrupt, le pchan est libéré
Sorry cant understand this, this is actually bad for non french speakers

> + */
> +
> +struct sdma_desc *sdma_report(struct sdma *sdma,
> +			      struct sdma_channel *schan,
> +			      enum sdma_report_status status)
> +{
> +	struct sdma_desc *sdesc = NULL;
> +	struct virt_dma_desc *vdesc;
> +	struct sdma_request *sreq;
> +
> +	switch (status) {
> +	case SDMA_REPORT_TRANSFER:
> +		/*
> +		 * We're done with the current transfer that was in this
> +		 * physical channel.
> +		 */
> +		vchan_cookie_complete(&schan->desc->vdesc);
> +
> +		/*
> +		 * Now, try to see if there's any queued transfer
> +		 * awaiting an available channel.
> +		 *
> +		 * If not, just bail out, and mark the pchan as
> +		 * available.
> +		 *
> +		 * If there's such a transfer, validate that the
> +		 * driver can handle it, and ask it to do the
> +		 * transfer.
> +		 */
> +		sreq = sdma_pop_queued_transfer(sdma, schan);
> +		if (!sreq) {
> +			list_add_tail(&schan->node, &sdma->avail_chans);
> +			return NULL;
> +		}
> +
> +		/* Mark the request as assigned to a particular channel */
> +		sreq->chan = schan;
> +
> +		/* Retrieve the next transfer descriptor */
> +		vdesc = vchan_next_desc(&sreq->vchan);
> +		schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> +
> +		break;
> +
> +	default:
> +		break;
since you have only one case, the switch seems overkill here!
Or the plan is to get chunk support added eventually, but then...

> +	}
> +
> +	return sdesc;
> +}
> +EXPORT_SYMBOL_GPL(sdma_report);
> +
> +static enum dma_status sdma_tx_status(struct dma_chan *chan,
> +				      dma_cookie_t cookie,
> +				      struct dma_tx_state *state)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct virt_dma_desc *vd;
> +	struct sdma_desc *desc;
> +	enum dma_status ret;
> +	unsigned long flags;
> +	size_t bytes = 0;
> +	void *lli;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	ret = dma_cookie_status(chan, cookie, state);
> +	if (ret == DMA_COMPLETE)
> +		goto out;
> +
> +	vd = vchan_find_desc(&sreq->vchan, cookie);
> +	desc = to_sdma_desc(&vd->tx);
> +
> +	if (vd) {
> +		lli = desc->v_lli;
> +		while (true) {
> +			bytes += sdma->ops->lli_size(lli);
> +
> +			if (!sdma->ops->lli_has_next(lli))
> +				break;
> +
> +			lli = sdma->ops->lli_next(lli);
wouldn't it be nicer to just use a link list here which is embedded in a generic
struct which in turn can be embedded in a driver specific one, so we actually
don't care what driver specific implementation is... that way we cna get rid
of lli_size and lli_has_next and lli_next ones. Further that will allow
drivers/framework to break a txn into multiple smaller txns internally.

> +		}
> +	} else if (chan) {
> +		bytes = sdma->ops->channel_residue(schan);
> +	}
> +
> +	dma_set_residue(state, bytes);
> +
> +out:
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return ret;
> +};
> +
> +static int sdma_config(struct dma_chan *chan,
> +		       struct dma_slave_config *config)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +	memcpy(&sreq->cfg, config, sizeof(*config));
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_pause(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel, just
> +	 * pause the channel.
> +	 *
> +	 * If not, remove the request from the pending list.
The current API pauses a channel, not a request, so this part isn't clear to
me, cna you pls explain...

> +	 */
> +	if (schan) {
> +		sdma->ops->channel_pause(schan);
make sure you document this and rest of the calls can be invoked with lock
held!
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_del_init(&sreq->node);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_resume(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel, just
> +	 * resume the channel.
> +	 *
> +	 * If not, add the request from the pending list.
same here too
> +	 */
> +	if (schan) {
> +		sdma->ops->channel_resume(schan);
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_add_tail(&sreq->node, &sdma->pend_reqs);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sdma_terminate(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma_channel *schan = sreq->chan;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * If the request is currently scheduled on a channel,
> +	 * terminate the channel.
> +	 *
> +	 * If not, prevent the request from being scheduled.
> +	 */
> +	if (schan) {
> +		sdma->ops->channel_terminate(schan);
> +	} else {
> +		spin_lock(&sdma->lock);
> +		list_del_init(&sreq->node);
> +		spin_unlock(&sdma->lock);
> +	}
> +
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	/*
> +	 * Flush all the pending descriptors from our vchan
> +	 */
> +	vchan_free_chan_resources(&sreq->vchan);
I think cleanup is not right we need more work here.
> +
> +	return 0;
> +}
> +
> +
> +static struct dma_async_tx_descriptor *sdma_prep_memcpy(struct dma_chan *chan,
> +							dma_addr_t dest, dma_addr_t src,
> +							size_t len, unsigned long flags)
> +{
> +	struct sdma_request *req = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct sdma_desc *desc;
> +	dma_addr_t p_lli;
> +	void *v_lli;
> +
> +	if (!len)
> +		return NULL;
> +
> +	/* Allocate our representation of a descriptor */
> +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> +	if (!desc)
> +		return NULL;
> +
> +	v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> +	if (!v_lli)
> +		goto err_desc_free;
> +
> +	/* Ask the driver to initialise its hardware descriptor */
> +	if (sdma->ops->lli_init(v_lli, req->private,
> +				SDMA_TRANSFER_MEMCPY,
> +				DMA_MEM_TO_MEM, src, dest, len,
> +				NULL))
> +		goto err_lli_free;
> +
> +	/* Create our single item LLI */
> +	sdma->ops->lli_queue(NULL, v_lli, p_lli);
> +	desc->p_lli = p_lli;
> +	desc->v_lli = v_lli;
> +
> +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
typically driver would do calculations for descriptor setting up the
registers values which cna be programmed, so a driver callback would be nice
here. That way lesser work in start function and which is good as I am
assuming start should be done from irq context

Also this needs to take into account the dma_slave_config, which is allowed
to me modified for subsequent txns
> +
> +err_lli_free:
> +	dma_pool_free(sdma->pool, v_lli, p_lli);
> +err_desc_free:
> +	kfree(desc);
> +
> +	return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *sdma_prep_slave_sg(struct dma_chan *chan,
> +							  struct scatterlist *sgl,
> +							  unsigned int sg_len,
> +							  enum dma_transfer_direction dir,
> +							  unsigned long flags, void *context)
> +{
> +	struct sdma_request *req = to_sdma_request(chan);
> +	struct dma_slave_config *config = &req->cfg;
> +	struct sdma *sdma = to_sdma(chan->device);
> +	void *v_lli, *prev_v_lli = NULL;
> +	struct scatterlist *sg;
> +	struct sdma_desc *desc;
> +	dma_addr_t p_lli;
> +	int i;
> +
> +	if (!sgl || !sg_len)
> +		return NULL;
> +
> +	/* Allocate our representation of a descriptor */
> +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> +	if (!desc)
> +		return NULL;
> +
> +	/*
> +	 * For each scatter list entry, build up our representation of
> +	 * the LLI, and ask the driver to create its hardware
> +	 * descriptor.
> +	 */
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> +		if (!v_lli)
> +			goto err_lli_free;
> +
> +		/* Ask the driver to initialise its hardware descriptor */
> +		if (sdma->ops->lli_init(v_lli, req->private,
> +					SDMA_TRANSFER_SLAVE,
> +					dir, sg_dma_address(sg),
> +					config->dst_addr, sg_dma_len(sg),
> +					config))
> +			goto err_lli_free;
> +
> +		/*
> +		 * If it's our first item, initialise our descriptor
> +		 * pointers to the lli.
> +		 *
> +		 * Otherwise, queue it to the end of the LLI.
> +		 */
> +		if (!prev_v_lli) {
> +			desc->p_lli = p_lli;
> +			desc->v_lli = v_lli;
> +			prev_v_lli = v_lli;
> +		} else {
> +			/* And to queue it at the end of its hardware LLI */
> +			prev_v_lli = sdma->ops->lli_queue(prev_v_lli, v_lli, p_lli);
> +		}
> +	}
> +
> +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
> +
> +err_lli_free:
> +#warning "Free the LLI"
why a non std preprocessor warning

> +
> +	kfree(desc);
> +	return NULL;
> +}
> +
> +static void sdma_issue_pending(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	struct sdma *sdma = to_sdma(chan->device);
> +	struct virt_dma_desc *vdesc;
> +	struct sdma_channel *schan;
> +	struct sdma_desc *sdesc;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +
> +	/* See if we have anything to do */
> +	if (!vchan_issue_pending(&sreq->vchan))
> +		goto out_chan_unlock;
> +
> +	/* Is some work in progress already? */
> +	if (sreq->chan)
> +		goto out_chan_unlock;
> +
> +	spin_lock(&sdma->lock);
> +
> +	/* Is there an available channel */
> +	if (list_empty(&sdma->avail_chans))
> +		goto out_main_unlock;
> +
> +	/*
> +	 * If there's no validate_request callback, it means that all
> +	 * channels can transfer any request. Pick the first available
> +	 * channel.
> +	 *
> +	 * Otherwise, iterate over all the pending channels and call
> +	 * validate_request.
> +	 */
> +	if (!sdma->ops->validate_request) {
> +		schan = list_first_entry(&sdma->avail_chans,
> +					 struct sdma_channel, node);
> +	} else {
> +		list_for_each_entry(schan, &sdma->avail_chans, node) {
> +			if (sdma->ops->validate_request(schan, sreq)) {
> +				list_del_init(&schan->node);
> +				break;
> +			}
> +		}
> +	}
why not call sdma_pop_queued_transfer() ?
> +
> +	if (!schan)
> +		goto out_main_unlock;
> +
> +	sreq->chan = schan;
> +
> +	/* Retrieve the next transfer descriptor */
> +	vdesc = vchan_next_desc(&sreq->vchan);
> +	schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> +
> +	sdma->ops->channel_start(schan, sdesc);
> +
> +out_main_unlock:
> +	spin_unlock(&sdma->lock);
> +out_chan_unlock:
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +}
> +
> +static void sdma_free_chan_resources(struct dma_chan *chan)
> +{
> +	struct sdma_request *sreq = to_sdma_request(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> +	list_del_init(&sreq->node);
> +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> +
> +	vchan_free_chan_resources(&sreq->vchan);
> +}
> +
> +static void sdma_free_desc(struct virt_dma_desc *vdesc)
> +{
> +#warning "Free the descriptors"
??

> +}
> +
> +struct sdma *sdma_alloc(struct device *dev,
> +			unsigned int channels,
> +			unsigned int requests,
> +			ssize_t lli_size,
> +			ssize_t priv_size)
> +{
> +	struct sdma *sdma;
> +	int ret, i;
> +
> +	sdma = devm_kzalloc(dev, sizeof(*sdma) + priv_size, GFP_KERNEL);
> +	if (!sdma) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	INIT_LIST_HEAD(&sdma->pend_reqs);
> +	INIT_LIST_HEAD(&sdma->avail_chans);
> +	spin_lock_init(&sdma->lock);
> +
> +	sdma->pool = dmam_pool_create(dev_name(dev), dev, lli_size, 4, 0);
> +	if (!sdma->pool) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	sdma->channels_nr = channels;
> +	sdma->channels = devm_kcalloc(dev, channels, sizeof(*sdma->channels), GFP_KERNEL);
> +	if (!sdma->channels) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < channels; i++) {
> +		struct sdma_channel *schan = &sdma->channels[i];
> +
> +		list_add_tail(&schan->node, &sdma->avail_chans);
> +		schan->index = i;
> +	}
> +
> +	sdma->requests_nr = requests;
> +	sdma->requests = devm_kcalloc(dev, requests, sizeof(*sdma->requests), GFP_KERNEL);
> +	if (!sdma->channels) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	INIT_LIST_HEAD(&sdma->ddev.channels);
> +
> +	for (i = 0; i < requests; i++) {
> +		struct sdma_request *sreq = &sdma->requests[i];
> +
> +		INIT_LIST_HEAD(&sreq->node);
> +		sreq->vchan.desc_free = sdma_free_desc;
> +		vchan_init(&sreq->vchan, &sdma->ddev);
> +	}
> +
> +	return sdma;
> +
> +out:
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(sdma_alloc);
> +
> +void sdma_free(struct sdma *sdma)
> +{
> +	return;
> +}
> +EXPORT_SYMBOL(sdma_free);
> +
> +int sdma_register(struct sdma *sdma,
> +		  struct sdma_ops *ops)
> +{
> +	struct dma_device *ddev = &sdma->ddev;
> +
> +	sdma->ops = ops;
> +
> +	ddev->device_config = sdma_config;
> +	ddev->device_tx_status = sdma_tx_status;
> +	ddev->device_issue_pending = sdma_issue_pending;
> +	ddev->device_free_chan_resources = sdma_free_chan_resources;
> +
> +	if (ops->channel_pause)
> +		ddev->device_pause = sdma_pause;
> +
> +	if (ops->channel_resume)
> +		ddev->device_resume = sdma_resume;
> +
> +	if (ops->channel_terminate)
> +		ddev->device_terminate_all = sdma_terminate;
> +
> +	if (dma_has_cap(DMA_SLAVE, ddev->cap_mask))
> +		ddev->device_prep_slave_sg = sdma_prep_slave_sg;
> +
> +	if (dma_has_cap(DMA_MEMCPY, ddev->cap_mask))
> +		ddev->device_prep_dma_memcpy = sdma_prep_memcpy;
> +
> +	dma_async_device_register(ddev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdma_register);
> +
> +int sdma_unregister(struct sdma *sdma)
> +{
> +	dma_async_device_unregister(&sdma->ddev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdma_unregister);
> diff --git a/drivers/dma/scheduled-dma.h b/drivers/dma/scheduled-dma.h
> new file mode 100644
> index 000000000000..d24c8143b2b6
> --- /dev/null
> +++ b/drivers/dma/scheduled-dma.h
> @@ -0,0 +1,140 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include "virt-dma.h"
> +
> +#ifndef _SCHEDULED_DMA_H_
> +#define _SCHEDULED_DMA_H_
> +
> +enum sdma_transfer_type {
> +	SDMA_TRANSFER_MEMCPY,
> +	SDMA_TRANSFER_SLAVE,
> +};
> +
> +enum sdma_report_status {
> +	SDMA_REPORT_CHUNK,
> +	SDMA_REPORT_TRANSFER,
> +};
can you pls add kernel style documentation for these and below.
> +
> +struct sdma_desc {
> +	struct virt_dma_desc	vdesc;
> +
> +	/* Entry point to our LLI */
> +	dma_addr_t		p_lli;
> +	void			*v_lli;
> +};
> +
> +struct sdma_channel {
> +	struct sdma_desc	*desc;
> +	unsigned int		index;
> +	struct list_head	node;
> +	void			*private;
> +};
> +
> +struct sdma_request {
> +	struct dma_slave_config	cfg;
> +	struct list_head	node;
> +	struct virt_dma_chan	vchan;
> +
> +	struct sdma_channel	*chan;
> +	void			*private;
> +};
> +
> +struct sdma_ops {
> +	/* LLI management operations */
> +	bool			(*lli_has_next)(void *v_lli);
> +	void			*(*lli_next)(void *v_lli);
> +	int			(*lli_init)(void *v_lli, void *sreq_priv,
> +					    enum sdma_transfer_type type,
> +					    enum dma_transfer_direction dir,
> +					    dma_addr_t src,
> +					    dma_addr_t dst, u32 len,
> +					    struct dma_slave_config *config);
> +	void			*(*lli_queue)(void *prev_v_lli, void *v_lli, dma_addr_t p_lli);
> +	size_t			(*lli_size)(void *v_lli);
if we can manage these using the standard list then it would simplify the
code a lot as well

> +
> +	/* Scheduler helper */
> +	struct sdma_request	*(*validate_request)(struct sdma_channel *chan,
> +						     struct sdma_request *req);
> +
> +	/* Transfer Management Functions */
> +	int			(*channel_pause)(struct sdma_channel *chan);
> +	int			(*channel_resume)(struct sdma_channel *chan);
> +	int			(*channel_start)(struct sdma_channel *chan, struct sdma_desc *sdesc);
> +	int			(*channel_terminate)(struct sdma_channel *chan);
> +	size_t			(*channel_residue)(struct sdma_channel *chan);
> +};
> +
> +struct sdma {
> +	struct dma_device	ddev;
> +	struct sdma_ops		*ops;
> +
> +	struct dma_pool		*pool;
> +
> +	struct sdma_channel	*channels;
> +	int			channels_nr;
> +	struct sdma_request	*requests;
> +	int			requests_nr;
> +
> +	struct list_head	avail_chans;
> +	struct list_head	pend_reqs;
> +
> +	spinlock_t		lock;
> +
> +	unsigned long		private[];
> +};
> +
> +static inline struct sdma *to_sdma(struct dma_device *d)
> +{
> +	return container_of(d, struct sdma, ddev);
> +}
> +
> +static inline struct sdma_request *to_sdma_request(struct dma_chan *chan)
> +{
> +	return container_of(chan, struct sdma_request, vchan.chan);
> +}
> +
> +static inline struct sdma_desc *to_sdma_desc(struct dma_async_tx_descriptor *tx)
> +{
> +	return container_of(tx, struct sdma_desc, vdesc.tx);
> +}
> +
> +static inline void *sdma_priv(struct sdma *sdma)
> +{
> +	return (void*)sdma->private;
> +}
> +
> +static inline void sdma_set_chan_private(struct sdma *sdma, void *ptr)
> +{
> +	int i;
> +
> +	for (i = 0; i < sdma->channels_nr; i++) {
> +		struct sdma_channel *schan = &sdma->channels[i];
> +
> +		schan->private = ptr;
> +	}
> +}
> +
> +struct sdma_desc *sdma_report(struct sdma *sdma,
> +			      struct sdma_channel *chan,
> +			      enum sdma_report_status status);
> +
> +struct sdma *sdma_alloc(struct device *dev,
> +			unsigned int channels,
> +			unsigned int requests,
> +			ssize_t lli_size,
> +			ssize_t priv_size);
> +void sdma_free(struct sdma *sdma);
> +
> +int sdma_register(struct sdma *sdma,
> +		  struct sdma_ops *ops);
> +int sdma_unregister(struct sdma *sdma);
> +
> +#endif /* _SCHEDULED_DMA_H_ */
> -- 
> 2.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Mon, Mar 23, 2015 at 10:15:03AM +0100, Ludovic Desroches wrote:
> On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:
> 
> [...]
> 
> > +/*
> > + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> > + *
> > + * Flow normal:
> > + *   - Election d'un pchan (Framework)
> > + *   - Push d'un descripteur vers le pchan (Driver)
> > + *   - idle....
> > + *   - interrupt (driver)
> > + *   - Transfert terminé, notification vers le framework (driver)
> > + *   - Nouveau transfert dans la queue?
> > + *     + Si oui, on est reparti
> > + *     + Si non, on sort de l'interrupt, le pchan est libéré
> > + */
> 
> As I am French too, I totally agree to translate comments in French but
> I am not sure it will satisfy everyone :)

Damn.... :)

I actually noticed it after sending the patches, and was hoping it
would go unnoticed, unfortunately, it hasn't :)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 10791 bytes --]

Hi Ludo,

On Mon, Mar 23, 2015 at 02:25:13PM +0100, Ludovic Desroches wrote:
> On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:
> > This framework aims at easing the development of dmaengine drivers by providing
> > generic implementations of the functions usually required by dmaengine, while
> > abstracting away most of the logic required.
> 
> For sure it will ease writing new dmaengine drivers! Moreover, adopting
> this framework will convert the driver to use virtual channels isn't it?

Yes, it relies on virt-dma

> > It is very relevant for controllers that have more requests than channels,
> > where you need to have some scheduling that is usually very bug prone, and
> > shouldn't be implemented in each and every driver.
> > 
> > This introduces a new set of callbacks for drivers to implement the device
> > specific behaviour. These new sets of callbacks aims at providing both how to
> > handle channel related operations (start the transfer of a new descriptor,
> > pause, resume, etc.) and the LLI related operations (iterator and various
> > accessors).
> > 
> > So far, the only transfer types supported are memcpy and slave transfers, but
> > eventually should support new transfer types as new drivers get converted.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/dma/Kconfig         |   4 +
> >  drivers/dma/Makefile        |   1 +
> >  drivers/dma/scheduled-dma.c | 571 ++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/dma/scheduled-dma.h | 140 +++++++++++
> >  4 files changed, 716 insertions(+)
> >  create mode 100644 drivers/dma/scheduled-dma.c
> >  create mode 100644 drivers/dma/scheduled-dma.h
> > 
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index a874b6ec6650..032bf5fcd58b 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -406,6 +406,7 @@ config DMA_SUN6I
> >  	depends on RESET_CONTROLLER
> >  	select DMA_ENGINE
> >  	select DMA_VIRTUAL_CHANNELS
> > +	select DMA_SCHEDULED
> >  	help
> >  	  Support for the DMA engine first found in Allwinner A31 SoCs.
> >  
> > @@ -431,6 +432,9 @@ config DMA_ENGINE
> >  config DMA_VIRTUAL_CHANNELS
> >  	tristate
> >  
> > +config DMA_SCHEDULED
> > +	bool
> > +
> 
> I think, it should select DMA_VIRTUAL_CHANNELS

Yep, indeed.

> >  config DMA_ACPI
> >  	def_bool y
> >  	depends on ACPI
> > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > index f915f61ec574..1db31814c749 100644
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
> >  obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
> >  obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
> >  obj-$(CONFIG_DMA_OF) += of-dma.o
> > +obj-$(CONFIG_DMA_SCHEDULED) += scheduled-dma.o
> >  
> >  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
> >  obj-$(CONFIG_DMATEST) += dmatest.o
> > diff --git a/drivers/dma/scheduled-dma.c b/drivers/dma/scheduled-dma.c
> > new file mode 100644
> > index 000000000000..482d04f2ccbc
> > --- /dev/null
> > +++ b/drivers/dma/scheduled-dma.c
> > @@ -0,0 +1,571 @@
> > +/*
> > + * Copyright (C) 2015 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/dmaengine.h>
> > +#include <linux/dmapool.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +
> > +#include "scheduled-dma.h"
> > +#include "virt-dma.h"
> > +
> > +static struct sdma_request *sdma_pop_queued_transfer(struct sdma *sdma,
> > +						     struct sdma_channel *schan)
> > +{
> > +	struct sdma_request *sreq = NULL;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sdma->lock, flags);
> > +
> > +	/* No requests are awaiting an available channel */
> > +	if (list_empty(&sdma->pend_reqs))
> > +		goto out;
> > +
> > +	/*
> > +	 * If we don't have any validate_request callback, any request
> > +	 * can be dispatched to any channel.
> > +	 *
> > +	 * Remove the first entry and return it.
> > +	 */
> > +	if (!sdma->ops->validate_request) {
> > +		sreq = list_first_entry(&sdma->pend_reqs,
> > +					struct sdma_request, node);
> > +		list_del_init(&sreq->node);
> > +		goto out;
> > +	}
> > +
> > +	list_for_each_entry(sreq, &sdma->pend_reqs, node) {
> > +		/*
> > +		 * Ask the driver to validate that the request can
> > +		 * happen on the channel.
> > +		 */
> > +		if (sdma->ops->validate_request(schan, sreq)) {
> > +			list_del_init(&sreq->node);
> > +			goto out;
> > +		}
> > +
> > +		/* Otherwise, just keep looping */
> > +	}
> > +
> > +out:
> > +	spin_unlock_irqrestore(&sdma->lock, flags);
> > +
> > +	return sreq;
> > +}
> > +
> > +/*
> > + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> > + *
> > + * Flow normal:
> > + *   - Election d'un pchan (Framework)
> > + *   - Push d'un descripteur vers le pchan (Driver)
> > + *   - idle....
> > + *   - interrupt (driver)
> > + *   - Transfert terminé, notification vers le framework (driver)
> > + *   - Nouveau transfert dans la queue?
> > + *     + Si oui, on est reparti
> > + *     + Si non, on sort de l'interrupt, le pchan est libéré
> > + */
> > +
> > +struct sdma_desc *sdma_report(struct sdma *sdma,
> > +			      struct sdma_channel *schan,
> > +			      enum sdma_report_status status)
> > +{
> > +	struct sdma_desc *sdesc = NULL;
> > +	struct virt_dma_desc *vdesc;
> > +	struct sdma_request *sreq;
> > +
> > +	switch (status) {
> > +	case SDMA_REPORT_TRANSFER:
> > +		/*
> > +		 * We're done with the current transfer that was in this
> > +		 * physical channel.
> > +		 */
> > +		vchan_cookie_complete(&schan->desc->vdesc);
> > +
> > +		/*
> > +		 * Now, try to see if there's any queued transfer
> > +		 * awaiting an available channel.
> > +		 *
> > +		 * If not, just bail out, and mark the pchan as
> > +		 * available.
> > +		 *
> > +		 * If there's such a transfer, validate that the
> > +		 * driver can handle it, and ask it to do the
> > +		 * transfer.
> > +		 */
> > +		sreq = sdma_pop_queued_transfer(sdma, schan);
> > +		if (!sreq) {
> > +			list_add_tail(&schan->node, &sdma->avail_chans);
> > +			return NULL;
> > +		}
> > +
> > +		/* Mark the request as assigned to a particular channel */
> > +		sreq->chan = schan;
> > +
> > +		/* Retrieve the next transfer descriptor */
> > +		vdesc = vchan_next_desc(&sreq->vchan);
> > +		schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> > +
> > +		break;
> > +
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return sdesc;
> > +}
> > +EXPORT_SYMBOL_GPL(sdma_report);
> > +
> > +static enum dma_status sdma_tx_status(struct dma_chan *chan,
> > +				      dma_cookie_t cookie,
> > +				      struct dma_tx_state *state)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct virt_dma_desc *vd;
> > +	struct sdma_desc *desc;
> > +	enum dma_status ret;
> > +	unsigned long flags;
> > +	size_t bytes = 0;
> > +	void *lli;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	ret = dma_cookie_status(chan, cookie, state);
> > +	if (ret == DMA_COMPLETE)
> > +		goto out;
> > +
> > +	vd = vchan_find_desc(&sreq->vchan, cookie);
> > +	desc = to_sdma_desc(&vd->tx);
> > +
> > +	if (vd) {
> > +		lli = desc->v_lli;
> > +		while (true) {
> > +			bytes += sdma->ops->lli_size(lli);
> > +
> > +			if (!sdma->ops->lli_has_next(lli))
> > +				break;
> > +
> > +			lli = sdma->ops->lli_next(lli);
> > +		}
> > +	} else if (chan) {
> > +		bytes = sdma->ops->channel_residue(schan);
> > +	}
> > +
> > +	dma_set_residue(state, bytes);
> > +
> > +out:
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return ret;
> > +};
> > +
> > +static int sdma_config(struct dma_chan *chan,
> > +		       struct dma_slave_config *config)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +	memcpy(&sreq->cfg, config, sizeof(*config));
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sdma_pause(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * If the request is currently scheduled on a channel, just
> > +	 * pause the channel.
> > +	 *
> > +	 * If not, remove the request from the pending list.
> > +	 */
> > +	if (schan) {
> > +		sdma->ops->channel_pause(schan);
> > +	} else {
> > +		spin_lock(&sdma->lock);
> > +		list_del_init(&sreq->node);
> > +		spin_unlock(&sdma->lock);
> > +	}
> > +
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sdma_resume(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * If the request is currently scheduled on a channel, just
> > +	 * resume the channel.
> > +	 *
> > +	 * If not, add the request from the pending list.
> > +	 */
> 
> Is such a case possible?

Yes, it's possible, if you have more requests than channels, and all
channels are currently busy with other transfers, we can end up in a
case where we have a requests where issue_pending has been called, but
is not currently scheduled on any channel, for an unknown amount of
time.

In such a case, you might very well end up with clients calling
pause/resume on these requests.

> The request should be already in the pending list, isn't it?

Not really, it would have been removed by sdma_pause.

> By the way, I may have missed something but where do you add a
> request to the pending list? I have seen it only in the resume case.

It should be in issue_pending and pause. If not, it's a bug :)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 26556 bytes --]

On Mon, Mar 23, 2015 at 10:08:43PM +0530, Vinod Koul wrote:
> On Sat, Mar 21, 2015 at 12:42:06PM -0700, Maxime Ripard wrote:
> > This framework aims at easing the development of dmaengine drivers by providing
> > generic implementations of the functions usually required by dmaengine, while
> > abstracting away most of the logic required.
> > 
> > It is very relevant for controllers that have more requests than channels,
> > where you need to have some scheduling that is usually very bug prone, and
> > shouldn't be implemented in each and every driver.
> > 
> > This introduces a new set of callbacks for drivers to implement the device
> > specific behaviour. These new sets of callbacks aims at providing both how to
> > handle channel related operations (start the transfer of a new descriptor,
> > pause, resume, etc.) and the LLI related operations (iterator and various
> > accessors).
> > 
> > So far, the only transfer types supported are memcpy and slave transfers, but
> > eventually should support new transfer types as new drivers get converted.
> A good direction indeed, few comments below...
> 
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/dma/Kconfig         |   4 +
> >  drivers/dma/Makefile        |   1 +
> >  drivers/dma/scheduled-dma.c | 571 ++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/dma/scheduled-dma.h | 140 +++++++++++
> >  4 files changed, 716 insertions(+)
> >  create mode 100644 drivers/dma/scheduled-dma.c
> >  create mode 100644 drivers/dma/scheduled-dma.h
> > 
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index a874b6ec6650..032bf5fcd58b 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -406,6 +406,7 @@ config DMA_SUN6I
> >  	depends on RESET_CONTROLLER
> >  	select DMA_ENGINE
> >  	select DMA_VIRTUAL_CHANNELS
> > +	select DMA_SCHEDULED
> ummm, selecting without converting driver?

Yeah, that should be in the second patch.

> >  	help
> >  	  Support for the DMA engine first found in Allwinner A31 SoCs.
> >  
> > @@ -431,6 +432,9 @@ config DMA_ENGINE
> >  config DMA_VIRTUAL_CHANNELS
> >  	tristate
> >  
> > +config DMA_SCHEDULED
> > +	bool
>
> it might make sense to do select DMA_VIRTUAL_CHANNELS but this is fine too.

Yep, indeed.

> > +
> >  config DMA_ACPI
> >  	def_bool y
> >  	depends on ACPI
> > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > index f915f61ec574..1db31814c749 100644
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
> >  obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
> >  obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
> >  obj-$(CONFIG_DMA_OF) += of-dma.o
> > +obj-$(CONFIG_DMA_SCHEDULED) += scheduled-dma.o
> >  
> >  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
> >  obj-$(CONFIG_DMATEST) += dmatest.o
> > diff --git a/drivers/dma/scheduled-dma.c b/drivers/dma/scheduled-dma.c
> > new file mode 100644
> > index 000000000000..482d04f2ccbc
> > --- /dev/null
> > +++ b/drivers/dma/scheduled-dma.c
> > @@ -0,0 +1,571 @@
> > +/*
> > + * Copyright (C) 2015 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/dmaengine.h>
> > +#include <linux/dmapool.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +
> > +#include "scheduled-dma.h"
> > +#include "virt-dma.h"
> > +
> > +static struct sdma_request *sdma_pop_queued_transfer(struct sdma *sdma,
> > +						     struct sdma_channel *schan)
> > +{
> > +	struct sdma_request *sreq = NULL;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sdma->lock, flags);
> > +
> > +	/* No requests are awaiting an available channel */
> > +	if (list_empty(&sdma->pend_reqs))
> > +		goto out;
> > +
> > +	/*
> > +	 * If we don't have any validate_request callback, any request
> > +	 * can be dispatched to any channel.
> > +	 *
> > +	 * Remove the first entry and return it.
> > +	 */
> > +	if (!sdma->ops->validate_request) {
> > +		sreq = list_first_entry(&sdma->pend_reqs,
> > +					struct sdma_request, node);
> > +		list_del_init(&sreq->node);
> > +		goto out;
> > +	}
> > +
> > +	list_for_each_entry(sreq, &sdma->pend_reqs, node) {
> > +		/*
> > +		 * Ask the driver to validate that the request can
> > +		 * happen on the channel.
> > +		 */
> > +		if (sdma->ops->validate_request(schan, sreq)) {
> > +			list_del_init(&sreq->node);
> > +			goto out;
> > +		}
> > +
> > +		/* Otherwise, just keep looping */
> > +	}
> > +
> > +out:
> > +	spin_unlock_irqrestore(&sdma->lock, flags);
> > +
> > +	return sreq;
> > +}
> > +
> > +/*
> > + * Besoin d'une fonction pour pusher un descriptor vers un pchan
> > + *
> > + * Flow normal:
> > + *   - Election d'un pchan (Framework)
> > + *   - Push d'un descripteur vers le pchan (Driver)
> > + *   - idle....
> > + *   - interrupt (driver)
> > + *   - Transfert terminé, notification vers le framework (driver)
> > + *   - Nouveau transfert dans la queue?
> > + *     + Si oui, on est reparti
> > + *     + Si non, on sort de l'interrupt, le pchan est libéré
>
> Sorry cant understand this, this is actually bad for non french speakers

My bad, it was some early french comments that I forgot to
remove. This is obviously not something that should be merged.

> > + */
> > +
> > +struct sdma_desc *sdma_report(struct sdma *sdma,
> > +			      struct sdma_channel *schan,
> > +			      enum sdma_report_status status)
> > +{
> > +	struct sdma_desc *sdesc = NULL;
> > +	struct virt_dma_desc *vdesc;
> > +	struct sdma_request *sreq;
> > +
> > +	switch (status) {
> > +	case SDMA_REPORT_TRANSFER:
> > +		/*
> > +		 * We're done with the current transfer that was in this
> > +		 * physical channel.
> > +		 */
> > +		vchan_cookie_complete(&schan->desc->vdesc);
> > +
> > +		/*
> > +		 * Now, try to see if there's any queued transfer
> > +		 * awaiting an available channel.
> > +		 *
> > +		 * If not, just bail out, and mark the pchan as
> > +		 * available.
> > +		 *
> > +		 * If there's such a transfer, validate that the
> > +		 * driver can handle it, and ask it to do the
> > +		 * transfer.
> > +		 */
> > +		sreq = sdma_pop_queued_transfer(sdma, schan);
> > +		if (!sreq) {
> > +			list_add_tail(&schan->node, &sdma->avail_chans);
> > +			return NULL;
> > +		}
> > +
> > +		/* Mark the request as assigned to a particular channel */
> > +		sreq->chan = schan;
> > +
> > +		/* Retrieve the next transfer descriptor */
> > +		vdesc = vchan_next_desc(&sreq->vchan);
> > +		schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> > +
> > +		break;
> > +
> > +	default:
> > +		break;
>
> since you have only one case, the switch seems overkill here!
> Or the plan is to get chunk support added eventually, but then...

The plan is also to support other reporting points. But it might not
really make sense, and we might need separate functions for different
reporting points. I don't really know what makes the more sense here,
and yeah, we might just have a function to report the end of a
transfer, and start from there until we have more use cases.
 
> > +	}
> > +
> > +	return sdesc;
> > +}
> > +EXPORT_SYMBOL_GPL(sdma_report);
> > +
> > +static enum dma_status sdma_tx_status(struct dma_chan *chan,
> > +				      dma_cookie_t cookie,
> > +				      struct dma_tx_state *state)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct virt_dma_desc *vd;
> > +	struct sdma_desc *desc;
> > +	enum dma_status ret;
> > +	unsigned long flags;
> > +	size_t bytes = 0;
> > +	void *lli;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	ret = dma_cookie_status(chan, cookie, state);
> > +	if (ret == DMA_COMPLETE)
> > +		goto out;
> > +
> > +	vd = vchan_find_desc(&sreq->vchan, cookie);
> > +	desc = to_sdma_desc(&vd->tx);
> > +
> > +	if (vd) {
> > +		lli = desc->v_lli;
> > +		while (true) {
> > +			bytes += sdma->ops->lli_size(lli);
> > +
> > +			if (!sdma->ops->lli_has_next(lli))
> > +				break;
> > +
> > +			lli = sdma->ops->lli_next(lli);
>
> wouldn't it be nicer to just use a link list here which is embedded
> in a generic struct which in turn can be embedded in a driver
> specific one, so we actually don't care what driver specific
> implementation is...

I'm not sure to get what you mean here. We will always have to care
about the driver specific implementation.

> that way we cna get rid of lli_size and lli_has_next and lli_next
> ones. Further that will allow drivers/framework to break a txn into
> multiple smaller txns internally.

I chose this accessors/iterators pattern because it actually make this
very easy to add new features on the LLI (like debugging, we just have
to add a new lli_dump) and that works.

We can also manage more easily the LLI, by allocating ourself the LLI
items, and freeing it afterwards, which is something that also is
often wrong, or at least addressed in different ways from one driver
to another (for example, the Atmel drivers have their own descriptor
allocation logic, while the others mostly rely on the dma_pools. Which
one is right, I don't know, but the right one should be shared by
everyone).

That would also be much easier to add software-emulated LLI, since you
just have to set the lli management functions to some common library
functions, and we're done.

I feel like it would be much more flexible, and would remove logic
from the driver itself, which is the point of this whole
"sub-framework".

> > +		}
> > +	} else if (chan) {
> > +		bytes = sdma->ops->channel_residue(schan);
> > +	}
> > +
> > +	dma_set_residue(state, bytes);
> > +
> > +out:
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return ret;
> > +};
> > +
> > +static int sdma_config(struct dma_chan *chan,
> > +		       struct dma_slave_config *config)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +	memcpy(&sreq->cfg, config, sizeof(*config));
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sdma_pause(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * If the request is currently scheduled on a channel, just
> > +	 * pause the channel.
> > +	 *
> > +	 * If not, remove the request from the pending list.
>
> The current API pauses a channel, not a request, so this part isn't clear to
> me, cna you pls explain...

Yeah, the point is that it exposes as much dma_chan as there is
requests. You might have less physical channels in your dma
controller, in which case the client drivers would have a dma_chan
that is not mapped 1:1 to a physical channel.

That means that the dma_chan you're pausing (and resuming,
terminating, etc.) might not be active, and just stored on some
list. In such a case, you won't have to pause a physical channel, just
prevent it from being scheduled on a new channel whenever one will
become available.
 
> > +	 */
> > +	if (schan) {
> > +		sdma->ops->channel_pause(schan);
>
> make sure you document this and rest of the calls can be invoked
> with lock held!

Yep, a big part of the path to the v1 will be to add comments and
documentation.

> > +	} else {
> > +		spin_lock(&sdma->lock);
> > +		list_del_init(&sreq->node);
> > +		spin_unlock(&sdma->lock);
> > +	}
> > +
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sdma_resume(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * If the request is currently scheduled on a channel, just
> > +	 * resume the channel.
> > +	 *
> > +	 * If not, add the request from the pending list.
> same here too
> > +	 */
> > +	if (schan) {
> > +		sdma->ops->channel_resume(schan);
> > +	} else {
> > +		spin_lock(&sdma->lock);
> > +		list_add_tail(&sreq->node, &sdma->pend_reqs);
> > +		spin_unlock(&sdma->lock);
> > +	}
> > +
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sdma_terminate(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma_channel *schan = sreq->chan;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * If the request is currently scheduled on a channel,
> > +	 * terminate the channel.
> > +	 *
> > +	 * If not, prevent the request from being scheduled.
> > +	 */
> > +	if (schan) {
> > +		sdma->ops->channel_terminate(schan);
> > +	} else {
> > +		spin_lock(&sdma->lock);
> > +		list_del_init(&sreq->node);
> > +		spin_unlock(&sdma->lock);
> > +	}
> > +
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	/*
> > +	 * Flush all the pending descriptors from our vchan
> > +	 */
> > +	vchan_free_chan_resources(&sreq->vchan);
>
> I think cleanup is not right we need more work here.

That might be an issue then. This code was taken from the sun6i
driver, which in turn was inspired by the k3 driver. What's wrong
here?

> > +
> > +	return 0;
> > +}
> > +
> > +
> > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(struct dma_chan *chan,
> > +							dma_addr_t dest, dma_addr_t src,
> > +							size_t len, unsigned long flags)
> > +{
> > +	struct sdma_request *req = to_sdma_request(chan);
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	struct sdma_desc *desc;
> > +	dma_addr_t p_lli;
> > +	void *v_lli;
> > +
> > +	if (!len)
> > +		return NULL;
> > +
> > +	/* Allocate our representation of a descriptor */
> > +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > +	if (!desc)
> > +		return NULL;
> > +
> > +	v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> > +	if (!v_lli)
> > +		goto err_desc_free;
> > +
> > +	/* Ask the driver to initialise its hardware descriptor */
> > +	if (sdma->ops->lli_init(v_lli, req->private,
> > +				SDMA_TRANSFER_MEMCPY,
> > +				DMA_MEM_TO_MEM, src, dest, len,
> > +				NULL))
> > +		goto err_lli_free;
> > +
> > +	/* Create our single item LLI */
> > +	sdma->ops->lli_queue(NULL, v_lli, p_lli);
> > +	desc->p_lli = p_lli;
> > +	desc->v_lli = v_lli;
> > +
> > +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
>
> typically driver would do calculations for descriptor setting up the
> registers values which cna be programmed, so a driver callback would be nice
> here. That way lesser work in start function and which is good as I am
> assuming start should be done from irq context

I'm not really sure that would make sense. You don't have the
guarantee that the dma_chan pointer you are passing to it is actually
mapped to a physical channel at this time.

And I thought that the prep functions could be called from irq context
too?

> Also this needs to take into account the dma_slave_config, which is allowed
> to me modified for subsequent txns

memcpy doesn't rely on dma_slave_config, does it?

> > +
> > +err_lli_free:
> > +	dma_pool_free(sdma->pool, v_lli, p_lli);
> > +err_desc_free:
> > +	kfree(desc);
> > +
> > +	return NULL;
> > +}
> > +
> > +static struct dma_async_tx_descriptor *sdma_prep_slave_sg(struct dma_chan *chan,
> > +							  struct scatterlist *sgl,
> > +							  unsigned int sg_len,
> > +							  enum dma_transfer_direction dir,
> > +							  unsigned long flags, void *context)
> > +{
> > +	struct sdma_request *req = to_sdma_request(chan);
> > +	struct dma_slave_config *config = &req->cfg;
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	void *v_lli, *prev_v_lli = NULL;
> > +	struct scatterlist *sg;
> > +	struct sdma_desc *desc;
> > +	dma_addr_t p_lli;
> > +	int i;
> > +
> > +	if (!sgl || !sg_len)
> > +		return NULL;
> > +
> > +	/* Allocate our representation of a descriptor */
> > +	desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > +	if (!desc)
> > +		return NULL;
> > +
> > +	/*
> > +	 * For each scatter list entry, build up our representation of
> > +	 * the LLI, and ask the driver to create its hardware
> > +	 * descriptor.
> > +	 */
> > +	for_each_sg(sgl, sg, sg_len, i) {
> > +		v_lli = dma_pool_alloc(sdma->pool, GFP_NOWAIT, &p_lli);
> > +		if (!v_lli)
> > +			goto err_lli_free;
> > +
> > +		/* Ask the driver to initialise its hardware descriptor */
> > +		if (sdma->ops->lli_init(v_lli, req->private,
> > +					SDMA_TRANSFER_SLAVE,
> > +					dir, sg_dma_address(sg),
> > +					config->dst_addr, sg_dma_len(sg),
> > +					config))
> > +			goto err_lli_free;
> > +
> > +		/*
> > +		 * If it's our first item, initialise our descriptor
> > +		 * pointers to the lli.
> > +		 *
> > +		 * Otherwise, queue it to the end of the LLI.
> > +		 */
> > +		if (!prev_v_lli) {
> > +			desc->p_lli = p_lli;
> > +			desc->v_lli = v_lli;
> > +			prev_v_lli = v_lli;
> > +		} else {
> > +			/* And to queue it at the end of its hardware LLI */
> > +			prev_v_lli = sdma->ops->lli_queue(prev_v_lli, v_lli, p_lli);
> > +		}
> > +	}
> > +
> > +	return vchan_tx_prep(&req->vchan, &desc->vdesc, flags);
> > +
> > +err_lli_free:
> > +#warning "Free the LLI"
> why a non std preprocessor warning

Just to put an excerpt on the fact that this is a TODO, that have not
been overlooked but just not implemented yet.

> > +
> > +	kfree(desc);
> > +	return NULL;
> > +}
> > +
> > +static void sdma_issue_pending(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	struct sdma *sdma = to_sdma(chan->device);
> > +	struct virt_dma_desc *vdesc;
> > +	struct sdma_channel *schan;
> > +	struct sdma_desc *sdesc;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +
> > +	/* See if we have anything to do */
> > +	if (!vchan_issue_pending(&sreq->vchan))
> > +		goto out_chan_unlock;
> > +
> > +	/* Is some work in progress already? */
> > +	if (sreq->chan)
> > +		goto out_chan_unlock;
> > +
> > +	spin_lock(&sdma->lock);
> > +
> > +	/* Is there an available channel */
> > +	if (list_empty(&sdma->avail_chans))
> > +		goto out_main_unlock;
> > +
> > +	/*
> > +	 * If there's no validate_request callback, it means that all
> > +	 * channels can transfer any request. Pick the first available
> > +	 * channel.
> > +	 *
> > +	 * Otherwise, iterate over all the pending channels and call
> > +	 * validate_request.
> > +	 */
> > +	if (!sdma->ops->validate_request) {
> > +		schan = list_first_entry(&sdma->avail_chans,
> > +					 struct sdma_channel, node);
> > +	} else {
> > +		list_for_each_entry(schan, &sdma->avail_chans, node) {
> > +			if (sdma->ops->validate_request(schan, sreq)) {
> > +				list_del_init(&schan->node);
> > +				break;
> > +			}
> > +		}
> > +	}
> why not call sdma_pop_queued_transfer() ?

pop_queued_transfer tries to elect a request for a given (newly)
available channel.

We're on the opposite case here, we try to elect a channel for a given
request.

But it's true it can be turned into a function of its own.

> > +
> > +	if (!schan)
> > +		goto out_main_unlock;
> > +
> > +	sreq->chan = schan;
> > +
> > +	/* Retrieve the next transfer descriptor */
> > +	vdesc = vchan_next_desc(&sreq->vchan);
> > +	schan->desc = sdesc = to_sdma_desc(&vdesc->tx);
> > +
> > +	sdma->ops->channel_start(schan, sdesc);
> > +
> > +out_main_unlock:
> > +	spin_unlock(&sdma->lock);
> > +out_chan_unlock:
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +}
> > +
> > +static void sdma_free_chan_resources(struct dma_chan *chan)
> > +{
> > +	struct sdma_request *sreq = to_sdma_request(chan);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&sreq->vchan.lock, flags);
> > +	list_del_init(&sreq->node);
> > +	spin_unlock_irqrestore(&sreq->vchan.lock, flags);
> > +
> > +	vchan_free_chan_resources(&sreq->vchan);
> > +}
> > +
> > +static void sdma_free_desc(struct virt_dma_desc *vdesc)
> > +{
> > +#warning "Free the descriptors"
> ??
> 
> > +}
> > +
> > +struct sdma *sdma_alloc(struct device *dev,
> > +			unsigned int channels,
> > +			unsigned int requests,
> > +			ssize_t lli_size,
> > +			ssize_t priv_size)
> > +{
> > +	struct sdma *sdma;
> > +	int ret, i;
> > +
> > +	sdma = devm_kzalloc(dev, sizeof(*sdma) + priv_size, GFP_KERNEL);
> > +	if (!sdma) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +	INIT_LIST_HEAD(&sdma->pend_reqs);
> > +	INIT_LIST_HEAD(&sdma->avail_chans);
> > +	spin_lock_init(&sdma->lock);
> > +
> > +	sdma->pool = dmam_pool_create(dev_name(dev), dev, lli_size, 4, 0);
> > +	if (!sdma->pool) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	sdma->channels_nr = channels;
> > +	sdma->channels = devm_kcalloc(dev, channels, sizeof(*sdma->channels), GFP_KERNEL);
> > +	if (!sdma->channels) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < channels; i++) {
> > +		struct sdma_channel *schan = &sdma->channels[i];
> > +
> > +		list_add_tail(&schan->node, &sdma->avail_chans);
> > +		schan->index = i;
> > +	}
> > +
> > +	sdma->requests_nr = requests;
> > +	sdma->requests = devm_kcalloc(dev, requests, sizeof(*sdma->requests), GFP_KERNEL);
> > +	if (!sdma->channels) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	INIT_LIST_HEAD(&sdma->ddev.channels);
> > +
> > +	for (i = 0; i < requests; i++) {
> > +		struct sdma_request *sreq = &sdma->requests[i];
> > +
> > +		INIT_LIST_HEAD(&sreq->node);
> > +		sreq->vchan.desc_free = sdma_free_desc;
> > +		vchan_init(&sreq->vchan, &sdma->ddev);
> > +	}
> > +
> > +	return sdma;
> > +
> > +out:
> > +	return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(sdma_alloc);
> > +
> > +void sdma_free(struct sdma *sdma)
> > +{
> > +	return;
> > +}
> > +EXPORT_SYMBOL(sdma_free);
> > +
> > +int sdma_register(struct sdma *sdma,
> > +		  struct sdma_ops *ops)
> > +{
> > +	struct dma_device *ddev = &sdma->ddev;
> > +
> > +	sdma->ops = ops;
> > +
> > +	ddev->device_config = sdma_config;
> > +	ddev->device_tx_status = sdma_tx_status;
> > +	ddev->device_issue_pending = sdma_issue_pending;
> > +	ddev->device_free_chan_resources = sdma_free_chan_resources;
> > +
> > +	if (ops->channel_pause)
> > +		ddev->device_pause = sdma_pause;
> > +
> > +	if (ops->channel_resume)
> > +		ddev->device_resume = sdma_resume;
> > +
> > +	if (ops->channel_terminate)
> > +		ddev->device_terminate_all = sdma_terminate;
> > +
> > +	if (dma_has_cap(DMA_SLAVE, ddev->cap_mask))
> > +		ddev->device_prep_slave_sg = sdma_prep_slave_sg;
> > +
> > +	if (dma_has_cap(DMA_MEMCPY, ddev->cap_mask))
> > +		ddev->device_prep_dma_memcpy = sdma_prep_memcpy;
> > +
> > +	dma_async_device_register(ddev);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(sdma_register);
> > +
> > +int sdma_unregister(struct sdma *sdma)
> > +{
> > +	dma_async_device_unregister(&sdma->ddev);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(sdma_unregister);
> > diff --git a/drivers/dma/scheduled-dma.h b/drivers/dma/scheduled-dma.h
> > new file mode 100644
> > index 000000000000..d24c8143b2b6
> > --- /dev/null
> > +++ b/drivers/dma/scheduled-dma.h
> > @@ -0,0 +1,140 @@
> > +/*
> > + * Copyright (C) 2015 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include "virt-dma.h"
> > +
> > +#ifndef _SCHEDULED_DMA_H_
> > +#define _SCHEDULED_DMA_H_
> > +
> > +enum sdma_transfer_type {
> > +	SDMA_TRANSFER_MEMCPY,
> > +	SDMA_TRANSFER_SLAVE,
> > +};
> > +
> > +enum sdma_report_status {
> > +	SDMA_REPORT_CHUNK,
> > +	SDMA_REPORT_TRANSFER,
> > +};
> can you pls add kernel style documentation for these and below.
> > +
> > +struct sdma_desc {
> > +	struct virt_dma_desc	vdesc;
> > +
> > +	/* Entry point to our LLI */
> > +	dma_addr_t		p_lli;
> > +	void			*v_lli;
> > +};
> > +
> > +struct sdma_channel {
> > +	struct sdma_desc	*desc;
> > +	unsigned int		index;
> > +	struct list_head	node;
> > +	void			*private;
> > +};
> > +
> > +struct sdma_request {
> > +	struct dma_slave_config	cfg;
> > +	struct list_head	node;
> > +	struct virt_dma_chan	vchan;
> > +
> > +	struct sdma_channel	*chan;
> > +	void			*private;
> > +};
> > +
> > +struct sdma_ops {
> > +	/* LLI management operations */
> > +	bool			(*lli_has_next)(void *v_lli);
> > +	void			*(*lli_next)(void *v_lli);
> > +	int			(*lli_init)(void *v_lli, void *sreq_priv,
> > +					    enum sdma_transfer_type type,
> > +					    enum dma_transfer_direction dir,
> > +					    dma_addr_t src,
> > +					    dma_addr_t dst, u32 len,
> > +					    struct dma_slave_config *config);
> > +	void			*(*lli_queue)(void *prev_v_lli, void *v_lli, dma_addr_t p_lli);
> > +	size_t			(*lli_size)(void *v_lli);
> if we can manage these using the standard list then it would simplify the
> code a lot as well

Yep, that's on my TODO :)

Thanks for your review!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Vinod Koul]

[-- Attachment #1: Type: text/plain, Size: 3425 bytes --]

On Mon, Mar 23, 2015 at 02:51:40PM -0700, Maxime Ripard wrote:
> > > +	if (vd) {
> > > +		lli = desc->v_lli;
> > > +		while (true) {
> > > +			bytes += sdma->ops->lli_size(lli);
> > > +
> > > +			if (!sdma->ops->lli_has_next(lli))
> > > +				break;
> > > +
> > > +			lli = sdma->ops->lli_next(lli);
> >
> > wouldn't it be nicer to just use a link list here which is embedded
> > in a generic struct which in turn can be embedded in a driver
> > specific one, so we actually don't care what driver specific
> > implementation is...
> 
> I'm not sure to get what you mean here. We will always have to care
> about the driver specific implementation.
by providing a callback driver can do whatever it wants whereas the framework
does generic stuff

> 
> > that way we cna get rid of lli_size and lli_has_next and lli_next
> > ones. Further that will allow drivers/framework to break a txn into
> > multiple smaller txns internally.
> 
> I chose this accessors/iterators pattern because it actually make this
> very easy to add new features on the LLI (like debugging, we just have
> to add a new lli_dump) and that works.
> 
> We can also manage more easily the LLI, by allocating ourself the LLI
> items, and freeing it afterwards, which is something that also is
> often wrong, or at least addressed in different ways from one driver
> to another (for example, the Atmel drivers have their own descriptor
> allocation logic, while the others mostly rely on the dma_pools. Which
> one is right, I don't know, but the right one should be shared by
> everyone).
> 
> That would also be much easier to add software-emulated LLI, since you
> just have to set the lli management functions to some common library
> functions, and we're done.
> 
> I feel like it would be much more flexible, and would remove logic
> from the driver itself, which is the point of this whole
> "sub-framework".
Yes the intent is right, I was thinking that using a kernel link frees us
from managing the list, lesser chances of bugs that way


> > typically driver would do calculations for descriptor setting up the
> > registers values which cna be programmed, so a driver callback would be nice
> > here. That way lesser work in start function and which is good as I am
> > assuming start should be done from irq context
> 
> I'm not really sure that would make sense. You don't have the
> guarantee that the dma_chan pointer you are passing to it is actually
> mapped to a physical channel at this time.
> 
> And I thought that the prep functions could be called from irq context
> too?
Yes that is true but if you have descriptor prepared and only need to
program the hw that it is the fastest way. Plus the HW is idle so faster we
can submit a new transaction the better it is. Btw the recommendation is
that new txn should be submitted in ISR and not wait till tasklet, most
driver do latter with few doing former.
The computation part of registers to be programmed should be anyway
independent of channel, while actual programming and starting of channel we
need to ensure right channel and thereby offsets are used :)

> 
> > Also this needs to take into account the dma_slave_config, which is allowed
> > to me modified for subsequent txns
> 
> memcpy doesn't rely on dma_slave_config, does it?
Nope but slave does :) this comment is for slave txn :)

-- 
~Vinod


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH RFC 1/2] dmaengine: Introduce scheduled DMA framework

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5304 bytes --]

On Tue, Mar 24, 2015 at 09:50:43PM +0530, Vinod Koul wrote:
> On Mon, Mar 23, 2015 at 02:51:40PM -0700, Maxime Ripard wrote:
> > > > +	if (vd) {
> > > > +		lli = desc->v_lli;
> > > > +		while (true) {
> > > > +			bytes += sdma->ops->lli_size(lli);
> > > > +
> > > > +			if (!sdma->ops->lli_has_next(lli))
> > > > +				break;
> > > > +
> > > > +			lli = sdma->ops->lli_next(lli);
> > >
> > > wouldn't it be nicer to just use a link list here which is embedded
> > > in a generic struct which in turn can be embedded in a driver
> > > specific one, so we actually don't care what driver specific
> > > implementation is...
> > 
> > I'm not sure to get what you mean here. We will always have to care
> > about the driver specific implementation.
>
> by providing a callback driver can do whatever it wants whereas the framework
> does generic stuff

It's kind of the whole point. LLI can be either actual list, maps,
tables, or even not supported by hardware at all and should be
emulated.

On this, letting the driver do whatever is needed seems like a good
thing.

> > > that way we cna get rid of lli_size and lli_has_next and lli_next
> > > ones. Further that will allow drivers/framework to break a txn into
> > > multiple smaller txns internally.
> > 
> > I chose this accessors/iterators pattern because it actually make this
> > very easy to add new features on the LLI (like debugging, we just have
> > to add a new lli_dump) and that works.
> > 
> > We can also manage more easily the LLI, by allocating ourself the LLI
> > items, and freeing it afterwards, which is something that also is
> > often wrong, or at least addressed in different ways from one driver
> > to another (for example, the Atmel drivers have their own descriptor
> > allocation logic, while the others mostly rely on the dma_pools. Which
> > one is right, I don't know, but the right one should be shared by
> > everyone).
> > 
> > That would also be much easier to add software-emulated LLI, since you
> > just have to set the lli management functions to some common library
> > functions, and we're done.
> > 
> > I feel like it would be much more flexible, and would remove logic
> > from the driver itself, which is the point of this whole
> > "sub-framework".
>
> Yes the intent is right, I was thinking that using a kernel link frees us
> from managing the list, lesser chances of bugs that way

A generic structure embedded in a driver specific structure wouldn't
even cover all cases. For example, in tx_status, we do want to access
the hardware descriptor size, and possibly iterate over the LLI
itself, so we would still have to have some accessors and iterators.

Given that, I'm not sure that duplicating this info in several places
is a good idea.

> > > typically driver would do calculations for descriptor setting up the
> > > registers values which cna be programmed, so a driver callback would be nice
> > > here. That way lesser work in start function and which is good as I am
> > > assuming start should be done from irq context
> > 
> > I'm not really sure that would make sense. You don't have the
> > guarantee that the dma_chan pointer you are passing to it is actually
> > mapped to a physical channel at this time.
> > 
> > And I thought that the prep functions could be called from irq context
> > too?
>
> Yes that is true but if you have descriptor prepared and only need to
> program the hw that it is the fastest way. Plus the HW is idle so faster we
> can submit a new transaction the better it is.

You don't really know at this point whether the hardware is idle, or
on which channel it's actually going to be scheduled to.

If it's just to prepare and store some registers value, I guess it
might make sense. I don't really know to what should we attach this
kind of precomputed values, to the request? the descriptor? the LLI
itself?

> Btw the recommendation is that new txn should be submitted in ISR
> and not wait till tasklet, most driver do latter with few doing
> former.

Yes, that's one of the things that my driver took into account, since
most drivers get this wrong (including the one I wrote).

sdma_report is expected to be called from an interrupt context, and
will try to find a new descriptor to run on the channel which just got
reported as free. If such a descriptor is available, it will return it
to the driver, that is expected to run it on that channel.

Otherwise, we just put the channel back in the available list for
future use.

> The computation part of registers to be programmed should be anyway
> independent of channel, while actual programming and starting of channel we
> need to ensure right channel and thereby offsets are used :)

Yep, that part is hopefully covered :)

> > > Also this needs to take into account the dma_slave_config, which is allowed
> > > to me modified for subsequent txns
> > 
> > memcpy doesn't rely on dma_slave_config, does it?
> Nope but slave does :) this comment is for slave txn :)

You were actually commenting on prep_memcpy. slave_sg actually takes
it into account.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]