[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Barry Song]

the dma engine of sirfsoc supports interleaved mode, but if we set
xlen=width instead xlen<width, it will work as non-interleaved. as
most clients of sirf dma driver still don't need interleaved mode,
so here we still need to implement prep_slave_sg entry so that users
like uart, spi can use these APIs instead of interleaved API.

the dma engine of sirfsoc doesn't support hardware s/g, so here we
are using the list of desc nodes to do SW s/g. when dma operations
finish, driver will re-start the next desc automatically.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/dma/sirf-dma.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
index 6aec3ad..c226c79 100644
--- a/drivers/dma/sirf-dma.c
+++ b/drivers/dma/sirf-dma.c
@@ -577,6 +577,62 @@ err_dir:
 }
 
 static struct dma_async_tx_descriptor *
+sirfsoc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+	unsigned int sg_len, enum dma_transfer_direction direction,
+	unsigned long flags, void *context)
+{
+	struct sirfsoc_dma_chan *schan = dma_chan_to_sirfsoc_dma_chan(chan);
+	struct sirfsoc_dma_desc *first_sdesc;
+	struct sirfsoc_dma_desc *sdesc;
+	struct list_head *l;
+	unsigned long iflags;
+	struct scatterlist *sg;
+	int desc_cnt = 0;
+	int i;
+	int ret;
+
+	/*
+	 * the hardware doesn't support sg, here we use software list
+	 * to simulate sg, so make sure we have enough desc nodes
+	 */
+	spin_lock_irqsave(&schan->lock, iflags);
+	list_for_each(l, &schan->free)
+		desc_cnt++;
+	if (desc_cnt < sg_len) {
+		spin_unlock_irqrestore(&schan->lock, iflags);
+		pr_err("sirfsoc DMA channel busy\n");
+		ret = -EBUSY;
+		goto err;
+	}
+
+	first_sdesc = list_first_entry(&schan->free, struct sirfsoc_dma_desc,
+			node);
+
+	for_each_sg(sgl, sg, sg_len, i) {
+		dma_addr_t addr = sg_dma_address(sg);
+		unsigned int len = sg_dma_len(sg);
+
+		sdesc = list_first_entry(&schan->free, struct sirfsoc_dma_desc,
+			node);
+		list_del(&sdesc->node);
+
+		sdesc->addr = addr;
+		sdesc->dir = (direction == DMA_MEM_TO_DEV ? 1 : 0);
+		sdesc->cyclic = 0;
+		sdesc->xlen = len / SIRFSOC_DMA_WORD_LEN;
+		sdesc->width = sdesc->xlen;
+		sdesc->ylen = 0;
+
+		list_add_tail(&sdesc->node, &schan->prepared);
+	}
+	spin_unlock_irqrestore(&schan->lock, iflags);
+
+	return &first_sdesc->desc;
+err:
+	return ERR_PTR(ret);
+}
+
+static struct dma_async_tx_descriptor *
 sirfsoc_dma_prep_cyclic(struct dma_chan *chan, dma_addr_t addr,
 	size_t buf_len, size_t period_len,
 	enum dma_transfer_direction direction, unsigned long flags, void *context)
@@ -711,6 +767,7 @@ static int sirfsoc_dma_probe(struct platform_device *op)
 	dma->device_control = sirfsoc_dma_control;
 	dma->device_tx_status = sirfsoc_dma_tx_status;
 	dma->device_prep_interleaved_dma = sirfsoc_dma_prep_interleaved;
+	dma->device_prep_slave_sg = sirfsoc_dma_prep_slave_sg;
 	dma->device_prep_dma_cyclic = sirfsoc_dma_prep_cyclic;
 
 	INIT_LIST_HEAD(&dma->channels);
-- 
1.8.2.3

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Vinod Koul]

