RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
-Original Message-
From: Jassi Brar [mailto:jassisinghb...@gmail.com]
Sent: Tuesday, July 12, 2011 6:15 PM
To: Raju, Sundaram
Cc: Linus Walleij; linux-arm-ker...@lists.infradead.org; linux-
ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
li...@arm.linux.org.uk; dan.j.willi...@intel.com; linux-omap@vger.kernel.org
Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
configuration
On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram sunda...@ti.com wrote:
-Original Message-
From: Jassi Brar [mailto:jassisinghb...@gmail.com]
Sent: Tuesday, July 12, 2011 4:51 PM
To: Linus Walleij
Cc: Raju, Sundaram; linux-arm-ker...@lists.infradead.org; linux-
ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
li...@arm.linux.org.uk; dan.j.willi...@intel.com; linux-
o...@vger.kernel.org
Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
configuration
On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij linus.wall...@linaro.org
wrote:
On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar jassisinghb...@gmail.com
wrote:
1) Striding, in one form or other, is supported by other DMACs as well.
The number will only increase in future.
Are we to add VENDOR_DMA_STRIDE_CONFIG for each case ?
If we are sure about this and striding will work in a similar way on all
then let's have the enum named DMA_STRIDE_CONFIG and move the
passed-in struct to linux/dmaengine.h) then?
Would that be:
struct dma_stride_config {
u32 read_bytes;
u32 skip_bytes;
};
Or something more complex?
Well, I am not sure if striding needs any special treatment at all.
Why not have client drivers prepare and submit sg-list.
Let the DMAC drivers interpret/parse the sg-list and program it
as strides if the h/w supports it.
If anything, we should make preparation and submission of sg-list
as efficient as possible.
Jassi,
sg_lists describe only a bunch of disjoint buffers. But what if the
DMAC can skip and read the bytes within each of the buffers in
the sg_list? (like TI EDMAC and TI SDMAC)
How can that information be passed to the offload
engine driver from the client?
OK, I overlooked.
We do need something new to handle these ultra-fine-grained sg-lists.
But still we shouldn't add SoC specific API to the common sub-systems.
Maybe a new api to pass fixed-format variable-length encoded message
to the DMAC drivers?
Which could be interpreted by DMAC drivers to extract all the needed xfer
parameters from the 'header' section and instructions to program the xfers
in the DMAC from the variable length body of the 'message' buffer.
It might sound complicated but we can have helpers to make the job easy.
Btw, the regular single/sg-list xfers could also be expressed by this method.
Do you expect this variable length body of the message to be DMAC
independent? I don't think so. In that case how is this different from what
we have here already?
If it can be DMAC independent, can you illustrate more on how this can
be done? But the point to note is, if this can be made DMAC independent
then the control command we have also can be made DMAC independent
by generalizing the configuration structure passed to it.
~ Sundaram
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: Tuesday, July 12, 2011 3:28 PM
To: Dan Williams
Cc: Raju, Sundaram; linux-arm-ker...@lists.infradead.org; linux-
ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
li...@arm.linux.org.uk; linux-omap@vger.kernel.org
Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
configuration
On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams dan.j.willi...@intel.com
wrote:
On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij linus.wall...@linaro.org
wrote:
...and I suspect the slave device drivers that use TI DMA are not
expected to ever work with other dmaengines? Likely the case, but
just wondering out loud.
Typically the OMAP/TI drivers are one-to-one with this specific DMA
controller, but they *can* support controllers without stride options, and
notice that striding will only be used for the display driver IIRC,
pseudo-code:
ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
(unsigned long) my_stride_config);
if (ret) {
/*
* OK no striding on this DMA engine, fall back to something else,
* such as creating an SGlist which emulates the striding with one
* sglist element per stride.
*/
}
By injecting an error in the stride config path this can even be
properly tested. So it will become an optional acceleration.
Yes, this is exactly what I also wanted to say. :)
But if the client driver does not implement a fallback like this then that
driver will work only with TI DMA and not any other dmaengines.
(Mentioning this because, there are client drivers which are tightly
coupled to this special configuration)
Keeping this in mind, I had started the original discussion with
the suggestion of modifying the existing prepare API and adding
an extra argument to it, which can be used to pass special configuration.
And I also wanted to generalize that configuration passed.
Anyways that design also will come down to this same path, and instead
of modifying the existing API signatures, I think this is the best way
we can go.
Regards,
Sundaram
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: Tuesday, July 12, 2011 3:33 PM
To: Jassi Brar
Cc: Raju, Sundaram; linux-arm-ker...@lists.infradead.org; linux-
ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
li...@arm.linux.org.uk; dan.j.willi...@intel.com; linux-omap@vger.kernel.org
Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
configuration
On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar jassisinghb...@gmail.com
wrote:
1) Striding, in one form or other, is supported by other DMACs as well.
The number will only increase in future.
Are we to add VENDOR_DMA_STRIDE_CONFIG for each case ?
If we are sure about this and striding will work in a similar way on all
then let's have the enum named DMA_STRIDE_CONFIG and move the
passed-in struct to linux/dmaengine.h) then?
Would that be:
struct dma_stride_config {
u32 read_bytes;
u32 skip_bytes;
};
Or something more complex?
When I started this discussion on stride config, I received comments like
this is too specific to TI DMAC, and there are not many DMACs which can
do this. I actually wanted to generalize the configuration passed and put
a comment on it similar to the one on top of dma_slave_config, which says
|
|/**
snip
| * The rationale for adding configuration information to this struct
| * is as follows: if it is likely that most DMA slave controllers in
| * the world will support the configuration option, then make it
| * generic. If not: if it is fixed so that it be sent in static from
| * the platform data, then prefer to do that. Else, if it is neither
| * fixed at runtime, nor generic enough (such as bus mastership on
| * some CPU family and whatnot) then create a custom slave config
| * struct and pass that, then make this config a member of that
| * struct, if applicable.
| */
|
If any other DMAC can do similar stride configuration,
then we can generalize it.
Till we generalize this stride configuration I think a custom
configuration aligned between the client driver and
the offload engine driver can be used.
2) As Dan noted, client drivers are going to have ifdef hackery in
order to be common
to other SoCs.
Don't think so, why? This is a runtime config entirely, and I just illustrated
in mail to Dan how that can be handled by falling back to a sglist I believe?
We can *maybe* even put the fallback code into dmaengine, so that an
emulated sglist in place for the DMAengine is done automatically of the
DMA controller does not support striding.
Good Idea.
But the client might always have a better way to handle this fallback than
this suggested fallback code in dmaengine, which will be a common
implementation based on the received sg_list and the DMAC capabilities.
If this is done then preference should be provided to the client's fallback
implementation, if present.
3) TI may not have just one DMAC IP used in all the SoCs. So if you want
vendor specific defines anyway, please atleast also add DMAC version to it.
Something like
DMA_SLAVE_CONFIG,
FSLDMA_EXTERNAL_START,
+ TI_DMA_v1_STRIDE_CONFIG,
Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
a lot of sense.
Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and
one for SDMAC in OMAP series of SoCs.
Regards,
Sundaram
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
-Original Message-
From: Jassi Brar [mailto:jassisinghb...@gmail.com]
Sent: Tuesday, July 12, 2011 4:51 PM
To: Linus Walleij
Cc: Raju, Sundaram; linux-arm-ker...@lists.infradead.org; linux-
ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
li...@arm.linux.org.uk; dan.j.willi...@intel.com; linux-omap@vger.kernel.org
Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
configuration
On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij linus.wall...@linaro.org
wrote:
On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar jassisinghb...@gmail.com
wrote:
1) Striding, in one form or other, is supported by other DMACs as well.
The number will only increase in future.
Are we to add VENDOR_DMA_STRIDE_CONFIG for each case ?
If we are sure about this and striding will work in a similar way on all
then let's have the enum named DMA_STRIDE_CONFIG and move the
passed-in struct to linux/dmaengine.h) then?
Would that be:
struct dma_stride_config {
u32 read_bytes;
u32 skip_bytes;
};
Or something more complex?
Well, I am not sure if striding needs any special treatment at all.
Why not have client drivers prepare and submit sg-list.
Let the DMAC drivers interpret/parse the sg-list and program it
as strides if the h/w supports it.
If anything, we should make preparation and submission of sg-list
as efficient as possible.
Jassi,
sg_lists describe only a bunch of disjoint buffers. But what if the
DMAC can skip and read the bytes within each of the buffers in
the sg_list? (like TI EDMAC and TI SDMAC)
How can that information be passed to the offload
engine driver from the client?
~Sundaram
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
[RFC] dmaengine: Moving TI SDMA driver to dmaengine - design plan
Hi, I am planning to move TI SDMA driver in OMAP tree into the dmaengine framework. The first immediate issue of concern I noticed is the huge number of client drivers that use the existing SDMA driver. More than 15 client drivers are using the current SDMA driver. Moving the SDMA driver along with all of these client drivers at a single stretch seems a humungous task. I noticed a model in the existing DMA drivers in dmaengine framework that will over come this issue. I would like to follow the Freescale i.MX DMA driver model, where imx-dma.c in drivers/dma which implements all the dmaengine hooks, internally uses the APIs in dma-v1.c file in arch/arm/mach-imx. All APIs in dma-v1.c are also exported out for direct use by clients. Advantages of this model: 1. No compilation breaks for any of the existing client drivers. 2. Client drivers can be moved one by one to the new SDMA driver after complete testing on all OMAP boards. We can mark the old SDMA driver APIs as deprecated and insist on all client drivers to be moved to the new SDMA driver in the dmaengine framework soon. I have made the preliminary changes in code for this design and I am in the phase of testing a few client drivers. If this is a good way to proceed then I will send out my patches with the client drivers I have changed for review once I am done with the testing. Let me know your thoughts. Regards, Sundaram -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] dmaengine: Moving TI SDMA driver to dmaengine - design plan
-Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Friday, July 08, 2011 3:34 PM To: Raju, Sundaram Cc: linux-arm-ker...@lists.infradead.org; linux-omap@vger.kernel.org; Dan; Shilimkar, Santosh; linux-ker...@vger.kernel.org Subject: Re: [RFC] dmaengine: Moving TI SDMA driver to dmaengine - design plan On Fri, Jul 08, 2011 at 01:52:17PM +0530, Raju, Sundaram wrote: I am planning to move TI SDMA driver in OMAP tree into the dmaengine framework. The first immediate issue of concern I noticed is the huge number of client drivers that use the existing SDMA driver. More than 15 client drivers are using the current SDMA driver. Moving the SDMA driver along with all of these client drivers at a single stretch seems a humungous task. I noticed a model in the existing DMA drivers in dmaengine framework that will over come this issue. It _is_ sane to build a dmaengine driver on top of the existing SoC private API, then convert the drivers to DMA engine, and then cleanup the resulting DMA engine driver. Yes, that is what I thought. Thanks. What we must make sure though is that the DMA engine slave API (which isn't well documented) is correctly implemented before drivers are converted over to use the DMA engine support code, otherwise we may end up with lots of drivers that require re-fixing several times over. Very true! Agreed! I will send over the patches for review once I am done with testing. Regards, Sundaram -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
-Original Message-
From: Koul, Vinod [mailto:vinod.k...@intel.com]
Sent: Thursday, June 16, 2011 11:15 AM
To: Raju, Sundaram
Cc: Russell King - ARM Linux; Linus Walleij; Dan; davinci-linux-open-
sou...@linux.davincidsp.com; linux-omap@vger.kernel.org; linux-
ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Subject: RE: [RFC] dmaengine: add new api for preparing simple slave transfer
On Tue, 2011-06-14 at 12:12 +0530, Raju, Sundaram wrote:
snip
Okay now things are more clear on what you are trying to do...
Vinod,
Sorry for the delayed response. I was OOO and not well.
Thanks for going through the design documents and giving options.
2) I think this can be achieved in two ways:
a) you use current standard sg_list mechanism, the dmac driver parses
the list and programs the dma offsets into dmac
Pros: you can use existing APIs, no changes to i/f.
If dmac has this capability they program dmac accordingly
Cons: you need to create sg-list in client driver
This option does not satisfy the requirements.
As Russell has pointed out
The overall conclusion which I'm coming to is that we already support
what you're asking for, but the problem is that we're using different
(and I'd argue standard) terminology to describe what we have.
The only issue which I see that we don't cover is the case where you want
to describe a single buffer which is organised as N bytes to be
transferred,
M following bytes to be skipped, N bytes to be transferred, M bytes to be
skipped. I doubt there are many controllers which can be programmed with
both 'N' and 'M' parameters directly.
Yes this is what I wanted to communicate and discuss in the list.
Thanks for describing it in the standard terminology, and pointing me in the
right direction.
The sg_list caters to all the parameters that a client driver has to pass
to the DMAC driver except for the STRIDE related info of skipping certain
bytes within a single buffer entry of the sg_list.
b) create a new api to describe these offset values, something like:
prep_buffer_offset(struct offset_description *buffer,.)
I would not like to change the current API for this and rather have a
new API for this, this should better then overriding current.
Yes, it would be better not to change the existing API.
This option seems good. But new API has to be added for this option.
Linus, had suggested something similar, but different,
Using the control API with a new dma_ctrl_cmd enum
defined for TI DMAC special configuration to be passed.
| Sundaram is this how your controller works?
| I mean the hardware can skip over sequences like this?
|
| When we added the config interface to DMAengine I originally included
| a custom config call, but Dan wanted me to keep it out until we
| had some specific usecase for it. FSLDMA recently started
| to use it.
|
| Notice how dmaengine_slave_config() is implemented:
|
| static inline int dmaengine_slave_config(struct dma_chan *chan,
| struct dma_slave_config *config)
| {
| return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
| (unsigned long)config);
| }
|
| So what is passed to the driver is just an unsigned long.
|
| This is actually modeled to be ioctl()-like so you can pass in a
| custom config to the same callback on the device driver,
| just use some other enumerator than DMA_SLAVE_CONFIG,
| say like FSLDMA already does with FSLDMA_EXTERNAL_START.
|
| Just put some enumerator in enum dma_ctrl_cmd in
| dmaengine.h such as SDMA_TEXAS_STRIDE_CONFIG and call
| like this:
|
| /* However that config struct needs to look, basically */
| static struct sdma_ti_stride_cgf = {
| take = M,
| skip = N,
| };
|
| ret = chan-device-device_control(chan, SDMA_TEXAS_STRIDE_CONFIG,
| sdma_ti_stride_cfg);
|
| Or something like this.
I think this is better option than your 2b. This requires just an addition of
one more enum in the dma_ctrl_cmd. What do you think about this?
If Dan and you are okay with this I will send a small patch for this asap.
Regards,
Sundaram
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
Russell, Thanks for all the quick pointers and the summary of how memory-to-peripheral transfers are expected to operate. Ok, what I'm envisioning is that your term chunk size means register width, and you view that as one dimension. We already describe this. A frame is a collection of chunks. That's already described - the number of chunks in a buffer is the buffer size divided by the chunk size. Buffers must be a multiple of the chunk size. Then we have a collection of frames. These can be non-contiguous. That's effectively described by our scatterlist. snip The overall conclusion which I'm coming to is that we already support what you're asking for, but the problem is that we're using different (and I'd argue standard) terminology to describe what we have. The only issue which I see that we don't cover is the case where you want to describe a single buffer which is organised as N bytes to be transferred, M following bytes to be skipped, N bytes to be transferred, M bytes to be skipped. I doubt there are many controllers which can be programmed with both 'N' and 'M' parameters directly. Yes this is what I wanted to communicate and discuss in the list. Thanks for describing it in the standard terminology, and pointing me in the right direction. Also please note that if the gap between the buffers in a scatterlist is uniform and that can again be skipped by the DMAC automatically by programming that gap size, then that feature also is not available in the current framework. I understand it can be done with the existing scatterlist, but just writing a value in a DMAC register is much better if available, than preparing a scatterlist and passing to the API. Please, don't generate special cases. Design proper APIs from the outset rather than abusing what's already there. So no, don't abuse the address width stuff. In any case, the address width stuff must still be used to describe the peripherals FIFO register. I did not intend to abuse the existing address width. It might look like that because of how I described it here. I agree that the dma_slave_config is for peripheral related properties to be stored. I was pointing out that the chunk size variable in the dma_buffer_config I proposed will be in most cases equal to FIFO register width, to describe what I actually meant by chunk size. IIUC, you have a buffer with gaps in between (given by above params). Is your DMA h/w capable of parsing this buffer and directly do a transfer based on above parameters (without any work for SW), or you are doing this in your dma driver and then programming a list of buffers? In former case (although i haven't seen such dma controller yet), can you share the datasheet? It would make very little sense to change the current API for your extra special case. This special case needs to be handled differently rather than making rule out of it!! Yes, Vinod. This is directly handled in the DMAC h/w. This capability is present in 2 TI DMACs. EDMA (Enhanced DMA) in all TI DaVinci SoCs and the SDMA (System DMA) in all TI OMAP SoCs. The drivers of these controllers are present in the respective DaVinci tree and OMAP tree under the SoC folders. quote SDMA and EDMA are TI SoC specific DMA controllers. Their drivers have been maintained in the respective SoC folders till now. arch/arm/plat-omap/dma.c arch/arm/mach-davinci/dma.c /quote The Manual of the EDMA controller in TI DaVinci SoCs is available at http://www.ti.com/litv/pdf/sprue23d Section 2.2 in the page 23 explains how transfers are made based on the gaps programmed. It also explains how the 3D buffer is internally split in EDMA based on the gaps programmed. The complete Reference manual of TI OMAP SoCs is available at http://www.ti.com/litv/pdf/spruf98s Chapter 9 in this document describes the SDMA controller. Section 9.4.3 in page 981 explains the various address modes, how the address is incremented by the DMAC and about the gaps in between buffers and frames and how the DMAC handles them. Linus, Is it really so bad? It is a custom configuration after all. Even if there were many DMACs out there supporting it, we'd probably model it like this, just pass something like DMA_STRIDE_CONFIG instead of something named after a specific slave controller. This way DMACs that didn't support striding could NACK a transfer for device drivers requesting it and then it could figure out what to do. If we can get some indication as to whether more DMACs can do this kind of stuff, we'd maybe like to introduce DMA_STRIDE_CONFIG already now. I wanted to discuss this feature in the list and get this feature added to the current dmaengine framework. If the current APIs can handle this feature, then its very good and I will follow that only. If what you suggest is the right way to go then I am okay. IMHO the framework should always handle the complex case and individual implementations should
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
Linus,
Thanks for the pointers.
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: Monday, June 13, 2011 7:43 PM
To: Raju, Sundaram
Cc: Russell King - ARM Linux; Koul, Vinod; Dan; davinci-linux-open-
sou...@linux.davincidsp.com; linux-omap@vger.kernel.org; linux-
ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Subject: Re: [RFC] dmaengine: add new api for preparing simple slave transfer
On Fri, Jun 10, 2011 at 3:33 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Fri, Jun 10, 2011 at 05:18:46PM +0530, Raju, Sundaram wrote:
Now DMACs capable of 3D transfer, do transfer of the whole 1D
buffer per sync received or even whole 2D buffer per sync received
(based on the sync rate programmed in the DMAC).
The only issue which I see that we don't cover is the case where you want
to describe a single buffer which is organised as N bytes to be transferred,
M following bytes to be skipped, N bytes to be transferred, M bytes to be
skipped. I doubt there are many controllers which can be programmed with
both 'N' and 'M' parameters directly.
Sundaram is this how your controller works?
I mean the hardware can skip over sequences like this?
When we added the config interface to DMAengine I originally included
a custom config call, but Dan wanted me to keep it out until we
had some specific usecase for it. FSLDMA recently started
to use it.
Notice how dmaengine_slave_config() is implemented:
static inline int dmaengine_slave_config(struct dma_chan *chan,
struct dma_slave_config *config)
{
return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
(unsigned long)config);
}
So what is passed to the driver is just an unsigned long.
This is actually modeled to be ioctl()-like so you can pass in a
custom config to the same callback on the device driver,
just use some other enumerator than DMA_SLAVE_CONFIG,
say like FSLDMA already does with FSLDMA_EXTERNAL_START.
Just put some enumerator in enum dma_ctrl_cmd in
dmaengine.h such as SDMA_TEXAS_STRIDE_CONFIG and call
like this:
/* However that config struct needs to look, basically */
static struct sdma_ti_stride_cgf = {
take = M,
skip = N,
};
ret = chan-device-device_control(chan, SDMA_TEXAS_STRIDE_CONFIG,
sdma_ti_stride_cfg);
Or something like this.
Yes, the hardware can skip over sequences like that.
I also thought about your suggestion, at first before submitting the RFC.
But I dint pursue this because, this ioctl() call has to be made before
every single prepare call.
I may have to end up using this if we decide not to change the API
signature for prepare APIs.
I actually intend to use this for all DMAC related ioctl(). Configuring
the DMAC and programming various modes etc specific to the
DMAC. I suppose this is the only way to do it.
Let me know if there is any other way to do it.
Thanks,
Sundaram
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
I think I should have tried to explain my case with a specific example.
I tried to generalize it and it has confused and misled every one.
I have tried again now. :)
snip
A simple single buffer transfer (i.e. non sg transfer) can be done only
as a trivial case of the device_prep_slave_sg API. The client driver is
expected to prepare a sg list and provide to the dmaengine API for that
single buffer also.
We can avoid preparing a sg list in every driver which wants this by
having dmaengine.h provide a helper for this case:
static inline dma_async_tx_descriptor *dmaengine_prep_slave_single(
struct dma_chan *chan, void *buf, size_t len,
enum dma_data_direction dir, unsigned long flags)
{
struct scatterlist sg;
sg_init_one(sg, buf, len);
return chan-device-device_prep_slave_sg(chan, sg, 1, dir, flags);
}
That sounds good...
Yes, this should be sufficient if the only aim is to avoid preparing a sg list
in every driver which wants to transfer a single buffer.
But my main aim was to add more buffer related details to the API.
I have tried to explain a use case below, where this will be useful.
snip
I think also providing dmaengine_prep_slave_sg() and
dmaengine_prep_dma_cyclic() as wrappers to hide the chan- device-blah
bit would also be beneficial (and helps keep that detail out of the
users of DMA engine.)
Yes, this would really be helpful if someone is just looking at the
client drivers and are not familiar with the dmaengine's
internal data structures.
In a slave transfer, the client has to define all the buffer related
attributes and the peripheral related attributes.
I'd argue that it is incorrect to have drivers specify the buffer
related attributes - that makes the API unnecessarily complicated
to use, requiring two calls (one to configure the channel, and one
to prepare the transfer) each time it needs to be used.
I am not able to understand why 2 calls will be required?
Client configures the slave using dma_slave_config only once.
If the direction flag is removed then, this configuration doesn’t
have to be modified at all.
quote
So I think the slave transfer API must also have a provision to pass
the buffer configuration. The buffer attributes have to be passed
directly as an argument in the prepare API, unlike dma_slave_config
as they will be different for each buffer that is going to be
transferred.
/quote
The API is called only once in the current framework to prepare
the descriptor.
After adding any extra arguments in the prepare API, client will
still have to call it only once.
Not only that but the memory side of the transfer should be no business
of the driver - the driver itself should only specify the attributes
for the device being driven. The attributes for the memory side of the
transfer should be a property of the DMA engine itself.
Yes Russell, you are correct. Attributes of the memory side should
be a property of DMA engine itself.
What is meant here is that, client has told the DMAC how to
write to/ read from the slave peripheral by defining all the slave
properties in the dma_slave_config.
It is assumed that the buffer side has to be read/written to
byte after byte continuously by the DMAC.
I wanted to say client must have provisions to pass the details
on how it wants the DMAC to read/ write to the
buffer, if the capability is available in the DMAC.
DMACs have the capability to auto increment the
source/destination address accordingly after transferring
a byte of data and they automatically read/write to the
next byte location. In some DMACs you can program an offset also.
They read x bytes, skip y bytes and read the next x bytes.
This detail will be passed to the client driver by the application.
Then it is for the slave driver to communicate this to the
DMA engine, right?
I would like to see in the long term the dma_slave_config structure
lose its 'direction' argument, and the rest of the parameters used to
define the device side parameters only.
I am not sure why direction flag is required and can be done away with.
Both sg and cyclic API have a direction parameter and that should be
used. A channel can be used in any direction client wishes to.
I also agree. The direction flag in the dma_slave_config is redundant.
As Russell had already mentioned in another thread, this redundancy
forces DMAC drivers to check if the direction flag in the
dma_slave_config is same as that passed to the prepare API.
Also client drivers have to keep modifying the dma_slave_config
every time before they make a direction change in transfer.
But many a times a client may want to transfer from a single buffer to
the peripheral and most of the DMA controllers have the capability to
transfer data in chunks/frames directly at a stretch.
So far, I've only seen DMA controllers which operate on a linked list of
source, destination, length,
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
Vinod,
-Original Message-
From: Koul, Vinod [mailto:vinod.k...@intel.com]
Sent: Friday, June 10, 2011 11:39 AM
To: Raju, Sundaram; Dan
Cc: Russell King - ARM Linux; davinci-linux-open-
sou...@linux.davincidsp.com; linux-omap@vger.kernel.org; linux-
ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Subject: RE: [RFC] dmaengine: add new api for preparing simple slave transfer
snip
The above 2 APIs in the dmaengine framework expect all the
peripheral/slave related attributes to be filled in the
dma_slave_config data structure.
struct dma_slave_config {
enum dma_data_direction direction;
dma_addr_t src_addr;
dma_addr_t dst_addr;
enum dma_slave_buswidth src_addr_width;
enum dma_slave_buswidth dst_addr_width;
u32 src_maxburst;
u32 dst_maxburst;
};
This data structure is passed to the offload engine via the dma_chan
data structure in its private pointer.
No, this is passed thru control API you described above. Please read
Documentation/dmaengine.txt
Yes, Vinod its my mistake. I wanted to say control API only,
but just mixed it up with how the dma_slave_config is attached
to each dma_chan so that the offload drivers can use it while
preparing the descriptors.
Now coming to the buffer related attributes, sg list is a nice way to
represent a disjoint buffer; all the offload engines in drivers/dma
create a descriptor for each of the contiguous chunk in the sg list
buffer and pass it to the controller.
But many a times a client may want to transfer from a single buffer to
the peripheral and most of the DMA controllers have the capability to
transfer data in chunks/frames directly at a stretch.
All the attributes of a buffer, which are required for the transfer
can be programmed in single descriptor and submitted to the
DMA controller.
So I think the slave transfer API must also have a provision to pass
the buffer configuration. The buffer attributes have to be passed
directly as an argument in the prepare API, unlike dma_slave_config
as they will be different for each buffer that is going to be
transferred.
Can you articulate what attributes you are talking about. The
dma_slave_config parameters don't represent buffer attributes. They
represent the dma attributes on how you want to transfer. These things
like bus widths, burst lengths are usually constant for the slave
transfers, not sure why they should change per buffer?
I have tried to explain the attributes in the previous mail
I posted in this thread.
Yes, buffer attributes should not be represented in
the dma_slave_config. It is for slave related data.
That is why had mentioned that buffer configuration should be
a separate structure and passed in the prepare API.
See quoted below:
quote
struct dma_buffer_config {
u32 chunk_size; /* number of bytes in a chunk */
u32 frame_size; /* number of chunks in a frame */
/* u32 n_frames; number of frames in the buffer */
u32 inter_chunk_gap; /* number of bytes between end of a chunk
and the start of the next chunk */
u32 inter_frame_gap; /* number of bytes between end of a frame
and the start of the next frame */
bool sync_rate; /* 0 - a sync event is required from the
peripheral to transfer a chunk
1 - a sync event is required from the
peripheral to transfer a frame */
};
The patch to add a new API for single buffer transfers alone:
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
@@ -491,6 +492,10 @@ struct dma_device {
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_data_direction direction,
unsigned long flags);
+ struct dma_async_tx_descriptor *(*device_prep_slave)(
+ struct dma_chan *chan, dma_addr_t buf_addr,
+ unsigned int buf_len, void *buf_prop,
+ enum dma_data_direction direction, unsigned long flags);
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_data_direction direction);
/quote
Regards,
Sundaram
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
Russell, -Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Friday, June 10, 2011 4:13 PM To: Raju, Sundaram Cc: Koul, Vinod; Dan; davinci-linux-open-sou...@linux.davincidsp.com; linux- o...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-arm- ker...@lists.infradead.org Subject: Re: [RFC] dmaengine: add new api for preparing simple slave transfer On Fri, Jun 10, 2011 at 03:51:41PM +0530, Raju, Sundaram wrote: Consider a simple video use case of de-interlacing a video buffer. Odd lines have to be transferred first, and then the even lines are transferred separately. This can be done by programming the inter frame gap as the line size of the video buffer in the DMAC. This will require you to have only 2 descriptors, one for the odd lines and another for the even lines. This results in only 2 descriptors being written to DMAC registers. How would this be handled with DMACs which can't 'skip' bytes in the buffer? You would have to generate a list of LLIs separately describing each 'line' of video and chain them together. Correct. How do you handle the situation where a driver uses your new proposed API, but it doesn't support that in hardware. It should be handled the same way how a sg buffer is handled, if LLI chaining feature is not available in the hardware. It has to be submitted as a queue of separate descriptors to the DMAC, where each descriptor will be processed after the completion of the previous descriptor. Actually we can deduce the chunk_size from the dma_slave_config itself. It is either the src_addr_width or dst_addr_width based on the direction. Because at a stretch DMAC cannot transfer more than the slave register width. I think you're misinterpreting those fields. (dst|src)_addr_width tells the DMA controller the width of each transaction - whether to issue a byte, half-word, word or double-word read or write to the peripheral. It doesn't say how many of those to issue, it just says what the peripheral access size is to be. In other words, they describe the width of the FIFO register. Yes correct, I was just giving an example for considering or understanding a buffer in 3D and how each dimension should be. chunk_size = (src|dst)_addr_width, for a special case, i.e, if DMAC is programmed to transfer the entire 1D buffer per sync event received from the peripheral. In slave transfer, the peripheral is going to give a sync event to DMAC when the FIFO register is full|empty. Now DMACs capable of 3D transfer, do transfer of the whole 1D buffer per sync received or even whole 2D buffer per sync received (based on the sync rate programmed in the DMAC). So the DMAC has to be programmed for a 1D size (i.e. chunk size) equal to that of the width of the FIFO register if the sync rate programmed in DMAC is per chunk. Regards, Sundaram -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] dmaengine: add new api for preparing simple slave transfer
SDMA and EDMA are TI SoC specific DMA controllers. Their drivers have been
maintained in the respective SoC folders till now.
arch/arm/plat-omap/dma.c
arch/arm/mach-davinci/dma.c
I have gone through the existing offload engine (DMA) drivers in drivers/dma
which do slave transfers. I would like to move SDMA and EDMA also to dmaengine
framework.
I believe that even though the dmaengine framework addresses and supports most
of the required use cases of a client driver to a DMA controller, some
extensions are required in it to make it still more generic.
Current framework contains two APIs to prepare for slave transfers:
struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_data_direction direction,
unsigned long flags);
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_data_direction direction);
and one control API.
int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
unsigned long arg);
A simple single buffer transfer (i.e. non sg transfer) can be done only as a
trivial case of the device_prep_slave_sg API. The client driver is expected to
prepare a sg list and provide to the dmaengine API for that single buffer also.
In a slave transfer, the client has to define all the buffer related attributes
and the peripheral related attributes.
The above 2 APIs in the dmaengine framework expect all the peripheral/slave
related attributes to be filled in the dma_slave_config data structure.
struct dma_slave_config {
enum dma_data_direction direction;
dma_addr_t src_addr;
dma_addr_t dst_addr;
enum dma_slave_buswidth src_addr_width;
enum dma_slave_buswidth dst_addr_width;
u32 src_maxburst;
u32 dst_maxburst;
};
This data structure is passed to the offload engine via the dma_chan data
structure in its private pointer.
Now coming to the buffer related attributes, sg list is a nice way to represent
a disjoint buffer; all the offload engines in drivers/dma create a descriptor
for each of the contiguous chunk in the sg list buffer and pass it to the
controller.
But many a times a client may want to transfer from a single buffer to the
peripheral and most of the DMA controllers have the capability to transfer data
in chunks/frames directly at a stretch. All the attributes of a buffer, which
are required for the transfer can be programmed in single descriptor and
submitted to the DMA controller.
So I think the slave transfer API must also have a provision to pass the buffer
configuration. The buffer attributes have to be passed directly as an argument
in the prepare API, unlike dma_slave_config as they will be different for each
buffer that is going to be transferred.
It is a stretch and impractical to use a highly segmented buffer (like the one
described below) in a sglist. This is because sg list itself is a
representation of a disjoint buffer collection in terms of smaller buffers. Now
then each of these smaller buffers can have different buffer configurations
(like described below) and we are not going to go down that road now.
Hence it makes sense to pass these buffer attributes for only a single buffer
transfer and not a sg list. This can be done by
OPTION #1
1. Adding a pointer of the dma_buffer_config data structure in the
device_prep_slave_sg API
2. Ensuring that it will be ignored if a sg list passed. Only when a single
buffer is passed (in the sg list) then this buffer configuration will be used.
3. Any client that wants to do a sg transfer can simply ignore this buffer
configuration and pass NULL.
The main disadvantage of this option is that all the existing drivers need to
be updated since the API signature is changed.
Now it might even be better to have a separate API for non sg transfers.
This is OPTION #2
Advantages of this option are
1. No change required in the existing drivers that use device_prep_slave_sg
API.
2. If any offload engine wants to prepare a simple buffer transfer differently
and not as a trivial case of a sg list, this will be useful. I know this can be
done using 2 different implementations inside the device_prep_slave_sg itself,
but I think it's cleaner to have different APIs.
I have provided the generic buffer configuration I can think of, here and also
a new API definition, if it makes sense to have one.
This buffer configuration might not be completely generic, and hence I ask all
of you to please provide comments to improve it.
Generic buffer description:
A generic buffer can be split into number of frames which contain number of
chunks inside them. The frames need not be contiguous, nor do the chunks inside
a frame.
RE: [RFC] dmaengine: add new api for preparing simple slave transfer
.
---
| Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n | Frame 0
---
| Inter Frame Gap |
---
| Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n | Frame 1
---
| Inter Frame Gap |
---
| |
---
| Inter Frame Gap |
---
| Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n | Frame m
---
Note: ICG = Inter Chunk Gap.
struct dma_buffer_config {
u32 chunk_size; /* number of bytes in a chunk */
u32 frame_size; /* number of chunks in a frame */
/* u32 n_frames; number of frames in the buffer */
u32 inter_chunk_gap; /* number of bytes between end of a chunk
and the start of the next chunk */
u32 inter_frame_gap; /* number of bytes between end of a frame
and the start of the next frame */
bool sync_rate; /* 0 - a sync event is required from the
peripheral to transfer a chunk
1 - a sync event is required from the
peripheral to transfer a frame */
};
The patch to add a new API for single buffer transfers alone:
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
@@ -491,6 +492,10 @@ struct dma_device {
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_data_direction direction,
unsigned long flags);
+ struct dma_async_tx_descriptor *(*device_prep_slave)(
+ struct dma_chan *chan, dma_addr_t buf_addr,
+ unsigned int buf_len, void *buf_prop,
+ enum dma_data_direction direction, unsigned long flags);
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_data_direction direction);
The number of frames (n_frames) can always be determined using all
other values in the dma_buffer_config along with the
buffer length (buf_len) passed in the API.
n_frames = buf_len / (chunk_sixe * frame_size);
Regards,
Sundaram
-Original Message-
From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
Sent: Thursday, June 09, 2011 6:17 PM
To: Raju, Sundaram
Cc: linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
davinci-linux-open-sou...@linux.davincidsp.com; linux-omap@vger.kernel.org
Subject: Re: [RFC] dmaengine: add new api for preparing simple slave transfer
Can you please re-post with sensible wrapping at or before column 72.
I'm not manually reformatting your entire message just so I can find
the relevant bits to reply to.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
