Skip to content
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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions include/zephyr/dmm/dmm.h
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).
*
Copy link
Member

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 or dma_handle from the time this function is called until dmm_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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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...)
* - 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: flushing is not necessary

Suggested change
* memory (for example flushing cache lines associated with the memory
* memory (for example writing back cache lines associated with the memory

* backing the DMA accessible buffers)
*
* @param[in] dev Device pointer
Copy link
Member

Choose a reason for hiding this comment

The 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 dma_handle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, that is the idea.

Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do.

* @param[in] flags Flags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these flags defined somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, TBD.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify here sth like

Suggested change
* @param[in] flags Flags
* @param[in] flags Flags reserved for future use

* @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,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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).
*
Copy link
Member

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 or dma_handle from the time this function is called until dmm_release_buffer_for_dma_in() is called.

Also I would clarify that dmm_release_buffer_for_dma_in() must be called after the DMA transfer is finished, before data is read by CPU.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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.
*
Copy link
Collaborator

Choose a reason for hiding this comment

The 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_ */