On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
> the dma engine of sirfsoc supports interleaved mode, but if we set
> xlen=width instead xlen<width, it will work as non-interleaved. as
> most clients of sirf dma driver still don't need interleaved mode,
> so here we still need to implement prep_slave_sg entry so that users
> like uart, spi can use these APIs instead of interleaved API.
Well in that case why dont you just create a wrapper on top of interleaved API
to make this work without driver changes

> the dma engine of sirfsoc doesn't support hardware s/g, so here we
> are using the list of desc nodes to do SW s/g. when dma operations
> finish, driver will re-start the next desc automatically.
> 
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/dma/sirf-dma.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
> index 6aec3ad..c226c79 100644
> --- a/drivers/dma/sirf-dma.c
> +++ b/drivers/dma/sirf-dma.c
> @@ -577,6 +577,62 @@ err_dir:
>  }
>  
>  static struct dma_async_tx_descriptor *
> +sirfsoc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> +	unsigned int sg_len, enum dma_transfer_direction direction,
> +	unsigned long flags, void *context)
> +{
> +	struct sirfsoc_dma_chan *schan = dma_chan_to_sirfsoc_dma_chan(chan);
> +	struct sirfsoc_dma_desc *first_sdesc;
> +	struct sirfsoc_dma_desc *sdesc;
> +	struct list_head *l;
> +	unsigned long iflags;
> +	struct scatterlist *sg;
> +	int desc_cnt = 0;
> +	int i;
> +	int ret;
> +
> +	/*
> +	 * the hardware doesn't support sg, here we use software list
> +	 * to simulate sg, so make sure we have enough desc nodes
> +	 */
> +	spin_lock_irqsave(&schan->lock, iflags);
> +	list_for_each(l, &schan->free)
> +		desc_cnt++;
why dont you allocate descriptors here. That will remove this limitation..

~Vinod

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Barry Song]

2013/8/26 Vinod Koul <vinod.koul@intel.com>:
> On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>> the dma engine of sirfsoc supports interleaved mode, but if we set
>> xlen=width instead xlen<width, it will work as non-interleaved. as
>> most clients of sirf dma driver still don't need interleaved mode,
>> so here we still need to implement prep_slave_sg entry so that users
>> like uart, spi can use these APIs instead of interleaved API.
> Well in that case why dont you just create a wrapper on top of interleaved API
> to make this work without driver changes

Vinod, do you mean using interleaved API to provide sg/single API if
specific drivers implement interleaved_dma but not implement sg_dma?

the problem is that is difficult to set right legal sgl[i].size,
sgl[i].icg and numf for all dmaengines as that depends on specific dma
hardware limitation.

>
>> the dma engine of sirfsoc doesn't support hardware s/g, so here we
>> are using the list of desc nodes to do SW s/g. when dma operations
>> finish, driver will re-start the next desc automatically.
>>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  drivers/dma/sirf-dma.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>>
>> diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
>> index 6aec3ad..c226c79 100644
>> --- a/drivers/dma/sirf-dma.c
>> +++ b/drivers/dma/sirf-dma.c
>> @@ -577,6 +577,62 @@ err_dir:
>>  }
>>
>>  static struct dma_async_tx_descriptor *
>> +sirfsoc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>> +     unsigned int sg_len, enum dma_transfer_direction direction,
>> +     unsigned long flags, void *context)
>> +{
>> +     struct sirfsoc_dma_chan *schan = dma_chan_to_sirfsoc_dma_chan(chan);
>> +     struct sirfsoc_dma_desc *first_sdesc;
>> +     struct sirfsoc_dma_desc *sdesc;
>> +     struct list_head *l;
>> +     unsigned long iflags;
>> +     struct scatterlist *sg;
>> +     int desc_cnt = 0;
>> +     int i;
>> +     int ret;
>> +
>> +     /*
>> +      * the hardware doesn't support sg, here we use software list
>> +      * to simulate sg, so make sure we have enough desc nodes
>> +      */
>> +     spin_lock_irqsave(&schan->lock, iflags);
>> +     list_for_each(l, &schan->free)
>> +             desc_cnt++;
> why dont you allocate descriptors here. That will remove this limitation..

i understand. here sirf user scenerios will never be over the
limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
too complex.

>
> ~Vinod

-barry

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Vinod Koul]

