[PATCH 0/3] staging: comedi: adl_pci9118: some dma transfer changes

[PATCH 0/3] staging: comedi: adl_pci9118: some dma transfer changes

[Ian Abbott]

For streaming acquisition on the analog input subdevice, this driver
normally uses DMA double buffering into two internal DMA buffers, so it
can switch buffers early after a DMA transfer has completed, while it
processes the completed DMA buffer.

PATCH 1 is just a bit of tidy up.

PATCH 2 avoids switching the DMA double buffer at the end of
acquisition.  For DMA single buffering, it avoids restarting the DMA
transfer if acquisition has ended normally or due to a comedi buffer
overflow error.  (However, for double buffering, the DMA transfer has
already been started on the other DMA buffer and will be stopped soon
after.)

PATCH 3 eliminates a possible defragmentation step in the DMA buffer
contents before they are transferred to the comedi buffer.  Instead, the
DMA buffer fragments are copied directly to the comedi buffer, or is
copied all in one go if the data is not fragmented.

This work is in response to Hartley's "PATCH 4/4] staging: comedi:
adl_pci9118: switch DMA buffers after writing samples", dated Mon, 10
Nov 2014 1959:17 -0500, message ID
<1415667477-28403-5-git-send-email-hsweeten@visionengravers.com>, which
was not applied.

These patches are against Greg's "staging-testing" branch.

1) staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a
   bit
2) staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart
3) staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation
   step

 drivers/staging/comedi/drivers/adl_pci9118.c | 157 +++++++++++++++++++++------
 1 file changed, 121 insertions(+), 36 deletions(-)

[PATCH 0/3] staging: comedi: adl_pci9118: some dma transfer changes

[Ian Abbott]

For streaming acquisition on the analog input subdevice, this driver
normally uses DMA double buffering into two internal DMA buffers, so it
can switch buffers early after a DMA transfer has completed, while it
processes the completed DMA buffer.

PATCH 1 is just a bit of tidy up.

PATCH 2 avoids switching the DMA double buffer at the end of
acquisition.  For DMA single buffering, it avoids restarting the DMA
transfer if acquisition has ended normally or due to a comedi buffer
overflow error.  (However, for double buffering, the DMA transfer has
already been started on the other DMA buffer and will be stopped soon
after.)

PATCH 3 eliminates a possible defragmentation step in the DMA buffer
contents before they are transferred to the comedi buffer.  Instead, the
DMA buffer fragments are copied directly to the comedi buffer, or is
copied all in one go if the data is not fragmented.

This work is in response to Hartley's "PATCH 4/4] staging: comedi:
adl_pci9118: switch DMA buffers after writing samples", dated Mon, 10
Nov 2014 1959:17 -0500, message ID
<1415667477-28403-5-git-send-email-hsweeten@visionengravers.com>, which
was not applied.

These patches are against Greg's "staging-testing" branch.

1) staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a
   bit
2) staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart
3) staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation
   step

 drivers/staging/comedi/drivers/adl_pci9118.c | 157 +++++++++++++++++++++------
 1 file changed, 121 insertions(+), 36 deletions(-)
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit

[Ian Abbott]

Eliminate the `next_dma_buf` variable in `interrupt_pci9118_ai_dma()`.
It holds the next value of `devpriv->dma_actbuf` when double buffering
is used, but we can just set that to the next value directly at the
point where the buffers are switched as the old value is not used
anywhere else.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index 5e0ff9d..aecfae8 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -608,16 +608,15 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 	struct comedi_cmd *cmd = &s->async->cmd;
 	struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
 	unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
-	unsigned int next_dma_buf;
 
