DPDK patches and discussions
 help / color / mirror / Atom feed
From: fengchengwen <fengchengwen@huawei.com>
To: Amit Prakash Shukla <amitprakashs@marvell.com>,
	Kevin Laatz <kevin.laatz@intel.com>,
	Bruce Richardson <bruce.richardson@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	"mb@smartsharesystems.com" <mb@smartsharesystems.com>,
	"conor.walsh@intel.com" <conor.walsh@intel.com>,
	Vamsi Krishna Attunuru <vattunuru@marvell.com>,
	"g.singh@nxp.com" <g.singh@nxp.com>,
	"sachin.saxena@oss.nxp.com" <sachin.saxena@oss.nxp.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
	"cheng1.jiang@intel.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
Date: Mon, 9 Oct 2023 16:59:04 +0800	[thread overview]
Message-ID: <45664b7f-4f85-8c5b-6e24-b4d0d1fcc2a7@huawei.com> (raw)
In-Reply-To: <PH0PR18MB5167290476A812F3F7DB90E6C8CEA@PH0PR18MB5167.namprd18.prod.outlook.com>

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)

  reply	other threads:[~2023-10-09  8:59 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-07  8:10 [PATCH v1 0/2] offload support to free dma " Amit Prakash Shukla
2023-09-07  8:10 ` [PATCH v1 1/2] dmadev: offload to free " Amit Prakash Shukla
2023-09-07  9:00   ` Amit Prakash Shukla
2023-09-18 11:12   ` Anoob Joseph
2023-09-07  8:10 ` [PATCH v1 2/2] test/dma: add source buffer offload free test Amit Prakash Shukla
2023-09-19 11:48   ` Anoob Joseph
2023-09-26  8:11     ` Amit Prakash Shukla
2023-09-26 12:17 ` [PATCH v2 0/2] offload support to free dma source buffer Amit Prakash Shukla
2023-09-26 12:17   ` [PATCH v2 1/2] dmadev: offload to free " Amit Prakash Shukla
2023-09-26 12:17   ` [PATCH v2 2/2] test/dma: add source buffer offload free test Amit Prakash Shukla
2023-09-28 11:50   ` [PATCH v3 0/2] offload support to free dma source buffer Amit Prakash Shukla
2023-09-28 11:50     ` [PATCH v3 1/2] dmadev: offload to free " Amit Prakash Shukla
2023-10-07  9:00       ` fengchengwen
2023-10-09  7:00         ` [EXT] " Amit Prakash Shukla
2023-10-09  8:59           ` fengchengwen [this message]
2023-10-09 12:01             ` Amit Prakash Shukla
2023-09-28 11:50     ` [PATCH v3 2/2] test/dma: add source buffer offload free test Amit Prakash Shukla
2023-09-28 12:08       ` Anoob Joseph
2023-10-07  9:32       ` fengchengwen
2023-10-09  7:07         ` [EXT] " Amit Prakash Shukla
2023-09-28 11:59     ` [PATCH v3 0/2] offload support to free dma source buffer Anoob Joseph
2023-10-05  7:21       ` Amit Prakash Shukla
2023-10-09 12:02     ` [PATCH v4 0/2] offload support to auto free dma buffer Amit Prakash Shukla
2023-10-09 12:02       ` [PATCH v4 1/2] dmadev: offload to auto free DMA buffer Amit Prakash Shukla
2023-10-10  1:40         ` fengchengwen
2023-10-16 11:28           ` [EXT] " Amit Prakash Shukla
2023-10-09 12:02       ` [PATCH v4 2/2] test/dma: auto free offload test to " Amit Prakash Shukla
2023-10-17  8:47       ` [PATCH v4 0/2] offload support to auto free dma buffer Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45664b7f-4f85-8c5b-6e24-b4d0d1fcc2a7@huawei.com \
    --to=fengchengwen@huawei.com \
    --cc=amitprakashs@marvell.com \
    --cc=anoobj@marvell.com \
    --cc=bruce.richardson@intel.com \
    --cc=cheng1.jiang@intel.com \
    --cc=conor.walsh@intel.com \
    --cc=dev@dpdk.org \
    --cc=g.singh@nxp.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=kevin.laatz@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=ndabilpuram@marvell.com \
    --cc=sachin.saxena@oss.nxp.com \
    --cc=vattunuru@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).