RE: [EXTERNAL] Re: [RFC v0 1/1] dmadev: provide priority configuration support

[Vamsi Krishna Attunuru <vattunuru@marvell.com>]

>-----Original Message-----
>From: fengchengwen <fengchengwen@huawei.com>
>Sent: Saturday, September 14, 2024 3:12 PM
>To: Vamsi Krishna Attunuru <vattunuru@marvell.com>;
>thomas@monjalon.net; bruce.richardson@intel.com;
>mb@smartsharesystems.com
>Cc: dev@dpdk.org; kevin.laatz@intel.com; Jerin Jacob <jerinj@marvell.com>;
>conor.walsh@intel.com; Gowrishankar Muthukrishnan
><gmuthukrishn@marvell.com>; Vidya Sagar Velumuri
><vvelumuri@marvell.com>; g.singh@nxp.com; sachin.saxena@oss.nxp.com;
>hemant.agrawal@nxp.com; Amit Prakash Shukla
><amitprakashs@marvell.com>
>Subject: [EXTERNAL] Re: [RFC v0 1/1] dmadev: provide priority configuration
>support
>
>On 2024/9/13 20: 10, Vamsi Krishna wrote: > From: Vamsi Attunuru
><vattunuru@ marvell. com> > > Some DMA controllers offer the ability to
>configure priority level > for the hardware command queues, allowing for the
>prioritization 
>On 2024/9/13 20:10, Vamsi Krishna wrote:
>> From: Vamsi Attunuru <vattunuru@marvell.com>
>>
>> Some DMA controllers offer the ability to configure priority level for
>> the hardware command queues, allowing for the prioritization of DMA
>> command execution based on queue importance.
>>
>> This patch introduces the necessary fields in the dmadev structures to
>> retrieve information about the hardware-supported priority levels and
>> to enable priority configuration from the application.
>>
>> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
>> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
>> ---
>>
>> Deprecation notice:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patches.dpdk.org_
>> project_dpdk_patch_20240730144612.2132848-2D1-2Damitprakashs-
>40marvell
>>
>.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=WllrYaumVkxaWjgKto6E
>_rtDQshh
>>
>Ihik2jkvzFyRhW8&m=c5golEA5oYRC2qs5aUbwKAGfIFWAsjeOW4OaQaRl9pE11
>PNXlmhg
>> 2MEnn4OYFN8g&s=3XTQJ_S2k_9fzXvPXLOYdj4VFExnzZ04_uiB1qBaFv0&e=
>>
>> * Assuming we do not anticipate any advanced scheduling schemes for
>dmadev queues,
>>   this RFC is intended to support a strict prioirty scheme.
>>
>>  doc/guides/rel_notes/release_24_11.rst | 15 +++------------
>>  lib/dmadev/rte_dmadev.c                | 10 ++++++++++
>>  lib/dmadev/rte_dmadev.h                | 11 +++++++++++
>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_24_11.rst
>> b/doc/guides/rel_notes/release_24_11.rst
>> index 0ff70d9057..dc711bbf98 100644
>> --- a/doc/guides/rel_notes/release_24_11.rst
>> +++ b/doc/guides/rel_notes/release_24_11.rst
>> @@ -88,18 +88,9 @@ API Changes
>>  ABI Changes
>>  -----------
>>
>> -.. This section should contain ABI changes. Sample format:
>> -
>> -   * sample: Add a short 1-2 sentence description of the ABI change
>> -     which was announced in the previous releases and made in this release.
>> -     Start with a scope label like "ethdev:".
>> -     Use fixed width quotes for ``function_names`` or ``struct_names``.
>> -     Use the past tense.
>> -
>> -   This section is a comment. Do not overwrite or remove it.
>> -   Also, make sure to start the actual text at the margin.
>> -   =======================================================
>> -
>> +* dmadev: Added ``nb_priorities`` field to ``rte_dma_info`` structure
>> +and
>> +  ``priority`` field to ``rte_dma_conf`` structure to get device
>> +supported
>> +  priority levels and configure required priority from the application.
>>
>>  Known Issues
>>  ------------
>> diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c index
>> 845727210f..053170f55a 100644
>> --- a/lib/dmadev/rte_dmadev.c
>> +++ b/lib/dmadev/rte_dmadev.c
>> @@ -497,6 +497,16 @@ rte_dma_configure(int16_t dev_id, const struct
>rte_dma_conf *dev_conf)
>>  		return -EINVAL;
>>  	}
>>
>> +	if (dev_info.nb_priorities == 1) {
>> +		RTE_DMA_LOG(ERR, "Device %d must support more than 1
>priority, or else 0", dev_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (dev_info.nb_priorities && (dev_conf->priority >=
>dev_info.nb_priorities)) {
>> +		RTE_DMA_LOG(ERR, "Device %d configure invalid priority",
>dev_id);
>> +		return -EINVAL;
>> +	}
>
>Please add trace support

Sure, will add it.

>
>> +
>>  	if (*dev->dev_ops->dev_configure == NULL)
>>  		return -ENOTSUP;
>>  	ret = (*dev->dev_ops->dev_configure)(dev, dev_conf, diff --git
>> a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
>> 5474a5281d..d59fcf1b61 100644
>> --- a/lib/dmadev/rte_dmadev.h
>> +++ b/lib/dmadev/rte_dmadev.h
>> @@ -297,6 +297,10 @@ struct rte_dma_info {
>>  	int16_t numa_node;
>>  	/** Number of virtual DMA channel configured. */
>>  	uint16_t nb_vchans;
>> +	/** Number of priority levels (must be > 1), if supported by DMA HW
>channel.
>> +	 * 0 otherwise.
>> +	 */
>> +	uint16_t nb_priorities;
>
>Suggest add priority schedule policy capability, for example:
>RTE_DMA_CAPA_PRIORITY_POLICY_SP

Sure, since strict priority is only supported and not explicitly defined as a capability. Will add it as capability.
>
>>  };
>>
>>  /**
>> @@ -332,6 +336,13 @@ struct rte_dma_conf {
>>  	 * @see RTE_DMA_CAPA_SILENT
>>  	 */
>>  	bool enable_silent;
>> +	/* The prioirty of the DMA HW channel.
>> +	 * This value cannot be greater than or equal to the field 'nb_priorities'
>> +	 * of struct rte_dma_info which get from rte_dma_info_get().
>> +	 * Among the values between '0' and 'nb_priorities - 1', lowest value
>> +	 * indicates higher priority and vice-versa.
>> +	 */
>> +	uint16_t priority;
>>  };
>>
>>  /**