On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
> >> the dma engine of sirfsoc supports interleaved mode, but if we set
> >> xlen=width instead xlen<width, it will work as non-interleaved. as
> >> most clients of sirf dma driver still don't need interleaved mode,
> >> so here we still need to implement prep_slave_sg entry so that users
> >> like uart, spi can use these APIs instead of interleaved API.
> > Well in that case why dont you just create a wrapper on top of interleaved API
> > to make this work without driver changes
> 
> Vinod, do you mean using interleaved API to provide sg/single API if
> specific drivers implement interleaved_dma but not implement sg_dma?
> 
> the problem is that is difficult to set right legal sgl[i].size,
> sgl[i].icg and numf for all dmaengines as that depends on specific dma
> hardware limitation.
The whole premise of doing the generic interleaved api was to ensure we can use
any of the usuages as a case of interleaved api. Can you explain how it would be
difficult?

> >> +     spin_lock_irqsave(&schan->lock, iflags);
> >> +     list_for_each(l, &schan->free)
> >> +             desc_cnt++;
> > why dont you allocate descriptors here. That will remove this limitation..
> 
> i understand. here sirf user scenerios will never be over the
> limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
> too complex.
I think you can make code simler by dynamically allocating and freeing
descriptors...

~Vinod
-- 

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Barry Song]

2013/9/2 Vinod Koul <vinod.koul@intel.com>:
> On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
>> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
>> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>> >> the dma engine of sirfsoc supports interleaved mode, but if we set
>> >> xlen=width instead xlen<width, it will work as non-interleaved. as
>> >> most clients of sirf dma driver still don't need interleaved mode,
>> >> so here we still need to implement prep_slave_sg entry so that users
>> >> like uart, spi can use these APIs instead of interleaved API.
>> > Well in that case why dont you just create a wrapper on top of interleaved API
>> > to make this work without driver changes
>>
>> Vinod, do you mean using interleaved API to provide sg/single API if
>> specific drivers implement interleaved_dma but not implement sg_dma?
>>
>> the problem is that is difficult to set right legal sgl[i].size,
>> sgl[i].icg and numf for all dmaengines as that depends on specific dma
>> hardware limitation.
> The whole premise of doing the generic interleaved api was to ensure we can use
> any of the usuages as a case of interleaved api. Can you explain how it would be
> difficult?

for example, if someone wants to do a 16Kbytes single transfer. how
will interleaved dma set transfer length and interval between every
row?
if it sets the transfer length to 16KB directly, the dma hardware
might not support such long a transfer at all.
so it might want to set as two 8KB transfer without interval between them.
0~8KB                empty interval
8KB-16KB          empty interval
or four 4KB transfer as
0~4KB                empty interval
4KB-8KB          empty interval
8KB-12KB          empty interval
12KB-16KB          empty interval

the problem is we don't know the hardware limitation of every dma
controllers for transferring length of each row.

>
>> >> +     spin_lock_irqsave(&schan->lock, iflags);
>> >> +     list_for_each(l, &schan->free)
>> >> +             desc_cnt++;
>> > why dont you allocate descriptors here. That will remove this limitation..
>>
>> i understand. here sirf user scenerios will never be over the
>> limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
>> too complex.
> I think you can make code simler by dynamically allocating and freeing
> descriptors...

when do you think is the right time to free? while the whole sg
finished? i always think the code will be ugly.

>
> ~Vinod
> --

-barry

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Vinod Koul]

On Tue, Sep 03, 2013 at 06:06:44PM +0800, Barry Song wrote:
> 2013/9/2 Vinod Koul <vinod.koul@intel.com>:
> > On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
> >> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
> >> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
> >> >> the dma engine of sirfsoc supports interleaved mode, but if we set
> >> >> xlen=width instead xlen<width, it will work as non-interleaved. as
> >> >> most clients of sirf dma driver still don't need interleaved mode,
> >> >> so here we still need to implement prep_slave_sg entry so that users
> >> >> like uart, spi can use these APIs instead of interleaved API.
> >> > Well in that case why dont you just create a wrapper on top of interleaved API
> >> > to make this work without driver changes
> >>
> >> Vinod, do you mean using interleaved API to provide sg/single API if
> >> specific drivers implement interleaved_dma but not implement sg_dma?
> >>
> >> the problem is that is difficult to set right legal sgl[i].size,
> >> sgl[i].icg and numf for all dmaengines as that depends on specific dma
> >> hardware limitation.
> > The whole premise of doing the generic interleaved api was to ensure we can use
> > any of the usuages as a case of interleaved api. Can you explain how it would be
> > difficult?
> 
> for example, if someone wants to do a 16Kbytes single transfer. how
> will interleaved dma set transfer length and interval between every
> row?
> if it sets the transfer length to 16KB directly, the dma hardware
> might not support such long a transfer at all.
> so it might want to set as two 8KB transfer without interval between them.
> 0~8KB                empty interval
> 8KB-16KB          empty interval
> or four 4KB transfer as
> 0~4KB                empty interval
> 4KB-8KB          empty interval
> 8KB-12KB          empty interval
> 12KB-16KB          empty interval
> 
> the problem is we don't know the hardware limitation of every dma
> controllers for transferring length of each row.
Why not:

Okay there are two ways to this
1. Advertize your capablity and let client use that to break. You cna do so
using dma_set/get_max_seg_size().

2. in DMAC, nothing stops you from breaking the given sg_list into smaller
chunks. So if you get a buffer exceeding what you do, you cna internally split
to multiple descriptors and chain those to parent one.

> 
> >
> >> >> +     spin_lock_irqsave(&schan->lock, iflags);
> >> >> +     list_for_each(l, &schan->free)
> >> >> +             desc_cnt++;
> >> > why dont you allocate descriptors here. That will remove this limitation..
> >>
> >> i understand. here sirf user scenerios will never be over the
> >> limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
> >> too complex.
> > I think you can make code simler by dynamically allocating and freeing
> > descriptors...
> 
> when do you think is the right time to free? while the whole sg
> finished? i always think the code will be ugly.
when the descriptor has been completed. You allocate in prepare. Bunch of driver
already do so... I dont think its so complicated

~Vinod
-- 

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Barry Song]

2013/9/3 Vinod Koul <vinod.koul@intel.com>:
> On Tue, Sep 03, 2013 at 06:06:44PM +0800, Barry Song wrote:
>> 2013/9/2 Vinod Koul <vinod.koul@intel.com>:
>> > On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
>> >> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
>> >> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>> >> >> the dma engine of sirfsoc supports interleaved mode, but if we set
>> >> >> xlen=width instead xlen<width, it will work as non-interleaved. as
>> >> >> most clients of sirf dma driver still don't need interleaved mode,
>> >> >> so here we still need to implement prep_slave_sg entry so that users
>> >> >> like uart, spi can use these APIs instead of interleaved API.
>> >> > Well in that case why dont you just create a wrapper on top of interleaved API
>> >> > to make this work without driver changes
>> >>
>> >> Vinod, do you mean using interleaved API to provide sg/single API if
>> >> specific drivers implement interleaved_dma but not implement sg_dma?
>> >>
>> >> the problem is that is difficult to set right legal sgl[i].size,
>> >> sgl[i].icg and numf for all dmaengines as that depends on specific dma
>> >> hardware limitation.
>> > The whole premise of doing the generic interleaved api was to ensure we can use
>> > any of the usuages as a case of interleaved api. Can you explain how it would be
>> > difficult?
>>
>> for example, if someone wants to do a 16Kbytes single transfer. how
>> will interleaved dma set transfer length and interval between every
>> row?
>> if it sets the transfer length to 16KB directly, the dma hardware
>> might not support such long a transfer at all.
>> so it might want to set as two 8KB transfer without interval between them.
>> 0~8KB                empty interval
>> 8KB-16KB          empty interval
>> or four 4KB transfer as
>> 0~4KB                empty interval
>> 4KB-8KB          empty interval
>> 8KB-12KB          empty interval
>> 12KB-16KB          empty interval
>>
>> the problem is we don't know the hardware limitation of every dma
>> controllers for transferring length of each row.
> Why not:
>
> Okay there are two ways to this
> 1. Advertize your capablity and let client use that to break. You cna do so
> using dma_set/get_max_seg_size().
>

