* [V2,3/5] dmaengine: dmatest: Add alignment parameter
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Add parameter "alignment" to allow setting the address alignment
manually. Having the ability to configure address alignment from
user space adds new testing capabilities where different alignments can
be configured for testing without having to modify the dma device
alignment properties.
If configured, the alignment value will override the device alignment
property of the target device.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
drivers/dma/dmatest.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 3c55190..c980865 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -79,6 +79,10 @@ static bool verbose;
module_param(verbose, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
+static int alignment = -1;
+module_param(alignment, int, 0644);
+MODULE_PARM_DESC(alignment, "Custom data address alignment taken as 2^(alignment) (default: not used (-1))");
+
/**
* struct dmatest_params - test parameters.
* @buf_size: size of the memcpy test buffer
@@ -103,6 +107,7 @@ struct dmatest_params {
int timeout;
bool noverify;
bool norandom;
+ int alignment;
};
/**
@@ -519,22 +524,26 @@ static int dmatest_func(void *data)
chan = thread->chan;
dev = chan->device;
if (thread->type == DMA_MEMCPY) {
- align = dev->copy_align;
+ align = params->alignment < 0 ? dev->copy_align :
+ params->alignment;
src_cnt = dst_cnt = 1;
} else if (thread->type == DMA_MEMSET) {
- align = dev->fill_align;
+ align = params->alignment < 0 ? dev->fill_align :
+ params->alignment;
src_cnt = dst_cnt = 1;
is_memset = true;
} else if (thread->type == DMA_XOR) {
/* force odd to ensure dst = src */
src_cnt = min_odd(params->xor_sources | 1, dev->max_xor);
dst_cnt = 1;
- align = dev->xor_align;
+ align = params->alignment < 0 ? dev->xor_align :
+ params->alignment;
} else if (thread->type == DMA_PQ) {
/* force odd to ensure dst = src */
src_cnt = min_odd(params->pq_sources | 1, dma_maxpq(dev, 0));
dst_cnt = 2;
- align = dev->pq_align;
+ align = params->alignment < 0 ? dev->pq_align :
+ params->alignment;
pq_coefs = kmalloc(params->pq_sources + 1, GFP_KERNEL);
if (!pq_coefs)
@@ -1032,6 +1041,7 @@ static void add_threaded_test(struct dmatest_info *info)
params->timeout = timeout;
params->noverify = noverify;
params->norandom = norandom;
+ params->alignment = alignment;
request_channels(info, DMA_MEMCPY);
request_channels(info, DMA_MEMSET);
^ permalink raw reply related
* [V2,2/5] dmaengine: dmatest: Use fixed point div to calculate iops
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Use fixed point division to calculate iops to prevent reporting 0 iops
when operations last for longer than a second.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
drivers/dma/dmatest.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 8ce3b06..3c55190 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -170,6 +170,13 @@ MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
#define PATTERN_COUNT_MASK 0x1f
#define PATTERN_MEMSET_IDX 0x01
+/* Fixed point arithmetic ops */
+#define FIXPT_SHIFT 8
+#define FIXPNT_MASK 0xFF
+#define FIXPT_TO_INT(a) ((a) >> FIXPT_SHIFT)
+#define INT_TO_FIXPT(a) ((a) << FIXPT_SHIFT)
+#define FIXPT_GET_FRAC(a) ((((a) & FIXPNT_MASK) * 100) >> FIXPT_SHIFT)
+
/* poor man's completion - we want to use wait_event_freezable() on it */
struct dmatest_done {
bool done;
@@ -446,13 +453,15 @@ static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
}
per_sec *= val;
+ per_sec = INT_TO_FIXPT(per_sec);
do_div(per_sec, runtime);
+
return per_sec;
}
static unsigned long long dmatest_KBs(s64 runtime, unsigned long long len)
{
- return dmatest_persec(runtime, len >> 10);
+ return FIXPT_TO_INT(dmatest_persec(runtime, len >> 10));
}
/*
@@ -493,6 +502,7 @@ static int dmatest_func(void *data)
ktime_t comparetime = 0;
s64 runtime = 0;
unsigned long long total_len = 0;
+ unsigned long long iops = 0;
u8 align = 0;
bool is_memset = false;
dma_addr_t *srcs;
@@ -833,9 +843,10 @@ static int dmatest_func(void *data)
err_srcs:
kfree(pq_coefs);
err_thread_type:
- pr_info("%s: summary %u tests, %u failures %llu iops %llu KB/s (%d)\n",
+ iops = dmatest_persec(runtime, total_tests);
+ pr_info("%s: summary %u tests, %u failures %llu.%02llu iops %llu KB/s (%d)\n",
current->comm, total_tests, failed_tests,
- dmatest_persec(runtime, total_tests),
+ FIXPT_TO_INT(iops), FIXPT_GET_FRAC(iops),
dmatest_KBs(runtime, total_len), ret);
/* terminate all transfers on specified channels */
^ permalink raw reply related
* [V2,1/5] dmaengine: dmatest: Add support for multi channel testing
From: Seraj Alijan @ 2018-08-31 11:03 UTC (permalink / raw)
To: vkoul
Cc: dmaengine, dan.j.williams, james.hartley, sifan.naeem, ed.blake,
Seraj Alijan
Add support for running tests on multiple channels simultaneously as the
driver currently limits to 1 channel per test run. This will add support
for stress testing DMA controllers with multi channel capabilities.
This is done by adding a callback function to the "channel" parameter
that registers the requested channel prior to the "run" parameter being
set to 1. Each time the "channel" parameter is populated with a new
dma channel, a new test is appended to the thread queue. Once the "run"
parameter is set to 1, the test will kick start all pending threads.
Signed-off-by: Seraj Alijan <seraj.alijan@sondrel.com>
---
drivers/dma/dmatest.c | 109 +++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 89 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index aa1712b..8ce3b06 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -27,11 +27,6 @@ static unsigned int test_buf_size = 16384;
module_param(test_buf_size, uint, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(test_buf_size, "Size of the memcpy test buffer");
-static char test_channel[20];
-module_param_string(channel, test_channel, sizeof(test_channel),
- S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
-
static char test_device[32];
module_param_string(device, test_device, sizeof(test_device),
S_IRUGO | S_IWUSR);
@@ -139,6 +134,21 @@ static bool dmatest_run;
module_param_cb(run, &run_ops, &dmatest_run, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(run, "Run the test (default: false)");
+static int dmatest_chan_set(const char *val, const struct kernel_param *kp);
+static int dmatest_chan_get(char *val, const struct kernel_param *kp);
+static const struct kernel_param_ops multi_chan_ops = {
+ .set = dmatest_chan_set,
+ .get = dmatest_chan_get,
+};
+
+static char test_channel[20];
+static struct kparam_string newchan_kps = {
+ .string = test_channel,
+ .maxlen = 20,
+};
+module_param_cb(channel, &multi_chan_ops, &newchan_kps, 0644);
+MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
+
/* Maximum amount of mismatched bytes in buffer to print */
#define MAX_ERROR_COUNT 32
@@ -179,6 +189,7 @@ struct dmatest_thread {
wait_queue_head_t done_wait;
struct dmatest_done test_done;
bool done;
+ bool pending;
};
struct dmatest_chan {
@@ -206,6 +217,22 @@ static bool is_threaded_test_run(struct dmatest_info *info)
return false;
}
+static bool is_threaded_test_pending(struct dmatest_info *info)
+{
+ struct dmatest_chan *dtc;
+
+ list_for_each_entry(dtc, &info->channels, node) {
+ struct dmatest_thread *thread;
+
+ list_for_each_entry(thread, &dtc->threads, node) {
+ if (thread->pending)
+ return true;
+ }
+ }
+
+ return false;
+}
+
static int dmatest_wait_get(char *val, const struct kernel_param *kp)
{
struct dmatest_info *info = &test_info;
@@ -476,6 +503,7 @@ static int dmatest_func(void *data)
ret = -ENOMEM;
smp_rmb();
+ thread->pending = false;
info = thread->info;
params = &info->params;
chan = thread->chan;
@@ -886,7 +914,7 @@ static int dmatest_add_threads(struct dmatest_info *info,
/* srcbuf and dstbuf are allocated by the thread itself */
get_task_struct(thread->task);
list_add_tail(&thread->node, &dtc->threads);
- wake_up_process(thread->task);
+ thread->pending = true;
}
return i;
@@ -932,7 +960,7 @@ static int dmatest_add_channel(struct dmatest_info *info,
thread_count += cnt > 0 ? cnt : 0;
}
- pr_info("Started %u threads using %s\n",
+ pr_info("Added %u threads using %s\n",
thread_count, dma_chan_name(chan));
list_add_tail(&dtc->node, &info->channels);
@@ -977,7 +1005,7 @@ static void request_channels(struct dmatest_info *info,
}
}
-static void run_threaded_test(struct dmatest_info *info)
+static void add_threaded_test(struct dmatest_info *info)
{
struct dmatest_params *params = &info->params;
@@ -1000,6 +1028,19 @@ static void run_threaded_test(struct dmatest_info *info)
request_channels(info, DMA_PQ);
}
+static void run_pending_tests(struct dmatest_info *info)
+{
+ struct dmatest_chan *dtc;
+
+ list_for_each_entry(dtc, &info->channels, node) {
+ struct dmatest_thread *thread;
+
+ list_for_each_entry(thread, &dtc->threads, node) {
+ wake_up_process(thread->task);
+ }
+ }
+}
+
static void stop_threaded_test(struct dmatest_info *info)
{
struct dmatest_chan *dtc, *_dtc;
@@ -1016,7 +1057,7 @@ static void stop_threaded_test(struct dmatest_info *info)
info->nr_channels = 0;
}
-static void restart_threaded_test(struct dmatest_info *info, bool run)
+static void start_threaded_tests(struct dmatest_info *info)
{
/* we might be called early to set run=, defer running until all
* parameters have been evaluated
@@ -1024,11 +1065,7 @@ static void restart_threaded_test(struct dmatest_info *info, bool run)
if (!info->did_init)
return;
- /* Stop any running test first */
- stop_threaded_test(info);
-
- /* Run test with new parameters */
- run_threaded_test(info);
+ run_pending_tests(info);
}
static int dmatest_run_get(char *val, const struct kernel_param *kp)
@@ -1039,7 +1076,8 @@ static int dmatest_run_get(char *val, const struct kernel_param *kp)
if (is_threaded_test_run(info)) {
dmatest_run = true;
} else {
- stop_threaded_test(info);
+ if (!is_threaded_test_pending(info))
+ stop_threaded_test(info);
dmatest_run = false;
}
mutex_unlock(&info->lock);
@@ -1057,18 +1095,48 @@ static int dmatest_run_set(const char *val, const struct kernel_param *kp)
if (ret) {
mutex_unlock(&info->lock);
return ret;
+ } else if (dmatest_run && is_threaded_test_pending(info)) {
+ start_threaded_tests(info);
}
- if (is_threaded_test_run(info))
- ret = -EBUSY;
- else if (dmatest_run)
- restart_threaded_test(info, dmatest_run);
+ mutex_unlock(&info->lock);
+
+ return ret;
+}
+
+static int dmatest_chan_set(const char *val, const struct kernel_param *kp)
+{
+ struct dmatest_info *info = &test_info;
+ int ret;
+ mutex_lock(&info->lock);
+ ret = param_set_copystring(val, kp);
+ if (ret) {
+ mutex_unlock(&info->lock);
+ return ret;
+ }
+ if (!is_threaded_test_run(info) && !is_threaded_test_pending(info))
+ stop_threaded_test(info);
+ add_threaded_test(info);
mutex_unlock(&info->lock);
return ret;
}
+static int dmatest_chan_get(char *val, const struct kernel_param *kp)
+{
+ struct dmatest_info *info = &test_info;
+
+ mutex_lock(&info->lock);
+ if (!is_threaded_test_run(info) && !is_threaded_test_pending(info)) {
+ stop_threaded_test(info);
+ strlcpy(test_channel, "", sizeof(test_channel));
+ }
+ mutex_unlock(&info->lock);
+
+ return param_get_string(val, kp);
+}
+
static int __init dmatest_init(void)
{
struct dmatest_info *info = &test_info;
@@ -1076,7 +1144,8 @@ static int __init dmatest_init(void)
if (dmatest_run) {
mutex_lock(&info->lock);
- run_threaded_test(info);
+ add_threaded_test(info);
+ run_pending_tests(info);
mutex_unlock(&info->lock);
}
^ permalink raw reply related
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Robin Gong @ 2018-08-31 9:49 UTC (permalink / raw)
To: Lucas Stach, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
Hi Lucas,
Seems I miss your previous mail. Thanks for your patch, but if move most jobs
of sdma_disable_channel_with_delay() into worker, that will bring another race condition
that upper driver such as Audio terminate channel and free resource of dma channel without
really channel stop, if dma transfer done interrupt come after that, oops or kernel cash may
be caught. Leave 'sdmac->desc = NULL' in the sdma_disable_channel_with_delay() may fix
such potential issue.
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2018年8月30日 21:22
> To: Vinod Koul <vkoul@kernel.org>
> Cc: Robin Gong <yibin.gong@nxp.com>; dmaengine@vger.kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; kernel@pengutronix.de;
> patchwork-lst@pengutronix.de
> Subject: [PATCH 3/4] dmaengine: imx-sdma: implement channel termination via
> worker
>
> The dmaengine documentation states that device_terminate_all may be
> asynchronous and need not wait for the active transfers to stop.
>
> This allows us to move most of the functionality currently implemented in the
> sdma channel termination function to run in a worker, outside of any atomic
> context. Moving this out of atomic context has two
> benefits: we can now sleep while waiting for the channel to terminate, instead
> of busy waiting and the freeing of the dma descriptors happens with IRQs
> enabled, getting rid of a warning in the dma mapping code.
>
> As the termination is now async, we need to implement the device_synchronize
> dma engine function which simply waits for the worker to finish its execution.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/dma/imx-sdma.c | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> 8d2fec8b16cc..a3ac216ede37 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -32,6 +32,7 @@
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> #include <linux/of_dma.h>
> +#include <linux/workqueue.h>
>
> #include <asm/irq.h>
> #include <linux/platform_data/dma-imx-sdma.h>
> @@ -375,6 +376,7 @@ struct sdma_channel {
> u32 shp_addr, per_addr;
> enum dma_status status;
> struct imx_dma_data data;
> + struct work_struct terminate_worker;
> };
>
> #define IMX_DMA_SG_LOOP BIT(0)
> @@ -1025,31 +1027,45 @@ static int sdma_disable_channel(struct dma_chan
> *chan)
>
> return 0;
> }
> -
> -static int sdma_disable_channel_with_delay(struct dma_chan *chan)
> +static void sdma_channel_terminate(struct work_struct *work)
> {
> - struct sdma_channel *sdmac = to_sdma_chan(chan);
> + struct sdma_channel *sdmac = container_of(work, struct sdma_channel,
> + terminate_worker);
> unsigned long flags;
> LIST_HEAD(head);
>
> - sdma_disable_channel(chan);
> - spin_lock_irqsave(&sdmac->vc.lock, flags);
> - vchan_get_all_descriptors(&sdmac->vc, &head);
> - sdmac->desc = NULL;
> - spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> - vchan_dma_desc_free_list(&sdmac->vc, &head);
> -
> /*
> * According to NXP R&D team a delay of one BD SDMA cost time
> * (maximum is 1ms) should be added after disable of the channel
> * bit, to ensure SDMA core has really been stopped after SDMA
> * clients call .device_terminate_all.
> */
> - mdelay(1);
> + usleep_range(1000, 2000);
> +
> + spin_lock_irqsave(&sdmac->vc.lock, flags);
> + vchan_get_all_descriptors(&sdmac->vc, &head);
> + sdmac->desc = NULL;
> + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> + vchan_dma_desc_free_list(&sdmac->vc, &head); }
> +
> +static int sdma_disable_channel_with_delay(struct dma_chan *chan) {
> + struct sdma_channel *sdmac = to_sdma_chan(chan);
> +
> + sdma_disable_channel(chan);
> + schedule_work(&sdmac->terminate_worker);
>
> return 0;
> }
>
> +static void sdma_channel_synchronize(struct dma_chan *chan) {
> + struct sdma_channel *sdmac = to_sdma_chan(chan);
> +
> + flush_work(&sdmac->terminate_worker);
> +}
> +
> static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
> {
> struct sdma_engine *sdma = sdmac->sdma; @@ -1993,6 +2009,7 @@
> static int sdma_probe(struct platform_device *pdev)
>
> sdmac->channel = i;
> sdmac->vc.desc_free = sdma_desc_free;
> + INIT_WORK(&sdmac->terminate_worker,
> sdma_channel_terminate);
> /*
> * Add the channel to the DMAC list. Do not add channel 0 though
> * because we need it internally in the SDMA driver. This also means
> @@ -2045,6 +2062,7 @@ static int sdma_probe(struct platform_device
> *pdev)
> sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic;
> sdma->dma_device.device_config = sdma_config;
> sdma->dma_device.device_terminate_all =
> sdma_disable_channel_with_delay;
> + sdma->dma_device.device_synchronize = sdma_channel_synchronize;
> sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS;
> sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
> sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
> --
> 2.18.0
^ permalink raw reply
* [v4,2/7] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
From: Vinod Koul @ 2018-08-30 13:27 UTC (permalink / raw)
To: Andrea Merello
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-kernel, Rob Herring, Mark Rutland, devicetree,
Radhey Shyam Pandey
On 30-08-18, 10:11, Andrea Merello wrote:
> On Wed, Aug 29, 2018 at 10:12 AM Andrea Merello
> <andrea.merello@gmail.com> wrote:
> >
> > On Mon, Aug 27, 2018 at 7:30 AM Vinod <vkoul@kernel.org> wrote:
> > >
> > > On 02-08-18, 16:10, Andrea Merello wrote:
> > >
> > > s/cylic/cyclic in patch title
> >
> > OK
> >
> > > > Whenever a single or cyclic transaction is prepared, the driver
> > > > could eventually split it over several SG descriptors in order
> > > > to deal with the HW maximum transfer length.
> > > >
> > > > This could end up in DMA operations starting from a misaligned
> > > > address. This seems fatal for the HW if DRE is not enabled.
> > >
> > > DRE?
> >
> > Stands for "Data Realignment Engine". I will add this string nearby
> > the acronym..
> >
> > > >
> > > > This patch eventually adjusts the transfer size in order to make sure
> > > > all operations start from an aligned address.
> > > >
> > > > Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > > > Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > > ---
> > > > Changes in v2:
> > > > - don't introduce copy_mask field, rather rely on already-esistent
> > > > copy_align field. Suggested by Radhey Shyam Pandey
> > > > - reword title
> > > > Changes in v3:
> > > > - fix bug introduced in v2: wrong copy size when DRE is enabled
> > > > - use implementation suggested by Radhey Shyam Pandey
> > > > Changes in v4:
> > > > - rework on the top of 1/6
> > > > ---
> > > > drivers/dma/xilinx/xilinx_dma.c | 22 ++++++++++++++++++----
> > > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > > > index a3aaa0e34cc7..aaa6de8a70e4 100644
> > > > --- a/drivers/dma/xilinx/xilinx_dma.c
> > > > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > > > @@ -954,15 +954,28 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
> > > >
> > > > /**
> > > > * xilinx_dma_calc_copysize - Calculate the amount of data to copy
> > > > + * @chan: Driver specific DMA channel
> > > > * @size: Total data that needs to be copied
> > > > * @done: Amount of data that has been already copied
> > > > *
> > > > * Return: Amount of data that has to be copied
> > > > */
> > > > -static int xilinx_dma_calc_copysize(int size, int done)
> > > > +static int xilinx_dma_calc_copysize(struct xilinx_dma_chan *chan,
> > > > + int size, int done)
> > >
> > > please align with opening brace
> >
> > OK
>
> Sorry for getting back on this.
> I've checked it, but it seems already aligned with opening brace in
> the original e-mail text I've sent. (4 tabs + 4 spaces).
Okay, please see that code looks fine, I will check after I apply
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Vinod Koul @ 2018-08-30 13:26 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, linux-kernel, lars, radheys
Hey Peter,
On 30-08-18, 11:57, Peter Ujfalusi wrote:
> On 2018-08-29 19:22, Vinod wrote:
> >>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> >>>> + * descriptor
> >>>> + * 3. submit the transfer
> >>>> + * - DMA_DEV_TO_MEM:
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> >>>> + * descriptor
> >>>> + * 3. submit the transfer
> >>>> + * 4. when the transfer is completed, the metadata should be available in the
> >>>> + * attached buffer
> >>>
> >>> I guess this is good to be moved into Documentation
> >>
> >> Should I create a new file for metadata? I guess it would make sense as the
> >> information is for both clients and engines.
> >
> > Hmm not sure, lets see how it looks as entries in these files, detailing
> > roles of clients and providers
>
> Update both client and provider documentation with tailoring the content
> for the audience?
Right :)
> >>> also we dont allow this for memcpy txn?
> >>
> >> I have not thought about that, but if I think about it it should be along the
> >> same lines as MEM_TO_DEV.
> >> I'll add the MEM_TO_MEM as well to the documentation.
> >
> > Okay and lets not implement it then..
>
> I'm not going to implement it, but the documentation could add that if
> metadata is used for MEM_TO_MEM then it is the same use case as with
> MEM_TO_DEV.
Yes that would be sensible
> >>>> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
> >>>> + * driver. The client driver can ask for the pointer, maximum size and the
> >>>> + * currently used size of the metadata and can directly update or read it.
> >>>> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
> >>>> + * provided as helper functions.
> >>>> + *
> >>>> + * Client drivers interested to use this mode can follow:
> >>>> + * - DMA_MEM_TO_DEV:
>
> Here, add DMA_MEM_TO_MEM
>
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
> >>>> + * metadata area
> >>>> + * 3. update the metadata at the pointer
> >>>> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
> >>>> + * of data the client has placed into the metadata buffer
> >>>> + * 5. submit the transfer
> >>>> + * - DMA_DEV_TO_MEM:
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. submit the transfer
> >>>> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
> >>>> + * pointer to the engine's metadata are
> >>>> + * 4. Read out the metadate from the pointer
> >>>> + *
> >>>> + * Note: the two mode is not compatible and clients must use one mode for a
> >>>> + * descriptor.
> >>>> + */
> >>>> +enum dma_desc_metadata_mode {
> >>>> + DESC_METADATA_CLIENT = (1 << 0),
> >>>> + DESC_METADATA_ENGINE = (1 << 1),
> >>>
> >>> BIT(x)
> >>
> >> OK, I followed what we have in the header to not mix (1 << x) and BIT(x)
> >
> > yeah lets update :)
>
> OK.
>
> >>>> +static inline int _desc_check_and_set_metadata_mode(
> >>>
> >>> why does this need to start with _ ?
> >>
> >> To scare people to use in client code ;)
> >
> > Lets not expose to them :D
>
> Sure, if the code moves to dmaengine.c it is granted.
>
> >>> Also I would like to see a use :-) before further comments.
> >>
> >> You mean the DMA driver and at least one client?
> >
> > DMA driver to _at_least_ start with. Client even better
>
> Hrm, I can send the DMA driver as RFC (not to merge, will not compile)
> but I need to do some excess cover letter and documentation since the
> UDMA is 'just' a piece in the data movement architecture and need to
> explain couple of things around it.
>
> I will need couple of days for that for sure.
If we are not merging, we can review it. I am not super excited about
merging w/o user of an API
> >> I have the DMA driver in my public facing branch [1], but it is not an easy
> >> read with it's close to 4k loc.
> >
> > It doesnt exist :P
>
> In this sense it does not, agree.
>
> >> The client is not in my branch and it is actually using an older version of
> >> the metadata support.
> >>
> >> The problem is that I don't know when I will be able to send the driver for
> >> review as all of this is targeting a brand new SoC (AM654) with completely new
> >> data movement architecture. There are lots of dependencies still need to be
> >> upstreamed before I can send something which at least compiles.
> >>
> >> I can offer snippets from the client driver, if that is good enough or a link
> >> to the public tree where it can be accessed, but it is not going to go
> >> upstream before the DMA driver.
> >
> > TBH that's not going to help much, lets come back to it when you need
> > this upstream.
>
> One of the reason I have sent the metadata support early is because
> Radhey was looking for similar thing for xilinx_dma and I already had
> the generic implementation of it which suits his case.
>
> I was planning to send the metadata support along with the DMA driver
> (and other core changes, new features).
>
> If not for me, then for Radhey's stake can the metadata support be
> considered as stand alone for now?
>
> I will send v2 as soon as I have it ready and I will send either v3 with
> the k3-udma DMA driver (UDMA drivers as not for merge) or standalone
> UDMA driver as RFC and for reference.
Okay, Radhey can take your patch and submit with his changes for merge.
^ permalink raw reply
* [4/4] dmaengine: imx-sdma: use GFP_NOWAIT for dma descriptor allocations
From: Lucas Stach @ 2018-08-30 13:22 UTC (permalink / raw)
To: Vinod Koul; +Cc: Robin Gong, dmaengine, linux-imx, kernel, patchwork-lst
DMA buffer descriptors aren't allocated from atomic context, so they
can use the less heavyweigth GFP_NOWAIT.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index a3ac216ede37..2f2e5aff1fdf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1210,7 +1210,7 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
int ret = 0;
desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
- GFP_ATOMIC);
+ GFP_NOWAIT);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
^ permalink raw reply related
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-08-30 13:22 UTC (permalink / raw)
To: Vinod Koul; +Cc: Robin Gong, dmaengine, linux-imx, kernel, patchwork-lst
The dmaengine documentation states that device_terminate_all may be
asynchronous and need not wait for the active transfers to stop.
This allows us to move most of the functionality currently implemented
in the sdma channel termination function to run in a worker, outside
of any atomic context. Moving this out of atomic context has two
benefits: we can now sleep while waiting for the channel to terminate,
instead of busy waiting and the freeing of the dma descriptors happens
with IRQs enabled, getting rid of a warning in the dma mapping code.
As the termination is now async, we need to implement the
device_synchronize dma engine function which simply waits for the
worker to finish its execution.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 40 +++++++++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8d2fec8b16cc..a3ac216ede37 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -32,6 +32,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_dma.h>
+#include <linux/workqueue.h>
#include <asm/irq.h>
#include <linux/platform_data/dma-imx-sdma.h>
@@ -375,6 +376,7 @@ struct sdma_channel {
u32 shp_addr, per_addr;
enum dma_status status;
struct imx_dma_data data;
+ struct work_struct terminate_worker;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -1025,31 +1027,45 @@ static int sdma_disable_channel(struct dma_chan *chan)
return 0;
}
-
-static int sdma_disable_channel_with_delay(struct dma_chan *chan)
+static void sdma_channel_terminate(struct work_struct *work)
{
- struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_channel *sdmac = container_of(work, struct sdma_channel,
+ terminate_worker);
unsigned long flags;
LIST_HEAD(head);
- sdma_disable_channel(chan);
- spin_lock_irqsave(&sdmac->vc.lock, flags);
- vchan_get_all_descriptors(&sdmac->vc, &head);
- sdmac->desc = NULL;
- spin_unlock_irqrestore(&sdmac->vc.lock, flags);
- vchan_dma_desc_free_list(&sdmac->vc, &head);
-
/*
* According to NXP R&D team a delay of one BD SDMA cost time
* (maximum is 1ms) should be added after disable of the channel
* bit, to ensure SDMA core has really been stopped after SDMA
* clients call .device_terminate_all.
*/
- mdelay(1);
+ usleep_range(1000, 2000);
+
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ vchan_get_all_descriptors(&sdmac->vc, &head);
+ sdmac->desc = NULL;
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
+ vchan_dma_desc_free_list(&sdmac->vc, &head);
+}
+
+static int sdma_disable_channel_with_delay(struct dma_chan *chan)
+{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+
+ sdma_disable_channel(chan);
+ schedule_work(&sdmac->terminate_worker);
return 0;
}
+static void sdma_channel_synchronize(struct dma_chan *chan)
+{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+
+ flush_work(&sdmac->terminate_worker);
+}
+
static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
{
struct sdma_engine *sdma = sdmac->sdma;
@@ -1993,6 +2009,7 @@ static int sdma_probe(struct platform_device *pdev)
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
+ INIT_WORK(&sdmac->terminate_worker, sdma_channel_terminate);
/*
* Add the channel to the DMAC list. Do not add channel 0 though
* because we need it internally in the SDMA driver. This also means
@@ -2045,6 +2062,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic;
sdma->dma_device.device_config = sdma_config;
sdma->dma_device.device_terminate_all = sdma_disable_channel_with_delay;
+ sdma->dma_device.device_synchronize = sdma_channel_synchronize;
sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS;
sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
^ permalink raw reply related
* [2/4] Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool"
From: Lucas Stach @ 2018-08-30 13:22 UTC (permalink / raw)
To: Vinod Koul; +Cc: Robin Gong, dmaengine, linux-imx, kernel, patchwork-lst
This reverts commit fe5b85c656bc. The SDMA engine needs the descriptors to
be contiguous in memory. As the dma pool API is only able to provide a
single descriptor per alloc invocation there is no guarantee that multiple
descriptors satisfy this requirement. Also the code in question is broken
as it only allocates memory for a single descriptor, without looking at the
number of descriptors required for the transfer, leading to out-of-bounds
accesses when the descriptors are written.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 3bca5e0c715f..8d2fec8b16cc 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -24,7 +24,6 @@
#include <linux/spinlock.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
-#include <linux/dmapool.h>
#include <linux/firmware.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
@@ -376,7 +375,6 @@ struct sdma_channel {
u32 shp_addr, per_addr;
enum dma_status status;
struct imx_dma_data data;
- struct dma_pool *bd_pool;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -1192,10 +1190,11 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
static int sdma_alloc_bd(struct sdma_desc *desc)
{
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
int ret = 0;
- desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
- &desc->bd_phys);
+ desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
+ GFP_ATOMIC);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
@@ -1206,7 +1205,9 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
static void sdma_free_bd(struct sdma_desc *desc)
{
- dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys);
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
+
+ dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
}
static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1272,10 +1273,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ahb;
- sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev,
- sizeof(struct sdma_buffer_descriptor),
- 32, 0);
-
return 0;
disable_clk_ahb:
@@ -1304,9 +1301,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
-
- dma_pool_destroy(sdmac->bd_pool);
- sdmac->bd_pool = NULL;
}
static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
^ permalink raw reply related
* [1/4] Revert "dmaengine: imx-sdma: Use GFP_NOWAIT for dma allocations"
From: Lucas Stach @ 2018-08-30 13:22 UTC (permalink / raw)
To: Vinod Koul; +Cc: Robin Gong, dmaengine, linux-imx, kernel, patchwork-lst
This reverts commit c1199875d327, as this depends on another commit
that is going to be reverted.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b4ec2d20e661..3bca5e0c715f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1194,8 +1194,8 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
{
int ret = 0;
- desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_NOWAIT,
- &desc->bd_phys);
+ desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
+ &desc->bd_phys);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
^ permalink raw reply related
* [v2,2/2] dmaengine: doc: Add sections for per descriptor metadata support
From: Peter Ujfalusi @ 2018-08-30 12:19 UTC (permalink / raw)
To: dan.j.williams, vkoul; +Cc: dmaengine, linux-kernel, lars, radheys
Update the provider and client documentation with details about the
metadata support.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Documentation/driver-api/dmaengine/client.rst | 70 +++++++++++++++++++
.../driver-api/dmaengine/provider.rst | 46 ++++++++++++
2 files changed, 116 insertions(+)
diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index fbbb2831f29f..584a39347cfe 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -151,6 +151,76 @@ The details of these operations are:
Note that callbacks will always be invoked from the DMA
engines tasklet, never from interrupt context.
+ Optional: per descriptor metadata
+ ---------------------------------
+ DMAengine provides two ways for metadata support.
+
+ DESC_METADATA_CLIENT
+
+ The metadata buffer is allocated/provided by the client driver and it is
+ attached to the descriptor.
+
+ .. code-block:: c
+
+ int dmaengine_desc_attach_metadata(struct dma_async_tx_descriptor *desc,
+ void *data, size_t len);
+
+ DESC_METADATA_ENGINE
+
+ The metadata buffer is allocated/managed by the DMA driver. The client
+ driver can ask for the pointer, maximum size and the currently used size of
+ the metadata and can directly update or read it.
+
+ .. code-block:: c
+
+ void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
+ size_t *payload_len, size_t *max_len);
+
+ int dmaengine_desc_set_metadata_len(struct dma_async_tx_descriptor *desc,
+ size_t payload_len);
+
+ Client drivers can query if a given mode is supported with:
+
+ .. code-block:: c
+
+ bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
+ enum dma_desc_metadata_mode mode);
+
+ Depending on the used mode client drivers must follow different flow.
+
+ DESC_METADATA_CLIENT
+
+ - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+ 1. prepare the descriptor (dmaengine_prep_*)
+ construct the metadata in the client's buffer
+ 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ descriptor
+ 3. submit the transfer
+ - DMA_DEV_TO_MEM:
+ 1. prepare the descriptor (dmaengine_prep_*)
+ 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ descriptor
+ 3. submit the transfer
+ 4. when the transfer is completed, the metadata should be available in the
+ attached buffer
+
+ DESC_METADATA_ENGINE
+
+ - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+ 1. prepare the descriptor (dmaengine_prep_*)
+ 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the
+ engine's metadata area
+ 3. update the metadata at the pointer
+ 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the
+ amount of data the client has placed into the metadata buffer
+ 5. submit the transfer
+ - DMA_DEV_TO_MEM:
+ 1. prepare the descriptor (dmaengine_prep_*)
+ 2. submit the transfer
+ 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
+ pointer to the engine's metadata are
+ 4. Read out the metadate from the pointer
+
4. Submit the transaction
Once the descriptor has been prepared and the callback information
diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index dfc4486b5743..502c59f75302 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -247,6 +247,52 @@ after each transfer. In case of a ring buffer, they may loop
(DMA_CYCLIC). Addresses pointing to a device's register (e.g. a FIFO)
are typically fixed.
+Per descriptor metadata support
+-------------------------------
+Some data movement architecure (DMA controller and peripherals) uses metadata
+associated with a transaction. The DMA controller role is to transfer the
+payload and the metadata alongside.
+The metadata itself is not used by the DMA engine itself, but it contains
+parameters, keys, vectors, etc for peripheral or from the peripheral.
+
+The DMAengine framework provides a generic ways to facilitate the metadata for
+descriptors. Depending on the architecture the DMA driver can implment either
+or both of the methods and it is up to the client driver to choose which one
+to use.
+
+- DESC_METADATA_CLIENT
+
+ The metadata buffer is allocated/provided by the client driver and it is
+ attached (via the dmaengine_desc_attach_metadata() helper to the descriptor.
+
+ From the DMA driver the following is expected for this mode:
+ - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM
+ The data from the provided metadata buffer should be prepared for the DMA
+ controller to be sent alongside of the payload data. Either by copying to a
+ hardware descriptor, or highly coupled packet.
+ - DMA_DEV_TO_MEM
+ On transfer completion the DMA driver must copy the metadata to the client
+ provided metadata buffer.
+
+- DESC_METADATA_ENGINE
+
+ The metadata buffer is allocated/managed by the DMA driver. The client driver
+ can ask for the pointer, maximum size and the currently used size of the
+ metadata and can directly update or read it. dmaengine_desc_get_metadata_ptr()
+ and dmaengine_desc_set_metadata_len() is provided as helper functions.
+
+ From the DMA driver the following is expected for this mode:
+ - get_metadata_ptr
+ Should return a pointer for the metadata buffer, the maximum size of the
+ metadata buffer and the currently used / valid (if any) bytes in the buffer.
+ - set_metadata_len
+ It is called by the clients after it have placed the metadata to the buffer
+ to let the DMA driver know the number of valid bytes provided.
+
+ Note: since the client will ask for the metadata pointer in the completion
+ callback (in DMA_DEV_TO_MEM case) the DMA driver must ensure that the
+ descriptor is not freed up prior the callback is called.
+
Device operations
-----------------
^ permalink raw reply related
* [v2,1/2] dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Peter Ujfalusi @ 2018-08-30 12:19 UTC (permalink / raw)
To: dan.j.williams, vkoul; +Cc: dmaengine, linux-kernel, lars, radheys
The metadata is best described as side band data or parameters traveling
alongside the data DMAd by the DMA engine. It is data
which is understood by the peripheral and the peripheral driver only, the
DMA engine see it only as data block and it is not interpreting it in any
way.
The metadata can be different per descriptor as it is a parameter for the
data being transferred.
If the DMA supports per descriptor metadata it can implement the attach,
get_ptr/set_len callbacks.
Client drivers must only use either attach or get_ptr/set_len to avoid
misconfiguration.
Client driver can check if a given metadata mode is supported by the
channel during probe time with
dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
and based on this information can use either mode.
Wrappers are also added for the metadata_ops.
To be used in DESC_METADATA_CLIENT mode:
dmaengine_desc_attach_metadata()
To be used in DESC_METADATA_ENGINE mode:
dmaengine_desc_get_metadata_ptr()
dmaengine_desc_set_metadata_len()
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/dmaengine.c | 73 ++++++++++++++++++++++++++
include/linux/dmaengine.h | 108 ++++++++++++++++++++++++++++++++++++++
2 files changed, 181 insertions(+)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index f1a441ab395d..53bd1eae23f2 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1306,6 +1306,79 @@ void dma_async_tx_descriptor_init(struct dma_async_tx_descriptor *tx,
}
EXPORT_SYMBOL(dma_async_tx_descriptor_init);
+static inline int desc_check_and_set_metadata_mode(
+ struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
+{
+ /* Make sure that the metadata mode is not mixed */
+ if (!desc->desc_metadata_mode) {
+ if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
+ desc->desc_metadata_mode = mode;
+ else
+ return -ENOTSUPP;
+ } else if (desc->desc_metadata_mode != mode) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int dmaengine_desc_attach_metadata(struct dma_async_tx_descriptor *desc,
+ void *data, size_t len)
+{
+ int ret;
+
+ if (!desc)
+ return -EINVAL;
+
+ ret = desc_check_and_set_metadata_mode(desc, DESC_METADATA_CLIENT);
+ if (ret)
+ return ret;
+
+ if (!desc->metadata_ops || !desc->metadata_ops->attach)
+ return -ENOTSUPP;
+
+ return desc->metadata_ops->attach(desc, data, len);
+}
+EXPORT_SYMBOL(dmaengine_desc_attach_metadata);
+
+void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
+ size_t *payload_len, size_t *max_len)
+{
+ int ret;
+
+ if (!desc)
+ return ERR_PTR(-EINVAL);
+
+ ret = desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
+ return ERR_PTR(-ENOTSUPP);
+
+ return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
+}
+EXPORT_SYMBOL(dmaengine_desc_get_metadata_ptr);
+
+int dmaengine_desc_set_metadata_len(struct dma_async_tx_descriptor *desc,
+ size_t payload_len)
+{
+ int ret;
+
+ if (!desc)
+ return -EINVAL;
+
+ ret = desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
+ if (ret)
+ return ret;
+
+ if (!desc->metadata_ops || !desc->metadata_ops->set_len)
+ return -ENOTSUPP;
+
+ return desc->metadata_ops->set_len(desc, payload_len);
+}
+EXPORT_SYMBOL(dmaengine_desc_set_metadata_len);
+
/* dma_wait_for_async_tx - spin wait for a transaction to complete
* @tx: in-flight transaction to wait on
*/
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 3db833a8c542..10ff71b13eef 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -231,6 +231,58 @@ typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t;
* @bytes_transferred: byte counter
*/
+/**
+ * enum dma_desc_metadata_mode - per descriptor metadata mode types supported
+ * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by the
+ * client driver and it is attached (via the dmaengine_desc_attach_metadata()
+ * helper) to the descriptor.
+ *
+ * Client drivers interested to use this mode can follow:
+ * - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+ * 1. prepare the descriptor (dmaengine_prep_*)
+ * construct the metadata in the client's buffer
+ * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ * descriptor
+ * 3. submit the transfer
+ * - DMA_DEV_TO_MEM:
+ * 1. prepare the descriptor (dmaengine_prep_*)
+ * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ * descriptor
+ * 3. submit the transfer
+ * 4. when the transfer is completed, the metadata should be available in the
+ * attached buffer
+ *
+ * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
+ * driver. The client driver can ask for the pointer, maximum size and the
+ * currently used size of the metadata and can directly update or read it.
+ * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
+ * provided as helper functions.
+ *
+ * Client drivers interested to use this mode can follow:
+ * - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+ * 1. prepare the descriptor (dmaengine_prep_*)
+ * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
+ * metadata area
+ * 3. update the metadata at the pointer
+ * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
+ * of data the client has placed into the metadata buffer
+ * 5. submit the transfer
+ * - DMA_DEV_TO_MEM:
+ * 1. prepare the descriptor (dmaengine_prep_*)
+ * 2. submit the transfer
+ * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
+ * pointer to the engine's metadata are
+ * 4. Read out the metadate from the pointer
+ *
+ * Note: the two mode is not compatible and clients must use one mode for a
+ * descriptor.
+ */
+enum dma_desc_metadata_mode {
+ DESC_METADATA_NONE = 0,
+ DESC_METADATA_CLIENT = BIT(0),
+ DESC_METADATA_ENGINE = BIT(1),
+};
+
struct dma_chan_percpu {
/* stats */
unsigned long memcpy_count;
@@ -494,6 +546,18 @@ struct dmaengine_unmap_data {
dma_addr_t addr[0];
};
+struct dma_async_tx_descriptor;
+
+struct dma_descriptor_metadata_ops {
+ int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
+ size_t len);
+
+ void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
+ size_t *payload_len, size_t *max_len);
+ int (*set_len)(struct dma_async_tx_descriptor *desc,
+ size_t payload_len);
+};
+
/**
* struct dma_async_tx_descriptor - async transaction descriptor
* ---dma generic offload fields---
@@ -507,6 +571,11 @@ struct dmaengine_unmap_data {
* descriptor pending. To be pushed on .issue_pending() call
* @callback: routine to call after this operation is complete
* @callback_param: general parameter to pass to the callback routine
+ * @desc_metadata_mode: core managed metadata mode to protect mixed use of
+ * DESC_METADATA_CLIENT or DESC_METADATA_ENGINE. Otherwise
+ * DESC_METADATA_NONE
+ * @metadata_ops: DMA driver provided metadata mode ops, need to be set by the
+ * DMA driver if metadata mode is supported with the descriptor
* ---async_tx api specific fields---
* @next: at completion submit this descriptor
* @parent: pointer to the next level up in the dependency chain
@@ -523,6 +592,8 @@ struct dma_async_tx_descriptor {
dma_async_tx_callback_result callback_result;
void *callback_param;
struct dmaengine_unmap_data *unmap;
+ enum dma_desc_metadata_mode desc_metadata_mode;
+ struct dma_descriptor_metadata_ops *metadata_ops;
#ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
struct dma_async_tx_descriptor *next;
struct dma_async_tx_descriptor *parent;
@@ -685,6 +756,7 @@ struct dma_filter {
* @global_node: list_head for global dma_device_list
* @filter: information for device/slave to filter function/param mapping
* @cap_mask: one or more dma_capability flags
+ * @desc_metadata_modes: supported metadata modes by the DMA device
* @max_xor: maximum number of xor sources, 0 if no capability
* @max_pq: maximum number of PQ sources and PQ-continue capability
* @copy_align: alignment shift for memcpy operations
@@ -749,6 +821,7 @@ struct dma_device {
struct list_head global_node;
struct dma_filter filter;
dma_cap_mask_t cap_mask;
+ enum dma_desc_metadata_mode desc_metadata_modes;
unsigned short max_xor;
unsigned short max_pq;
enum dmaengine_alignment copy_align;
@@ -935,6 +1008,41 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
len, flags);
}
+static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
+ enum dma_desc_metadata_mode mode)
+{
+ if (!chan)
+ return false;
+
+ return !!(chan->device->desc_metadata_modes & mode);
+}
+
+#ifdef CONFIG_DMA_ENGINE
+int dmaengine_desc_attach_metadata(struct dma_async_tx_descriptor *desc,
+ void *data, size_t len);
+void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
+ size_t *payload_len, size_t *max_len);
+int dmaengine_desc_set_metadata_len(struct dma_async_tx_descriptor *desc,
+ size_t payload_len);
+#else /* CONFIG_DMA_ENGINE */
+static inline int dmaengine_desc_attach_metadata(
+ struct dma_async_tx_descriptor *desc, void *data, size_t len)
+{
+ return -EINVAL;
+}
+static inline void *dmaengine_desc_get_metadata_ptr(
+ struct dma_async_tx_descriptor *desc, size_t *payload_len,
+ size_t *max_len)
+{
+ return NULL;
+}
+static inline int dmaengine_desc_set_metadata_len(
+ struct dma_async_tx_descriptor *desc, size_t payload_len)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_DMA_ENGINE */
+
/**
* dmaengine_terminate_all() - Terminate all active DMA transfers
* @chan: The channel for which to terminate the transfers
^ permalink raw reply related
* [4/6] dt-bindings: dmaengine: usb-dmac: Add binding for r8a774a1
From: Simon Horman @ 2018-08-30 12:09 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Vinod Koul, Rob Herring, Mark Rutland, Biju Das,
Greg Kroah-Hartman, dmaengine, devicetree, Geert Uytterhoeven,
Chris Paterson, linux-renesas-soc, linux-kernel
On Fri, Aug 24, 2018 at 08:56:13AM +0100, Fabrizio Castro wrote:
> From: Biju Das <biju.das@bp.renesas.com>
>
> This patch adds binding for r8a774a1 (RZ/G2M).
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Radhey Shyam Pandey @ 2018-08-30 9:32 UTC (permalink / raw)
To: Peter Ujfalusi, Vinod
Cc: dan.j.williams@intel.com, dmaengine@vger.kernel.org,
linux-kernel@vger.kernel.org, lars@metafoo.de
<snip>
> Vinod,
>
> On 2018-08-29 19:22, Vinod wrote:
> >>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to
> the
> >>>> + * descriptor
> >>>> + * 3. submit the transfer
> >>>> + * - DMA_DEV_TO_MEM:
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to
> the
> >>>> + * descriptor
> >>>> + * 3. submit the transfer
> >>>> + * 4. when the transfer is completed, the metadata should be available
> in the
> >>>> + * attached buffer
> >>>
> >>> I guess this is good to be moved into Documentation
> >>
> >> Should I create a new file for metadata? I guess it would make sense as the
> >> information is for both clients and engines.
> >
> > Hmm not sure, lets see how it looks as entries in these files, detailing
> > roles of clients and providers
>
> Update both client and provider documentation with tailoring the content
> for the audience?
>
> >>> also we dont allow this for memcpy txn?
> >>
> >> I have not thought about that, but if I think about it it should be along the
> >> same lines as MEM_TO_DEV.
> >> I'll add the MEM_TO_MEM as well to the documentation.
> >
> > Okay and lets not implement it then..
>
> I'm not going to implement it, but the documentation could add that if
> metadata is used for MEM_TO_MEM then it is the same use case as with
> MEM_TO_DEV.
>
> >
> >>
> >>>> + *
> >>>> + * @DESC_METADATA_ENGINE - the metadata buffer is
> allocated/managed by the DMA
> >>>> + * driver. The client driver can ask for the pointer, maximum size and
> the
> >>>> + * currently used size of the metadata and can directly update or read it.
> >>>> + * dmaengine_desc_get_metadata_ptr() and
> dmaengine_desc_set_metadata_len() is
> >>>> + * provided as helper functions.
> >>>> + *
> >>>> + * Client drivers interested to use this mode can follow:
> >>>> + * - DMA_MEM_TO_DEV:
>
> Here, add DMA_MEM_TO_MEM
>
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the
> engine's
> >>>> + * metadata area
> >>>> + * 3. update the metadata at the pointer
> >>>> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine
> the amount
> >>>> + * of data the client has placed into the metadata buffer
> >>>> + * 5. submit the transfer
> >>>> + * - DMA_DEV_TO_MEM:
> >>>> + * 1. prepare the descriptor (dmaengine_prep_*)
> >>>> + * 2. submit the transfer
> >>>> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr()
> to get the
> >>>> + * pointer to the engine's metadata are
> >>>> + * 4. Read out the metadate from the pointer
> >>>> + *
> >>>> + * Note: the two mode is not compatible and clients must use one mode
> for a
> >>>> + * descriptor.
> >>>> + */
> >>>> +enum dma_desc_metadata_mode {
> >>>> + DESC_METADATA_CLIENT = (1 << 0),
> >>>> + DESC_METADATA_ENGINE = (1 << 1),
> >>>
> >>> BIT(x)
> >>
> >> OK, I followed what we have in the header to not mix (1 << x) and BIT(x)
> >
> > yeah lets update :)
>
> OK.
>
> >>>> +static inline int _desc_check_and_set_metadata_mode(
> >>>
> >>> why does this need to start with _ ?
> >>
> >> To scare people to use in client code ;)
> >
> > Lets not expose to them :D
>
> Sure, if the code moves to dmaengine.c it is granted.
>
> >>> Also I would like to see a use :-) before further comments.
> >>
> >> You mean the DMA driver and at least one client?
> >
> > DMA driver to _at_least_ start with. Client even better
>
> Hrm, I can send the DMA driver as RFC (not to merge, will not compile)
> but I need to do some excess cover letter and documentation since the
> UDMA is 'just' a piece in the data movement architecture and need to
> explain couple of things around it.
>
> I will need couple of days for that for sure.
>
> >> I have the DMA driver in my public facing branch [1], but it is not an easy
> >> read with it's close to 4k loc.
> >
> > It doesnt exist :P
>
> In this sense it does not, agree.
>
> >> The client is not in my branch and it is actually using an older version of
> >> the metadata support.
> >>
> >> The problem is that I don't know when I will be able to send the driver for
> >> review as all of this is targeting a brand new SoC (AM654) with completely
> new
> >> data movement architecture. There are lots of dependencies still need to be
> >> upstreamed before I can send something which at least compiles.
> >>
> >> I can offer snippets from the client driver, if that is good enough or a link
> >> to the public tree where it can be accessed, but it is not going to go
> >> upstream before the DMA driver.
> >
> > TBH that's not going to help much, lets come back to it when you need
> > this upstream.
>
> One of the reason I have sent the metadata support early is because
> Radhey was looking for similar thing for xilinx_dma and I already had
> the generic implementation of it which suits his case.
>
> I was planning to send the metadata support along with the DMA driver
> (and other core changes, new features).
>
> If not for me, then for Radhey's stake can the metadata support be
> considered as stand alone for now?
Thanks, Peter. I was thinking to put the same request. Based on metadata_ops
RFC, I can send v2 of my earlier patchset[1]. Once it is acked, I will next
send client axiethernet driver[2] RFC to networking mailing list.
Let's wait for Vinod's inputs.
[1] https://www.spinics.net/lists/dmaengine/msg15208.html
[2] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>
> I will send v2 as soon as I have it ready and I will send either v3 with
> the k3-udma DMA driver (UDMA drivers as not for merge) or standalone
> UDMA driver as RFC and for reference.
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Peter Ujfalusi @ 2018-08-30 8:57 UTC (permalink / raw)
To: Vinod; +Cc: dan.j.williams, dmaengine, linux-kernel, lars, radheys
Vinod,
On 2018-08-29 19:22, Vinod wrote:
>>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>>>> + * descriptor
>>>> + * 3. submit the transfer
>>>> + * - DMA_DEV_TO_MEM:
>>>> + * 1. prepare the descriptor (dmaengine_prep_*)
>>>> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>>>> + * descriptor
>>>> + * 3. submit the transfer
>>>> + * 4. when the transfer is completed, the metadata should be available in the
>>>> + * attached buffer
>>>
>>> I guess this is good to be moved into Documentation
>>
>> Should I create a new file for metadata? I guess it would make sense as the
>> information is for both clients and engines.
>
> Hmm not sure, lets see how it looks as entries in these files, detailing
> roles of clients and providers
Update both client and provider documentation with tailoring the content
for the audience?
>>> also we dont allow this for memcpy txn?
>>
>> I have not thought about that, but if I think about it it should be along the
>> same lines as MEM_TO_DEV.
>> I'll add the MEM_TO_MEM as well to the documentation.
>
> Okay and lets not implement it then..
I'm not going to implement it, but the documentation could add that if
metadata is used for MEM_TO_MEM then it is the same use case as with
MEM_TO_DEV.
>
>>
>>>> + *
>>>> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
>>>> + * driver. The client driver can ask for the pointer, maximum size and the
>>>> + * currently used size of the metadata and can directly update or read it.
>>>> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
>>>> + * provided as helper functions.
>>>> + *
>>>> + * Client drivers interested to use this mode can follow:
>>>> + * - DMA_MEM_TO_DEV:
Here, add DMA_MEM_TO_MEM
>>>> + * 1. prepare the descriptor (dmaengine_prep_*)
>>>> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
>>>> + * metadata area
>>>> + * 3. update the metadata at the pointer
>>>> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
>>>> + * of data the client has placed into the metadata buffer
>>>> + * 5. submit the transfer
>>>> + * - DMA_DEV_TO_MEM:
>>>> + * 1. prepare the descriptor (dmaengine_prep_*)
>>>> + * 2. submit the transfer
>>>> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
>>>> + * pointer to the engine's metadata are
>>>> + * 4. Read out the metadate from the pointer
>>>> + *
>>>> + * Note: the two mode is not compatible and clients must use one mode for a
>>>> + * descriptor.
>>>> + */
>>>> +enum dma_desc_metadata_mode {
>>>> + DESC_METADATA_CLIENT = (1 << 0),
>>>> + DESC_METADATA_ENGINE = (1 << 1),
>>>
>>> BIT(x)
>>
>> OK, I followed what we have in the header to not mix (1 << x) and BIT(x)
>
> yeah lets update :)
OK.
>>>> +static inline int _desc_check_and_set_metadata_mode(
>>>
>>> why does this need to start with _ ?
>>
>> To scare people to use in client code ;)
>
> Lets not expose to them :D
Sure, if the code moves to dmaengine.c it is granted.
>>> Also I would like to see a use :-) before further comments.
>>
>> You mean the DMA driver and at least one client?
>
> DMA driver to _at_least_ start with. Client even better
Hrm, I can send the DMA driver as RFC (not to merge, will not compile)
but I need to do some excess cover letter and documentation since the
UDMA is 'just' a piece in the data movement architecture and need to
explain couple of things around it.
I will need couple of days for that for sure.
>> I have the DMA driver in my public facing branch [1], but it is not an easy
>> read with it's close to 4k loc.
>
> It doesnt exist :P
In this sense it does not, agree.
>> The client is not in my branch and it is actually using an older version of
>> the metadata support.
>>
>> The problem is that I don't know when I will be able to send the driver for
>> review as all of this is targeting a brand new SoC (AM654) with completely new
>> data movement architecture. There are lots of dependencies still need to be
>> upstreamed before I can send something which at least compiles.
>>
>> I can offer snippets from the client driver, if that is good enough or a link
>> to the public tree where it can be accessed, but it is not going to go
>> upstream before the DMA driver.
>
> TBH that's not going to help much, lets come back to it when you need
> this upstream.
One of the reason I have sent the metadata support early is because
Radhey was looking for similar thing for xilinx_dma and I already had
the generic implementation of it which suits his case.
I was planning to send the metadata support along with the DMA driver
(and other core changes, new features).
If not for me, then for Radhey's stake can the metadata support be
considered as stand alone for now?
I will send v2 as soon as I have it ready and I will send either v3 with
the k3-udma DMA driver (UDMA drivers as not for merge) or standalone
UDMA driver as RFC and for reference.
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [v4,2/7] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
From: Andrea Merello @ 2018-08-30 8:11 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-kernel, Rob Herring, Mark Rutland, devicetree,
Radhey Shyam Pandey
On Wed, Aug 29, 2018 at 10:12 AM Andrea Merello
<andrea.merello@gmail.com> wrote:
>
> On Mon, Aug 27, 2018 at 7:30 AM Vinod <vkoul@kernel.org> wrote:
> >
> > On 02-08-18, 16:10, Andrea Merello wrote:
> >
> > s/cylic/cyclic in patch title
>
> OK
>
> > > Whenever a single or cyclic transaction is prepared, the driver
> > > could eventually split it over several SG descriptors in order
> > > to deal with the HW maximum transfer length.
> > >
> > > This could end up in DMA operations starting from a misaligned
> > > address. This seems fatal for the HW if DRE is not enabled.
> >
> > DRE?
>
> Stands for "Data Realignment Engine". I will add this string nearby
> the acronym..
>
> > >
> > > This patch eventually adjusts the transfer size in order to make sure
> > > all operations start from an aligned address.
> > >
> > > Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > > Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > > ---
> > > Changes in v2:
> > > - don't introduce copy_mask field, rather rely on already-esistent
> > > copy_align field. Suggested by Radhey Shyam Pandey
> > > - reword title
> > > Changes in v3:
> > > - fix bug introduced in v2: wrong copy size when DRE is enabled
> > > - use implementation suggested by Radhey Shyam Pandey
> > > Changes in v4:
> > > - rework on the top of 1/6
> > > ---
> > > drivers/dma/xilinx/xilinx_dma.c | 22 ++++++++++++++++++----
> > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> > > index a3aaa0e34cc7..aaa6de8a70e4 100644
> > > --- a/drivers/dma/xilinx/xilinx_dma.c
> > > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > > @@ -954,15 +954,28 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
> > >
> > > /**
> > > * xilinx_dma_calc_copysize - Calculate the amount of data to copy
> > > + * @chan: Driver specific DMA channel
> > > * @size: Total data that needs to be copied
> > > * @done: Amount of data that has been already copied
> > > *
> > > * Return: Amount of data that has to be copied
> > > */
> > > -static int xilinx_dma_calc_copysize(int size, int done)
> > > +static int xilinx_dma_calc_copysize(struct xilinx_dma_chan *chan,
> > > + int size, int done)
> >
> > please align with opening brace
>
> OK
Sorry for getting back on this.
I've checked it, but it seems already aligned with opening brace in
the original e-mail text I've sent. (4 tabs + 4 spaces).
> > > {
> > > - return min_t(size_t, size - done,
> > > + size_t copy = min_t(size_t, size - done,
> > > XILINX_DMA_MAX_TRANS_LEN);
> > > +
> > > + if ((copy + done < size) &&
> > > + chan->xdev->common.copy_align) {
> > > + /*
> > > + * If this is not the last descriptor, make sure
> > > + * the next one will be properly aligned
> > > + */
> > > + copy = rounddown(copy,
> > > + (1 << chan->xdev->common.copy_align));
> > > + }
> > > + return copy;
> > > }
> > >
> > > /**
> > > @@ -1804,7 +1817,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> > > * Calculate the maximum number of bytes to transfer,
> > > * making sure it is less than the hw limit
> > > */
> > > - copy = xilinx_dma_calc_copysize(sg_dma_len(sg),
> > > + copy = xilinx_dma_calc_copysize(chan, sg_dma_len(sg),
> > > sg_used);
> > > hw = &segment->hw;
> > >
> > > @@ -1909,7 +1922,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
> > > * Calculate the maximum number of bytes to transfer,
> > > * making sure it is less than the hw limit
> > > */
> > > - copy = xilinx_dma_calc_copysize(period_len, sg_used);
> > > + copy = xilinx_dma_calc_copysize(chan,
> > > + period_len, sg_used);
> > > hw = &segment->hw;
> > > xilinx_axidma_buf(chan, hw, buf_addr, sg_used,
> > > period_len * i);
> > > --
> > > 2.17.1
> >
> > --
> > ~Vinod
^ permalink raw reply
* [v5,18/18] MIPS: JZ4740: DTS: Add DMA nodes
From: Paul Cercueil @ 2018-08-29 21:33 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
Add the devicetree nodes for the DMA core of the JZ4740 SoC, disabled
by default, as currently there are no clients for the DMA driver
(until the MMC driver and/or others get a devicetree node).
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Acked-by: Paul Burton <paul.burton@mips.com>
---
Notes:
v2: New patch in this series
v3: Modify node to comply with devicetree specification
v4: No change
v5: No change
arch/mips/boot/dts/ingenic/jz4740.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 26c6b561d6f7..6fb16fd24035 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -154,6 +154,21 @@
clock-names = "baud", "module";
};
+ dmac: dma-controller@13020000 {
+ compatible = "ingenic,jz4740-dma";
+ reg = <0x13020000 0xbc
+ 0x13020300 0x14>;
+ #dma-cells = <2>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <29>;
+
+ clocks = <&cgu JZ4740_CLK_DMA>;
+
+ /* Disable dmac until we have something that uses it */
+ status = "disabled";
+ };
+
uhc: uhc@13030000 {
compatible = "ingenic,jz4740-ohci", "generic-ohci";
reg = <0x13030000 0x1000>;
^ permalink raw reply related
* [v5,17/18] MIPS: JZ4770: DTS: Add DMA nodes
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
Add the two devicetree nodes for the two DMA cores of the JZ4770 SoC,
disabled by default, as currently there are no clients for the DMA
driver (until the MMC driver and/or others get a devicetree node).
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Acked-by: Paul Burton <paul.burton@mips.com>
---
Notes:
v2: No change
v3: Modify nodes to comply with devicetree specification
v4: No change
v5: No change
arch/mips/boot/dts/ingenic/jz4770.dtsi | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi b/arch/mips/boot/dts/ingenic/jz4770.dtsi
index 7c2804f3f5f1..49ede6c14ff3 100644
--- a/arch/mips/boot/dts/ingenic/jz4770.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi
@@ -196,6 +196,36 @@
status = "disabled";
};
+ dmac0: dma-controller@13420000 {
+ compatible = "ingenic,jz4770-dma";
+ reg = <0x13420000 0xC0
+ 0x13420300 0x20>;
+
+ #dma-cells = <1>;
+
+ clocks = <&cgu JZ4770_CLK_DMA>;
+ interrupt-parent = <&intc>;
+ interrupts = <24>;
+
+ /* Disable dmac0 until we have something that uses it */
+ status = "disabled";
+ };
+
+ dmac1: dma-controller@13420100 {
+ compatible = "ingenic,jz4770-dma";
+ reg = <0x13420100 0xC0
+ 0x13420400 0x20>;
+
+ #dma-cells = <1>;
+
+ clocks = <&cgu JZ4770_CLK_DMA>;
+ interrupt-parent = <&intc>;
+ interrupts = <23>;
+
+ /* Disable dmac1 until we have something that uses it */
+ status = "disabled";
+ };
+
uhc: uhc@13430000 {
compatible = "generic-ohci";
reg = <0x13430000 0x1000>;
^ permalink raw reply related
* [v5,16/18] MIPS: JZ4780: DTS: Update DMA node to match driver changes
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
The driver now accepts two memory resources, the first one for the
channel-specific registers, the second one for the controller-specific
registers.
Note that older devicetrees, without this commit, will still work with
the jz4780-dma driver.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: Update info about devicetree ABI compatibility in commit message
v3: No change
v4: No change
v5: No change
arch/mips/boot/dts/ingenic/jz4780.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index ce93d57f1b4d..b03cdec56de9 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -266,7 +266,8 @@
dma: dma@13420000 {
compatible = "ingenic,jz4780-dma";
- reg = <0x13420000 0x10000>;
+ reg = <0x13420000 0x400
+ 0x13421000 0x40>;
#dma-cells = <2>;
interrupt-parent = <&intc>;
^ permalink raw reply related
* [v5,15/18] dmaengine: dma-jz4780: Use dma_set_residue()
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Daniel Silsby, Paul Cercueil
From: Daniel Silsby <dansilsby@gmail.com>
This is the standard method provided by dmaengine header.
Signed-off-by: Daniel Silsby <dansilsby@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: No change
v3: No change
v4: Add my Signed-off-by
v5: No change
drivers/dma/dma-jz4780.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index b73d96166637..e1bb93dd32ba 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -639,6 +639,7 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
struct virt_dma_desc *vdesc;
enum dma_status status;
unsigned long flags;
+ unsigned long residue = 0;
status = dma_cookie_status(chan, cookie, txstate);
if ((status == DMA_COMPLETE) || (txstate == NULL))
@@ -649,13 +650,13 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
vdesc = vchan_find_desc(&jzchan->vchan, cookie);
if (vdesc) {
/* On the issued list, so hasn't been processed yet */
- txstate->residue = jz4780_dma_desc_residue(jzchan,
+ residue = jz4780_dma_desc_residue(jzchan,
to_jz4780_dma_desc(vdesc), 0);
} else if (cookie == jzchan->desc->vdesc.tx.cookie) {
- txstate->residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
+ residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
jzchan->curr_hwdesc + 1);
- } else
- txstate->residue = 0;
+ }
+ dma_set_residue(txstate, residue);
if (vdesc && jzchan->desc && vdesc == &jzchan->desc->vdesc
&& jzchan->desc->status & (JZ_DMA_DCS_AR | JZ_DMA_DCS_HLT))
^ permalink raw reply related
* [v5,14/18] dmaengine: dma-jz4780: Further residue status fix
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Daniel Silsby, Paul Cercueil
From: Daniel Silsby <dansilsby@gmail.com>
Func jz4780_dma_desc_residue() expects the index to the next hw
descriptor as its last parameter. Caller func jz4780_dma_tx_status(),
however, applied modulus before passing it. When the current hw
descriptor was last in the list, the index passed became zero.
The resulting excess of reported residue especially caused problems
with cyclic DMA transfer clients, i.e. ALSA AIC audio output, which
rely on this for determining current DMA location within buffer.
Combined with the recent and related residue-reporting fixes, spurious
ALSA audio underruns on jz4770 hardware are now fixed.
Signed-off-by: Daniel Silsby <dansilsby@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: No change
v3: No change
v4: Add my Signed-off-by
v5: No change
drivers/dma/dma-jz4780.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index d3b915ec8a09..b73d96166637 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -653,7 +653,7 @@ static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
to_jz4780_dma_desc(vdesc), 0);
} else if (cookie == jzchan->desc->vdesc.tx.cookie) {
txstate->residue = jz4780_dma_desc_residue(jzchan, jzchan->desc,
- (jzchan->curr_hwdesc + 1) % jzchan->desc->count);
+ jzchan->curr_hwdesc + 1);
} else
txstate->residue = 0;
^ permalink raw reply related
* [v5,13/18] dmaengine: dma-jz4780: Set DTCn register explicitly
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Daniel Silsby, Paul Cercueil
From: Daniel Silsby <dansilsby@gmail.com>
Normally, we wouldn't set the channel transfer count register directly
when using descriptor-driven transfers. However, there is no harm in
doing so, and it allows jz4780_dma_desc_residue() to report the correct
residue of an ongoing transfer, no matter when it is called.
Signed-off-by: Daniel Silsby <dansilsby@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: No change
v3: No change
v4: Add my Signed-off-by
v5: No change
drivers/dma/dma-jz4780.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index d055602a92ca..d3b915ec8a09 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -532,6 +532,15 @@ static void jz4780_dma_begin(struct jz4780_dma_chan *jzchan)
jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DRT,
jzchan->transfer_type);
+ /*
+ * Set the transfer count. This is redundant for a descriptor-driven
+ * transfer. However, there can be a delay between the transfer start
+ * time and when DTCn reg contains the new transfer count. Setting
+ * it explicitly ensures residue is computed correctly at all times.
+ */
+ jz4780_dma_chn_writel(jzdma, jzchan->id, JZ_DMA_REG_DTC,
+ jzchan->desc->desc[jzchan->curr_hwdesc].dtc);
+
/* Write descriptor address and initiate descriptor fetch. */
desc_phys = jzchan->desc->desc_phys +
(jzchan->curr_hwdesc * sizeof(*jzchan->desc->desc));
^ permalink raw reply related
* [v5,12/18] dmaengine: dma-jz4780: Simplify jz4780_dma_desc_residue()
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Daniel Silsby, Paul Cercueil
From: Daniel Silsby <dansilsby@gmail.com>
Simple cleanup, no changes to actual logic here.
Signed-off-by: Daniel Silsby <dansilsby@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: No change
v3: No change
v4: Add my Signed-off-by
v5: Use GENMASK macro
drivers/dma/dma-jz4780.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index bd3cecb800c5..d055602a92ca 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -610,22 +610,17 @@ static size_t jz4780_dma_desc_residue(struct jz4780_dma_chan *jzchan,
struct jz4780_dma_desc *desc, unsigned int next_sg)
{
struct jz4780_dma_dev *jzdma = jz4780_dma_chan_parent(jzchan);
- unsigned int residue, count;
+ unsigned int count = 0;
unsigned int i;
- residue = 0;
-
for (i = next_sg; i < desc->count; i++)
- residue += (desc->desc[i].dtc & GENMASK(23, 0)) <<
- jzchan->transfer_shift;
+ count += desc->desc[i].dtc & GENMASK(23, 0);
- if (next_sg != 0) {
- count = jz4780_dma_chn_readl(jzdma, jzchan->id,
+ if (next_sg != 0)
+ count += jz4780_dma_chn_readl(jzdma, jzchan->id,
JZ_DMA_REG_DTC);
- residue += count << jzchan->transfer_shift;
- }
- return residue;
+ return count << jzchan->transfer_shift;
}
static enum dma_status jz4780_dma_tx_status(struct dma_chan *chan,
^ permalink raw reply related
* [v5,11/18] dmaengine: dma-jz4780: Add missing residue DTC mask
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Daniel Silsby, Paul Cercueil
From: Daniel Silsby <dansilsby@gmail.com>
The 'dtc' word in jz DMA descriptors contains two fields: The
lowest 24 bits are the transfer count, and upper 8 bits are the DOA
offset to next descriptor. The upper 8 bits are now correctly masked
off when computing residue in jz4780_dma_desc_residue(). Note that
reads of the DTCn hardware reg are automatically masked this way.
Signed-off-by: Daniel Silsby <dansilsby@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
---
Notes:
v2: No change
v3: No change
v4: Add my Signed-off-by
v5: Use GENMASK macro
drivers/dma/dma-jz4780.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 3a4d0a4b550d..bd3cecb800c5 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -616,7 +616,8 @@ static size_t jz4780_dma_desc_residue(struct jz4780_dma_chan *jzchan,
residue = 0;
for (i = next_sg; i < desc->count; i++)
- residue += desc->desc[i].dtc << jzchan->transfer_shift;
+ residue += (desc->desc[i].dtc & GENMASK(23, 0)) <<
+ jzchan->transfer_shift;
if (next_sg != 0) {
count = jz4780_dma_chn_readl(jzdma, jzchan->id,
^ permalink raw reply related
* [v5,10/18] dmaengine: dma-jz4780: Enable Fast DMA to the AIC
From: Paul Cercueil @ 2018-08-29 21:32 UTC (permalink / raw)
To: Vinod Koul, Ralf Baechle, Paul Burton
Cc: od, dmaengine, devicetree, linux-kernel, linux-mips,
Paul Cercueil
With the fast DMA bit set, the DMA will transfer twice as much data
per clock period to the AIC, so there is little point not to set it.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Notes:
v2: No change
v3: No change
v4: No change
v5: No change
drivers/dma/dma-jz4780.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 565971c2a33c..3a4d0a4b550d 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -52,6 +52,7 @@
#define JZ_DMA_DMAC_DMAE BIT(0)
#define JZ_DMA_DMAC_AR BIT(2)
#define JZ_DMA_DMAC_HLT BIT(3)
+#define JZ_DMA_DMAC_FAIC BIT(27)
#define JZ_DMA_DMAC_FMSC BIT(31)
#define JZ_DMA_DRT_AUTO 0x8
@@ -923,8 +924,8 @@ static int jz4780_dma_probe(struct platform_device *pdev)
* Also set the FMSC bit - it increases MSC performance, so it makes
* little sense not to enable it.
*/
- jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC,
- JZ_DMA_DMAC_DMAE | JZ_DMA_DMAC_FMSC);
+ jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMAC, JZ_DMA_DMAC_DMAE |
+ JZ_DMA_DMAC_FAIC | JZ_DMA_DMAC_FMSC);
if (soc_data->flags & JZ_SOC_DATA_PROGRAMMABLE_DMA)
jz4780_dma_ctrl_writel(jzdma, JZ_DMA_REG_DMACP, 0);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox