[RFC]DMA API Update


Liu, Baohong
 

-----Original Message-----
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
Sent: Thursday, January 19, 2017 3:06 AM
To: Liu, Baohong <baohong.liu(a)intel.com>; devel(a)lists.zephyrproject.org
Subject: Re: [devel] [RFC]DMA API Update

On Wed, 2017-01-18 at 23:15 +0000, Liu, Baohong wrote:
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]

That still leaves the question as to who calls dma_channel_config
and how the parameters for that get into the system. Is each driver
that implements DMA support going to have a bunch of KConfig entries
to dma_slot, hankshake_polarity etc? Or will the SoC have a bunch of
Just to clarify.

The driver that implements DMA support (e.g. SPI driver) will need to determine
where it needs to get the parameters values for calling the DMA API. This is
beyond the scope of this RFC.

#defines (like we currently have for IRQ numbers) for setting these
DMA attributes (assuming they are fixed in hardware).
The DMA driver should have configuration entries for setting these
parameters and the values of these configurations should be determined
and set by an app level prj.conf, not by the SPI or DMA driver
So, when I add DMA support to my uart, everyone project that uses a UART,
or device attached through a UART, needs know the magic device specific
configuration required and add it their prj.conf?

E.g, to run the Zephyr UART tests I edit tests/drivers/uart/prj.conf to add the
DMA slot, handshaking polarity, etc., that are used for the UART + DMA
driver combination on the device I'm interested in?

Does prj.conf support #includes? That way we could at least provide an
include files per SoC for people to include to get all this config. Then again, if
we did that, why not just add the config direct to the SoC defconfig? Or even
better, just add it to SoC headers files like IRQ assignments and device
addresses? At least that way this SoC specific knowledge is more together in
one place. Currently it's spread between SoC and board files, and leaking into
device drivers, and now starting to push some of it into project files as well
seems like completely the wrong direction to take.

--
Tixy


Liu, Baohong
 

-----Original Message-----
From: Tseng, Kuo-Lang
Sent: Wednesday, January 18, 2017 3:06 PM
To: Liu, Baohong <baohong.liu(a)intel.com>; 'devel(a)lists.zephyrproject.org'
<devel(a)lists.zephyrproject.org>
Subject: RE: [RFC]DMA API Update

Baohong,

-----Original Message-----
From: Liu, Baohong [mailto:baohong.liu(a)intel.com]
Sent: Tuesday, January 17, 2017 4:52 PM


Hi everyone,

Based on the feedbacks, I updated the RFC.
[...]

/**
* @brief DMA channel configuration structure.
*
* config is a bit field with the following parts:
* dma_slot [ 0 : 5 ] --which peripheral and
direction(HW
specific)
* hankshake_polarity [ 6 ] --high or low
* channel_direction [ 7 : 9 ] --0-memory to memory, 1-
memory to
peripheral,
* 2-peripheral to memory ...
* complete_int_en [ 10 ] --completion interrupt enable
* block_int_en [ 11 ] --block completion interrupt
enable
* source_int_en [ 12 ] --source completion interrupt
enable
* dest_int_en [ 13 ] --destination completion interrupt
enable
* error_int_en [ 14 ] --error interrupt enable
* source_handshake [ 15 ] --HW or SW
* dest_handshake [ 16 ] --HW or SW
I thought the original idea (from ZEP-873) was to minimize the
dma_channel_config structure by removing SoC or driver specific entries like
the handshake interface above which may not be common in other DMA
engines. IMO, those should be kept inside the driver and not to be exposed
in API.
Probably, you was talking about handshake_polarity. I will remove it. By the way,
The elimination will not save any memory usage.

handshake_interface which is having a new name "dma_slot" can't be removed.
All the SoCs I have reviewed so far have this feature.


Kuo


Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2017-01-18 at 23:15 +0000, Liu, Baohong wrote:
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]

That still leaves the question as to who calls dma_channel_config and how
the parameters for that get into the system. Is each driver that implements
DMA support going to have a bunch of KConfig entries to dma_slot,
hankshake_polarity etc? Or will the SoC have a bunch of #defines (like we
currently have for IRQ numbers) for setting these DMA attributes (assuming
they are fixed in hardware).
The DMA driver should have configuration entries for setting these parameters and
the values of these configurations should be determined and set by an app level
prj.conf, not by the SPI or DMA driver
So, when I add DMA support to my uart, everyone project that uses a
UART, or device attached through a UART, needs know the magic device
specific configuration required and add it their prj.conf?

E.g, to run the Zephyr UART tests I edit tests/drivers/uart/prj.conf
to add the DMA slot, handshaking polarity, etc., that are used for the
UART + DMA driver combination on the device I'm interested in?

Does prj.conf support #includes? That way we could at least provide an
include files per SoC for people to include to get all this config. Then
again, if we did that, why not just add the config direct to the SoC
defconfig? Or even better, just add it to SoC headers files like IRQ
assignments and device addresses? At least that way this SoC specific
knowledge is more together in one place. Currently it's spread between
SoC and board files, and leaking into device drivers, and now starting
to push some of it into project files as well seems like completely the
wrong direction to take.

--
Tixy


Liu, Baohong
 

From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
Sent: Monday, January 16, 2017 7:16 AM
Subject: Re: [devel] [RFC]DMA API Update

On Sat, 2017-01-14 at 00:31 +0000, Liu, Baohong wrote:
Thanks for the feedback. See my reply inline.
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]

But finally, the big elephant in the room is: how are DMA APIs
actually expected to used? And how should the system be configured?
The same way as other device drivers. There will be a driver to
implement the APIs for each SoC. App does the config, and initiate the
transfer by calling the APIs.
As far as I can see there is no documentation or examples in Zephyr that give
a clue as to how DMA can be used with device drivers. So it's very difficult to
review a DMA API without some more explanation on it's expected use.
The DMA API can be used by drivers for DMA operations


Let me try some more specific questions...

Say I have a system which has:
A) a driver for a DMA Controller (it implement struct dma_driver_api)
B) a driver for some interface, e.g. SPI
C) an app which wants to talk to a device attached to that interface
e.g. and LCD controller
D) other things

Which of the above will call

- dma_channel_config
- dma_transfer_config
- dma_transfer_start
- dma_transfer_stop

?
IMO, ideally the SPI driver should call the DMA API to implement the DMA support.
However, I think this is up to the driver to decide to use the DMA API or to utilize a
HAL driver it depends on


The SPI subsystem interface doesn't have anything DMA related in it, so I can
only assume that DMA use will have to be hidden inside the SPI driver, and
for each spi_transceive request made made of it will setup and operate on
an appropriate struct dma_transfer_config? (Or use scatter-gather lists when
API's get that ;-)

If so, every device driver in zephyr that anyone wants to use with DMA will
need to add DMA support to it and make it configurable to
(optionally) use the generic DMA API. Or is it expected that all APIs like SPI
grow new functions for use with DMA specifically?
Either one is fine.


That still leaves the question as to who calls dma_channel_config and how
the parameters for that get into the system. Is each driver that implements
DMA support going to have a bunch of KConfig entries to dma_slot,
hankshake_polarity etc? Or will the SoC have a bunch of #defines (like we
currently have for IRQ numbers) for setting these DMA attributes (assuming
they are fixed in hardware).
The DMA driver should have configuration entries for setting these parameters and
the values of these configurations should be determined and set by an app level
prj.conf, not by the SPI or DMA driver


There's also the question as to how channel numbers are allocated. Is this
more per-driver KConfig options or should we have a DMA channel allocator
function so they can just be assigned at runtime?

I'm also thinking about the fact that there are likely to be more devices
capable of DMA than channels available so how about an API to free a
channel? E.g. dma_channel_config claims a channel for use and
dma_channel_free releases for someone else. That way even with static
allocation of channels we have the option of allocating the same channel to
two different devices if we know they can't be active at the same time.
The current API dma_channel_config can support this; when the channel is being used by
a driver, if another driver calls the dma_channel_config API, the API should return an
specific error code indicating the channel is not ready to be used.

--
Tixy


Liu, Baohong
 

From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
Sent: Wednesday, January 18, 2017 3:31 AM
Subject: Re: [devel] [RFC]DMA API Update

On Wed, 2017-01-18 at 01:14 +0000, Liu, Baohong wrote:
-----Original Message-----
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]
?

The SPI subsystem interface doesn't have anything DMA related in it,
so I can only assume that DMA use will have to be hidden inside the
SPI driver, and for each spi_transceive request made made of it will
setup and operate on an appropriate struct dma_transfer_config? (Or
use scatter-gather lists when API's get that ;-)

If so, every device driver in zephyr that anyone wants to use with
DMA will need to add DMA support to it and make it configurable to
(optionally) use the generic DMA API. Or is it expected that all
APIs like SPI grow new functions for use with DMA specifically?

That still leaves the question as to who calls dma_channel_config
and how the parameters for that get into the system. Is each driver
that implements DMA support going to have a bunch of KConfig entries
to dma_slot, hankshake_polarity etc? Or will the SoC have a bunch of
#defines (like we currently have for IRQ numbers) for setting these
DMA attributes (assuming they are fixed in hardware).

There's also the question as to how channel numbers are allocated.
Is this more per-driver KConfig options or should we have a DMA
channel allocator function so they can just be assigned at runtime?
What you said is the scenario of driver over driver. There are
existing examples In Zephyr.
Just to clarify my previous answer which is meant to say that there are existing
usage cases a driver use API from another driver. (e.g some sensor drivers uses
I2C driver API)

For the channel assignment, I think this should be decided and set by an app level
Prj.conf, not by driver Kconfig.


Where? I can only assume you have in mind the things in
ext/hal/qmsi/drivers which is only occurrence of DMA in the Zephyr tree I
can find. Those drivers look hard coded for use on a single SoC and have
driver specific APIs for DMA, and nothing in-tree uses those APIs.

Also, there is no app in-tree that actually uses DMA apart from
test_loop_transfer which just does a simple memory-to-memory DMA test.

If the idea is that people should follow the Quark example, then it looks to
me that your saying DMA support in drivers is something everyone has to
design for themselves on a per-driver basis? If so, there'll be no API
consistency between drivers and every app will be hard coded to a particular
set of drivers.

But I can't believe that is what is intended, so I must have mis- understood
something fundamental.
The DMA API is designed to solve that; ideally app or drivers should all use the DMA
API to implement DMA support

--
Tixy


Tseng, Kuo-Lang <kuo-lang.tseng@...>
 

Baohong,

-----Original Message-----
From: Liu, Baohong [mailto:baohong.liu(a)intel.com]
Sent: Tuesday, January 17, 2017 4:52 PM


Hi everyone,

Based on the feedbacks, I updated the RFC.
[...]

/**
* @brief DMA channel configuration structure.
*
* config is a bit field with the following parts:
* dma_slot [ 0 : 5 ] --which peripheral and direction(HW
specific)
* hankshake_polarity [ 6 ] --high or low
* channel_direction [ 7 : 9 ] --0-memory to memory, 1-memory to
peripheral,
* 2-peripheral to memory ...
* complete_int_en [ 10 ] --completion interrupt enable
* block_int_en [ 11 ] --block completion interrupt enable
* source_int_en [ 12 ] --source completion interrupt enable
* dest_int_en [ 13 ] --destination completion interrupt
enable
* error_int_en [ 14 ] --error interrupt enable
* source_handshake [ 15 ] --HW or SW
* dest_handshake [ 16 ] --HW or SW
I thought the original idea (from ZEP-873) was to minimize the dma_channel_config
structure by removing SoC or driver specific entries like the handshake interface
above which may not be common in other DMA engines. IMO, those should be kept
inside the driver and not to be exposed in API.

Kuo


Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2017-01-18 at 01:14 +0000, Liu, Baohong wrote:
-----Original Message-----
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]
?

The SPI subsystem interface doesn't have anything DMA related in it, so I can
only assume that DMA use will have to be hidden inside the SPI driver, and
for each spi_transceive request made made of it will setup and operate on
an appropriate struct dma_transfer_config? (Or use scatter-gather lists when
API's get that ;-)

If so, every device driver in zephyr that anyone wants to use with DMA will
need to add DMA support to it and make it configurable to
(optionally) use the generic DMA API. Or is it expected that all APIs like SPI
grow new functions for use with DMA specifically?

That still leaves the question as to who calls dma_channel_config and how
the parameters for that get into the system. Is each driver that implements
DMA support going to have a bunch of KConfig entries to dma_slot,
hankshake_polarity etc? Or will the SoC have a bunch of #defines (like we
currently have for IRQ numbers) for setting these DMA attributes (assuming
they are fixed in hardware).

There's also the question as to how channel numbers are allocated. Is this
more per-driver KConfig options or should we have a DMA channel allocator
function so they can just be assigned at runtime?
What you said is the scenario of driver over driver. There are existing examples
In Zephyr.
Where? I can only assume you have in mind the things in
ext/hal/qmsi/drivers which is only occurrence of DMA in the Zephyr tree
I can find. Those drivers look hard coded for use on a single SoC and
have driver specific APIs for DMA, and nothing in-tree uses those APIs.

Also, there is no app in-tree that actually uses DMA apart from
test_loop_transfer which just does a simple memory-to-memory DMA test.

If the idea is that people should follow the Quark example, then it
looks to me that your saying DMA support in drivers is something
everyone has to design for themselves on a per-driver basis? If so,
there'll be no API consistency between drivers and every app will be
hard coded to a particular set of drivers.

But I can't believe that is what is intended, so I must have mis-
understood something fundamental.

--
Tixy


Liu, Baohong
 

-----Original Message-----
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
Sent: Monday, January 16, 2017 7:16 AM
To: Liu, Baohong <baohong.liu(a)intel.com>; devel(a)lists.zephyrproject.org
Subject: Re: [devel] [RFC]DMA API Update

On Sat, 2017-01-14 at 00:31 +0000, Liu, Baohong wrote:
Thanks for the feedback. See my reply inline.
From: Jon Medhurst (Tixy) [mailto:tixy(a)linaro.org]
[...]

But finally, the big elephant in the room is: how are DMA APIs
actually expected to used? And how should the system be configured?
The same way as other device drivers. There will be a driver to
implement the APIs for each SoC. App does the config, and initiate the
transfer by calling the APIs.
As far as I can see there is no documentation or examples in Zephyr that give
a clue as to how DMA can be used with device drivers. So it's very difficult to
review a DMA API without some more explanation on it's expected use.

Let me try some more specific questions...

Say I have a system which has:
A) a driver for a DMA Controller (it implement struct dma_driver_api)
B) a driver for some interface, e.g. SPI
C) an app which wants to talk to a device attached to that interface
e.g. and LCD controller
D) other things

Which of the above will call

- dma_channel_config
- dma_transfer_config
- dma_transfer_start
- dma_transfer_stop
I updated the RFC.
The sequence is dma_channel_config, dma_transfer_start, and then callback.
So simple!

dma_transfer_stop is to abort an existing transfer.

?

The SPI subsystem interface doesn't have anything DMA related in it, so I can
only assume that DMA use will have to be hidden inside the SPI driver, and
for each spi_transceive request made made of it will setup and operate on
an appropriate struct dma_transfer_config? (Or use scatter-gather lists when
API's get that ;-)

If so, every device driver in zephyr that anyone wants to use with DMA will
need to add DMA support to it and make it configurable to
(optionally) use the generic DMA API. Or is it expected that all APIs like SPI
grow new functions for use with DMA specifically?

That still leaves the question as to who calls dma_channel_config and how
the parameters for that get into the system. Is each driver that implements
DMA support going to have a bunch of KConfig entries to dma_slot,
hankshake_polarity etc? Or will the SoC have a bunch of #defines (like we
currently have for IRQ numbers) for setting these DMA attributes (assuming
they are fixed in hardware).

There's also the question as to how channel numbers are allocated. Is this
more per-driver KConfig options or should we have a DMA channel allocator
function so they can just be assigned at runtime?
What you said is the scenario of driver over driver. There are existing examples
In Zephyr.


I'm also thinking about the fact that there are likely to be more devices
capable of DMA than channels available so how about an API to free a
channel? E.g. dma_channel_config claims a channel for use and
dma_channel_free releases for someone else. That way even with static
allocation of channels we have the option of allocating the same channel to
two different devices if we know they can't be active at the same time.
dma_slot is the one to dynamically connect a dma channel to a device. But, the
responsibility to prevent conflict is on user.


--
Tixy


Liu, Baohong
 

Hi everyone,

Based on the feedbacks, I updated the RFC.

Data Structure Definitions
-------

/**
* @brief DMA block configuration structure.
*
* config is a bit field with the following parts:
* source_gather_en [ 0 ] --enable/disable source gather
* dest_scatter_en [ 1 ] --enable/disable destination scatter
* source_inc_en [ 2 ] --enable/disable source address increment
* dest_inc_en [ 3 ] --enable/disable destination address increment
* source_reload_en [ 4 ] --reload source address at the end of block transfer
* dest_reload_en [ 5 ] --reload destination address at the end of block transfer
* fifo_mode_control [ 6 : 9 ] --How full of the fifo before transfer start. HW specific.
* flow_control_mode [ 10 ] --source request is served when data available or
* wait until destination request happens.
* RESERVED [ 11 : 31 ]
* source_address is block starting address at source
* source_gather_count is the continuous transfer count between gather boundaries
* source_gather_interval is the address adjustment at gather boundary
* dest_address is block starting address at destination
* dest_scatter_count is the continuous transfer count between scatter boundaries
* dest_scatter_interval is the address adjustment at scatter boundary
* block_size is the number of bytes to be transferred for this block.
*/
struct dma_block_config {
uint32_t config;
uint32_t source_address;
uint32_t source_gather_count;
uint32_t source_gather_interval;
uint32_t dest_address;
uint32_t dest_scatter_count;
uint32_t dest_scatter_interval;
uint32_t block_size;
struct dma_block_config *next_block;
}