this way is ok. does max_seg also match with interleaved mode?


> 2. in DMAC, nothing stops you from breaking the given sg_list into smaller
> chunks. So if you get a buffer exceeding what you do, you cna internally split
> to multiple descriptors and chain those to parent one.

this way still need hacking in dmac driver.

>
>>
>> >
>> >> >> +     spin_lock_irqsave(&schan->lock, iflags);
>> >> >> +     list_for_each(l, &schan->free)
>> >> >> +             desc_cnt++;
>> >> > why dont you allocate descriptors here. That will remove this limitation..
>> >>
>> >> i understand. here sirf user scenerios will never be over the
>> >> limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
>> >> too complex.
>> > I think you can make code simler by dynamically allocating and freeing
>> > descriptors...
>>
>> when do you think is the right time to free? while the whole sg
>> finished? i always think the code will be ugly.
> when the descriptor has been completed. You allocate in prepare. Bunch of driver
> already do so... I dont think its so complicated

here sirf dmac alloc all 16 desc in alloc_resoures and free all in
free_resources. if we can move the way you said, it is fine.
but it seems it means another separate patch for that.
>
> ~Vinod
> --

-barry

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Vinod Koul]

On Tue, Sep 03, 2013 at 08:42:36PM +0800, Barry Song wrote:
> > Why not:
> >
> > Okay there are two ways to this
> > 1. Advertize your capablity and let client use that to break. You cna do so
> > using dma_set/get_max_seg_size().
> >
> 
> this way is ok. does max_seg also match with interleaved mode?
i dont see a reason not to..
> 
> 
> > 2. in DMAC, nothing stops you from breaking the given sg_list into smaller
> > chunks. So if you get a buffer exceeding what you do, you cna internally split
> > to multiple descriptors and chain those to parent one.
> 
> this way still need hacking in dmac driver.
Yup but you get a driver which cna handle any buffers any lists, which is very
nice from users POV
> 
> >
> >>
> >> >
> >> >> >> +     spin_lock_irqsave(&schan->lock, iflags);
> >> >> >> +     list_for_each(l, &schan->free)
> >> >> >> +             desc_cnt++;
> >> >> > why dont you allocate descriptors here. That will remove this limitation..
> >> >>
> >> >> i understand. here sirf user scenerios will never be over the
> >> >> limitation of SIRFSOC_DMA_DESCRIPTORS = 16. so i don't want to make it
> >> >> too complex.
> >> > I think you can make code simler by dynamically allocating and freeing
> >> > descriptors...
> >>
> >> when do you think is the right time to free? while the whole sg
> >> finished? i always think the code will be ugly.
> > when the descriptor has been completed. You allocate in prepare. Bunch of driver
> > already do so... I dont think its so complicated
> 
> here sirf dmac alloc all 16 desc in alloc_resoures and free all in
> free_resources. if we can move the way you said, it is fine.
> but it seems it means another separate patch for that.
Sure...

~Vinod
-- 

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Jassi Brar]

