[PATCH 0/3] dma: Indicate residue granularity in dma_slave_caps

[PATCH 0/3] dma: Indicate residue granularity in dma_slave_caps

[Lars-Peter Clausen]

Hi,

This patch series introduces a new field to the dma_slave_caps struct which
exposes the granularity with which the reported residue is updated, updates the
pl330 DMA driver to set this field and lets the generic dmaengine PCM audio
driver check that field.

The reason for introducing the field is that for audio we need to know the
granularity with which the reported residue is updated. The uncertainty of the
ALSA period pointer depends on the granularity of the reported residue. E.g. if
residue is only updated once every period this means that the position in the
buffer from/to which the hardware is currently reading/writing could be
anywhere between the reported PCM pointer and the reported PCM pointer plus one
period size (plus some extra margin for interrupt latency, etc). Certain
algorithms (e.g. pulseaudio's timer based scheduling) only work correctly if the
uncertainty of the PCM pointer is low enough. Exposing this information from the
DMA driver makes it possible for applications to switch to alternative modes of
operation which also work with a high granularity.

The patch series introduces four levels of granularity, which I think will cover
most DMA cores. I'd like some feedback though if anybody can think of a DMA
controller for which none of the levels would be correct. The four levels are:
	* DESCRIPTOR: The DMA channel is only able to tell whether a descriptor has
	  been completed or not, which means residue reporting is not supported by
	  this channel. The residue field of the dma_tx_state field will always be
	  0.
	* SEGMENT: The DMA channel updates the residue field after each successfully
	  completed segment of the transfer (For cyclic transfers this is after each
	  period). This is typically implemented by having the hardware generate an
	  interrupt after each transferred segment and then the driver updates the
	  outstanding residue by the size of the segment. Another possibility is if
	  the hardware supports SG and the segment descriptor has a field which gets
	  set by the hardware after the segment has been completed. The driver then
	  counts the number of segments which do not have the flag set to compute
	  the residue.
	* BURST: The DMA channel updates the residue field after each transferred
	  burst. This is typically only supported if the hardware has a progress
	  register of some sort (E.g. a register with the current read/write address
	  or a register with the amount of bursts/beats/bytes that have been
	  transferred or still need to be transferred).
	* BYTE: Same as BURST but with a byte level granularity.

I've only included the last one for completeness and I'm not sure if we really
need it. Even if the hardware is able to report the amount of transferred data
with a byte level granularity the lower bits will not carry too much
meaning. When using burst transfers the lower bits will remain the same for
longer periods and then rapidly increment during the burst transfer. The upper
bits though will increment with the actual frequency with which the data is
consumed. And that's what applications are typically interested in.

- Lars

Lars-Peter Clausen (3):
  dma: Indicate residue granularity in dma_slave_caps
  dma: pl330: Set residue_granularity
  ASoC: generic-dmaengine-pcm: Check DMA residue granularity

 drivers/dma/pl330.c                   |  1 +
 include/linux/dmaengine.h             | 17 ++++++++++++++++
 sound/soc/soc-generic-dmaengine-pcm.c | 37 +++++++++++++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 2 deletions(-)

-- 
1.8.0

[PATCH 1/3] dma: Indicate residue granularity in dma_slave_caps

[Lars-Peter Clausen]

This patch adds a new field to the dma_slave_caps struct which indicates the
granularity with which the driver is able to update the residue field of the
dma_tx_state struct. Making this information available to dmaengine users allows
them to make better decisions on how to operate. E.g. for audio certain features
like wakeup less operation or timer based scheduling only make sense and work
correctly if the reported residue is fine-grained enough.

Right now four different levels of granularity are supported:
	* DESCRIPTOR: The DMA channel is only able to tell whether a descriptor has
	  been completed or not, which means residue reporting is not supported by
	  this channel. The residue field of the dma_tx_state field will always be
	  0.
	* SEGMENT: The DMA channel updates the residue field after each successfully
	  completed segment of the transfer (For cyclic transfers this is after each
	  period). This is typically implemented by having the hardware generate an
	  interrupt after each transferred segment and then the drivers updates the
	  outstanding residue by the size of the segment. Another possibility is if
	  the hardware supports SG and the segment descriptor has a field which gets
	  set after the segment has been completed. The driver then counts the
	  number of segments without the flag set to compute the residue.
	* BURST: The DMA channel updates the residue field after each transferred
	  burst. This is typically only supported if the hardware has a progress
	  register of some sort (E.g. a register with the current read/write address
	  or a register with the amount of bursts/beats/bytes that have been
	  transferred or still need to be transferred).
	* BYTE: Same as BURST but with a byte level granularity.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/linux/dmaengine.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 41cf0c3..b7a14d0 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -363,6 +363,21 @@ struct dma_slave_config {
 	unsigned int slave_id;
 };
 
+/**
+ * enum dma_residue_granularity - Granularity of the reported transfer residue
+ * @DMA_RESIDUE_GRANULATRITY_DESCRIPTOR: Residue reporting is not supported.
+ * @DMA_RESIDUE_GRANULATRITY_SEGMENT: Residue is updated with each completed
+ *  segment in the descriptor.
+ * @DMA_RESIDUE_GRANULATRITY_BURST: Residue is updated with each transfered burst
+ * @DMA_RESIDUE_GRANULATRITY_BYTE: Residue is updated with each transfered byte
+ */
+enum dma_residue_granularity {
+	DMA_RESIDUE_GRANULARITY_DESCRIPTOR = 0,
+	DMA_RESIDUE_GRANULARITY_SEGMENT = 1,
+	DMA_RESIDUE_GRANULARITY_BURST = 2,
+	DMA_RESIDUE_GRANULARITY_BYTE = 3,
+};
+
 /* struct dma_slave_caps - expose capabilities of a slave channel only
  *
  * @src_addr_widths: bit mask of src addr widths the channel supports
@@ -373,6 +388,7 @@ struct dma_slave_config {
  * 	should be checked by controller as well
  * @cmd_pause: true, if pause and thereby resume is supported
  * @cmd_terminate: true, if terminate cmd is supported
+ * @residue_granularity: granularity of the reported transfer residue
  */
 struct dma_slave_caps {
 	u32 src_addr_widths;
@@ -380,6 +396,7 @@ struct dma_slave_caps {
 	u32 directions;
 	bool cmd_pause;
 	bool cmd_terminate;
+	enum dma_residue_granularity residue_granularity;
 };
 
 static inline const char *dma_chan_name(struct dma_chan *chan)
-- 
1.8.0

[PATCH 2/3] dma: pl330: Set residue_granularity

[Lars-Peter Clausen]

The pl330 driver currently does not support residue reporting, so set the
residue granularity to DMA_RESIDUE_GRANULARITY_DESCRIPTOR.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/dma/pl330.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 7adaf3a..05b597a 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2890,6 +2890,7 @@ static int pl330_dma_device_slave_caps(struct dma_chan *dchan,
 	caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
 	caps->cmd_pause = false;
 	caps->cmd_terminate = true;
+	caps->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
 
 	return 0;
 }
-- 
1.8.0

[PATCH 3/3] ASoC: generic-dmaengine-pcm: Check DMA residue granularity

[Lars-Peter Clausen]

The dmaengine framework now exposes the granularity with which it is able to
report the transfer residue for a certain DMA channel. Check the granularity in
the generic dmaengine PCM driver and
	a) Set the SNDRV_PCM_INFO_BATCH if the granularity is per period or worse.
	b) Fallback to the (race condition prone) period counting if the driver does
	not support any residue reporting.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-generic-dmaengine-pcm.c | 37 +++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 87e8635..7bdee7b 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -144,6 +144,8 @@ static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
 	if (ret == 0) {
 		if (dma_caps.cmd_pause)
 			hw.info |= SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME;
+		if (dma_caps.residue_granularity <= DMA_RESIDUE_GRANULARITY_SEGMENT)
+			hw.info |= SNDRV_PCM_INFO_BATCH;
 	}
 
 	return snd_soc_set_runtime_hwparams(substream, &hw);