/**
* @brief DMA channel configuration structure.
*
* config is a bit field with the following parts:
* dma_slot [ 0 : 5 ] --which peripheral and direction(HW specific)
* hankshake_polarity [ 6 ] --high or low
* channel_direction [ 7 : 9 ] --0-memory to memory, 1-memory to peripheral,
* 2-peripheral to memory ...
* complete_int_en [ 10 ] --completion interrupt enable
* block_int_en [ 11 ] --block completion interrupt enable
* source_int_en [ 12 ] --source completion interrupt enable
* dest_int_en [ 13 ] --destination completion interrupt enable
* error_int_en [ 14 ] --error interrupt enable
* source_handshake [ 15 ] --HW or SW
* dest_handshake [ 16 ] --HW or SW
* channel_priority [ 17 : 20 ] --DMA channel priority
* source_chaining_en [ 21 ] --enable/disable source block chaining
* dest_chaining_en [ 22 ] --enable/disable destination block chaining
* RESERVED [ 23 : 31 ]
* config_size is a bit field with the following parts:
* source_data_size [ 0 : 7 ] --number of bytes
* dest_data_size [ 8 : 15 ] -- number of bytes
* source_burst_length [ 16 : 23 ] -- number of source data units
* dest_burst_length [ 24 : 31 ] -- number of destination data units
* dma_callback is the callback function pointer
*/
struct dma_channel_config {
uint32_t config;
uint32_t config_size;
uint32_t block_count;
struct dma_block_config *block_head;
void (*dma_callback)(struct device *dev, uint32_t channel, int error_code);
}