On 3 September 2013 15:36, Barry Song <21cnbao@gmail.com> wrote:
> 2013/9/2 Vinod Koul <vinod.koul@intel.com>:
>> On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
>>> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
>>> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>>> >> the dma engine of sirfsoc supports interleaved mode, but if we set
>>> >> xlen=width instead xlen<width, it will work as non-interleaved. as
>>> >> most clients of sirf dma driver still don't need interleaved mode,
>>> >> so here we still need to implement prep_slave_sg entry so that users
>>> >> like uart, spi can use these APIs instead of interleaved API.
>>> > Well in that case why dont you just create a wrapper on top of interleaved API
>>> > to make this work without driver changes
>>>
>>> Vinod, do you mean using interleaved API to provide sg/single API if
>>> specific drivers implement interleaved_dma but not implement sg_dma?
>>>
>>> the problem is that is difficult to set right legal sgl[i].size,
>>> sgl[i].icg and numf for all dmaengines as that depends on specific dma
>>> hardware limitation.
>> The whole premise of doing the generic interleaved api was to ensure we can use
>> any of the usuages as a case of interleaved api. Can you explain how it would be
>> difficult?
>
> for example, if someone wants to do a 16Kbytes single transfer. how
> will interleaved dma set transfer length and interval between every
> row?
> if it sets the transfer length to 16KB directly, the dma hardware
> might not support such long a transfer at all.
> so it might want to set as two 8KB transfer without interval between them.
> 0~8KB                empty interval
> 8KB-16KB          empty interval
> or four 4KB transfer as
> 0~4KB                empty interval
> 4KB-8KB          empty interval
> 8KB-12KB          empty interval
> 12KB-16KB          empty interval
>
> the problem is we don't know the hardware limitation of every dma
> controllers for transferring length of each row.
>
A client is only interested in transferring the total amount data and
if/when it wants notified. How the dma controller divides the big
transfer, shouldn't matter to the client. In fact it would be bad for
a client to care about the working of a dma controller.
IOW, the 16KB transfer could be divided into 4 parts by the dmac that
supports max 4KB transfers, into 8 parts if max is 2KB and so on...
the client shouldn't care.

-jassi

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Vinod Koul]

On Tue, Sep 03, 2013 at 05:25:40PM +0530, Jassi Brar wrote:
> > for example, if someone wants to do a 16Kbytes single transfer. how
> > will interleaved dma set transfer length and interval between every
> > row?
> > if it sets the transfer length to 16KB directly, the dma hardware
> > might not support such long a transfer at all.
> > so it might want to set as two 8KB transfer without interval between them.
> > 0~8KB                empty interval
> > 8KB-16KB          empty interval
> > or four 4KB transfer as
> > 0~4KB                empty interval
> > 4KB-8KB          empty interval
> > 8KB-12KB          empty interval
> > 12KB-16KB          empty interval
> >
> > the problem is we don't know the hardware limitation of every dma
> > controllers for transferring length of each row.
> >
> A client is only interested in transferring the total amount data and
> if/when it wants notified. How the dma controller divides the big
> transfer, shouldn't matter to the client. In fact it would be bad for
> a client to care about the working of a dma controller.
> IOW, the 16KB transfer could be divided into 4 parts by the dmac that
> supports max 4KB transfers, into 8 parts if max is 2KB and so on...
> the client shouldn't care.
That is also a correct approach. I agree that dma driver should be able to
handle any lengths and split that to multiple descriptors. 

But I think in interleaved API, clients should know the capablity otherwise dma
driver will have redo the whole list again...

~Vinod
-- 

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Jassi Brar]