-	if (devpriv->dma_doublebuf) {	/*
-					 * switch DMA buffers if is used
-					 * double buffering
-					 */
-		next_dma_buf = 1 - devpriv->dma_actbuf;
-		pci9118_amcc_setup_dma(dev, next_dma_buf);
-		if (devpriv->ai_do == 4)
-			interrupt_pci9118_ai_mode4_switch(dev, next_dma_buf);
+	/* switch DMA buffers and restart DMA if double buffering */
+	if (devpriv->dma_doublebuf) {
+		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
+		pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
+		if (devpriv->ai_do == 4) {
+			interrupt_pci9118_ai_mode4_switch(dev,
+							  devpriv->dma_actbuf);
+		}
 	}
 
 	if (nsamples) {
@@ -631,11 +630,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 			s->async->events |= COMEDI_CB_EOA;
 	}
 
-	if (devpriv->dma_doublebuf) {
-		/* switch dma buffers */
-		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
-	} else {
-		/* restart DMA if is not used double buffering */
+	/* restart DMA if not double buffering */
+	if (!devpriv->dma_doublebuf) {
 		pci9118_amcc_setup_dma(dev, 0);
 		if (devpriv->ai_do == 4)
 			interrupt_pci9118_ai_mode4_switch(dev, 0);
-- 
2.1.3

[PATCH 1/3] staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit

[Ian Abbott]

Eliminate the `next_dma_buf` variable in `interrupt_pci9118_ai_dma()`.
It holds the next value of `devpriv->dma_actbuf` when double buffering
is used, but we can just set that to the next value directly at the
point where the buffers are switched as the old value is not used
anywhere else.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index 5e0ff9d..aecfae8 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -608,16 +608,15 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 	struct comedi_cmd *cmd = &s->async->cmd;
 	struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
 	unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
-	unsigned int next_dma_buf;
 
-	if (devpriv->dma_doublebuf) {	/*
-					 * switch DMA buffers if is used
-					 * double buffering
-					 */
-		next_dma_buf = 1 - devpriv->dma_actbuf;
-		pci9118_amcc_setup_dma(dev, next_dma_buf);
-		if (devpriv->ai_do == 4)
-			interrupt_pci9118_ai_mode4_switch(dev, next_dma_buf);
+	/* switch DMA buffers and restart DMA if double buffering */
+	if (devpriv->dma_doublebuf) {
+		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
+		pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
+		if (devpriv->ai_do == 4) {
+			interrupt_pci9118_ai_mode4_switch(dev,
+							  devpriv->dma_actbuf);
+		}
 	}
 
 	if (nsamples) {
@@ -631,11 +630,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 			s->async->events |= COMEDI_CB_EOA;
 	}
 
-	if (devpriv->dma_doublebuf) {
-		/* switch dma buffers */
-		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
-	} else {
-		/* restart DMA if is not used double buffering */
+	/* restart DMA if not double buffering */
+	if (!devpriv->dma_doublebuf) {
 		pci9118_amcc_setup_dma(dev, 0);
 		if (devpriv->ai_do == 4)
 			interrupt_pci9118_ai_mode4_switch(dev, 0);
-- 
2.1.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart

[Ian Abbott]

`interrupt_pci9118_ai_dma()` is called on interrupt to transfer data
from DMA buffers into the comedi async data buffer.  Currently it always
restarts DMA.  If double buffering, it restarts DMA on the next DMA
buffer before processing the current DMA buffer, otherwise it restarts
DMA on the same DMA buffer after it has been processed.

For single buffering we can avoid restarting the DMA transfer by
checking the async event flags after the current buffer has been
processed, which is easy.

For double buffering, we need to know how many valid samples there are
in the current buffer before it has been processed and determine whether
there is enough to complete the acquisition.  Call new function
`valid_samples_in_act_dma_buf()` to determine the number of valid
samples in the current DMA buffer, and compare that with the result of
`comedi_nsamples_left()` to determine if DMA needs to be restarted.
(`comedi_nsamples_left()` needs an upper bound to clamp to, so use the
number of valid samples in the DMA buffer plus one for our test.)

It is still possible for DMA to be restarted unnecessarily in the double
buffer case if a `COMEDI_CB_OVERFLOW` event occurs while copying to the
comedi async buffer, but it doesn't really matter.  The ongoing DMA
operation will get disabled when the subdevice's `cancel()` handler is
called when the events are handled later in the interrupt service
routine (as it does currently).

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 78 +++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index aecfae8..c0ea733 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -446,6 +446,62 @@ static void interrupt_pci9118_ai_mode4_switch(struct comedi_device *dev,
 	outl(devpriv->ai_cfg, dev->iobase + PCI9118_AI_CFG_REG);
 }
 
+static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
+						 struct comedi_subdevice *s,
+						 unsigned int n_raw_samples)
+{
+	struct pci9118_private *devpriv = dev->private;
+	struct comedi_cmd *cmd = &s->async->cmd;
+	unsigned int start_pos = devpriv->ai_add_front;
+	unsigned int stop_pos = start_pos + cmd->chanlist_len;
+	unsigned int span_len = stop_pos + devpriv->ai_add_back;
+	unsigned int dma_pos = devpriv->ai_act_dmapos;
+	unsigned int whole_spans, n_samples, x;
+
+	if (span_len == cmd->chanlist_len)
+		return n_raw_samples;	/* use all samples */
+
+	/*
+	 * Not all samples are to be used.  Buffer contents consist of a
+	 * possibly non-whole number of spans and a region of each span
+	 * is to be used.
+	 *
+	 * Account for samples in whole number of spans.
+	 */
+	whole_spans = n_raw_samples / span_len;
+	n_samples = whole_spans * cmd->chanlist_len;
+	n_raw_samples -= whole_spans * span_len;
+
+	/*
+	 * Deal with remaining samples which could overlap up to two spans.
+	 */
+	while (n_raw_samples) {
+		if (dma_pos < start_pos) {
+			/* Skip samples before start position. */
+			x = start_pos - dma_pos;
+			if (x > n_raw_samples)
+				x = n_raw_samples;
+			dma_pos += x;
+			n_raw_samples -= x;
+			if (!n_raw_samples)
+				break;
+		}
+		if (dma_pos < stop_pos) {
+			/* Include samples before stop position. */
+			x = stop_pos - dma_pos;
+			if (x > n_raw_samples)
+				x = n_raw_samples;
+			n_samples += x;
+			dma_pos += x;
+			n_raw_samples -= x;
+		}
+		/* Advance to next span. */
+		start_pos += span_len;
+		stop_pos += span_len;
+	}
+	return n_samples;
+}
+
 static unsigned int defragment_dma_buffer(struct comedi_device *dev,
 					  struct comedi_subdevice *s,
 					  unsigned short *dma_buffer,
@@ -607,10 +663,16 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 	struct pci9118_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
 	struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
-	unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
+	unsigned int n_all = comedi_bytes_to_samples(s, dmabuf->use_size);
+	unsigned int n_valid;
+	bool more_dma;
+
+	/* determine whether more DMA buffers to do after this one */
+	n_valid = valid_samples_in_act_dma_buf(dev, s, n_all);
+	more_dma = n_valid < comedi_nsamples_left(s, n_valid + 1);
 
 	/* switch DMA buffers and restart DMA if double buffering */
-	if (devpriv->dma_doublebuf) {
+	if (more_dma && devpriv->dma_doublebuf) {
 		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
 		pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
 		if (devpriv->ai_do == 4) {
@@ -619,10 +681,9 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 		}
 	}
 
-	if (nsamples) {
-		nsamples = defragment_dma_buffer(dev, s, dmabuf->virt,
-						 nsamples);
-		comedi_buf_write_samples(s, dmabuf->virt, nsamples);
+	if (n_all) {
+		n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
+		comedi_buf_write_samples(s, dmabuf->virt, n_valid);
 	}
 
 	if (!devpriv->ai_neverending) {
@@ -630,8 +691,11 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 			s->async->events |= COMEDI_CB_EOA;
 	}
 
+	if (s->async->events & COMEDI_CB_CANCEL_MASK)
+		more_dma = false;
+
 	/* restart DMA if not double buffering */
-	if (!devpriv->dma_doublebuf) {
+	if (more_dma && !devpriv->dma_doublebuf) {
 		pci9118_amcc_setup_dma(dev, 0);
 		if (devpriv->ai_do == 4)
 			interrupt_pci9118_ai_mode4_switch(dev, 0);
-- 
2.1.3

[PATCH 2/3] staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart

[Ian Abbott]

`interrupt_pci9118_ai_dma()` is called on interrupt to transfer data
from DMA buffers into the comedi async data buffer.  Currently it always
restarts DMA.  If double buffering, it restarts DMA on the next DMA
buffer before processing the current DMA buffer, otherwise it restarts
DMA on the same DMA buffer after it has been processed.

For single buffering we can avoid restarting the DMA transfer by
checking the async event flags after the current buffer has been
processed, which is easy.

For double buffering, we need to know how many valid samples there are
in the current buffer before it has been processed and determine whether
there is enough to complete the acquisition.  Call new function
`valid_samples_in_act_dma_buf()` to determine the number of valid
samples in the current DMA buffer, and compare that with the result of
`comedi_nsamples_left()` to determine if DMA needs to be restarted.
(`comedi_nsamples_left()` needs an upper bound to clamp to, so use the
number of valid samples in the DMA buffer plus one for our test.)

It is still possible for DMA to be restarted unnecessarily in the double
buffer case if a `COMEDI_CB_OVERFLOW` event occurs while copying to the
comedi async buffer, but it doesn't really matter.  The ongoing DMA
operation will get disabled when the subdevice's `cancel()` handler is
called when the events are handled later in the interrupt service
routine (as it does currently).

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 78 +++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index aecfae8..c0ea733 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -446,6 +446,62 @@ static void interrupt_pci9118_ai_mode4_switch(struct comedi_device *dev,
 	outl(devpriv->ai_cfg, dev->iobase + PCI9118_AI_CFG_REG);
 }
 
+static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
+						 struct comedi_subdevice *s,
+						 unsigned int n_raw_samples)
+{
+	struct pci9118_private *devpriv = dev->private;
+	struct comedi_cmd *cmd = &s->async->cmd;
+	unsigned int start_pos = devpriv->ai_add_front;
+	unsigned int stop_pos = start_pos + cmd->chanlist_len;
+	unsigned int span_len = stop_pos + devpriv->ai_add_back;
+	unsigned int dma_pos = devpriv->ai_act_dmapos;
+	unsigned int whole_spans, n_samples, x;
+
+	if (span_len == cmd->chanlist_len)
+		return n_raw_samples;	/* use all samples */
+
+	/*
+	 * Not all samples are to be used.  Buffer contents consist of a
+	 * possibly non-whole number of spans and a region of each span
+	 * is to be used.
+	 *
+	 * Account for samples in whole number of spans.
+	 */
+	whole_spans = n_raw_samples / span_len;
+	n_samples = whole_spans * cmd->chanlist_len;
+	n_raw_samples -= whole_spans * span_len;
+
+	/*
+	 * Deal with remaining samples which could overlap up to two spans.
+	 */
+	while (n_raw_samples) {
+		if (dma_pos < start_pos) {
+			/* Skip samples before start position. */
+			x = start_pos - dma_pos;
+			if (x > n_raw_samples)
+				x = n_raw_samples;
+			dma_pos += x;
+			n_raw_samples -= x;
+			if (!n_raw_samples)
+				break;
+		}
+		if (dma_pos < stop_pos) {
+			/* Include samples before stop position. */
+			x = stop_pos - dma_pos;
+			if (x > n_raw_samples)
+				x = n_raw_samples;
+			n_samples += x;
+			dma_pos += x;
+			n_raw_samples -= x;
+		}
+		/* Advance to next span. */
+		start_pos += span_len;
+		stop_pos += span_len;
+	}
+	return n_samples;
+}
+
 static unsigned int defragment_dma_buffer(struct comedi_device *dev,
 					  struct comedi_subdevice *s,
 					  unsigned short *dma_buffer,
@@ -607,10 +663,16 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 	struct pci9118_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
 	struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
-	unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
+	unsigned int n_all = comedi_bytes_to_samples(s, dmabuf->use_size);
+	unsigned int n_valid;
+	bool more_dma;
+
+	/* determine whether more DMA buffers to do after this one */
+	n_valid = valid_samples_in_act_dma_buf(dev, s, n_all);
+	more_dma = n_valid < comedi_nsamples_left(s, n_valid + 1);
 
 	/* switch DMA buffers and restart DMA if double buffering */
-	if (devpriv->dma_doublebuf) {
+	if (more_dma && devpriv->dma_doublebuf) {
 		devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
 		pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
 		if (devpriv->ai_do == 4) {
@@ -619,10 +681,9 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 		}
 	}
 
-	if (nsamples) {
-		nsamples = defragment_dma_buffer(dev, s, dmabuf->virt,
-						 nsamples);
-		comedi_buf_write_samples(s, dmabuf->virt, nsamples);
+	if (n_all) {
+		n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
+		comedi_buf_write_samples(s, dmabuf->virt, n_valid);
 	}
 
 	if (!devpriv->ai_neverending) {
@@ -630,8 +691,11 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 			s->async->events |= COMEDI_CB_EOA;
 	}
 
+	if (s->async->events & COMEDI_CB_CANCEL_MASK)
+		more_dma = false;
+
 	/* restart DMA if not double buffering */
-	if (!devpriv->dma_doublebuf) {
+	if (more_dma && !devpriv->dma_doublebuf) {
 		pci9118_amcc_setup_dma(dev, 0);
 		if (devpriv->ai_do == 4)
 			interrupt_pci9118_ai_mode4_switch(dev, 0);
-- 
2.1.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step

[Ian Abbott]

The DMA operations used by the driver may have been set up to acquire
data from unwanted channels in addition to the wanted channels.
Currently, `interrupt_pci9118_ai_dma()` calls `defragment_dma_buffer()`
to move all the wanted data to the start of the DMA buffer and then
calls `comedi_buf_write_samples()` to copy it all to the comedi async
buffer.  Those two functions used to be called from
`move_block_from_dma()` which was absorbed into
`interrupt_pci9118_ai_dma()`.

Reinstate `move_block_from_dma()` but rewrite it to copy data directly
from the wanted fragments of the DMA buffer to the comedi async buffer
without defragmenting the buffer first.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 67 +++++++++++++++++++---------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index c0ea733..2660358 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -502,29 +502,56 @@ static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
 	return n_samples;
 }
 
-static unsigned int defragment_dma_buffer(struct comedi_device *dev,
-					  struct comedi_subdevice *s,
-					  unsigned short *dma_buffer,
-					  unsigned int num_samples)
+static void move_block_from_dma(struct comedi_device *dev,
+				struct comedi_subdevice *s,
+				unsigned short *dma_buffer,
+				unsigned int n_raw_samples)
 {
 	struct pci9118_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
-	unsigned int i = 0, j = 0;
-	unsigned int start_pos = devpriv->ai_add_front,
-	    stop_pos = devpriv->ai_add_front + cmd->chanlist_len;
-	unsigned int raw_scanlen = devpriv->ai_add_front + cmd->chanlist_len +
-	    devpriv->ai_add_back;
+	unsigned int start_pos = devpriv->ai_add_front;
+	unsigned int stop_pos = start_pos + cmd->chanlist_len;
+	unsigned int span_len = stop_pos + devpriv->ai_add_back;
+	unsigned int dma_pos = devpriv->ai_act_dmapos;
+	unsigned int x;
 
-	for (i = 0; i < num_samples; i++) {
-		if (devpriv->ai_act_dmapos >= start_pos &&
-		    devpriv->ai_act_dmapos < stop_pos) {
-			dma_buffer[j++] = dma_buffer[i];
+	if (span_len == cmd->chanlist_len) {
+		/* All samples are to be copied. */
+		comedi_buf_write_samples(s, dma_buffer, n_raw_samples);
+		dma_pos += n_raw_samples;
+	} else {
+		/*
+		 * Not all samples are to be copied.  Buffer contents consist
+		 * of a possibly non-whole number of spans and a region of
+		 * each span is to be copied.
+		 */
+		while (n_raw_samples) {
+			if (dma_pos < start_pos) {
+				/* Skip samples before start position. */
+				x = start_pos - dma_pos;
+				if (x > n_raw_samples)
+					x = n_raw_samples;
+				dma_pos += x;
+				n_raw_samples -= x;
+				if (!n_raw_samples)
+					break;
+			}
+			if (dma_pos < stop_pos) {
+				/* Copy samples before stop position. */
+				x = stop_pos - dma_pos;
+				if (x > n_raw_samples)
+					x = n_raw_samples;
+				comedi_buf_write_samples(s, dma_buffer, x);
+				dma_pos += x;
+				n_raw_samples -= x;
+			}
+			/* Advance to next span. */
+			start_pos += span_len;
+			stop_pos += span_len;
 		}
-		devpriv->ai_act_dmapos++;
-		devpriv->ai_act_dmapos %= raw_scanlen;
 	}
-
-	return j;
+	/* Update position in span for next time. */
+	devpriv->ai_act_dmapos = dma_pos % span_len;
 }
 
 static void pci9118_exttrg_enable(struct comedi_device *dev, bool enable)
@@ -681,10 +708,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 		}
 	}
 
-	if (n_all) {
-		n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
-		comedi_buf_write_samples(s, dmabuf->virt, n_valid);
-	}
+	if (n_all)
+		move_block_from_dma(dev, s, dmabuf->virt, n_all);
 
 	if (!devpriv->ai_neverending) {
 		if (s->async->scans_done >= cmd->stop_arg)
-- 
2.1.3

[PATCH 3/3] staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step

[Ian Abbott]

The DMA operations used by the driver may have been set up to acquire
data from unwanted channels in addition to the wanted channels.
Currently, `interrupt_pci9118_ai_dma()` calls `defragment_dma_buffer()`
to move all the wanted data to the start of the DMA buffer and then
calls `comedi_buf_write_samples()` to copy it all to the comedi async
buffer.  Those two functions used to be called from
`move_block_from_dma()` which was absorbed into
`interrupt_pci9118_ai_dma()`.

Reinstate `move_block_from_dma()` but rewrite it to copy data directly
from the wanted fragments of the DMA buffer to the comedi async buffer
without defragmenting the buffer first.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/adl_pci9118.c | 67 +++++++++++++++++++---------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c
index c0ea733..2660358 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -502,29 +502,56 @@ static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
 	return n_samples;
 }
 
-static unsigned int defragment_dma_buffer(struct comedi_device *dev,
-					  struct comedi_subdevice *s,
-					  unsigned short *dma_buffer,
-					  unsigned int num_samples)
+static void move_block_from_dma(struct comedi_device *dev,
+				struct comedi_subdevice *s,
+				unsigned short *dma_buffer,
+				unsigned int n_raw_samples)
 {
 	struct pci9118_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
-	unsigned int i = 0, j = 0;
-	unsigned int start_pos = devpriv->ai_add_front,
-	    stop_pos = devpriv->ai_add_front + cmd->chanlist_len;
-	unsigned int raw_scanlen = devpriv->ai_add_front + cmd->chanlist_len +
-	    devpriv->ai_add_back;
+	unsigned int start_pos = devpriv->ai_add_front;
+	unsigned int stop_pos = start_pos + cmd->chanlist_len;
+	unsigned int span_len = stop_pos + devpriv->ai_add_back;
+	unsigned int dma_pos = devpriv->ai_act_dmapos;
+	unsigned int x;
 
-	for (i = 0; i < num_samples; i++) {
-		if (devpriv->ai_act_dmapos >= start_pos &&
-		    devpriv->ai_act_dmapos < stop_pos) {
-			dma_buffer[j++] = dma_buffer[i];
+	if (span_len == cmd->chanlist_len) {
+		/* All samples are to be copied. */
+		comedi_buf_write_samples(s, dma_buffer, n_raw_samples);
+		dma_pos += n_raw_samples;
+	} else {
+		/*
+		 * Not all samples are to be copied.  Buffer contents consist
+		 * of a possibly non-whole number of spans and a region of
+		 * each span is to be copied.
+		 */
+		while (n_raw_samples) {
+			if (dma_pos < start_pos) {
+				/* Skip samples before start position. */
+				x = start_pos - dma_pos;
+				if (x > n_raw_samples)
+					x = n_raw_samples;
+				dma_pos += x;
+				n_raw_samples -= x;
+				if (!n_raw_samples)
+					break;
+			}
+			if (dma_pos < stop_pos) {
+				/* Copy samples before stop position. */
+				x = stop_pos - dma_pos;
+				if (x > n_raw_samples)
+					x = n_raw_samples;
+				comedi_buf_write_samples(s, dma_buffer, x);
+				dma_pos += x;
+				n_raw_samples -= x;
+			}
+			/* Advance to next span. */
+			start_pos += span_len;
+			stop_pos += span_len;
 		}
-		devpriv->ai_act_dmapos++;
-		devpriv->ai_act_dmapos %= raw_scanlen;
 	}
-
-	return j;
+	/* Update position in span for next time. */
+	devpriv->ai_act_dmapos = dma_pos % span_len;
 }
 
 static void pci9118_exttrg_enable(struct comedi_device *dev, bool enable)
@@ -681,10 +708,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device *dev,
 		}
 	}
 
-	if (n_all) {
-		n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
-		comedi_buf_write_samples(s, dmabuf->virt, n_valid);
-	}
+	if (n_all)
+		move_block_from_dma(dev, s, dmabuf->virt, n_all);
 
 	if (!devpriv->ai_neverending) {
 		if (s->async->scans_done >= cmd->stop_arg)
-- 
2.1.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel