-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC][DNM] DMM: Skeleton for a DMA Memory Management subsystem #57602
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,123 @@ | ||||||
/* | ||||||
* Copyright (c) 2023 Carlo Caione <[email protected]> | ||||||
* | ||||||
* SPDX-License-Identifier: Apache-2.0 | ||||||
*/ | ||||||
|
||||||
#ifndef ZEPHYR_INCLUDE_DMM_DMM_H_ | ||||||
#define ZEPHYR_INCLUDE_DMM_DMM_H_ | ||||||
|
||||||
#include <stdio.h> | ||||||
#include <zephyr/device.h> | ||||||
#include <zephyr/kernel.h> | ||||||
|
||||||
#ifdef __cplusplus | ||||||
extern "C" { | ||||||
#endif | ||||||
|
||||||
/** @brief Get a DMA handle for outbound data transfer | ||||||
* | ||||||
* This function is used to translate the user-provided buffer into something | ||||||
* that is usable for a DMA outbound data transfer and return that to the user | ||||||
* as an opaque DMA handle (usually a physical/bus address). | ||||||
* | ||||||
* This function should: | ||||||
* | ||||||
* - allocate bounce buffers if the user-provided buffer cannot be used | ||||||
* directly by the DMA (for example the memory is not DMA accessible, not | ||||||
* aligned, not padded to cache line size, etc...) | ||||||
* - copy the data from the user-provided buffer to the bounce buffers | ||||||
* - translate CPU buffer addresses to DMA addresses if needed | ||||||
* - do the proper cache management if buffers are allocated in cacheable | ||||||
* memory (for example flushing cache lines associated with the memory | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: flushing is not necessary
Suggested change
|
||||||
* backing the DMA accessible buffers) | ||||||
* | ||||||
* @param[in] dev Device pointer | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which device is this? The device that is to perform DMA transfer from the buffer pointed by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, that is the idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add this information somewhere to the docstring to make it clear? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll do. |
||||||
* @param[in] flags Flags | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these flags defined somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, TBD. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you clarify here sth like
Suggested change
|
||||||
* @param[out] dma_handle DMA handle. | ||||||
* @param[in] buffer User-provided buffer. | ||||||
* @param[in] buffer_size Size of the user-provided buffer. | ||||||
* | ||||||
* @retval 0 on success | ||||||
* @retval other errno codes depending on the implementation | ||||||
*/ | ||||||
int dmm_get_buffer_for_dma_out(const struct device *dev, uint32_t flags, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would this possible to define where is the buffer comes from? say a memory pool point?or just a fixed physical address |
||||||
void **dma_handle, const void *buffer, | ||||||
size_t buffer_size); | ||||||
|
||||||
/** @brief Release a DMA handle (outbound data transfer) | ||||||
* | ||||||
* This function is used to release a DMA handle and the associated memory, | ||||||
* for example freeing the bounce buffers if used. | ||||||
* | ||||||
* @param[in] dev Device pointer | ||||||
* @param[in] flags Flags | ||||||
* @param[in] dma_handle DMA handle. | ||||||
* | ||||||
* @retval 0 on success | ||||||
* @retval other errno codes depending on the implementation | ||||||
*/ | ||||||
int dmm_release_buffer_for_dma_out(const struct device *dev, uint32_t flags, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if release a buffer why I need to know the flags? |
||||||
void *dma_handle); | ||||||
|
||||||
/** @brief Get a DMA handle for inbound data transfer | ||||||
* | ||||||
* This function is used to translate the user-provided buffer into something | ||||||
* that is usable for a DMA inbound data transfer and return that to the user | ||||||
* as an opaque DMA handle (usually a physical/bus address). | ||||||
* | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a paragraph that the user cannot access buffer pointed by Also I would clarify that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ack |
||||||
* This function should: | ||||||
* | ||||||
* - allocate bounce buffers if the user-provided buffer cannot be used | ||||||
* directly by the DMA (for example the memory is not DMA accessible, not | ||||||
* aligned, not padded to cache line size, etc...) | ||||||
* - translate CPU buffer addresses to DMA addresses if needed | ||||||
* - do the proper cache management if buffers are allocated in cacheable | ||||||
* memory (for example invalidating cache lines associated with the memory | ||||||
* backing the DMA accessible buffers) | ||||||
* | ||||||
* @param[in] dev Device pointer | ||||||
* @param[in] flags Flags | ||||||
Comment on lines
+79
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar ambiguity like in the outbound transfer function |
||||||
* @param[out] dma_handle DMA handle. | ||||||
* @param[in] buffer User-provided buffer. | ||||||
* @param[in] buffer_size Size of the user-provided buffer. | ||||||
* | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as out, can we add a parameter for where the buffer is comming from? |
||||||
* @retval 0 on success | ||||||
* @retval other errno codes depending on the implementation | ||||||
*/ | ||||||
int dmm_get_buffer_for_dma_in(const struct device *dev, uint32_t flags, | ||||||
void **dma_handle, const void *buffer, | ||||||
size_t buffer_size); | ||||||
|
||||||
/** @brief Get data and release a DMA handle (inbound data transfer) | ||||||
* | ||||||
* This function is used to retrieve the user-accessible data from the DMA | ||||||
* handle, release it and free the associated memory. | ||||||
* | ||||||
* This function should: | ||||||
* | ||||||
* - do the proper cache management if buffers are allocated in cacheable | ||||||
* memory (for example invalidating cache lines associated with the memory | ||||||
* backing the DMA accessible buffers) | ||||||
* - copy the data from the DMA buffer to the user-provided buffer | ||||||
* - update the @p buffer_size pointer with the size of the retrieved data | ||||||
* - free the bounce buffers if used | ||||||
* | ||||||
* @param[in] dev Device pointer | ||||||
* @param[in] flags Flags | ||||||
* @param[in] dma_handle DMA handle. | ||||||
* @param[out] buffer User-provided buffer | ||||||
* @param[in,out] buffer_size Size of the input buffer and returned data | ||||||
* | ||||||
* @retval 0 on success | ||||||
* @retval other errno codes depending on the implementation | ||||||
*/ | ||||||
int dmm_release_buffer_for_dma_in(const struct device *dev, uint32_t flags, | ||||||
void *dma_handle, void *buffer, | ||||||
size_t *buffer_size); | ||||||
|
||||||
#ifdef __cplusplus | ||||||
} | ||||||
#endif | ||||||
|
||||||
#endif /* ZEPHYR_INCLUDE_DMM_DMM_H_ */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a paragraph that the user cannot access buffer pointed by
buffer
ordma_handle
from the time this function is called untildmm_release_buffer_for_dma_out()
is called. Access includes reading from this buffer.Also I would clarify that
dmm_release_buffer_for_dma_out()
must be called after the DMA transfer is finished. Not calling this function can lead to memory leaks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack