public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] Use DMA for data transfers in JZ4740 MMC driver
@ 2014-07-21  4:37 Apelete Seketeli
  2014-07-21  4:37 ` [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers Apelete Seketeli
  2014-07-21  4:37 ` [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer Apelete Seketeli
  0 siblings, 2 replies; 9+ messages in thread
From: Apelete Seketeli @ 2014-07-21  4:37 UTC (permalink / raw)
  To: Chris Ball, Ulf Hansson, H Hartley Sweeten, Lars-Peter Clausen,
	Wei Yongjun, Alex Smith, Laurent Pinchart, linux-mmc
  Cc: linux-kernel

Hello,

MMC driver for JZ4740 SoC is currently relying on PIO mode only for
data transfers.

The patches that come as a follow-up of this message allow the use of
DMA for data transfers.

Changes since v5:
  - added a new patch to this series, on top of the previous one, to
    prepare next dma transfer in parallel with current transfer,

  - updated the benchmarks in this cover letter for the aforementioned
    patch.

Changes since v4:
  - prefix functions with jz4740_mmc_ instead of jz4740_ to keep
    consistency with the rest of the code,

  - test (host->sg_len == 0) instead of (host->sg_len < 0) after
    dma_map_sg() in jz4740_mmc_start_dma_transfer() according to
    dmaengine.txt instructions.

Changes since v3:
  - use DMA engine device instead of MMC device in dma_map_sg() and
    dma_unmap_sg() as the memory transfers will be performed by the
    DMA engine,

  - remove unnecessary mem_res == NULL check in jz4740_mmc_probe()
    since devm_ioremap_resource() handles the mem_res == NULL case and
    print a message,

  - added a check to host->use_dma in jz4740_mmc_remove() to avoid
    calling jz4740_release_dma_channels() unnecessarily.

Changes since v2:
  - declare sg_len member of struct jz4740_mmc_host as int instead of
    unsigned int.

Changes since v1:
  - remove blank line added by mistake in jz_mmc_prepare_data_transfer().

According to the following PIO vs DMA benchmarks there seems to be a
slight improvement in transfer speed with the Ben NanoNote booting
from SD card, while load average seems to be roughly on par.

No noticeable improvement in the DMA vs DMA + MMC asynchronous
requests case. The latter change should help reduce the impact of DMA
preparation overhead on fast SD cards performance though:

* With DMA + MMC asynchronous requests:

Test cases |             root@BenNanoNote:/# uptime              |   root@BenNanoNote:/# time zcat root/fedora-16.iso.gz > /dev/null && uptime
-----------|----------------------------------------------------------------------------------------------------------------------------------
Test run 1 | 00:01:35 up 1 min,  load average: 1.31, 0.44, 0.15  |  00:06:46 up 6 min,  load average: 2.53, 1.75, 0.81
Test run 2 | 00:10:09 up 1 min,  load average: 1.26, 0.42, 0.14  |  00:15:20 up 6 min,  load average: 2.46, 1.73, 0.80
Test run 3 | 00:31:22 up 1 min,  load average: 1.30, 0.44, 0.15  |  00:36:33 up 6 min,  load average: 2.45, 1.73, 0.80
-----------|----------------------------------------------------------------------------------------------------------------------------------
  Average  |             1 min,  load average: 1.29, 0.43, 0.14  |              6 min,  load average: 2.48, 1.73, 0.80


* With DMA:

Test cases |             root@BenNanoNote:/# uptime              |   root@BenNanoNote:/# time zcat root/fedora-16.iso.gz > /dev/null && uptime
-----------|----------------------------------------------------------------------------------------------------------------------------------
Test run 1 | 00:20:55 up 1 min,  load average: 1.26, 0.42, 0.14  |  00:26:10 up 6 min,  load average: 2.89, 1.94, 0.89
Test run 2 | 00:30:22 up 1 min,  load average: 1.16, 0.38, 0.13  |  00:35:34 up 6 min,  load average: 2.68, 1.86, 0.85
Test run 3 | 00:39:56 up 1 min,  load average: 1.16, 0.38, 0.13  |  00:45:06 up 6 min,  load average: 2.57, 1.76, 0.81
-----------|----------------------------------------------------------------------------------------------------------------------------------
  Average  |             1 min,  load average: 1.19, 0.39, 0.13  |              6 min,  load average: 2.71, 1.85, 0.85


* With PIO:

Test cases |             root@BenNanoNote:/# uptime              |   root@BenNanoNote:/# time zcat root/fedora-16.iso.gz > /dev/null && uptime
----------------------------------------------------------------------------------------------------------------------------------------------
Test run 1 | 00:50:47 up 1 min,  load average: 1.42, 0.49, 0.17  |  00:56:52 up 7 min,  load average: 2.47, 2.00, 0.98
Test run 2 | 01:00:19 up 1 min,  load average: 1.21, 0.39, 0.14  |  01:06:29 up 7 min,  load average: 2.45, 1.96, 0.96
Test run 3 | 01:11:27 up 1 min,  load average: 1.15, 0.36, 0.12  |  01:17:33 up 7 min,  load average: 2.63, 2.01, 0.97
-----------|----------------------------------------------------------------------------------------------------------------------------------
  Average  |             1 min,  load average: 1.26, 0.41, 0.14  |              7 min,  load average: 2.52, 1.99, 0.97


Changes were rebased on top of linux master branch, built and tested
successfully.

The following changes since commit 9a3c414:

  Linux 3.16-rc6

are available in the git repository at:

  git://git.seketeli.net/~apelete/linux.git jz4740-mmc-dma

Apelete Seketeli (2):
  mmc: jz4740: add dma infrastructure for data transfers
  mmc: jz4740: prepare next dma transfer in parallel with current
    transfer

 drivers/mmc/host/jz4740_mmc.c |  268 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 260 insertions(+), 8 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers
  2014-07-21  4:37 [PATCH v6 0/2] Use DMA for data transfers in JZ4740 MMC driver Apelete Seketeli
@ 2014-07-21  4:37 ` Apelete Seketeli
  2014-08-12 16:21   ` Ulf Hansson
  2014-07-21  4:37 ` [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer Apelete Seketeli
  1 sibling, 1 reply; 9+ messages in thread
From: Apelete Seketeli @ 2014-07-21  4:37 UTC (permalink / raw)
  To: Chris Ball, Ulf Hansson, H Hartley Sweeten, Lars-Peter Clausen,
	Wei Yongjun, Alex Smith, Laurent Pinchart, linux-mmc
  Cc: linux-kernel

Until now the MMC driver for JZ4740 SoC was relying on PIO mode only
for data transfers.
This patch allows the use of DMA for data trasnfers in addition to PIO
mode by relying on DMA Engine.

DMA tranfers performance might be further improved by taking advantage
of the asynchronous request capability of the MMC framework.

Signed-off-by: Apelete Seketeli <apelete@seketeli.net>
---
 drivers/mmc/host/jz4740_mmc.c |  174 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 166 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 537d6c7..049b133 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -30,7 +30,9 @@
 #include <asm/mach-jz4740/gpio.h>
 #include <asm/cacheflush.h>
 #include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 
+#include <asm/mach-jz4740/dma.h>
 #include <asm/mach-jz4740/jz4740_mmc.h>
 
 #define JZ_REG_MMC_STRPCL	0x00
@@ -122,6 +124,7 @@ struct jz4740_mmc_host {
 	int card_detect_irq;
 
 	void __iomem *base;
+	struct resource *mem_res;
 	struct mmc_request *req;
 	struct mmc_command *cmd;
 
@@ -136,8 +139,138 @@ struct jz4740_mmc_host {
 	struct timer_list timeout_timer;
 	struct sg_mapping_iter miter;
 	enum jz4740_mmc_state state;
+
+	/* DMA support */
+	struct dma_chan *dma_rx;
+	struct dma_chan *dma_tx;
+	bool use_dma;
+	int sg_len;
+
+/* The DMA trigger level is 8 words, that is to say, the DMA read
+ * trigger is when data words in MSC_RXFIFO is >= 8 and the DMA write
+ * trigger is when data words in MSC_TXFIFO is < 8.
+ */
+#define JZ4740_MMC_FIFO_HALF_SIZE 8
 };
 
+/*----------------------------------------------------------------------------*/
+/* DMA infrastructure */
+
+static void jz4740_mmc_release_dma_channels(struct jz4740_mmc_host *host)
+{
+	if (!host->use_dma)
+		return;
+
+	dma_release_channel(host->dma_tx);
+	dma_release_channel(host->dma_rx);
+}
+
+static int jz4740_mmc_acquire_dma_channels(struct jz4740_mmc_host *host)
+{
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	host->dma_tx = dma_request_channel(mask, NULL, host);
+	if (!host->dma_tx) {
+		dev_err(mmc_dev(host->mmc), "Failed to get dma_tx channel\n");
+		return -ENODEV;
+	}
+
+	host->dma_rx = dma_request_channel(mask, NULL, host);
+	if (!host->dma_rx) {
+		dev_err(mmc_dev(host->mmc), "Failed to get dma_rx channel\n");
+		goto free_master_write;
+	}
+
+	return 0;
+
+free_master_write:
+	dma_release_channel(host->dma_tx);
+	return -ENODEV;
+}
+
+static inline int jz4740_mmc_get_dma_dir(struct mmc_data *data)
+{
+	return (data->flags & MMC_DATA_READ) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+}
+
+static void jz4740_mmc_dma_unmap(struct jz4740_mmc_host *host,
+				 struct mmc_data *data)
+{
+	struct dma_chan *chan;
+	enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
+
+	if (dir == DMA_TO_DEVICE)
+		chan = host->dma_tx;
+	else
+		chan = host->dma_rx;
+
+	dma_unmap_sg(chan->device->dev, data->sg, data->sg_len, dir);
+}
+
+static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
+					 struct mmc_data *data)
+{
+	struct dma_chan *chan;
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config conf = {
+		.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+		.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+		.src_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
+		.dst_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
+	};
+	enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
+
+	if (dir == DMA_TO_DEVICE) {
+		conf.direction = DMA_MEM_TO_DEV;
+		conf.dst_addr = host->mem_res->start + JZ_REG_MMC_TXFIFO;
+		conf.slave_id = JZ4740_DMA_TYPE_MMC_TRANSMIT;
+		chan = host->dma_tx;
+	} else {
+		conf.direction = DMA_DEV_TO_MEM;
+		conf.src_addr = host->mem_res->start + JZ_REG_MMC_RXFIFO;
+		conf.slave_id = JZ4740_DMA_TYPE_MMC_RECEIVE;
+		chan = host->dma_rx;
+	}
+
+	host->sg_len = dma_map_sg(chan->device->dev,
+				  data->sg,
+				  data->sg_len,
+				  dir);
+
+	if (host->sg_len == 0) {
+		dev_err(mmc_dev(host->mmc),
+			"Failed to map scatterlist for DMA operation\n");
+		return -EINVAL;
+	}
+
+	dmaengine_slave_config(chan, &conf);
+	desc = dmaengine_prep_slave_sg(chan,
+				       data->sg,
+				       host->sg_len,
+				       conf.direction,
+				       DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc) {
+		dev_err(mmc_dev(host->mmc),
+			"Failed to allocate DMA %s descriptor",
+			 conf.direction == DMA_MEM_TO_DEV ? "TX" : "RX");
+		goto dma_unmap;
+	}
+
+	dmaengine_submit(desc);
+	dma_async_issue_pending(chan);
+
+	return 0;
+
+dma_unmap:
+	jz4740_mmc_dma_unmap(host, data);
+	return -ENOMEM;
+}
+
+/*----------------------------------------------------------------------------*/
+
 static void jz4740_mmc_set_irq_enabled(struct jz4740_mmc_host *host,
 	unsigned int irq, bool enabled)
 {
@@ -442,6 +575,8 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host,
 			cmdat |= JZ_MMC_CMDAT_WRITE;
 		if (cmd->data->flags & MMC_DATA_STREAM)
 			cmdat |= JZ_MMC_CMDAT_STREAM;
+		if (host->use_dma)
+			cmdat |= JZ_MMC_CMDAT_DMA_EN;
 
 		writew(cmd->data->blksz, host->base + JZ_REG_MMC_BLKLEN);
 		writew(cmd->data->blocks, host->base + JZ_REG_MMC_NOB);
@@ -474,6 +609,7 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
 	struct jz4740_mmc_host *host = (struct jz4740_mmc_host *)devid;
 	struct mmc_command *cmd = host->req->cmd;
 	struct mmc_request *req = host->req;
+	struct mmc_data *data = cmd->data;
 	bool timeout = false;
 
 	if (cmd->error)
@@ -484,23 +620,32 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
 		if (cmd->flags & MMC_RSP_PRESENT)
 			jz4740_mmc_read_response(host, cmd);
 
-		if (!cmd->data)
+		if (!data)
 			break;
 
 		jz_mmc_prepare_data_transfer(host);
 
 	case JZ4740_MMC_STATE_TRANSFER_DATA:
-		if (cmd->data->flags & MMC_DATA_READ)
-			timeout = jz4740_mmc_read_data(host, cmd->data);
+		if (host->use_dma) {
+			/* Use DMA if enabled, data transfer direction was
+			 * defined  before in jz_mmc_prepare_data_transfer().
+			 */
+			timeout = jz4740_mmc_start_dma_transfer(host, data);
+			data->bytes_xfered = data->blocks * data->blksz;
+		} else if (data->flags & MMC_DATA_READ)
+			/* If DMA is not enabled, rely on data flags
+			 * to establish data transfer direction.
+			 */
+			timeout = jz4740_mmc_read_data(host, data);
 		else
-			timeout = jz4740_mmc_write_data(host, cmd->data);
+			timeout = jz4740_mmc_write_data(host, data);
 
 		if (unlikely(timeout)) {
 			host->state = JZ4740_MMC_STATE_TRANSFER_DATA;
 			break;
 		}
 
-		jz4740_mmc_transfer_check_state(host, cmd->data);
+		jz4740_mmc_transfer_check_state(host, data);
 
 		timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_DATA_TRAN_DONE);
 		if (unlikely(timeout)) {
@@ -757,7 +902,6 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
 	struct mmc_host *mmc;
 	struct jz4740_mmc_host *host;
 	struct jz4740_mmc_platform_data *pdata;
-	struct resource *res;
 
 	pdata = pdev->dev.platform_data;
 
@@ -784,10 +928,11 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
 		goto err_free_host;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	host->base = devm_ioremap_resource(&pdev->dev, res);
+	host->mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	host->base = devm_ioremap_resource(&pdev->dev, host->mem_res);
 	if (IS_ERR(host->base)) {
 		ret = PTR_ERR(host->base);
+		dev_err(&pdev->dev, "Failed to ioremap base memory\n");
 		goto err_free_host;
 	}
 
@@ -834,6 +979,10 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
 	/* It is not important when it times out, it just needs to timeout. */
 	set_timer_slack(&host->timeout_timer, HZ);
 
+	host->use_dma = true;
+	if (host->use_dma && jz4740_mmc_acquire_dma_channels(host) != 0)
+		host->use_dma = false;
+
 	platform_set_drvdata(pdev, host);
 	ret = mmc_add_host(mmc);
 
@@ -843,6 +992,10 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
 	}
 	dev_info(&pdev->dev, "JZ SD/MMC card driver registered\n");
 
+	dev_info(&pdev->dev, "Using %s, %d-bit mode\n",
+		 host->use_dma ? "DMA" : "PIO",
+		 (mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1);
+
 	return 0;
 
 err_free_irq:
@@ -850,6 +1003,8 @@ err_free_irq:
 err_free_gpios:
 	jz4740_mmc_free_gpios(pdev);
 err_gpio_bulk_free:
+	if (host->use_dma)
+		jz4740_mmc_release_dma_channels(host);
 	jz_gpio_bulk_free(jz4740_mmc_pins, jz4740_mmc_num_pins(host));
 err_free_host:
 	mmc_free_host(mmc);
@@ -872,6 +1027,9 @@ static int jz4740_mmc_remove(struct platform_device *pdev)
 	jz4740_mmc_free_gpios(pdev);
 	jz_gpio_bulk_free(jz4740_mmc_pins, jz4740_mmc_num_pins(host));
 
+	if (host->use_dma)
+		jz4740_mmc_release_dma_channels(host);
+
 	mmc_free_host(host->mmc);
 
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-07-21  4:37 [PATCH v6 0/2] Use DMA for data transfers in JZ4740 MMC driver Apelete Seketeli
  2014-07-21  4:37 ` [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers Apelete Seketeli
@ 2014-07-21  4:37 ` Apelete Seketeli
  2014-08-12 16:22   ` Ulf Hansson
  2014-08-12 18:12   ` Chris Ball
  1 sibling, 2 replies; 9+ messages in thread
From: Apelete Seketeli @ 2014-07-21  4:37 UTC (permalink / raw)
  To: Chris Ball, Ulf Hansson, H Hartley Sweeten, Lars-Peter Clausen,
	Wei Yongjun, Alex Smith, Laurent Pinchart, linux-mmc
  Cc: linux-kernel

Make use of the MMC asynchronous request capability to prepare the
next DMA transfer request in parallel with the current transfer.
This is done by adding pre-request and post-request callbacks that are
used by the MMC framework during an active data transfer.

It should help reduce the impact of DMA preparation overhead on the SD
card performance.

Signed-off-by: Apelete Seketeli <apelete@seketeli.net>
---
 drivers/mmc/host/jz4740_mmc.c |  138 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 116 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 049b133..14738cd 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -114,6 +114,11 @@ enum jz4740_mmc_state {
 	JZ4740_MMC_STATE_DONE,
 };
 
+struct jz4740_mmc_host_next {
+	int sg_len;
+	s32 cookie;
+};
+
 struct jz4740_mmc_host {
 	struct mmc_host *mmc;
 	struct platform_device *pdev;
@@ -143,6 +148,7 @@ struct jz4740_mmc_host {
 	/* DMA support */
 	struct dma_chan *dma_rx;
 	struct dma_chan *dma_tx;
+	struct jz4740_mmc_host_next next_data;
 	bool use_dma;
 	int sg_len;
 
@@ -184,6 +190,9 @@ static int jz4740_mmc_acquire_dma_channels(struct jz4740_mmc_host *host)
 		goto free_master_write;
 	}
 
+	/* Initialize DMA pre request cookie */
+	host->next_data.cookie = 1;
+
 	return 0;
 
 free_master_write:
@@ -196,23 +205,72 @@ static inline int jz4740_mmc_get_dma_dir(struct mmc_data *data)
 	return (data->flags & MMC_DATA_READ) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 }
 
+static inline struct dma_chan *jz4740_mmc_get_dma_chan(struct jz4740_mmc_host *host,
+						       struct mmc_data *data)
+{
+	return (data->flags & MMC_DATA_READ) ? host->dma_rx : host->dma_tx;
+}
+
 static void jz4740_mmc_dma_unmap(struct jz4740_mmc_host *host,
 				 struct mmc_data *data)
 {
-	struct dma_chan *chan;
+	struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
 	enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
 
-	if (dir == DMA_TO_DEVICE)
-		chan = host->dma_tx;
-	else
-		chan = host->dma_rx;
-
 	dma_unmap_sg(chan->device->dev, data->sg, data->sg_len, dir);
 }
 
+/* Prepares DMA data for current/next transfer, returns non-zero on failure */
+static int jz4740_mmc_prepare_dma_data(struct jz4740_mmc_host *host,
+				       struct mmc_data *data,
+				       struct jz4740_mmc_host_next *next,
+				       struct dma_chan *chan)
+{
+	struct jz4740_mmc_host_next *next_data = &host->next_data;
+	enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
+	int sg_len;
+
+	if (!next && data->host_cookie &&
+	    data->host_cookie != host->next_data.cookie) {
+		dev_warn(mmc_dev(host->mmc),
+			 "[%s] invalid cookie: data->host_cookie %d host->next_data.cookie %d\n",
+			 __func__,
+			 data->host_cookie,
+			 host->next_data.cookie);
+		data->host_cookie = 0;
+	}
+
+	/* Check if next job is already prepared */
+	if (next || data->host_cookie != host->next_data.cookie) {
+		sg_len = dma_map_sg(chan->device->dev,
+				    data->sg,
+				    data->sg_len,
+				    dir);
+
+	} else {
+		sg_len = next_data->sg_len;
+		next_data->sg_len = 0;
+	}
+
+	if (sg_len <= 0) {
+		dev_err(mmc_dev(host->mmc),
+			"Failed to map scatterlist for DMA operation\n");
+		return -EINVAL;
+	}
+
+	if (next) {
+		next->sg_len = sg_len;
+		data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
+	} else
+		host->sg_len = sg_len;
+
+	return 0;
+}
+
 static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
 					 struct mmc_data *data)
 {
+	int ret;
 	struct dma_chan *chan;
 	struct dma_async_tx_descriptor *desc;
 	struct dma_slave_config conf = {
@@ -221,9 +279,8 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
 		.src_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
 		.dst_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
 	};
-	enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
 
-	if (dir == DMA_TO_DEVICE) {
+	if (data->flags & MMC_DATA_WRITE) {
 		conf.direction = DMA_MEM_TO_DEV;
 		conf.dst_addr = host->mem_res->start + JZ_REG_MMC_TXFIFO;
 		conf.slave_id = JZ4740_DMA_TYPE_MMC_TRANSMIT;
@@ -235,16 +292,9 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
 		chan = host->dma_rx;
 	}
 
-	host->sg_len = dma_map_sg(chan->device->dev,
-				  data->sg,
-				  data->sg_len,
-				  dir);
-
-	if (host->sg_len == 0) {
-		dev_err(mmc_dev(host->mmc),
-			"Failed to map scatterlist for DMA operation\n");
-		return -EINVAL;
-	}
+	ret = jz4740_mmc_prepare_dma_data(host, data, NULL, chan);
+	if (ret)
+		return ret;
 
 	dmaengine_slave_config(chan, &conf);
 	desc = dmaengine_prep_slave_sg(chan,
@@ -269,6 +319,43 @@ dma_unmap:
 	return -ENOMEM;
 }
 
+static void jz4740_mmc_pre_request(struct mmc_host *mmc,
+				   struct mmc_request *mrq,
+				   bool is_first_req)
+{
+	struct jz4740_mmc_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+	struct jz4740_mmc_host_next *next_data = &host->next_data;
+
+	BUG_ON(data->host_cookie);
+
+	if (host->use_dma) {
+		struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
+
+		if (jz4740_mmc_prepare_dma_data(host, data, next_data, chan))
+			data->host_cookie = 0;
+	}
+}
+
+static void jz4740_mmc_post_request(struct mmc_host *mmc,
+				    struct mmc_request *mrq,
+				    int err)
+{
+	struct jz4740_mmc_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+
+	if (host->use_dma && data->host_cookie) {
+		jz4740_mmc_dma_unmap(host, data);
+		data->host_cookie = 0;
+	}
+
+	if (err) {
+		struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
+
+		dmaengine_terminate_all(chan);
+	}
+}
+
 /*----------------------------------------------------------------------------*/
 
 static void jz4740_mmc_set_irq_enabled(struct jz4740_mmc_host *host,
@@ -627,14 +714,19 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
 
 	case JZ4740_MMC_STATE_TRANSFER_DATA:
 		if (host->use_dma) {
-			/* Use DMA if enabled, data transfer direction was
-			 * defined  before in jz_mmc_prepare_data_transfer().
+			/* Use DMA if enabled.
+			 * Data transfer direction is defined later by
+			 * relying on data flags in
+			 * jz4740_mmc_prepare_dma_data() and
+			 * jz4740_mmc_start_dma_transfer().
 			 */
 			timeout = jz4740_mmc_start_dma_transfer(host, data);
 			data->bytes_xfered = data->blocks * data->blksz;
 		} else if (data->flags & MMC_DATA_READ)
-			/* If DMA is not enabled, rely on data flags
-			 * to establish data transfer direction.
+			/* Use PIO if DMA is not enabled.
+			 * Data transfer direction was defined before
+			 * by relying on data flags in
+			 * jz_mmc_prepare_data_transfer().
 			 */
 			timeout = jz4740_mmc_read_data(host, data);
 		else
@@ -809,6 +901,8 @@ static void jz4740_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 static const struct mmc_host_ops jz4740_mmc_ops = {
 	.request	= jz4740_mmc_request,
+	.pre_req	= jz4740_mmc_pre_request,
+	.post_req	= jz4740_mmc_post_request,
 	.set_ios	= jz4740_mmc_set_ios,
 	.get_ro		= mmc_gpio_get_ro,
 	.get_cd		= mmc_gpio_get_cd,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers
  2014-07-21  4:37 ` [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers Apelete Seketeli
@ 2014-08-12 16:21   ` Ulf Hansson
  0 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2014-08-12 16:21 UTC (permalink / raw)
  To: Apelete Seketeli
  Cc: Chris Ball, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc,
	linux-kernel@vger.kernel.org

On 21 July 2014 06:37, Apelete Seketeli <apelete@seketeli.net> wrote:
> Until now the MMC driver for JZ4740 SoC was relying on PIO mode only
> for data transfers.
> This patch allows the use of DMA for data trasnfers in addition to PIO
> mode by relying on DMA Engine.
>
> DMA tranfers performance might be further improved by taking advantage
> of the asynchronous request capability of the MMC framework.
>
> Signed-off-by: Apelete Seketeli <apelete@seketeli.net>

Thanks! Queued for 3.18.

Kind regards
Uffe

> ---
>  drivers/mmc/host/jz4740_mmc.c |  174 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 166 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 537d6c7..049b133 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -30,7 +30,9 @@
>  #include <asm/mach-jz4740/gpio.h>
>  #include <asm/cacheflush.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
>
> +#include <asm/mach-jz4740/dma.h>
>  #include <asm/mach-jz4740/jz4740_mmc.h>
>
>  #define JZ_REG_MMC_STRPCL      0x00
> @@ -122,6 +124,7 @@ struct jz4740_mmc_host {
>         int card_detect_irq;
>
>         void __iomem *base;
> +       struct resource *mem_res;
>         struct mmc_request *req;
>         struct mmc_command *cmd;
>
> @@ -136,8 +139,138 @@ struct jz4740_mmc_host {
>         struct timer_list timeout_timer;
>         struct sg_mapping_iter miter;
>         enum jz4740_mmc_state state;
> +
> +       /* DMA support */
> +       struct dma_chan *dma_rx;
> +       struct dma_chan *dma_tx;
> +       bool use_dma;
> +       int sg_len;
> +
> +/* The DMA trigger level is 8 words, that is to say, the DMA read
> + * trigger is when data words in MSC_RXFIFO is >= 8 and the DMA write
> + * trigger is when data words in MSC_TXFIFO is < 8.
> + */
> +#define JZ4740_MMC_FIFO_HALF_SIZE 8
>  };
>
> +/*----------------------------------------------------------------------------*/
> +/* DMA infrastructure */
> +
> +static void jz4740_mmc_release_dma_channels(struct jz4740_mmc_host *host)
> +{
> +       if (!host->use_dma)
> +               return;
> +
> +       dma_release_channel(host->dma_tx);
> +       dma_release_channel(host->dma_rx);
> +}
> +
> +static int jz4740_mmc_acquire_dma_channels(struct jz4740_mmc_host *host)
> +{
> +       dma_cap_mask_t mask;
> +
> +       dma_cap_zero(mask);
> +       dma_cap_set(DMA_SLAVE, mask);
> +
> +       host->dma_tx = dma_request_channel(mask, NULL, host);
> +       if (!host->dma_tx) {
> +               dev_err(mmc_dev(host->mmc), "Failed to get dma_tx channel\n");
> +               return -ENODEV;
> +       }
> +
> +       host->dma_rx = dma_request_channel(mask, NULL, host);
> +       if (!host->dma_rx) {
> +               dev_err(mmc_dev(host->mmc), "Failed to get dma_rx channel\n");
> +               goto free_master_write;
> +       }
> +
> +       return 0;
> +
> +free_master_write:
> +       dma_release_channel(host->dma_tx);
> +       return -ENODEV;
> +}
> +
> +static inline int jz4740_mmc_get_dma_dir(struct mmc_data *data)
> +{
> +       return (data->flags & MMC_DATA_READ) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +}
> +
> +static void jz4740_mmc_dma_unmap(struct jz4740_mmc_host *host,
> +                                struct mmc_data *data)
> +{
> +       struct dma_chan *chan;
> +       enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
> +
> +       if (dir == DMA_TO_DEVICE)
> +               chan = host->dma_tx;
> +       else
> +               chan = host->dma_rx;
> +
> +       dma_unmap_sg(chan->device->dev, data->sg, data->sg_len, dir);
> +}
> +
> +static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
> +                                        struct mmc_data *data)
> +{
> +       struct dma_chan *chan;
> +       struct dma_async_tx_descriptor *desc;
> +       struct dma_slave_config conf = {
> +               .src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
> +               .dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
> +               .src_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
> +               .dst_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
> +       };
> +       enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
> +
> +       if (dir == DMA_TO_DEVICE) {
> +               conf.direction = DMA_MEM_TO_DEV;
> +               conf.dst_addr = host->mem_res->start + JZ_REG_MMC_TXFIFO;
> +               conf.slave_id = JZ4740_DMA_TYPE_MMC_TRANSMIT;
> +               chan = host->dma_tx;
> +       } else {
> +               conf.direction = DMA_DEV_TO_MEM;
> +               conf.src_addr = host->mem_res->start + JZ_REG_MMC_RXFIFO;
> +               conf.slave_id = JZ4740_DMA_TYPE_MMC_RECEIVE;
> +               chan = host->dma_rx;
> +       }
> +
> +       host->sg_len = dma_map_sg(chan->device->dev,
> +                                 data->sg,
> +                                 data->sg_len,
> +                                 dir);
> +
> +       if (host->sg_len == 0) {
> +               dev_err(mmc_dev(host->mmc),
> +                       "Failed to map scatterlist for DMA operation\n");
> +               return -EINVAL;
> +       }
> +
> +       dmaengine_slave_config(chan, &conf);
> +       desc = dmaengine_prep_slave_sg(chan,
> +                                      data->sg,
> +                                      host->sg_len,
> +                                      conf.direction,
> +                                      DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +       if (!desc) {
> +               dev_err(mmc_dev(host->mmc),
> +                       "Failed to allocate DMA %s descriptor",
> +                        conf.direction == DMA_MEM_TO_DEV ? "TX" : "RX");
> +               goto dma_unmap;
> +       }
> +
> +       dmaengine_submit(desc);
> +       dma_async_issue_pending(chan);
> +
> +       return 0;
> +
> +dma_unmap:
> +       jz4740_mmc_dma_unmap(host, data);
> +       return -ENOMEM;
> +}
> +
> +/*----------------------------------------------------------------------------*/
> +
>  static void jz4740_mmc_set_irq_enabled(struct jz4740_mmc_host *host,
>         unsigned int irq, bool enabled)
>  {
> @@ -442,6 +575,8 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host,
>                         cmdat |= JZ_MMC_CMDAT_WRITE;
>                 if (cmd->data->flags & MMC_DATA_STREAM)
>                         cmdat |= JZ_MMC_CMDAT_STREAM;
> +               if (host->use_dma)
> +                       cmdat |= JZ_MMC_CMDAT_DMA_EN;
>
>                 writew(cmd->data->blksz, host->base + JZ_REG_MMC_BLKLEN);
>                 writew(cmd->data->blocks, host->base + JZ_REG_MMC_NOB);
> @@ -474,6 +609,7 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
>         struct jz4740_mmc_host *host = (struct jz4740_mmc_host *)devid;
>         struct mmc_command *cmd = host->req->cmd;
>         struct mmc_request *req = host->req;
> +       struct mmc_data *data = cmd->data;
>         bool timeout = false;
>
>         if (cmd->error)
> @@ -484,23 +620,32 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
>                 if (cmd->flags & MMC_RSP_PRESENT)
>                         jz4740_mmc_read_response(host, cmd);
>
> -               if (!cmd->data)
> +               if (!data)
>                         break;
>
>                 jz_mmc_prepare_data_transfer(host);
>
>         case JZ4740_MMC_STATE_TRANSFER_DATA:
> -               if (cmd->data->flags & MMC_DATA_READ)
> -                       timeout = jz4740_mmc_read_data(host, cmd->data);
> +               if (host->use_dma) {
> +                       /* Use DMA if enabled, data transfer direction was
> +                        * defined  before in jz_mmc_prepare_data_transfer().
> +                        */
> +                       timeout = jz4740_mmc_start_dma_transfer(host, data);
> +                       data->bytes_xfered = data->blocks * data->blksz;
> +               } else if (data->flags & MMC_DATA_READ)
> +                       /* If DMA is not enabled, rely on data flags
> +                        * to establish data transfer direction.
> +                        */
> +                       timeout = jz4740_mmc_read_data(host, data);
>                 else
> -                       timeout = jz4740_mmc_write_data(host, cmd->data);
> +                       timeout = jz4740_mmc_write_data(host, data);
>
>                 if (unlikely(timeout)) {
>                         host->state = JZ4740_MMC_STATE_TRANSFER_DATA;
>                         break;
>                 }
>
> -               jz4740_mmc_transfer_check_state(host, cmd->data);
> +               jz4740_mmc_transfer_check_state(host, data);
>
>                 timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_DATA_TRAN_DONE);
>                 if (unlikely(timeout)) {
> @@ -757,7 +902,6 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
>         struct mmc_host *mmc;
>         struct jz4740_mmc_host *host;
>         struct jz4740_mmc_platform_data *pdata;
> -       struct resource *res;
>
>         pdata = pdev->dev.platform_data;
>
> @@ -784,10 +928,11 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
>                 goto err_free_host;
>         }
>
> -       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       host->base = devm_ioremap_resource(&pdev->dev, res);
> +       host->mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       host->base = devm_ioremap_resource(&pdev->dev, host->mem_res);
>         if (IS_ERR(host->base)) {
>                 ret = PTR_ERR(host->base);
> +               dev_err(&pdev->dev, "Failed to ioremap base memory\n");
>                 goto err_free_host;
>         }
>
> @@ -834,6 +979,10 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
>         /* It is not important when it times out, it just needs to timeout. */
>         set_timer_slack(&host->timeout_timer, HZ);
>
> +       host->use_dma = true;
> +       if (host->use_dma && jz4740_mmc_acquire_dma_channels(host) != 0)
> +               host->use_dma = false;
> +
>         platform_set_drvdata(pdev, host);
>         ret = mmc_add_host(mmc);
>
> @@ -843,6 +992,10 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
>         }
>         dev_info(&pdev->dev, "JZ SD/MMC card driver registered\n");
>
> +       dev_info(&pdev->dev, "Using %s, %d-bit mode\n",
> +                host->use_dma ? "DMA" : "PIO",
> +                (mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1);
> +
>         return 0;
>
>  err_free_irq:
> @@ -850,6 +1003,8 @@ err_free_irq:
>  err_free_gpios:
>         jz4740_mmc_free_gpios(pdev);
>  err_gpio_bulk_free:
> +       if (host->use_dma)
> +               jz4740_mmc_release_dma_channels(host);
>         jz_gpio_bulk_free(jz4740_mmc_pins, jz4740_mmc_num_pins(host));
>  err_free_host:
>         mmc_free_host(mmc);
> @@ -872,6 +1027,9 @@ static int jz4740_mmc_remove(struct platform_device *pdev)
>         jz4740_mmc_free_gpios(pdev);
>         jz_gpio_bulk_free(jz4740_mmc_pins, jz4740_mmc_num_pins(host));
>
> +       if (host->use_dma)
> +               jz4740_mmc_release_dma_channels(host);
> +
>         mmc_free_host(host->mmc);
>
>         return 0;
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-07-21  4:37 ` [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer Apelete Seketeli
@ 2014-08-12 16:22   ` Ulf Hansson
  2014-08-12 18:10     ` Apelete Seketeli
  2014-08-12 18:12   ` Chris Ball
  1 sibling, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2014-08-12 16:22 UTC (permalink / raw)
  To: Apelete Seketeli
  Cc: Chris Ball, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc,
	linux-kernel@vger.kernel.org

On 21 July 2014 06:37, Apelete Seketeli <apelete@seketeli.net> wrote:
> Make use of the MMC asynchronous request capability to prepare the
> next DMA transfer request in parallel with the current transfer.
> This is done by adding pre-request and post-request callbacks that are
> used by the MMC framework during an active data transfer.
>
> It should help reduce the impact of DMA preparation overhead on the SD
> card performance.
>
> Signed-off-by: Apelete Seketeli <apelete@seketeli.net>

Thanks! Queued for 3.18.

Kind regards
Uffe


> ---
>  drivers/mmc/host/jz4740_mmc.c |  138 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 116 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 049b133..14738cd 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -114,6 +114,11 @@ enum jz4740_mmc_state {
>         JZ4740_MMC_STATE_DONE,
>  };
>
> +struct jz4740_mmc_host_next {
> +       int sg_len;
> +       s32 cookie;
> +};
> +
>  struct jz4740_mmc_host {
>         struct mmc_host *mmc;
>         struct platform_device *pdev;
> @@ -143,6 +148,7 @@ struct jz4740_mmc_host {
>         /* DMA support */
>         struct dma_chan *dma_rx;
>         struct dma_chan *dma_tx;
> +       struct jz4740_mmc_host_next next_data;
>         bool use_dma;
>         int sg_len;
>
> @@ -184,6 +190,9 @@ static int jz4740_mmc_acquire_dma_channels(struct jz4740_mmc_host *host)
>                 goto free_master_write;
>         }
>
> +       /* Initialize DMA pre request cookie */
> +       host->next_data.cookie = 1;
> +
>         return 0;
>
>  free_master_write:
> @@ -196,23 +205,72 @@ static inline int jz4740_mmc_get_dma_dir(struct mmc_data *data)
>         return (data->flags & MMC_DATA_READ) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>  }
>
> +static inline struct dma_chan *jz4740_mmc_get_dma_chan(struct jz4740_mmc_host *host,
> +                                                      struct mmc_data *data)
> +{
> +       return (data->flags & MMC_DATA_READ) ? host->dma_rx : host->dma_tx;
> +}
> +
>  static void jz4740_mmc_dma_unmap(struct jz4740_mmc_host *host,
>                                  struct mmc_data *data)
>  {
> -       struct dma_chan *chan;
> +       struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
>         enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
>
> -       if (dir == DMA_TO_DEVICE)
> -               chan = host->dma_tx;
> -       else
> -               chan = host->dma_rx;
> -
>         dma_unmap_sg(chan->device->dev, data->sg, data->sg_len, dir);
>  }
>
> +/* Prepares DMA data for current/next transfer, returns non-zero on failure */
> +static int jz4740_mmc_prepare_dma_data(struct jz4740_mmc_host *host,
> +                                      struct mmc_data *data,
> +                                      struct jz4740_mmc_host_next *next,
> +                                      struct dma_chan *chan)
> +{
> +       struct jz4740_mmc_host_next *next_data = &host->next_data;
> +       enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
> +       int sg_len;
> +
> +       if (!next && data->host_cookie &&
> +           data->host_cookie != host->next_data.cookie) {
> +               dev_warn(mmc_dev(host->mmc),
> +                        "[%s] invalid cookie: data->host_cookie %d host->next_data.cookie %d\n",
> +                        __func__,
> +                        data->host_cookie,
> +                        host->next_data.cookie);
> +               data->host_cookie = 0;
> +       }
> +
> +       /* Check if next job is already prepared */
> +       if (next || data->host_cookie != host->next_data.cookie) {
> +               sg_len = dma_map_sg(chan->device->dev,
> +                                   data->sg,
> +                                   data->sg_len,
> +                                   dir);
> +
> +       } else {
> +               sg_len = next_data->sg_len;
> +               next_data->sg_len = 0;
> +       }
> +
> +       if (sg_len <= 0) {
> +               dev_err(mmc_dev(host->mmc),
> +                       "Failed to map scatterlist for DMA operation\n");
> +               return -EINVAL;
> +       }
> +
> +       if (next) {
> +               next->sg_len = sg_len;
> +               data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
> +       } else
> +               host->sg_len = sg_len;
> +
> +       return 0;
> +}
> +
>  static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
>                                          struct mmc_data *data)
>  {
> +       int ret;
>         struct dma_chan *chan;
>         struct dma_async_tx_descriptor *desc;
>         struct dma_slave_config conf = {
> @@ -221,9 +279,8 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
>                 .src_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
>                 .dst_maxburst = JZ4740_MMC_FIFO_HALF_SIZE,
>         };
> -       enum dma_data_direction dir = jz4740_mmc_get_dma_dir(data);
>
> -       if (dir == DMA_TO_DEVICE) {
> +       if (data->flags & MMC_DATA_WRITE) {
>                 conf.direction = DMA_MEM_TO_DEV;
>                 conf.dst_addr = host->mem_res->start + JZ_REG_MMC_TXFIFO;
>                 conf.slave_id = JZ4740_DMA_TYPE_MMC_TRANSMIT;
> @@ -235,16 +292,9 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
>                 chan = host->dma_rx;
>         }
>
> -       host->sg_len = dma_map_sg(chan->device->dev,
> -                                 data->sg,
> -                                 data->sg_len,
> -                                 dir);
> -
> -       if (host->sg_len == 0) {
> -               dev_err(mmc_dev(host->mmc),
> -                       "Failed to map scatterlist for DMA operation\n");
> -               return -EINVAL;
> -       }
> +       ret = jz4740_mmc_prepare_dma_data(host, data, NULL, chan);
> +       if (ret)
> +               return ret;
>
>         dmaengine_slave_config(chan, &conf);
>         desc = dmaengine_prep_slave_sg(chan,
> @@ -269,6 +319,43 @@ dma_unmap:
>         return -ENOMEM;
>  }
>
> +static void jz4740_mmc_pre_request(struct mmc_host *mmc,
> +                                  struct mmc_request *mrq,
> +                                  bool is_first_req)
> +{
> +       struct jz4740_mmc_host *host = mmc_priv(mmc);
> +       struct mmc_data *data = mrq->data;
> +       struct jz4740_mmc_host_next *next_data = &host->next_data;
> +
> +       BUG_ON(data->host_cookie);
> +
> +       if (host->use_dma) {
> +               struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
> +
> +               if (jz4740_mmc_prepare_dma_data(host, data, next_data, chan))
> +                       data->host_cookie = 0;
> +       }
> +}
> +
> +static void jz4740_mmc_post_request(struct mmc_host *mmc,
> +                                   struct mmc_request *mrq,
> +                                   int err)
> +{
> +       struct jz4740_mmc_host *host = mmc_priv(mmc);
> +       struct mmc_data *data = mrq->data;
> +
> +       if (host->use_dma && data->host_cookie) {
> +               jz4740_mmc_dma_unmap(host, data);
> +               data->host_cookie = 0;
> +       }
> +
> +       if (err) {
> +               struct dma_chan *chan = jz4740_mmc_get_dma_chan(host, data);
> +
> +               dmaengine_terminate_all(chan);
> +       }
> +}
> +
>  /*----------------------------------------------------------------------------*/
>
>  static void jz4740_mmc_set_irq_enabled(struct jz4740_mmc_host *host,
> @@ -627,14 +714,19 @@ static irqreturn_t jz_mmc_irq_worker(int irq, void *devid)
>
>         case JZ4740_MMC_STATE_TRANSFER_DATA:
>                 if (host->use_dma) {
> -                       /* Use DMA if enabled, data transfer direction was
> -                        * defined  before in jz_mmc_prepare_data_transfer().
> +                       /* Use DMA if enabled.
> +                        * Data transfer direction is defined later by
> +                        * relying on data flags in
> +                        * jz4740_mmc_prepare_dma_data() and
> +                        * jz4740_mmc_start_dma_transfer().
>                          */
>                         timeout = jz4740_mmc_start_dma_transfer(host, data);
>                         data->bytes_xfered = data->blocks * data->blksz;
>                 } else if (data->flags & MMC_DATA_READ)
> -                       /* If DMA is not enabled, rely on data flags
> -                        * to establish data transfer direction.
> +                       /* Use PIO if DMA is not enabled.
> +                        * Data transfer direction was defined before
> +                        * by relying on data flags in
> +                        * jz_mmc_prepare_data_transfer().
>                          */
>                         timeout = jz4740_mmc_read_data(host, data);
>                 else
> @@ -809,6 +901,8 @@ static void jz4740_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>
>  static const struct mmc_host_ops jz4740_mmc_ops = {
>         .request        = jz4740_mmc_request,
> +       .pre_req        = jz4740_mmc_pre_request,
> +       .post_req       = jz4740_mmc_post_request,
>         .set_ios        = jz4740_mmc_set_ios,
>         .get_ro         = mmc_gpio_get_ro,
>         .get_cd         = mmc_gpio_get_cd,
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-08-12 16:22   ` Ulf Hansson
@ 2014-08-12 18:10     ` Apelete Seketeli
  2014-08-12 20:15       ` Ulf Hansson
  0 siblings, 1 reply; 9+ messages in thread
From: Apelete Seketeli @ 2014-08-12 18:10 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Chris Ball, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc,
	linux-kernel@vger.kernel.org

On Tue, Aug-12-2014 at 06:22:04 PM +0200, Ulf Hansson wrote:
> On 21 July 2014 06:37, Apelete Seketeli <apelete@seketeli.net> wrote:
> > Make use of the MMC asynchronous request capability to prepare the
> > next DMA transfer request in parallel with the current transfer.
> > This is done by adding pre-request and post-request callbacks that are
> > used by the MMC framework during an active data transfer.
> >
> > It should help reduce the impact of DMA preparation overhead on the SD
> > card performance.
> >
> > Signed-off-by: Apelete Seketeli <apelete@seketeli.net>
> 
> Thanks! Queued for 3.18.

W00t \o/ <-- hacker joy. Thank you :).

Just out of curiosity, why not pulling these for 3.17 ?

Cheers.
-- 
        Apelete

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-07-21  4:37 ` [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer Apelete Seketeli
  2014-08-12 16:22   ` Ulf Hansson
@ 2014-08-12 18:12   ` Chris Ball
  2014-08-12 20:02     ` Apelete Seketeli
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Ball @ 2014-08-12 18:12 UTC (permalink / raw)
  To: Apelete Seketeli
  Cc: Ulf Hansson, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc, linux-kernel

Hi Apelete,

On Mon, Jul 21 2014, Apelete Seketeli wrote:
> It should help reduce the impact of DMA preparation overhead on the SD
> card performance.

Did you do any benchmarking to check that this is true?  (If so, it'd
be good to note what performance change you saw in the commit message.)

Thanks!

- Chris.
-- 
Chris Ball   <http://printf.net/>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-08-12 18:12   ` Chris Ball
@ 2014-08-12 20:02     ` Apelete Seketeli
  0 siblings, 0 replies; 9+ messages in thread
From: Apelete Seketeli @ 2014-08-12 20:02 UTC (permalink / raw)
  To: Chris Ball
  Cc: Ulf Hansson, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc, linux-kernel

Hi Chris,

On Tue, Aug-12-2014 at 07:12:33 PM +0100, Chris Ball wrote:
> Hi Apelete,
> 
> On Mon, Jul 21 2014, Apelete Seketeli wrote:
> > It should help reduce the impact of DMA preparation overhead on the SD
> > card performance.
> 
> Did you do any benchmarking to check that this is true?  (If so, it'd
> be good to note what performance change you saw in the commit message.)

I actually did a few quick benchmarks, posted the results in the cover
letter: https://lkml.org/lkml/2014/7/21/11.
As you can see the improvement is measurable, although not significant
with zcat.

Cheers.
-- 
        Apelete

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer
  2014-08-12 18:10     ` Apelete Seketeli
@ 2014-08-12 20:15       ` Ulf Hansson
  0 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2014-08-12 20:15 UTC (permalink / raw)
  To: Apelete Seketeli
  Cc: Chris Ball, H Hartley Sweeten, Lars-Peter Clausen, Wei Yongjun,
	Alex Smith, Laurent Pinchart, linux-mmc,
	linux-kernel@vger.kernel.org

On 12 August 2014 20:10, Apelete Seketeli <apelete@seketeli.net> wrote:
> On Tue, Aug-12-2014 at 06:22:04 PM +0200, Ulf Hansson wrote:
>> On 21 July 2014 06:37, Apelete Seketeli <apelete@seketeli.net> wrote:
>> > Make use of the MMC asynchronous request capability to prepare the
>> > next DMA transfer request in parallel with the current transfer.
>> > This is done by adding pre-request and post-request callbacks that are
>> > used by the MMC framework during an active data transfer.
>> >
>> > It should help reduce the impact of DMA preparation overhead on the SD
>> > card performance.
>> >
>> > Signed-off-by: Apelete Seketeli <apelete@seketeli.net>
>>
>> Thanks! Queued for 3.18.
>
> W00t \o/ <-- hacker joy. Thank you :).
>
> Just out of curiosity, why not pulling these for 3.17 ?

I have had a vacation period. Since these are quite big changes, I
think they deserve some testing in linux-next and did not want to pull
in some late changes for 3.17.

Kind regards
Uffe

>
> Cheers.
> --
>         Apelete

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-08-12 20:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-21  4:37 [PATCH v6 0/2] Use DMA for data transfers in JZ4740 MMC driver Apelete Seketeli
2014-07-21  4:37 ` [PATCH v6 1/2] mmc: jz4740: add dma infrastructure for data transfers Apelete Seketeli
2014-08-12 16:21   ` Ulf Hansson
2014-07-21  4:37 ` [PATCH v6 2/2] mmc: jz4740: prepare next dma transfer in parallel with current transfer Apelete Seketeli
2014-08-12 16:22   ` Ulf Hansson
2014-08-12 18:10     ` Apelete Seketeli
2014-08-12 20:15       ` Ulf Hansson
2014-08-12 18:12   ` Chris Ball
2014-08-12 20:02     ` Apelete Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox