[v3,1/2] dmadev: offload to free source buffer
Checks
Commit Message
This changeset adds support in DMA library to free source DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config when the DMA transfer direction is
configured as RTE_DMA_DIR_MEM_TO_DEV.
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
Comments
Hi Amit,
On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> This changeset adds support in DMA library to free source DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config when the DMA transfer direction is
> configured as RTE_DMA_DIR_MEM_TO_DEV.
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> ---
> lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
> index b157ab7600..f7a6af2528 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
> #define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33)
> /** Support fill operation. */
> #define RTE_DMA_CAPA_OPS_FILL RTE_BIT64(34)
> +/** Support for source buffer free for mem to dev transfer.
Support auto free source buffer once the M2D (memory-to-device) transfer completed.
> + *
> + * @note Even though the DMA driver has this capability, it may not support all
> + * mempool drivers. If the mempool is not supported by the DMA driver,
> + * rte_dma_vchan_setup() will fail.
In addition to hardware support, there are requirements for buffer attribute (e.g. only specific mempool support it).
> + **/
> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE RTE_BIT64(35)
1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not new OPS.
2) the name is too long. how abort RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better name.
> /**@}*/
>
> /**
> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
> * @see struct rte_dma_port_param
> */
> struct rte_dma_port_param dst_port;
> + /** Mempool from which source buffer is allocated. Mempool info is used
> + * for freeing source buffer by hardware when configured direction is
> + * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
> + * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
> + * rte_dma_copy_sg().
> + *
> + * @note If the mempool is not supported by the DMA device,
> + * rte_dma_vchan_setup() will fail.
> + *
> + * @see RTE_DMA_OP_FLAG_FREE_SBUF
> + */
> + struct rte_mempool *mem_to_dev_src_buf_pool;
> +
> };
Suggest add one extra struct e.g.
struct rte_dma_auto_free_param {
union {
struct rte_mempool *pool;
}
uint64_t reserved[2]; /**< Reserved for future fields. */
};
In the above conf, we could add a new field: struct rte_dma_auto_free_param m2d_buf
>
> /**
> @@ -818,6 +838,13 @@ struct rte_dma_sge {
> * capability bit for this, driver should not return error if this flag was set.
> */
> #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
> +/** Mem to dev source buffer free flag.
> + * Used for freeing source DMA buffer by hardware when the transfer direction is
> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> + *
> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> + */
> +#define RTE_DMA_OP_FLAG_FREE_SBUF RTE_BIT64(3)
Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF
> /**@}*/
>
> /**
>
The S in SBUF seem useless, because it should not auto free dstbuf in logically.
Maybe we should direct use auto-free (just like silent)
Hi Chengwen,
Thanks for the review and feedback. Please find my reply in-line.
Thanks,
Amit Shukla
> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Saturday, October 7, 2023 2:31 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Kevin Laatz
> <kevin.laatz@intel.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> mb@smartsharesystems.com; conor.walsh@intel.com; Vamsi Krishna
> Attunuru <vattunuru@marvell.com>; g.singh@nxp.com;
> sachin.saxena@oss.nxp.com; hemant.agrawal@nxp.com;
> cheng1.jiang@intel.com; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Anoob Joseph <anoobj@marvell.com>
> Subject: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Amit,
>
> On 2023/9/28 19:50, Amit Prakash Shukla wrote:
> > This changeset adds support in DMA library to free source DMA buffer
> > by hardware. On a supported hardware, application can pass on the
> > mempool information as part of vchan config when the DMA transfer
> > direction is configured as RTE_DMA_DIR_MEM_TO_DEV.
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> > lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
> > b157ab7600..f7a6af2528 100644
> > --- a/lib/dmadev/rte_dmadev.h
> > +++ b/lib/dmadev/rte_dmadev.h
> > @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
> > #define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33)
> > /** Support fill operation. */
> > #define RTE_DMA_CAPA_OPS_FILL RTE_BIT64(34)
> > +/** Support for source buffer free for mem to dev transfer.
>
> Support auto free source buffer once the M2D (memory-to-device) transfer
> completed.
Ack
>
> > + *
> > + * @note Even though the DMA driver has this capability, it may not
> > + support all
> > + * mempool drivers. If the mempool is not supported by the DMA
> > + driver,
> > + * rte_dma_vchan_setup() will fail.
>
> In addition to hardware support, there are requirements for buffer attribute
> (e.g. only specific mempool support it).
>
> > + **/
> > +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
> RTE_BIT64(35)
>
> 1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not
> new OPS.
> 2) the name is too long. how abort
> RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better
> name.
Ack.
>
> > /**@}*/
> >
> > /**
> > @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
> > * @see struct rte_dma_port_param
> > */
> > struct rte_dma_port_param dst_port;
> > + /** Mempool from which source buffer is allocated. Mempool info is
> used
> > + * for freeing source buffer by hardware when configured direction is
> > + * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
> hardware,
> > + * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
> rte_dma_copy and
> > + * rte_dma_copy_sg().
> > + *
> > + * @note If the mempool is not supported by the DMA device,
> > + * rte_dma_vchan_setup() will fail.
> > + *
> > + * @see RTE_DMA_OP_FLAG_FREE_SBUF
> > + */
> > + struct rte_mempool *mem_to_dev_src_buf_pool;
> > +
> > };
>
> Suggest add one extra struct e.g.
> struct rte_dma_auto_free_param {
> union {
> struct rte_mempool *pool;
> }
> uint64_t reserved[2]; /**< Reserved for future fields. */ };
>
> In the above conf, we could add a new field: struct
> rte_dma_auto_free_param m2d_buf
Ack, will add new struct.
struct rte_dma_auto_free_buf_param {
struct rte_mempool *pool;
uint64_t reserved[2]; /**< Reserved for future fields. */ };
};
struct rte_dma_auto_free_buf_param m2d_sbuf;
>
> >
> > /**
> > @@ -818,6 +838,13 @@ struct rte_dma_sge {
> > * capability bit for this, driver should not return error if this flag was set.
> > */
> > #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
> > +/** Mem to dev source buffer free flag.
> > + * Used for freeing source DMA buffer by hardware when the transfer
> > +direction is
> > + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> > + *
> > + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> > + */
> > +#define RTE_DMA_OP_FLAG_FREE_SBUF RTE_BIT64(3)
>
> Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF
Ack
>
> > /**@}*/
> >
> > /**
> >
>
> The S in SBUF seem useless, because it should not auto free dstbuf in
> logically.
>
> Maybe we should direct use auto-free (just like silent)
Hi Amit,
Rethink again, I suggest use auto-free to denote this feature.
So we could re-define as:
RTE_DMA_CAPA_M2D_AUTO_FREE
RTE_DMA_OP_FLAG_AUTO_FREE
struct rte_dma_auto_free_param {
union {
struct {
struct rte_mempool *pool;
} m2d;
};
uint64_t reserved[2]; /**< Reserved for future fields. */ };
};
note: because one vchan only support one DMA direction, so we could use union.
struct rte_dma_vchan_conf {
enum rte_dma_direction direction;
struct rte_dma_port_param src_port;
struct rte_dma_port_param dst_port;
struct rte_dma_auto_free_param auto_free;
};
As for later extension, if support other type auto-free, so could:
1. add one capa: RTE_DMA_CAPA_XXX_AUTO_FREE
2. modify rte_dma_auto_free_param:
struct rte_dma_auto_free_param {
union {
struct {
struct rte_mempool *pool;
} m2d;
struct {
// params;
} xxx;
};
uint64_t reserved[2]; /**< Reserved for future fields. */ };
};
Thanks.
On 2023/10/9 15:00, Amit Prakash Shukla wrote:
> Hi Chengwen,
>
> Thanks for the review and feedback. Please find my reply in-line.
>
> Thanks,
> Amit Shukla
>
>> -----Original Message-----
>> From: fengchengwen <fengchengwen@huawei.com>
>> Sent: Saturday, October 7, 2023 2:31 PM
>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Kevin Laatz
>> <kevin.laatz@intel.com>; Bruce Richardson <bruce.richardson@intel.com>
>> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> mb@smartsharesystems.com; conor.walsh@intel.com; Vamsi Krishna
>> Attunuru <vattunuru@marvell.com>; g.singh@nxp.com;
>> sachin.saxena@oss.nxp.com; hemant.agrawal@nxp.com;
>> cheng1.jiang@intel.com; Nithin Kumar Dabilpuram
>> <ndabilpuram@marvell.com>; Anoob Joseph <anoobj@marvell.com>
>> Subject: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> Hi Amit,
>>
>> On 2023/9/28 19:50, Amit Prakash Shukla wrote:
>>> This changeset adds support in DMA library to free source DMA buffer
>>> by hardware. On a supported hardware, application can pass on the
>>> mempool information as part of vchan config when the DMA transfer
>>> direction is configured as RTE_DMA_DIR_MEM_TO_DEV.
>>>
>>> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> Acked-by: Anoob Joseph <anoobj@marvell.com>
>>> ---
>>> lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++
>>> 1 file changed, 27 insertions(+)
>>>
>>> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
>>> b157ab7600..f7a6af2528 100644
>>> --- a/lib/dmadev/rte_dmadev.h
>>> +++ b/lib/dmadev/rte_dmadev.h
>>> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>>> #define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33)
>>> /** Support fill operation. */
>>> #define RTE_DMA_CAPA_OPS_FILL RTE_BIT64(34)
>>> +/** Support for source buffer free for mem to dev transfer.
>>
>> Support auto free source buffer once the M2D (memory-to-device) transfer
>> completed.
>
> Ack
>
>>
>>> + *
>>> + * @note Even though the DMA driver has this capability, it may not
>>> + support all
>>> + * mempool drivers. If the mempool is not supported by the DMA
>>> + driver,
>>> + * rte_dma_vchan_setup() will fail.
>>
>> In addition to hardware support, there are requirements for buffer attribute
>> (e.g. only specific mempool support it).
>>
>>> + **/
>>> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
>> RTE_BIT64(35)
>>
>> 1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not
>> new OPS.
>> 2) the name is too long. how abort
>> RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better
>> name.
>
> Ack.
>
>>
>>> /**@}*/
>>>
>>> /**
>>> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>>> * @see struct rte_dma_port_param
>>> */
>>> struct rte_dma_port_param dst_port;
>>> + /** Mempool from which source buffer is allocated. Mempool info is
>> used
>>> + * for freeing source buffer by hardware when configured direction is
>>> + * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
>> hardware,
>>> + * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
>> rte_dma_copy and
>>> + * rte_dma_copy_sg().
>>> + *
>>> + * @note If the mempool is not supported by the DMA device,
>>> + * rte_dma_vchan_setup() will fail.
>>> + *
>>> + * @see RTE_DMA_OP_FLAG_FREE_SBUF
>>> + */
>>> + struct rte_mempool *mem_to_dev_src_buf_pool;
>>> +
>>> };
>>
>> Suggest add one extra struct e.g.
>> struct rte_dma_auto_free_param {
>> union {
>> struct rte_mempool *pool;
>> }
>> uint64_t reserved[2]; /**< Reserved for future fields. */ };
>>
>> In the above conf, we could add a new field: struct
>> rte_dma_auto_free_param m2d_buf
>
> Ack, will add new struct.
> struct rte_dma_auto_free_buf_param {
> struct rte_mempool *pool;
> uint64_t reserved[2]; /**< Reserved for future fields. */ };
> };
>
> struct rte_dma_auto_free_buf_param m2d_sbuf;
>
>>
>>>
>>> /**
>>> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>>> * capability bit for this, driver should not return error if this flag was set.
>>> */
>>> #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
>>> +/** Mem to dev source buffer free flag.
>>> + * Used for freeing source DMA buffer by hardware when the transfer
>>> +direction is
>>> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
>>> + *
>>> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
>>> + */
>>> +#define RTE_DMA_OP_FLAG_FREE_SBUF RTE_BIT64(3)
>>
>> Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF
>
> Ack
>
>>
>>> /**@}*/
>>>
>>> /**
>>>
>>
>> The S in SBUF seem useless, because it should not auto free dstbuf in
>> logically.
>>
>> Maybe we should direct use auto-free (just like silent)
Hi Chengwen,
Ack, I will make the changes in next version of the patch.
Thanks,
Amit Shukla
> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Monday, October 9, 2023 2:29 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Kevin Laatz
> <kevin.laatz@intel.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> mb@smartsharesystems.com; conor.walsh@intel.com; Vamsi Krishna
> Attunuru <vattunuru@marvell.com>; g.singh@nxp.com;
> sachin.saxena@oss.nxp.com; hemant.agrawal@nxp.com;
> cheng1.jiang@intel.com; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Anoob Joseph <anoobj@marvell.com>
> Subject: Re: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer
>
> Hi Amit,
>
> Rethink again, I suggest use auto-free to denote this feature.
>
> So we could re-define as:
> RTE_DMA_CAPA_M2D_AUTO_FREE
> RTE_DMA_OP_FLAG_AUTO_FREE
>
> struct rte_dma_auto_free_param {
> union {
> struct {
> struct rte_mempool *pool;
> } m2d;
> };
> uint64_t reserved[2]; /**< Reserved for future fields. */ }; };
> note: because one vchan only support one DMA direction, so we could use
> union.
>
> struct rte_dma_vchan_conf {
> enum rte_dma_direction direction;
> struct rte_dma_port_param src_port;
> struct rte_dma_port_param dst_port;
> struct rte_dma_auto_free_param auto_free; };
>
>
> As for later extension, if support other type auto-free, so could:
> 1. add one capa: RTE_DMA_CAPA_XXX_AUTO_FREE 2. modify
> rte_dma_auto_free_param:
> struct rte_dma_auto_free_param {
> union {
> struct {
> struct rte_mempool *pool;
> } m2d;
> struct {
> // params;
> } xxx;
> };
> uint64_t reserved[2]; /**< Reserved for future fields. */ };
> };
>
>
> Thanks.
>
<snip>
@@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
#define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33)
/** Support fill operation. */
#define RTE_DMA_CAPA_OPS_FILL RTE_BIT64(34)
+/** Support for source buffer free for mem to dev transfer.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ **/
+#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE RTE_BIT64(35)
/**@}*/
/**
@@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
* @see struct rte_dma_port_param
*/
struct rte_dma_port_param dst_port;
+ /** Mempool from which source buffer is allocated. Mempool info is used
+ * for freeing source buffer by hardware when configured direction is
+ * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
+ * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
+ * rte_dma_copy_sg().
+ *
+ * @note If the mempool is not supported by the DMA device,
+ * rte_dma_vchan_setup() will fail.
+ *
+ * @see RTE_DMA_OP_FLAG_FREE_SBUF
+ */
+ struct rte_mempool *mem_to_dev_src_buf_pool;
+
};
/**
@@ -818,6 +838,13 @@ struct rte_dma_sge {
* capability bit for this, driver should not return error if this flag was set.
*/
#define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
+/** Mem to dev source buffer free flag.
+ * Used for freeing source DMA buffer by hardware when the transfer direction is
+ * configured as RTE_DMA_DIR_MEM_TO_DEV.
+ *
+ * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
+ */
+#define RTE_DMA_OP_FLAG_FREE_SBUF RTE_BIT64(3)
/**@}*/
/**