[PATCH v2 00/14] dma: edma: Fixes for cyclic (audio) operation

classic Classic list List threaded Threaded
45 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 00/14] dma: edma: Fixes for cyclic (audio) operation

Peter Ujfalusi
Hi,

This is basically a resend of the previous series:
https://lkml.org/lkml/2014/3/13/119
with removed ASoC patches (most of them are applied already).

Changes since v1:
- ASoC patches removed
- Comments from Andriy Shevchenko addressed
- patch added to fix cases when src/dst_maxburst is set to 0

Adding support for DMA pause/resume
Possibility to select non default event queue/TC for cyclic (audio) dma
channels: all devices using the eDMA via dmaengine was assigned to the default
EQ/TC (mmc, i2c, spi, etc, and audio). This is not optimal from system
performance point of view since sharing the same EQ/TC can cause latency spikes
for cyclic channels (long DMA transfers for MMC for example).

While debugging the edma to get things sorted out I noticed that the debug was
too verbose and the important information was hidden even when the we did not
asked for verbose dmaengine debug.
I have included some debug cleanups for the edma dmaengine driver also.

Regards,
Peter
---
Peter Ujfalusi (14):
  platform_data: edma: Be precise with the paRAM struct
  dma: edma: Correct the handling of src/dst_maxburst == 0
  dma: edma: Add support for DMA_PAUSE/RESUME operation
  dma: edma: Set DMA_CYCLIC capability flag
  arm: common: edma: Select event queue 1 as default when booted with DT
  arm: common: edma: Save the number of event queues/TCs
  arm: common: edma: API to request non default queue for a channel
  DMA: edma: Use different eventq for cyclic channels
  dma: edma: Implement device_slave_caps callback
  dma: edma: Simplify direction configuration in edma_config_pset()
  dma: edma: Reduce debug print verbosity for non verbose debugging
  dma: edma: Prefix debug prints where the text were identical in prep
    callbacks
  dma: edma: Add channel number to debug prints
  dma: edma: Print the direction value as well when it is not supported

 arch/arm/common/edma.c             | 34 +++++++++++++-
 drivers/dma/edma.c                 | 96 +++++++++++++++++++++++++++++---------
 include/linux/platform_data/edma.h | 20 ++++----
 3 files changed, 119 insertions(+), 31 deletions(-)

--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 01/14] platform_data: edma: Be precise with the paRAM struct

Peter Ujfalusi
The edmacc_param struct should follow the layout of the paRAM area in the
HW. Be explicit on the size of the fields (u32) and also mark the struct
as packed to avoid any padding on non 32bit architectures.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 include/linux/platform_data/edma.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index f50821cb64be..923f8a3e4ce0 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -43,15 +43,15 @@
 
 /* PaRAM slots are laid out like this */
 struct edmacc_param {
- unsigned int opt;
- unsigned int src;
- unsigned int a_b_cnt;
- unsigned int dst;
- unsigned int src_dst_bidx;
- unsigned int link_bcntrld;
- unsigned int src_dst_cidx;
- unsigned int ccnt;
-};
+ u32 opt;
+ u32 src;
+ u32 a_b_cnt;
+ u32 dst;
+ u32 src_dst_bidx;
+ u32 link_bcntrld;
+ u32 src_dst_cidx;
+ u32 ccnt;
+} __packed;
 
 /* fields in edmacc_param.opt */
 #define SAM BIT(0)
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 02/14] dma: edma: Correct the handling of src/dst_maxburst == 0

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
When clients asks for maxburst = 0 it is basically the same case as if they
were asking for maxburst = 1 since in both case ASYNC need to be used and
the eDMA is expected to write/read one word per DMA request.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index cd04eb7b182e..2742867fd1e6 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -285,6 +285,10 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
  int absync;
 
  acnt = dev_width;