Remove struct dma_transfer_config

API Interfaces
-------

/**
* @brief Configure individual channel for DMA transfer.
*
* @param dev Pointer to the device structure for the driver instance.
* @param channel Numeric identification of the channel to configure
* @param config Data structure containing the intended configuration for the
* selected channel
*
* @retval 0 If successful.
* @retval Negative errno code if failure.
*/
static inline int dma_channel_config(struct device *dev, uint32_t channel,
struct dma_channel_config *config)
{
}

/**
* @brief Enables DMA channel and starts the transfer, the channel must be
* configured beforehand.
*
* @param dev Pointer to the device structure for the driver instance.
* @param channel Numeric identification of the channel where the transfer will
* be processed
*
* @retval 0 If successful.
* @retval Negative errno code if failure.
*/
static inline int dma_transfer_start(struct device *dev, uint32_t channel)
{
}

/**
* @brief Stops the DMA transfer and disables the channel.
*
* @param dev Pointer to the device structure for the driver instance.
* @param channel Numeric identification of the channel where the transfer was
* being processed
*
* @retval 0 If successful.
* @retval Negative errno code if failure.
*/
static inline int dma_transfer_stop(struct device *dev, uint32_t channel)
{
}

Remove API interface dma_transfer_config().

Suggestions and comments are welcome.