On 3 September 2013 17:38, Vinod Koul <vinod.koul@intel.com> wrote:
> On Tue, Sep 03, 2013 at 05:25:40PM +0530, Jassi Brar wrote:
>> > for example, if someone wants to do a 16Kbytes single transfer. how
>> > will interleaved dma set transfer length and interval between every
>> > row?
>> > if it sets the transfer length to 16KB directly, the dma hardware
>> > might not support such long a transfer at all.
>> > so it might want to set as two 8KB transfer without interval between them.
>> > 0~8KB                empty interval
>> > 8KB-16KB          empty interval
>> > or four 4KB transfer as
>> > 0~4KB                empty interval
>> > 4KB-8KB          empty interval
>> > 8KB-12KB          empty interval
>> > 12KB-16KB          empty interval
>> >
>> > the problem is we don't know the hardware limitation of every dma
>> > controllers for transferring length of each row.
>> >
>> A client is only interested in transferring the total amount data and
>> if/when it wants notified. How the dma controller divides the big
>> transfer, shouldn't matter to the client. In fact it would be bad for
>> a client to care about the working of a dma controller.
>> IOW, the 16KB transfer could be divided into 4 parts by the dmac that
>> supports max 4KB transfers, into 8 parts if max is 2KB and so on...
>> the client shouldn't care.
> That is also a correct approach. I agree that dma driver should be able to
> handle any lengths and split that to multiple descriptors.
>
> But I think in interleaved API, clients should know the capablity otherwise dma
> driver will have redo the whole list again...
>
I am not sure if the clients should do the mapping from arbitrary data
pattern onto how the dmac is to be programmed. The client is no more
generic (it just passes dmac specific data in a generic way), and the
approach also entails yet another api for dmacs to express their
capability.

As we know, the interleaved api was meant for clients to express
arbitrary data arrangement in memory. It's not for clients passing
transfer info in dmac specific format.

-jassi

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Barry Song]

2013/9/3 Jassi Brar <jaswinder.singh@linaro.org>:
> On 3 September 2013 15:36, Barry Song <21cnbao@gmail.com> wrote:
>> 2013/9/2 Vinod Koul <vinod.koul@intel.com>:
>>> On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
>>>> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
>>>> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>>>> >> the dma engine of sirfsoc supports interleaved mode, but if we set
>>>> >> xlen=width instead xlen<width, it will work as non-interleaved. as
>>>> >> most clients of sirf dma driver still don't need interleaved mode,
>>>> >> so here we still need to implement prep_slave_sg entry so that users
>>>> >> like uart, spi can use these APIs instead of interleaved API.
>>>> > Well in that case why dont you just create a wrapper on top of interleaved API
>>>> > to make this work without driver changes
>>>>
>>>> Vinod, do you mean using interleaved API to provide sg/single API if
>>>> specific drivers implement interleaved_dma but not implement sg_dma?
>>>>
>>>> the problem is that is difficult to set right legal sgl[i].size,
>>>> sgl[i].icg and numf for all dmaengines as that depends on specific dma
>>>> hardware limitation.
>>> The whole premise of doing the generic interleaved api was to ensure we can use
>>> any of the usuages as a case of interleaved api. Can you explain how it would be
>>> difficult?
>>
>> for example, if someone wants to do a 16Kbytes single transfer. how
>> will interleaved dma set transfer length and interval between every
>> row?
>> if it sets the transfer length to 16KB directly, the dma hardware
>> might not support such long a transfer at all.
>> so it might want to set as two 8KB transfer without interval between them.
>> 0~8KB                empty interval
>> 8KB-16KB          empty interval
>> or four 4KB transfer as
>> 0~4KB                empty interval
>> 4KB-8KB          empty interval
>> 8KB-12KB          empty interval
>> 12KB-16KB          empty interval
>>
>> the problem is we don't know the hardware limitation of every dma
>> controllers for transferring length of each row.
>>
> A client is only interested in transferring the total amount data and
> if/when it wants notified. How the dma controller divides the big
> transfer, shouldn't matter to the client. In fact it would be bad for
> a client to care about the working of a dma controller.
> IOW, the 16KB transfer could be divided into 4 parts by the dmac that
> supports max 4KB transfers, into 8 parts if max is 2KB and so on...
> the client shouldn't care.

here my point is that we don't expose dma transferring hardware
details to clients. i actually wanted to hide that in dma driver not
dma client.
the only difference between vinod and me is that Vinod's point is
hiding that in dma core, my way is hiding it in dma driver.

btw, interleaved_dma clients do care about how the interleaved
transfer will happen. but single_dma client doesn't care.

>
> -jassi

-barry

[PATCH] dmaengine: sirf: add dmaengine_prep_slave_single/sg support

[Jassi Brar]

On 3 September 2013 18:08, Barry Song <21cnbao@gmail.com> wrote:
> 2013/9/3 Jassi Brar <jaswinder.singh@linaro.org>:
>> On 3 September 2013 15:36, Barry Song <21cnbao@gmail.com> wrote:
>>> 2013/9/2 Vinod Koul <vinod.koul@intel.com>:
>>>> On Sun, Sep 01, 2013 at 09:02:01PM +0800, Barry Song wrote:
>>>>> 2013/8/26 Vinod Koul <vinod.koul@intel.com>:
>>>>> > On Sun, Aug 25, 2013 at 08:57:13PM +0800, Barry Song wrote:
>>>>> >> the dma engine of sirfsoc supports interleaved mode, but if we set
>>>>> >> xlen=width instead xlen<width, it will work as non-interleaved. as
>>>>> >> most clients of sirf dma driver still don't need interleaved mode,
>>>>> >> so here we still need to implement prep_slave_sg entry so that users
>>>>> >> like uart, spi can use these APIs instead of interleaved API.
>>>>> > Well in that case why dont you just create a wrapper on top of interleaved API
>>>>> > to make this work without driver changes
>>>>>
>>>>> Vinod, do you mean using interleaved API to provide sg/single API if
>>>>> specific drivers implement interleaved_dma but not implement sg_dma?
>>>>>
>>>>> the problem is that is difficult to set right legal sgl[i].size,
>>>>> sgl[i].icg and numf for all dmaengines as that depends on specific dma
>>>>> hardware limitation.
>>>> The whole premise of doing the generic interleaved api was to ensure we can use
>>>> any of the usuages as a case of interleaved api. Can you explain how it would be
>>>> difficult?
>>>
>>> for example, if someone wants to do a 16Kbytes single transfer. how
>>> will interleaved dma set transfer length and interval between every
>>> row?
>>> if it sets the transfer length to 16KB directly, the dma hardware
>>> might not support such long a transfer at all.
>>> so it might want to set as two 8KB transfer without interval between them.
>>> 0~8KB                empty interval
>>> 8KB-16KB          empty interval
>>> or four 4KB transfer as
>>> 0~4KB                empty interval
>>> 4KB-8KB          empty interval
>>> 8KB-12KB          empty interval
>>> 12KB-16KB          empty interval
>>>
>>> the problem is we don't know the hardware limitation of every dma
>>> controllers for transferring length of each row.
>>>
>> A client is only interested in transferring the total amount data and
>> if/when it wants notified. How the dma controller divides the big
>> transfer, shouldn't matter to the client. In fact it would be bad for
>> a client to care about the working of a dma controller.
>> IOW, the 16KB transfer could be divided into 4 parts by the dmac that
>> supports max 4KB transfers, into 8 parts if max is 2KB and so on...
>> the client shouldn't care.
>
> here my point is that we don't expose dma transferring hardware
> details to clients. i actually wanted to hide that in dma driver not
> dma client.
> the only difference between vinod and me is that Vinod's point is
> hiding that in dma core, my way is hiding it in dma driver.
>
How big is the effort to convert the clients to use interleaved api?

> btw, interleaved_dma clients do care about how the interleaved
> transfer will happen
>
No, there is difference. Interleaved api only tells how the data, to
be transferred, is laid out in memory. It doesn't not specify how the
dma controller actually does the transfer.

-jassi