dma: axi-dmac: Split too large segments

dma: axi-dmac: Split too large segments

[Ardelean, Alexandru]

On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote:
> 

My bad here.
I took this patch from our kernel tree and sent it.
I assumed it works, since it works in our tree.
I'll take a look and see about the order of patches, and which one(s)
need(s) to be sent before this one

Thanks
Alex

> 
> Hi Lars-Peter,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.0-rc4 next-20190215]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
> 
> url:    
> https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/dma-axi-dmac-Split-too-large-segments/20190216-160002
> config: nds32-allyesconfig (attached as .config)
> compiler: nds32le-linux-gcc (GCC) 6.4.0
> reproduce:
>         wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=6.4.0 make.cross ARCH=nds32
> 
> All errors (new ones prefixed by >>):
> 
>    drivers//dma/dma-axi-dmac.c: In function 'axi_dmac_prep_slave_sg':
> > > drivers//dma/dma-axi-dmac.c:443:12: error: implicit declaration of
> > > function 'sg_nents_for_dma' [-Werror=implicit-function-declaration]
> 
>      num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
>                ^~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors
> 
> vim +/sg_nents_for_dma +443 drivers//dma/dma-axi-dmac.c
> 
>    427
>    428  static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
>    429          struct dma_chan *c, struct scatterlist *sgl,
>    430          unsigned int sg_len, enum dma_transfer_direction
> direction,
>    431          unsigned long flags, void *context)
>    432  {
>    433          struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
>    434          struct axi_dmac_desc *desc;
>    435          struct axi_dmac_sg *dsg;
>    436          struct scatterlist *sg;
>    437          unsigned int num_sgs;
>    438          unsigned int i;
>    439
>    440          if (direction != chan->direction)
>    441                  return NULL;
>    442
>  > 443          num_sgs = sg_nents_for_dma(sgl, sg_len, chan-
> >max_length);
>    444          desc = axi_dmac_alloc_desc(num_sgs);
>    445          if (!desc)
>    446                  return NULL;
>    447
>    448          dsg = desc->sg;
>    449
>    450          for_each_sg(sgl, sg, sg_len, i) {
>    451                  if (!axi_dmac_check_addr(chan,
> sg_dma_address(sg)) ||
>    452                      !axi_dmac_check_len(chan, sg_dma_len(sg))) {
>    453                          kfree(desc);
>    454                          return NULL;
>    455                  }
>    456
>    457                  dsg = axi_dmac_fill_linear_sg(chan, direction,
> sg_dma_address(sg), 1,
>    458                          sg_dma_len(sg), dsg);
>    459          }
>    460
>    461          desc->cyclic = false;
>    462
>    463          return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
>    464  }
>    465
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation

dma: axi-dmac: Split too large segments

[Vinod Koul]

On 18-02-19, 08:34, Lars-Peter Clausen wrote:
> On 2/18/19 8:28 AM, Ardelean, Alexandru wrote:
> > On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote:
> >>
> > 
> > My bad here.
> > I took this patch from our kernel tree and sent it.
> > I assumed it works, since it works in our tree.
> > I'll take a look and see about the order of patches, and which one(s)
> > need(s) to be sent before this one
> 
> https://github.com/analogdevicesinc/linux/commit/414a555896b2abca3518c3c878f21e7b1ea95904#diff-b5b2fd5430b4f9aff1ae97bae60dd5c5
> 
> That patch was sent upstream, not sure why it was never merged.
> 
> But if necessary you can also re-write this patch to not rely on the
> other one.

Or send this patch upstream if you find it very useful :)

dma: axi-dmac: Split too large segments

[Lars-Peter Clausen]

On 2/18/19 8:28 AM, Ardelean, Alexandru wrote:
> On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote:
>>
> 
> My bad here.
> I took this patch from our kernel tree and sent it.
> I assumed it works, since it works in our tree.
> I'll take a look and see about the order of patches, and which one(s)
> need(s) to be sent before this one