+
+ /* src/dst_maxburst == 0 is the same case as src/dst_maxburst == 1 */
+ if (!burst)
+ burst = 1;
  /*
  * If the maxburst is equal to the fifo width, use
  * A-synced transfers. This allows for large contiguous
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 03/14] dma: edma: Add support for DMA_PAUSE/RESUME operation

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
Pause/Resume can be used by the audio stack when the stream is paused/resumed
The edma platform code has support for this and the legacy audio stack used
this.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 2742867fd1e6..7891378a03f0 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -240,6 +240,26 @@ static int edma_slave_config(struct edma_chan *echan,
  return 0;
 }
 
+static int edma_dma_pause(struct edma_chan *echan)
+{
+ /* Pause/Resume only allowed with cyclic mode */
+ if (!echan->edesc->cyclic)
+ return -EINVAL;
+
+ edma_pause(echan->ch_num);
+ return 0;
+}
+
+static int edma_dma_resume(struct edma_chan *echan)
+{
+ /* Pause/Resume only allowed with cyclic mode */
+ if (!echan->edesc->cyclic)
+ return -EINVAL;
+
+ edma_resume(echan->ch_num);
+ return 0;
+}
+
 static int edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
  unsigned long arg)
 {
@@ -255,6 +275,14 @@ static int edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
  config = (struct dma_slave_config *)arg;
  ret = edma_slave_config(echan, config);
  break;
+ case DMA_PAUSE:
+ ret = edma_dma_pause(echan);
+ break;
+
+ case DMA_RESUME:
+ ret = edma_dma_resume(echan);
+ break;
+
  default:
  ret = -ENOSYS;
  }
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 04/14] dma: edma: Set DMA_CYCLIC capability flag

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
Indicate that the edma dmaengine driver has support for cyclic mode.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 arch/arm/common/edma.c | 1 +
 drivers/dma/edma.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 41bca32409fc..86a8b263278f 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1574,6 +1574,7 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
  return ERR_PTR(ret);
 
  dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
+ dma_cap_set(DMA_CYCLIC, edma_filter_info.dma_cap);
  of_dma_controller_register(dev->of_node, of_dma_simple_xlate,
    &edma_filter_info);
 
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 7891378a03f0..1dd9e8806975 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -891,6 +891,7 @@ static int edma_probe(struct platform_device *pdev)
 
  dma_cap_zero(ecc->dma_slave.cap_mask);
  dma_cap_set(DMA_SLAVE, ecc->dma_slave.cap_mask);
+ dma_cap_set(DMA_CYCLIC, ecc->dma_slave.cap_mask);
 
  edma_dma_init(ecc, &ecc->dma_slave, &pdev->dev);
 
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
Use the EVENTQ_1 for default and leave the EVENTQ_0 to be used by high
priority channels, like audio.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 arch/arm/common/edma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 86a8b263278f..19520e2519d9 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1546,7 +1546,8 @@ static int edma_of_parse_dt(struct device *dev,
 
  pdata->queue_priority_mapping = queue_priority_map;
 
- pdata->default_queue = 0;
+ /* select queue 1 as default */
+ pdata->default_queue = EVENTQ_1;
 
  prop = of_find_property(node, "ti,edma-xbar-event-map", &sz);
  if (prop)
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 06/14] arm: common: edma: Save the number of event queues/TCs

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
For later use save the number of queues available for the CC.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 arch/arm/common/edma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 19520e2519d9..be267b2080be 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1770,6 +1770,9 @@ static int edma_probe(struct platform_device *pdev)
  map_queue_tc(j, queue_tc_mapping[i][0],
  queue_tc_mapping[i][1]);
 
+ /* Save the number of TCs */
+ edma_cc[j]->num_tc = i;
+
  /* Event queue priority mapping */
  for (i = 0; queue_priority_mapping[i][0] != -1; i++)
  assign_priority_to_queue(j,
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 07/14] arm: common: edma: API to request non default queue for a channel

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
When using eDMA3 via dmaengine all dma channels will use the default queue.
Since during request time we do not have means to change this it need to be done
later, before the DMA has been started.
With the added function it is possible to move the channel to a non default
queue if it is possible, otherwise (when only one EQ/TC is available for the CC)
the default queue is going to be used.
For example: For optimal system performance the audio (cyclic) DMA should
be placed to a separate queue which is different than what the rest of the
system is using.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 arch/arm/common/edma.c             | 27 +++++++++++++++++++++++++++
 include/linux/platform_data/edma.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index be267b2080be..eaf6dd19f082 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -712,6 +712,33 @@ int edma_alloc_channel(int channel,
 }
 EXPORT_SYMBOL(edma_alloc_channel);
 
+/**
+ * edma_request_non_default_queue - try to map the channel to non default queue
+ * @channel: dma channel returned from edma_alloc_channel()
+ *
+ * For certain type of applications like audio it is preferred to not use the
+ * default event queue/tc to avoid eDMA caused latency.
+ *
+ * This function will iterate through the event queues available for the CC and
+ * picks the first EQ/TC which is not set as the default for the CC
+ */
+void edma_request_non_default_queue(int channel)
+{
+ unsigned ctlr = EDMA_CTLR(channel);
+ enum dma_event_q eventq_no = EVENTQ_DEFAULT;
+ int i;
+
+ for (i = 0; i < edma_cc[ctlr]->num_tc; i++) {
+ if (i != edma_cc[ctlr]->default_queue) {
+ eventq_no = i;
+ break;
+ }
+ }
+
+ channel = EDMA_CHAN_SLOT(channel);
+ map_dmach_queue(ctlr, channel, eventq_no);
+}
+EXPORT_SYMBOL(edma_request_non_default_queue);
 
 /**
  * edma_free_channel - deallocate DMA channel
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 923f8a3e4ce0..5d0a1b98f205 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -117,6 +117,8 @@ int edma_alloc_channel(int channel,
  void *data, enum dma_event_q);
 void edma_free_channel(unsigned channel);
 
+void edma_request_non_default_queue(int channel);
+
 /* alloc/free parameter RAM slots */
 int edma_alloc_slot(unsigned ctlr, int slot);
 void edma_free_slot(unsigned slot);
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 08/14] DMA: edma: Use different eventq for cyclic channels

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
To improve latency with cyclic DMA operation it is preferred to
use different eventq/tc than the default which is used by all
other drivers (mmc, spi, i2c, etc).
When preparing the cyclic dma ask for non default queue for the
channel which is going to be used with cyclic mode.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 1dd9e8806975..10048b40fac8 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -628,6 +628,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  edesc->pset[i].opt |= TCINTEN;
  }
 
+ /* Use different eventq/tc for cyclic DMA to reduce latency */
+ edma_request_non_default_queue(echan->ch_num);
+
  return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
 }
 
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 09/14] dma: edma: Implement device_slave_caps callback

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
With the callback implemented omap-dma can provide information to client
drivers regarding to supported address widths, directions, residue
granularity, etc.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 10048b40fac8..855766672aa9 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -855,6 +855,23 @@ static void __init edma_chan_init(struct edma_cc *ecc,
  }
 }
 
+#define EDMA_DMA_BUSWIDTHS (BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES))
+
+static int edma_dma_device_slave_caps(struct dma_chan *dchan,
+      struct dma_slave_caps *caps)
+{
+ caps->src_addr_widths = EDMA_DMA_BUSWIDTHS;
+ caps->dstn_addr_widths = EDMA_DMA_BUSWIDTHS;
+ caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+ caps->cmd_pause = true;
+ caps->cmd_terminate = true;
+ caps->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
+
+ return 0;
+}
+
 static void edma_dma_init(struct edma_cc *ecc, struct dma_device *dma,
   struct device *dev)
 {
@@ -865,6 +882,7 @@ static void edma_dma_init(struct edma_cc *ecc, struct dma_device *dma,
  dma->device_issue_pending = edma_issue_pending;
  dma->device_tx_status = edma_tx_status;
  dma->device_control = edma_control;
+ dma->device_slave_caps = edma_dma_device_slave_caps;
  dma->dev = dev;
 
  INIT_LIST_HEAD(&dma->channels);
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 10/14] dma: edma: Simplify direction configuration in edma_config_pset()

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
check for the direction has been already done in the function calling
edma_config_pset().
The error reporting is redundant and also the "else if ()" can be removed.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 855766672aa9..d954099650ae 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -372,14 +372,12 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
  src_cidx = cidx;
  dst_bidx = 0;
  dst_cidx = 0;
- } else if (direction == DMA_DEV_TO_MEM)  {
+ } else {
+ /* DMA_DEV_TO_MEM */
  src_bidx = 0;
  src_cidx = 0;
  dst_bidx = acnt;
  dst_cidx = cidx;
- } else {
- dev_err(dev, "%s: direction not implemented yet\n", __func__);
- return -EINVAL;
  }
 
  pset->opt = EDMA_TCC(EDMA_CHAN_SLOT(echan->ch_num));
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 11/14] dma: edma: Reduce debug print verbosity for non verbose debugging

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
Do not print the paRAM information when verbose debugging is not asked and
also reduce the number of lines printed in edma_prep_dma_cyclic()

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index d954099650ae..e8de2e84a60a 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -141,7 +141,7 @@ static void edma_execute(struct edma_chan *echan)
  for (i = 0; i < nslots; i++) {
  j = i + edesc->processed;
  edma_write_slot(echan->slot[i], &edesc->pset[j]);
- dev_dbg(echan->vchan.chan.device->dev,
+ dev_vdbg(echan->vchan.chan.device->dev,
  "\n pset[%d]:\n"
  "  chnum\t%d\n"
  "  slot\t%d\n"
@@ -558,9 +558,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  edesc->cyclic = 1;
  edesc->pset_nr = nslots;
 
- dev_dbg(dev, "%s: nslots=%d\n", __func__, nslots);
- dev_dbg(dev, "%s: period_len=%d\n", __func__, period_len);
- dev_dbg(dev, "%s: buf_len=%d\n", __func__, buf_len);
+ dev_dbg(dev, "%s: channel=%d nslots=%d period_len=%zu buf_len=%zu\n",
+ __func__, echan->ch_num, nslots, period_len, buf_len);
 
  for (i = 0; i < nslots; i++) {
  /* Allocate a PaRAM slot, if needed */
@@ -594,8 +593,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  else
  src_addr += period_len;
 
- dev_dbg(dev, "%s: Configure period %d of buf:\n", __func__, i);
- dev_dbg(dev,
+ dev_vdbg(dev, "%s: Configure period %d of buf:\n", __func__, i);
+ dev_vdbg(dev,
  "\n pset[%d]:\n"
  "  chnum\t%d\n"
  "  slot\t%d\n"
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 12/14] dma: edma: Prefix debug prints where the text were identical in prep callbacks

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
prep_slave_sg and prep_dma_cyclic callbacks have mostly same failure cases
with the same texts printed in case we hit them. It helps when debugging if
we know exactly which callback generated the errors.
At the same time change the debug level for descriptor allocation failure
from dbg to err since all other error cases are dev_err and this failure is
similarly fatal as the other ones.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index e8de2e84a60a..7feb6231276e 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -434,14 +434,14 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
  }
 
  if (dev_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) {
- dev_err(dev, "Undefined slave buswidth\n");
+ dev_err(dev, "%s: Undefined slave buswidth\n", __func__);
  return NULL;
  }
 
  edesc = kzalloc(sizeof(*edesc) + sg_len *
  sizeof(edesc->pset[0]), GFP_ATOMIC);
  if (!edesc) {
- dev_dbg(dev, "Failed to allocate a descriptor\n");
+ dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__);
  return NULL;
  }
 
@@ -457,7 +457,8 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
  EDMA_SLOT_ANY);
  if (echan->slot[i] < 0) {
  kfree(edesc);
- dev_err(dev, "Failed to allocate slot\n");
+ dev_err(dev, "%s: Failed to allocate slot\n",
+ __func__);
  return NULL;
  }
  }
@@ -526,7 +527,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  }
 
  if (dev_width == DMA_SLAVE_BUSWIDTH_UNDEFINED) {
- dev_err(dev, "Undefined slave buswidth\n");
+ dev_err(dev, "%s: Undefined slave buswidth\n", __func__);
  return NULL;
  }
 
@@ -551,7 +552,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  edesc = kzalloc(sizeof(*edesc) + nslots *
  sizeof(edesc->pset[0]), GFP_ATOMIC);
  if (!edesc) {
- dev_dbg(dev, "Failed to allocate a descriptor\n");
+ dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__);
  return NULL;
  }
 
@@ -569,7 +570,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  EDMA_SLOT_ANY);
  if (echan->slot[i] < 0) {
  kfree(edesc);
- dev_err(dev, "Failed to allocate slot\n");
+ dev_err(dev, "%s: Failed to allocate slot\n",
+ __func__);
  return NULL;
  }
  }
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 13/14] dma: edma: Add channel number to debug prints

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
It helps to identify issues if we have some information regarding to the
channel which the event is associated.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 7feb6231276e..2413237840f8 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -185,7 +185,8 @@ static void edma_execute(struct edma_chan *echan)
  edma_resume(echan->ch_num);
 
  if (edesc->processed <= MAX_NR_SG) {
- dev_dbg(dev, "first transfer starting %d\n", echan->ch_num);
+ dev_dbg(dev, "first transfer starting on channel %d\n",
+ echan->ch_num);
  edma_start(echan->ch_num);
  }
 
@@ -195,7 +196,7 @@ static void edma_execute(struct edma_chan *echan)
  * MAX_NR_SG
  */
  if (echan->missed) {
- dev_dbg(dev, "missed event in execute detected\n");
+ dev_dbg(dev, "missed event on channel %d\n", echan->ch_num);
  edma_clean_channel(echan->ch_num);
  edma_stop(echan->ch_num);
  edma_start(echan->ch_num);
@@ -736,7 +737,7 @@ static int edma_alloc_chan_resources(struct dma_chan *chan)
  echan->alloced = true;
  echan->slot[0] = echan->ch_num;
 
- dev_dbg(dev, "allocated channel for %u:%u\n",
+ dev_dbg(dev, "allocated channel %d for %u:%u\n", echan->ch_num,
  EDMA_CTLR(echan->ch_num), EDMA_CHAN_SLOT(echan->ch_num));
 
  return 0;
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 14/14] dma: edma: Print the direction value as well when it is not supported

Peter Ujfalusi
In reply to this post by Peter Ujfalusi
In case of not supported direction it is better to print the direction also.
It is unlikely, but in such an event it helps with the debugging.

Signed-off-by: Peter Ujfalusi <[hidden email]>
---
 drivers/dma/edma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 2413237840f8..912bf0190f45 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -430,7 +430,7 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg(
  dev_width = echan->cfg.dst_addr_width;
  burst = echan->cfg.dst_maxburst;
  } else {
- dev_err(dev, "%s: bad direction?\n", __func__);
+ dev_err(dev, "%s: bad direction: %d\n", __func__, direction);
  return NULL;
  }
 
@@ -523,7 +523,7 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
  dev_width = echan->cfg.dst_addr_width;
  burst = echan->cfg.dst_maxburst;
  } else {
- dev_err(dev, "%s: bad direction?\n", __func__);
+ dev_err(dev, "%s: bad direction: %d\n", __func__, direction);
  return NULL;
  }
 
--
1.9.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 05/14] arm: common: edma: Select event queue 1 as default when booted with DT

Joel Fernandes
In reply to this post by Peter Ujfalusi
On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
> Use the EVENTQ_1 for default and leave the EVENTQ_0 to be used by high
> priority channels, like audio.
>
> Signed-off-by: Peter Ujfalusi <[hidden email]>

This looks good, though another way to do it would be to leave default
to Queue 0. Put audio in Queue 1, and change QUEPRI to make Queue 1 as
higher priority.

This is fine,
Acked-by: Joel Fernandes <[hidden email]>

Regards,
-Joel
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 08/14] DMA: edma: Use different eventq for cyclic channels

Joel Fernandes
In reply to this post by Peter Ujfalusi
On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:

> To improve latency with cyclic DMA operation it is preferred to
> use different eventq/tc than the default which is used by all
> other drivers (mmc, spi, i2c, etc).
> When preparing the cyclic dma ask for non default queue for the
> channel which is going to be used with cyclic mode.
>
> Signed-off-by: Peter Ujfalusi <[hidden email]>
> ---
>  drivers/dma/edma.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 1dd9e8806975..10048b40fac8 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -628,6 +628,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic(
>   edesc->pset[i].opt |= TCINTEN;
>   }
>  
> + /* Use different eventq/tc for cyclic DMA to reduce latency */
> + edma_request_non_default_queue(echan->ch_num);
> +
>   return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
>  }
>  
>

Is there any way to guarantee that the non-default queue is of the
highest priority, or in other words default queue is of lowest priority.
I know you set queue 1 as default because by default 0 is higher
priority. And then assigning non-default queue.

When assigning default to Queue 1, it would be good to also call
assign_priority_to_queue and set QUEPRI to 7 for Queue 1. Since 0, 2 and
4 are all non-defaults.

Thanks,
-Joel
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 10/14] dma: edma: Simplify direction configuration in edma_config_pset()

Joel Fernandes
In reply to this post by Peter Ujfalusi
On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
> We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
> check for the direction has been already done in the function calling
> edma_config_pset().
> The error reporting is redundant and also the "else if ()" can be removed.
>

NAK. Please don't do this. I have been working on MEM_TO_MEM support as
well so leave it as it is for future.

Thanks,
-Joel

> Signed-off-by: Peter Ujfalusi <[hidden email]>
> ---
>  drivers/dma/edma.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 855766672aa9..d954099650ae 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -372,14 +372,12 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
>   src_cidx = cidx;
>   dst_bidx = 0;
>   dst_cidx = 0;
> - } else if (direction == DMA_DEV_TO_MEM)  {
> + } else {
> + /* DMA_DEV_TO_MEM */
>   src_bidx = 0;
>   src_cidx = 0;
>   dst_bidx = acnt;
>   dst_cidx = cidx;
> - } else {
> - dev_err(dev, "%s: direction not implemented yet\n", __func__);
> - return -EINVAL;
>   }
>  
>   pset->opt = EDMA_TCC(EDMA_CHAN_SLOT(echan->ch_num));
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 00/14] dma: edma: Fixes for cyclic (audio) operation

