On 23-08-18, 16:07, Peter Ujfalusi wrote:
> The metadata is best described as side band data or parameters traveling
> alongside the data DMAd by the DMA engine. It is data
> which is understood by the peripheral and the peripheral driver only, the
> DMA engine see it only as data block and it is not interpreting it in any
> way.
>
> The metadata can be different per descriptor as it is a parameter for the
> data being transferred.
>
> If the DMA supports per descriptor metadata it can implement the attach,
> get_ptr/set_len callbacks.
>
> Client drivers must only use either attach or get_ptr/set_len to avoid
> miss configuration.
misconfiguration?
> Client driver can check if a given metadata mode is supported by the
> channel during probe time with
> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
>
> and based on this information can use either mode.
>
> Wrappers are also added for the metadata_ops.
>
> To be used in DESC_METADATA_CLIENT mode:
> dmaengine_desc_attach_metadata()
>
> To be used in DESC_METADATA_ENGINE mode:
> dmaengine_desc_get_metadata_ptr()
> dmaengine_desc_set_metadata_len()
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
> ---
> Hi,
>
> Changes since rfc:
> - DESC_METADATA_EMBEDDED renamed to DESC_METADATA_ENGINE
> - Use flow is added for both CLIENT and ENGINE metadata modes
>
> Regards,
> Peter
>
> include/linux/dmaengine.h | 144 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 3db833a8c542..f809635cfeaa 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -231,6 +231,57 @@ typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t;
> * @bytes_transferred: byte counter
> */
>
> +/**
> + * enum dma_desc_metadata_mode - per descriptor metadata mode types supported
> + * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by the
> + * client driver and it is attached (via the dmaengine_desc_attach_metadata()
> + * helper) to the descriptor.
> + *
> + * Client drivers interested to use this mode can follow:
> + * - DMA_MEM_TO_DEV:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * construct the metadata in the clinet's buffer
typo clinet
> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> + * descriptor
> + * 3. submit the transfer
> + * - DMA_DEV_TO_MEM:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. use dmaengine_desc_attach_metadata() to attach the buffer to the
> + * descriptor
> + * 3. submit the transfer
> + * 4. when the transfer is completed, the metadata should be available in the
> + * attached buffer
I guess this is good to be moved into Documentation
also we dont allow this for memcpy txn?
> + *
> + * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
> + * driver. The client driver can ask for the pointer, maximum size and the
> + * currently used size of the metadata and can directly update or read it.
> + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
> + * provided as helper functions.
> + *
> + * Client drivers interested to use this mode can follow:
> + * - DMA_MEM_TO_DEV:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the engine's
> + * metadata area
> + * 3. update the metadata at the pointer
> + * 4. use dmaengine_desc_set_metadata_len() to tell the DMA engine the amount
> + * of data the client has placed into the metadata buffer
> + * 5. submit the transfer
> + * - DMA_DEV_TO_MEM:
> + * 1. prepare the descriptor (dmaengine_prep_*)
> + * 2. submit the transfer
> + * 3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
> + * pointer to the engine's metadata are
> + * 4. Read out the metadate from the pointer
> + *
> + * Note: the two mode is not compatible and clients must use one mode for a
> + * descriptor.
> + */
> +enum dma_desc_metadata_mode {
> + DESC_METADATA_CLIENT = (1 << 0),
> + DESC_METADATA_ENGINE = (1 << 1),
BIT(x)
> +};
> +
> struct dma_chan_percpu {
> /* stats */
> unsigned long memcpy_count;
> @@ -494,6 +545,18 @@ struct dmaengine_unmap_data {
> dma_addr_t addr[0];
> };
>
> +struct dma_async_tx_descriptor;
> +
> +struct dma_descriptor_metadata_ops {
> + int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
> + size_t len);
> +
> + void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
> + size_t *payload_len, size_t *max_len);
> + int (*set_len)(struct dma_async_tx_descriptor *desc,
> + size_t payload_len);
> +};
> +
> /**
> * struct dma_async_tx_descriptor - async transaction descriptor
> * ---dma generic offload fields---
> @@ -523,6 +586,8 @@ struct dma_async_tx_descriptor {
> dma_async_tx_callback_result callback_result;
> void *callback_param;
> struct dmaengine_unmap_data *unmap;
> + enum dma_desc_metadata_mode desc_metadata_mode;
> + struct dma_descriptor_metadata_ops *metadata_ops;
> #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
> struct dma_async_tx_descriptor *next;
> struct dma_async_tx_descriptor *parent;
> @@ -685,6 +750,7 @@ struct dma_filter {
> * @global_node: list_head for global dma_device_list
> * @filter: information for device/slave to filter function/param mapping
> * @cap_mask: one or more dma_capability flags
> + * @desc_metadata_modes: supported metadata modes by the DMA device
> * @max_xor: maximum number of xor sources, 0 if no capability
> * @max_pq: maximum number of PQ sources and PQ-continue capability
> * @copy_align: alignment shift for memcpy operations
> @@ -749,6 +815,7 @@ struct dma_device {
> struct list_head global_node;
> struct dma_filter filter;
> dma_cap_mask_t cap_mask;
> + enum dma_desc_metadata_mode desc_metadata_modes;
> unsigned short max_xor;
> unsigned short max_pq;
> enum dmaengine_alignment copy_align;
> @@ -935,6 +1002,83 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
> len, flags);
> }
>
> +static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
> + enum dma_desc_metadata_mode mode)
> +{
> + return !!(chan->device->desc_metadata_modes & mode);
> +}
> +
> +static inline int _desc_check_and_set_metadata_mode(
why does this need to start with _ ?
> + struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
> +{
> + /* Make sure that the metadata mode is not mixed */
> + if (!desc->desc_metadata_mode) {
> + if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
> + desc->desc_metadata_mode = mode;
> + else
> + return -ENOTSUPP;
> + } else if (desc->desc_metadata_mode != mode) {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static inline int dmaengine_desc_attach_metadata(
> + struct dma_async_tx_descriptor *desc, void *data, size_t len)
> +{
> + int ret;
> +
> + if (!desc)
> + return -EINVAL;
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_CLIENT);
> + if (ret)
> + return ret;
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->attach)
> + return -ENOTSUPP;
> +
> + return desc->metadata_ops->attach(desc, data, len);
> +}
> +
> +static inline void *dmaengine_desc_get_metadata_ptr(
> + struct dma_async_tx_descriptor *desc, size_t *payload_len,
> + size_t *max_len)
> +{
> + int ret;
> +
> + if (!desc)
> + return ERR_PTR(-EINVAL);
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
> + return ERR_PTR(-ENOTSUPP);
> +
> + return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
> +}
> +
> +static inline int dmaengine_desc_set_metadata_len(
> + struct dma_async_tx_descriptor *desc, size_t payload_len)
> +{
> + int ret;
> +
> + if (!desc)
> + return -EINVAL;
> +
> + ret = _desc_check_and_set_metadata_mode(desc, DESC_METADATA_ENGINE);
> + if (ret)
> + return ret;
> +
> + if (!desc->metadata_ops || !desc->metadata_ops->set_len)
> + return -ENOTSUPP;
> +
> + return desc->metadata_ops->set_len(desc, payload_len);
> +}
thats bit too much code for a header file :( Lets move it to C file
please. We can utilize local dmaengine.h and not expose all these and
possible misuse by clients
Also I would like to see a use :-) before further comments.
--
~Vinod
