As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
Signed-off-by: Alexandre Bounine <redacted>
Cc: Jassi Brar <redacted>
Cc: Russell King <redacted>
Cc: Kumar Gala <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <redacted>
---
include/linux/dmaengine.h | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
quoted hunk
Signed-off-by: Alexandre Bounine <redacted>
Cc: Jassi Brar <redacted>
Cc: Russell King <redacted>
Cc: Kumar Gala <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <redacted>
---
include/linux/dmaengine.h | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
On Mon, 2012-01-30 at 08:55 -0800, Bounine, Alexandre wrote:
On Monday, January 30, 2012 at 4:31 AM, Vinod Koul wrote:
quoted
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
quoted
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
I agree that cyclic and interleaved calls may benefit from adding that parameter as well.
Benefits to the cyclic call are straightforward - same as dma_slave.
Adding a context parameter to the interleaved transfers may be more future proofing option
than an immediate need. Memcopy and other calls that deal with local memory transfers
probably should be left untouched.
What if we limit modifications to:
1) three calls (slave, cyclic and interleaved) OR
2) two (slave and cyclic) at this moment?
I am just more focused on dma_slave just because it fits well to provide RDMA
over RapidIO fabric.
If everybody agrees, I can go ahead and make changes to all three at once.
For now we need at least slave and cyclic, so pls go ahead and make these changes.
For interleaved, we might need it sooner [1], but I would think it would
need few more changes to the API, so it can be rolled as part of those
changes.
--
~Vinod
[1]: https://lkml.org/lkml/2012/1/30/48
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
quoted
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
Yes, we (shdma.c) also need to pass additional slave configuration
information to the dmaengine driver and I also was thinking about
extending the existing API, but my ideas were going more in the direction
of adding a parameter to __dma_request_channel() along the lines of
-struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
+struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param,
+ struct dma_slave_desc *slave);
where struct dma_slave_desc would basically just contain an opaque slave
ID and would be embedded by dmaengine drivers in their specific
slave-configuration types. The main difference to the API change being
proposed here is, that I was going to configure DMA channels only once for
each slave upon channel allocation, whereas the proposed change does this
on a per-transfer basis. Therefore my question: do I understand the
explanation of the RapidIO DMA architecture from the above quoted thread
right, that DMA channels can be freely used by client drivers, without
binding them to specific DSP cards? In which case you, probably, don't use
DMA_PRIVATE? But that the decision - to which DSP card this specific
transfer is directed - is made by configuring the DMA channels rather than
embedded in the actual data?
Thanks
Guennadi
quoted
Signed-off-by: Alexandre Bounine <redacted>
Cc: Jassi Brar <redacted>
Cc: Russell King <redacted>
Cc: Kumar Gala <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <redacted>
---
include/linux/dmaengine.h | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
On Wed, 2012-02-01 at 01:09 +0100, Guennadi Liakhovetski wrote:
On Mon, 30 Jan 2012, Vinod Koul wrote:
quoted
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
quoted
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
Yes, we (shdma.c) also need to pass additional slave configuration
information to the dmaengine driver and I also was thinking about
extending the existing API, but my ideas were going more in the direction
of adding a parameter to __dma_request_channel() along the lines of
So your question is more on the lines of channel mapping/allocation?
The approach here is to pass controller specific parameters which are
required to setup the respective transfer. Since this is dependent on
each transfer, this needs to be passed in respective prepare.
The two things are completely orthogonal and shouldn't be clubbed.
For your issue we need a separate debate on how to solve this... I am
open to ideas...
--
~Vinod
On Wed, 2012-02-01 at 01:09 +0100, Guennadi Liakhovetski wrote:
quoted
On Mon, 30 Jan 2012, Vinod Koul wrote:
quoted
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
quoted
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
Yes, we (shdma.c) also need to pass additional slave configuration
information to the dmaengine driver and I also was thinking about
extending the existing API, but my ideas were going more in the direction
of adding a parameter to __dma_request_channel() along the lines of
So your question is more on the lines of channel mapping/allocation?
The approach here is to pass controller specific parameters which are
required to setup the respective transfer. Since this is dependent on
each transfer, this needs to be passed in respective prepare.
The two things are completely orthogonal and shouldn't be clubbed.
For your issue we need a separate debate on how to solve this... I am
open to ideas...
Well, I'm not sure whether they are necessarily always orthogonal, they
don't seem so in my case at least. We definitely can use our approach -
configure the channel during allocation. I _think_ we could also perform
the configuration on a per-transfer basis, during the prepare stage, as
this RFC is suggesting, but that definitely would require reworking the
driver somewhat and changing the concept. The current concept is a fixed
DMA channel allocation to slaves for as long as the slave is using DMA.
This is simpler, avoids some overhead during operation and fits well with
the dmaengine PRIVATE channel concept. So, given the choice, we would
prefer to perform the configuration during channel allocation.
Maybe there are cases, where the driver absolutely needs this additional
information during allocation, in which case my proposal would be the only
way to go for them.
I'll post an RFC soon - stay tuned:-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
On Wed, 2012-02-01 at 12:58 +0100, Guennadi Liakhovetski wrote:
quoted
The two things are completely orthogonal and shouldn't be clubbed.
For your issue we need a separate debate on how to solve this... I am
open to ideas...
Well, I'm not sure whether they are necessarily always orthogonal, they
don't seem so in my case at least. We definitely can use our approach -
configure the channel during allocation. I _think_ we could also perform
the configuration on a per-transfer basis, during the prepare stage, as
this RFC is suggesting, but that definitely would require reworking the
driver somewhat and changing the concept. The current concept is a fixed
DMA channel allocation to slaves for as long as the slave is using DMA.
This is simpler, avoids some overhead during operation and fits well with
the dmaengine PRIVATE channel concept. So, given the choice, we would
prefer to perform the configuration during channel allocation.
Maybe there are cases, where the driver absolutely needs this additional
information during allocation, in which case my proposal would be the only
way to go for them.
what are you trying to address, sending controller specific information
at allocation or the channel allocation itself. I kind of sense both.
But apprach here is discussed is to pass paramters which are required
for each transfer, not static for a channel, hence the additional
controller specific parameter in respective prepare.