Joel Fernandes
In reply to this post by Peter Ujfalusi
Hi Peter,

Other than patches 8/14 and 10/14 which I responded to, you could add my
Acked-by, or add it to the series itself once you make the changes and
drop 10.

Acked-by: Joel Fernandes <[hidden email]>

Thanks,
-Joel

On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:

> Hi,
>
> This is basically a resend of the previous series:
> https://lkml.org/lkml/2014/3/13/119
> with removed ASoC patches (most of them are applied already).
>
> Changes since v1:
> - ASoC patches removed
> - Comments from Andriy Shevchenko addressed
> - patch added to fix cases when src/dst_maxburst is set to 0
>
> Adding support for DMA pause/resume
> Possibility to select non default event queue/TC for cyclic (audio) dma
> channels: all devices using the eDMA via dmaengine was assigned to the default
> EQ/TC (mmc, i2c, spi, etc, and audio). This is not optimal from system
> performance point of view since sharing the same EQ/TC can cause latency spikes
> for cyclic channels (long DMA transfers for MMC for example).
>
> While debugging the edma to get things sorted out I noticed that the debug was
> too verbose and the important information was hidden even when the we did not
> asked for verbose dmaengine debug.
> I have included some debug cleanups for the edma dmaengine driver also.
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (14):
>   platform_data: edma: Be precise with the paRAM struct
>   dma: edma: Correct the handling of src/dst_maxburst == 0
>   dma: edma: Add support for DMA_PAUSE/RESUME operation
>   dma: edma: Set DMA_CYCLIC capability flag
>   arm: common: edma: Select event queue 1 as default when booted with DT
>   arm: common: edma: Save the number of event queues/TCs
>   arm: common: edma: API to request non default queue for a channel
>   DMA: edma: Use different eventq for cyclic channels
>   dma: edma: Implement device_slave_caps callback
>   dma: edma: Simplify direction configuration in edma_config_pset()
>   dma: edma: Reduce debug print verbosity for non verbose debugging
>   dma: edma: Prefix debug prints where the text were identical in prep
>     callbacks
>   dma: edma: Add channel number to debug prints
>   dma: edma: Print the direction value as well when it is not supported
>
>  arch/arm/common/edma.c             | 34 +++++++++++++-
>  drivers/dma/edma.c                 | 96 +++++++++++++++++++++++++++++---------
>  include/linux/platform_data/edma.h | 20 ++++----
>  3 files changed, 119 insertions(+), 31 deletions(-)
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 10/14] dma: edma: Simplify direction configuration in edma_config_pset()

Peter Ujfalusi
In reply to this post by Joel Fernandes
On 04/11/2014 01:40 AM, Joel Fernandes wrote:
> On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
>> We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
>> check for the direction has been already done in the function calling
>> edma_config_pset().
>> The error reporting is redundant and also the "else if ()" can be removed.
>>
>
> NAK. Please don't do this. I have been working on MEM_TO_MEM support as
> well so leave it as it is for future.

Sure. It is still valid to say that the error else {} will never going to
happen since you have the same check in the calling function and they already
filtered the non implemented direction.

I'll leave this out from v3.

--
P├ęter

>
> Thanks,
> -Joel
>
>> Signed-off-by: Peter Ujfalusi <[hidden email]>
>> ---
>>  drivers/dma/edma.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
>> index 855766672aa9..d954099650ae 100644
>> --- a/drivers/dma/edma.c
>> +++ b/drivers/dma/edma.c
>> @@ -372,14 +372,12 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
>>   src_cidx = cidx;
>>   dst_bidx = 0;
>>   dst_cidx = 0;
>> - } else if (direction == DMA_DEV_TO_MEM)  {
>> + } else {
>> + /* DMA_DEV_TO_MEM */
>>   src_bidx = 0;
>>   src_cidx = 0;
>>   dst_bidx = acnt;
>>   dst_cidx = cidx;
>> - } else {
>> - dev_err(dev, "%s: direction not implemented yet\n", __func__);
>> - return -EINVAL;
>>   }
>>  
>>   pset->opt = EDMA_TCC(EDMA_CHAN_SLOT(echan->ch_num));
>>
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
123