https://github.com/analogdevicesinc/linux/commit/414a555896b2abca3518c3c878f21e7b1ea95904#diff-b5b2fd5430b4f9aff1ae97bae60dd5c5

That patch was sent upstream, not sure why it was never merged.

But if necessary you can also re-write this patch to not rely on the
other one.

dma: axi-dmac: Split too large segments

[kbuild test robot]

Hi Lars-Peter,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190215]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/dma-axi-dmac-Split-too-large-segments/20190216-160002
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/dma/dma-axi-dmac.c: In function 'axi_dmac_prep_slave_sg':
>> drivers/dma/dma-axi-dmac.c:443:12: error: implicit declaration of function 'sg_nents_for_dma'; did you mean 'sg_nents_for_len'? [-Werror=implicit-function-declaration]
     num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
               ^~~~~~~~~~~~~~~~
               sg_nents_for_len
   cc1: some warnings being treated as errors

vim +443 drivers/dma/dma-axi-dmac.c

   427	
   428	static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
   429		struct dma_chan *c, struct scatterlist *sgl,
   430		unsigned int sg_len, enum dma_transfer_direction direction,
   431		unsigned long flags, void *context)
   432	{
   433		struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
   434		struct axi_dmac_desc *desc;
   435		struct axi_dmac_sg *dsg;
   436		struct scatterlist *sg;
   437		unsigned int num_sgs;
   438		unsigned int i;
   439	
   440		if (direction != chan->direction)
   441			return NULL;
   442	
 > 443		num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
   444		desc = axi_dmac_alloc_desc(num_sgs);
   445		if (!desc)
   446			return NULL;
   447	
   448		dsg = desc->sg;
   449	
   450		for_each_sg(sgl, sg, sg_len, i) {
   451			if (!axi_dmac_check_addr(chan, sg_dma_address(sg)) ||
   452			    !axi_dmac_check_len(chan, sg_dma_len(sg))) {
   453				kfree(desc);
   454				return NULL;
   455			}
   456	
   457			dsg = axi_dmac_fill_linear_sg(chan, direction, sg_dma_address(sg), 1,
   458				sg_dma_len(sg), dsg);
   459		}
   460	
   461		desc->cyclic = false;
   462	
   463		return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
   464	}
   465
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

dma: axi-dmac: Split too large segments

[Alexandru Ardelean]

From: Lars-Peter Clausen <lars@metafoo.de>

The axi-dmac driver currently rejects transfers with segments that are
larger than what the hardware can handle.

Re-work the driver so that these large segments are split into multiple
segments instead where each segment is smaller or equal to the maximum
segment size.

This allows the driver to handle transfers with segments of arbitrary size.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/dma/dma-axi-dmac.c | 80 ++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index ffc0adc2f6ce..6a41e1f49077 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -83,6 +83,7 @@ struct axi_dmac_sg {
 	unsigned int dest_stride;
 	unsigned int src_stride;
 	unsigned int id;
+	bool last;
 	bool schedule_when_free;
 };
 
@@ -166,7 +167,7 @@ static int axi_dmac_dest_is_mem(struct axi_dmac_chan *chan)
 
 static bool axi_dmac_check_len(struct axi_dmac_chan *chan, unsigned int len)
 {
-	if (len == 0 || len > chan->max_length)
+	if (len == 0)
 		return false;
 	if ((len & chan->align_mask) != 0) /* Not aligned */
 		return false;
@@ -379,6 +380,50 @@ static struct axi_dmac_desc *axi_dmac_alloc_desc(unsigned int num_sgs)
 	return desc;
 }
 
+static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
+	enum dma_transfer_direction direction, dma_addr_t addr,
+	unsigned int num_periods, unsigned int period_len,
+	struct axi_dmac_sg *sg)
+{
+	unsigned int num_segments, i;
+	unsigned int segment_size;
+	unsigned int len;
+
+	/* Split into multiple equally sized segments if necessary */
+	num_segments = DIV_ROUND_UP(period_len, chan->max_length);
+	segment_size = DIV_ROUND_UP(period_len, num_segments);
+	/* Take care of alignment */
+	segment_size = ((segment_size - 1) | chan->align_mask) + 1;
+
+	for (i = 0; i < num_periods; i++) {
+		len = period_len;
+
+		while (len > segment_size) {
+			if (direction == DMA_DEV_TO_MEM)
+				sg->dest_addr = addr;
+			else
+				sg->src_addr = addr;
+			sg->x_len = segment_size;
+			sg->y_len = 1;
+			sg++;
+			addr += segment_size;
+			len -= segment_size;
+		}
+
+		if (direction == DMA_DEV_TO_MEM)
+			sg->dest_addr = addr;
+		else
+			sg->src_addr = addr;
+		sg->x_len = len;
+		sg->y_len = 1;
+		sg->last = true;
+		sg++;
+		addr += len;
+	}
+
+	return sg;
+}
+
 static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
 	struct dma_chan *c, struct scatterlist *sgl,
 	unsigned int sg_len, enum dma_transfer_direction direction,
@@ -386,16 +431,21 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
 {
 	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
 	struct axi_dmac_desc *desc;
+	struct axi_dmac_sg *dsg;
 	struct scatterlist *sg;
+	unsigned int num_sgs;
 	unsigned int i;
 
 	if (direction != chan->direction)
 		return NULL;
 
-	desc = axi_dmac_alloc_desc(sg_len);
+	num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
+	desc = axi_dmac_alloc_desc(num_sgs);
 	if (!desc)
 		return NULL;
 
+	dsg = desc->sg;
+
 	for_each_sg(sgl, sg, sg_len, i) {
 		if (!axi_dmac_check_addr(chan, sg_dma_address(sg)) ||
 		    !axi_dmac_check_len(chan, sg_dma_len(sg))) {
@@ -403,12 +453,8 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
 			return NULL;
 		}
 
-		if (direction == DMA_DEV_TO_MEM)
-			desc->sg[i].dest_addr = sg_dma_address(sg);
-		else
-			desc->sg[i].src_addr = sg_dma_address(sg);
-		desc->sg[i].x_len = sg_dma_len(sg);
-		desc->sg[i].y_len = 1;
+		dsg = axi_dmac_fill_linear_sg(chan, direction, sg_dma_address(sg), 1,
+			sg_dma_len(sg), dsg);
 	}
 
 	desc->cyclic = false;
@@ -423,7 +469,7 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_dma_cyclic(
 {
 	struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
 	struct axi_dmac_desc *desc;
-	unsigned int num_periods, i;
+	unsigned int num_periods, num_segments;
 
 	if (direction != chan->direction)
 		return NULL;
@@ -436,20 +482,14 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_dma_cyclic(
 		return NULL;
 
 	num_periods = buf_len / period_len;
+	num_segments = DIV_ROUND_UP(period_len, chan->max_length);
 
-	desc = axi_dmac_alloc_desc(num_periods);
+	desc = axi_dmac_alloc_desc(num_periods * num_segments);
 	if (!desc)
 		return NULL;
 
-	for (i = 0; i < num_periods; i++) {
-		if (direction == DMA_DEV_TO_MEM)
-			desc->sg[i].dest_addr = buf_addr;
-		else
-			desc->sg[i].src_addr = buf_addr;
-		desc->sg[i].x_len = period_len;
-		desc->sg[i].y_len = 1;
-		buf_addr += period_len;
-	}
+	axi_dmac_fill_linear_sg(chan, direction, buf_addr, num_periods,
+		buf_len, desc->sg);
 
 	desc->cyclic = true;
 
@@ -647,7 +687,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
 	of_node_put(of_channels);
 
 	pdev->dev.dma_parms = &dmac->dma_parms;
-	dma_set_max_seg_size(&pdev->dev, dmac->chan.max_length);
+	dma_set_max_seg_size(&pdev->dev, UINT_MAX);
 
 	dma_dev = &dmac->dma_dev;
 	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);