@@ -282,6 +284,27 @@ static const struct snd_soc_platform_driver dmaengine_no_residue_pcm_platform =
 	.probe_order	= SND_SOC_COMP_ORDER_LATE,
 };
 
+static bool dmaengine_pcm_can_report_residue(struct dmaengine_pcm *pcm)
+{
+	struct dma_slave_caps dma_caps;
+	int ret;
+	int i;
+
+	for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) {
+		if (!pcm->chan[i])
+			continue;
+
+		ret = dma_get_slave_caps(pcm->chan[i], &dma_caps);
+		if (ret != 0)
+			continue;
+
+		if (dma_caps.residue_granularity == DMA_RESIDUE_GRANULARITY_DESCRIPTOR)
+			return false;
+	}
+
+	return true;
+}
+
 static const char * const dmaengine_pcm_dma_channel_names[] = {
 	[SNDRV_PCM_STREAM_PLAYBACK] = "tx",
 	[SNDRV_PCM_STREAM_CAPTURE] = "rx",
@@ -323,11 +346,21 @@ int snd_dmaengine_pcm_register(struct device *dev,
 	if (!pcm)
 		return -ENOMEM;
 
+	dmaengine_pcm_request_chan_of(pcm, dev);
+
+	/*
+	 * This will only return false if we know for sure that at least one
+	 * channel does not support residue reporting. If the compat path is
+	 * used for requesting the channels or the dma driver does not implement
+	 * the slave_caps API we rely on the caller to set the NO_RESIDUE flag
+	 * in case residue reporting is not supported.
+	 */
+	if (!dmaengine_pcm_can_report_residue(pcm))
+		flags |= SND_DMAENGINE_PCM_FLAG_NO_RESIDUE;
+
 	pcm->config = config;
 	pcm->flags = flags;
 
-	dmaengine_pcm_request_chan_of(pcm, dev);
-
 	if (flags & SND_DMAENGINE_PCM_FLAG_NO_RESIDUE)
 		return snd_soc_add_platform(dev, &pcm->platform,
 				&dmaengine_no_residue_pcm_platform);
-- 
1.8.0

Re: [PATCH 0/3] dma: Indicate residue granularity in dma_slave_caps

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1725 bytes --]

On Sun, Dec 08, 2013 at 05:18:06PM +0100, Lars-Peter Clausen wrote:

Please word wrap a bit earlier, you're right at 80 columns which causes
issues when quoted.

> ALSA period pointer depends on the granularity of the reported residue. E.g. if
> residue is only updated once every period this means that the position in the
> buffer from/to which the hardware is currently reading/writing could be
> anywhere between the reported PCM pointer and the reported PCM pointer plus one

Since there's quite a bit of development going on with the DMA stuff
it'd be good if this could be merged via ASoC (either itself or as a
shared branch) so we can get the ASoC changes in easily.

> 	* BYTE: Same as BURST but with a byte level granularity.

> I've only included the last one for completeness and I'm not sure if we really
> need it. Even if the hardware is able to report the amount of transferred data
> with a byte level granularity the lower bits will not carry too much
> meaning. When using burst transfers the lower bits will remain the same for
> longer periods and then rapidly increment during the burst transfer. The upper
> bits though will increment with the actual frequency with which the data is
> consumed. And that's what applications are typically interested in.

I share your concerns about the usefulness here, and once you get to
this point you're starting to worry about if it means bytes on the
device side or the memory side which is really angels dancing on the
head of a pin sort of thing.  That can apply to burst sizes as well
where it gets a bit more relevant, we might want to say which we mean
there (I'd assume right now it's just the same as our current burst
configuration) if it even matters.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]