-----Original Message-----
From: Liu, Baohong
Sent: Wednesday, January 11, 2017 11:50 AM
To: devel(a)lists.zephyrproject.org
Subject: [RFC]DMA API Update


Hi everyone,

I would like to propose the update for DMA API.
(https://jira.zephyrproject.org/browse/ZEP-873).

Known Problems
--------

Struct dma_channel_config consists of quite a few variables of enum types,
two callback function pointers, and one callback user data pointer.

- The enums can be collapsed into a bit field.
This will reduce the size of the data structure.

- The two callback function pointers (for normal and error cases) can be
combined.
A parameter of error code can be used to differentiate the two cases. So,
the
current callback parameter of user data pointer will be replaced by two new
parameters. One for error code and the other one for channel id.

- Callback data pointer can be removed.
Currently, this field holds user data. It can be moved to driver data structure.
The callback function can dig out the information from dev pointer and
channel id passed to it.

Proposed Solution
--------

-- The following are the enums we have now.

handshake_interface /* which peripheral and direction*/
hankshake_polarity /* high or low*/
channel_direction /* memory to memory, memory to peripheral ... */
Source_transfer_width /* source data size */
Destination_transfer_width /* destination data size */
Source_burst_length /* source burst length */
Destination_burst_length /* destination burst length */

All these will be collapsed into a bit field. Some of them will have new names.

Besides the above, three new items will be added.
source_handshake /* HW or SW */
dest_handshake /* HW or SW */
channel_priority /* DMA channel priority */

-- For the callback function, there will be three parameters. They are device
pointer,
channel_id, and error code. As said above, the error callback will be
removed.

-- The callback_data pointer will be removed.

Final API Format
--------

/**
* @brief DMA configuration structure.
*
* config is a bit field with the following parts:
* dma_slot [ 0 : 5 ] --which peripheral and
direction(HW specific)
* hankshake_polarity [ 6 ] --high or low
* channel_direction [ 7 : 9 ] --0-memory to memory, 1-
memory to peripheral,
* 2-peripheral to memory
* source_data_size [ 10 : 12 ] --source data size(8,16,32,64,128
and 256 bits)
* dest_data_size [ 13 : 15 ] --destination data
size(8,16,32,64,128 and 256 bits)
* source_burst_length [ 16 : 18 ] --number of source data
unit(1,4,8,16,32,64,128 and 256)
* dest_burst_length [ 19 : 21 ] -- number of dest data
unit(1,4,8,16,32,64,128 and 256)
* source_handshake [ 22 ] --HW or SW
* dest_handshake [ 23 ] --HW or SW
* channel_priority [ 24 : 27 ] --DMA channel priority
* RESERVED [ 28 : 31 ]
* dma_callback is the callback function pointer
*/
struct dma_channel_config {
uint32_t config;
void (*dma_callback)(struct device *dev, uint32_t channel, int
error_code); }

The remaining parts will stay the same.
No change to the structure dma_transfer_config.
No change to the API function prototypes:
dma_channel_config(...)
dma_transfer_config(...)
dma_transfer_start(...)
dma_transfer_stop(...)

(Note: for xx_data_size and xx_burst_length in the above bit field, the value
will only serve as an index. For example, for source_data_size, bit field values
of 0, 1, 2, 3, 4 and 5 will correspond to sizes of 8,16,32,64,128 and 256 bits.)

Suggestions and comments are welcome.