| Message ID | 20180815105704.26498-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor | expand |
> -----Original Message----- > From: Peter Ujfalusi <peter.ujfalusi@ti.com> > Sent: Wednesday, August 15, 2018 4:27 PM > To: dan.j.williams@intel.com; vkoul@kernel.org > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; > lars@metafoo.de; Radhey Shyam Pandey <radheys@xilinx.com> > Subject: [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor > > 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. > > 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_EMBEDDED); > > 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_EMBEDDED mode: > dmaengine_desc_get_metadata_ptr() > dmaengine_desc_set_metadata_len() > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > include/linux/dmaengine.h | 112 > ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 112 insertions(+) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 3db833a8c542..2200f8985adf 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -231,6 +231,25 @@ 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. > + * @DESC_METADATA_EMBEDDED - the metadata buffer is > allocated/managed by the DMA Just a thought - We can rename it to DESC_METADATA_ENGINE? i.e metadata allocation place - > dma client/engine. > + * driver. The client driver can ask for the pointer, maximum size and the > + * currently used size of the metadata and can directly updata or read it. /s/updata/update > + * dmaengine_desc_get_metadata_ptr() and > dmaengine_desc_set_metadata_len() is > + * provided as helper functions. It will be helpful if we add description for both DESC_METADATA_EMBEDDED modes i.e DMA_DEV_TO_MEM and MEM_TO_DEV types. I think in DEV_TO_MEM we don't need to set_metadata_len(). Length will provided by DMA engine. > + * > + * 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_EMBEDDED = (1 << 1), > +}; > + > struct dma_chan_percpu { > /* stats */ > unsigned long memcpy_count; > @@ -494,6 +513,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 +554,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 +718,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 +783,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 +970,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( > + 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) { Minor nit - we can refactor this code to have failure path early. > + 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_EMBEDDED); > + 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_EMBEDDED); > + if (ret) > + return ret; > + > + if (!desc->metadata_ops || !desc->metadata_ops->set_len) > + return -ENOTSUPP; > + > + return desc->metadata_ops->set_len(desc, payload_len); > +} > + > /** > * dmaengine_terminate_all() - Terminate all active DMA transfers > * @chan: The channel for which to terminate the transfers > -- > Peter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Radhey, On 2018-08-16 16:29, Radhey Shyam Pandey wrote: >> +/** >> + * 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. >> + * @DESC_METADATA_EMBEDDED - the metadata buffer is >> allocated/managed by the DMA > Just a thought - We can rename it to DESC_METADATA_ENGINE? > i.e metadata allocation place - > dma client/engine. Sounds good. > >> + * driver. The client driver can ask for the pointer, maximum size and the >> + * currently used size of the metadata and can directly updata or read it. > /s/updata/update > >> + * dmaengine_desc_get_metadata_ptr() and >> dmaengine_desc_set_metadata_len() is >> + * provided as helper functions. > It will be helpful if we add description for both DESC_METADATA_EMBEDDED > modes i.e DMA_DEV_TO_MEM and MEM_TO_DEV types. I think in DEV_TO_MEM > we don't need to set_metadata_len(). Length will provided by DMA engine. I agree, it is better to extend the explanation on the two modes. ... >> +static inline int _desc_check_and_set_metadata_mode( >> + 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) { > Minor nit - we can refactor this code to have failure path early. I don't think it would help readability, to move the failure case first: if (desc->desc_metadata_mode && (desc->desc_metadata_mode != mode)) { return -EINVAL; } else if (!desc->desc_metadata_mode) { if (dmaengine_is_metadata_mode_supported(desc->chan, mode)) desc->desc_metadata_mode = mode; else return -ENOTSUPP; } return 0; > >> + 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; >> +} - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 3db833a8c542..2200f8985adf 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -231,6 +231,25 @@ 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. + * @DESC_METADATA_EMBEDDED - 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 updata or read it. + * dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is + * provided as helper functions. + * + * 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_EMBEDDED = (1 << 1), +}; + struct dma_chan_percpu { /* stats */ unsigned long memcpy_count; @@ -494,6 +513,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 +554,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 +718,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 +783,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 +970,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( + 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_EMBEDDED); + 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_EMBEDDED); + if (ret) + return ret; + + if (!desc->metadata_ops || !desc->metadata_ops->set_len) + return -ENOTSUPP; + + return desc->metadata_ops->set_len(desc, payload_len); +} + /** * dmaengine_terminate_all() - Terminate all active DMA transfers * @chan: The channel for which to terminate the transfers
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. 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_EMBEDDED); 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_EMBEDDED mode: dmaengine_desc_get_metadata_ptr() dmaengine_desc_set_metadata_len() Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- include/linux/dmaengine.h | 112 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)