[v2] lib/dmadev: get DMA device using device ID
Checks
Commit Message
DMA library has a function to get DMA device based on device name but
there is no function to get DMA device using device id.
Added a function that lookup for the dma device using device id and
returns the pointer to the same.
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
v2:
- Arranged api in alphabetical order in version.map
lib/dmadev/rte_dmadev.c | 9 +++++++++
lib/dmadev/rte_dmadev_pmd.h | 14 ++++++++++++++
lib/dmadev/version.map | 1 +
3 files changed, 24 insertions(+)
Comments
>
> DMA library has a function to get DMA device based on device name but
> there is no function to get DMA device using device id.
>
> Added a function that lookup for the dma device using device id and returns
> the pointer to the same.
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
Hi Thomas,
Gentle ping.
Could you please consider merging this patch. The driver series https://patches.dpdk.org/project/dpdk/patch/20231208082835.2817601-3-amitprakashs@marvell.com/ is dependent on this patch.
Thanks,
Amit Shukla
> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Monday, January 8, 2024 8:17 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Chengwen Feng
> <fengchengwen@huawei.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
> Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Vamsi
> Krishna Attunuru <vattunuru@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; mb@smartsharesystems.com; Amit Prakash
> Shukla <amitprakashs@marvell.com>
> Subject: RE: [PATCH v2] lib/dmadev: get DMA device using device ID
>
> >
> > DMA library has a function to get DMA device based on device name but
> > there is no function to get DMA device using device id.
> >
> > Added a function that lookup for the dma device using device id and
> > returns the pointer to the same.
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
>
19/12/2023 12:00, Amit Prakash Shukla:
> +struct rte_dma_dev *
> +rte_dma_pmd_get_dev_by_id(const int dev_id)
const does not make sense here for an int parameter.
> +{
> + if (!rte_dma_is_valid(dev_id))
> + return NULL;
> +
> + return &rte_dma_devices[dev_id];
> +}
[...]
> +/**
> + * @internal
> + * Get the rte_dma_dev structure device pointer for the device id.
> + *
> + * @param dev_id
> + * Device ID value to select the device structure.
This comment is not explanatory.
What is an ID? Where does it come from?
Where can we see such ID for DMA device?
> + *
> + * @return
> + * - rte_dma_dev structure pointer for the given device ID on success, NULL
> + * otherwise.
> + */
> +__rte_internal
> +struct rte_dma_dev *rte_dma_pmd_get_dev_by_id(const int dev_id);
Again, const does not make sense here.
Chengwen, please can you comment this patch as you maintain dmadev?
Hi Thomas,
On 2024/2/9 0:25, Thomas Monjalon wrote:
> 19/12/2023 12:00, Amit Prakash Shukla:
>> +struct rte_dma_dev *
>> +rte_dma_pmd_get_dev_by_id(const int dev_id)
> const does not make sense here for an int parameter.
I think it could be OK with const even if the parameter is not pointer.
However, most DPDK APIs do not have const for simple types (e.g.
int/uin16_t).
In this aspect, I think it's also OK to remove const of here for
consistency.
>
>> +{
>> + if (!rte_dma_is_valid(dev_id))
>> + return NULL;
>> +
>> + return &rte_dma_devices[dev_id];
>> +}
> [...]
>> +/**
>> + * @internal
>> + * Get the rte_dma_dev structure device pointer for the device id.
>> + *
>> + * @param dev_id
>> + * Device ID value to select the device structure.
> This comment is not explanatory.
> What is an ID? Where does it come from?
> Where can we see such ID for DMA device?
This new API is used in the event-dma driver of cnxk [1]:
The rte_event_dma_adapter_vchan_add has parameter of dma_dev_id, and it then
invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id, vchan,
event),
at cnxk driver, this ops will check whether the DMA is
cnxk_dmadev_pci_driver.
I think this is because the cnxk's event-and-dma implement has deep coupling
(because the cnxk's event device could interact with another vendor's
dma device).
Maybe we should think of a better way to solve this kind of coupling
problem.
Thanks
[1]
https://patches.dpdk.org/project/dpdk/patch/20231208082835.2817601-3-amitprakashs@marvell.com/
>
>> + *
>> + * @return
>> + * - rte_dma_dev structure pointer for the given device ID on success, NULL
>> + * otherwise.
>> + */
>> +__rte_internal
>> +struct rte_dma_dev *rte_dma_pmd_get_dev_by_id(const int dev_id);
> Again, const does not make sense here.
>
> Chengwen, please can you comment this patch as you maintain dmadev?
>
>
Hi Thomas and Chengwen,
Thank you for the review and feedback. Please find my comment in-line.
Thanks,
Amit Shukla
> ----------------------------------------------------------------------
> Hi Thomas,
>
>
> On 2024/2/9 0:25, Thomas Monjalon wrote:
> > 19/12/2023 12:00, Amit Prakash Shukla:
> >> +struct rte_dma_dev *
> >> +rte_dma_pmd_get_dev_by_id(const int dev_id)
> > const does not make sense here for an int parameter.
>
>
> I think it could be OK with const even if the parameter is not pointer.
>
> However, most DPDK APIs do not have const for simple types (e.g.
> int/uin16_t).
>
> In this aspect, I think it's also OK to remove const of here for consistency.
>
>
> >
> >> +{
> >> + if (!rte_dma_is_valid(dev_id))
> >> + return NULL;
> >> +
> >> + return &rte_dma_devices[dev_id];
> >> +}
> > [...]
> >> +/**
> >> + * @internal
> >> + * Get the rte_dma_dev structure device pointer for the device id.
> >> + *
> >> + * @param dev_id
> >> + * Device ID value to select the device structure.
> > This comment is not explanatory.
> > What is an ID? Where does it come from?
> > Where can we see such ID for DMA device?
>
> This new API is used in the event-dma driver of cnxk [1]:
>
> The rte_event_dma_adapter_vchan_add has parameter of dma_dev_id, and it
> then
>
> invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id, vchan,
> event),
>
> at cnxk driver, this ops will check whether the DMA is
> cnxk_dmadev_pci_driver.
>
> I think this is because the cnxk's event-and-dma implement has deep coupling
>
> (because the cnxk's event device could interact with another vendor's
> dma device).
>
>
> Maybe we should think of a better way to solve this kind of coupling
> problem.
Id, is the DMA dev id which is used in looking up DMA dev. This API is in-line with the other libraries.
Crypto library has an api rte_cryptodev_pmd_get_dev to get crypto device based on device id.
>
>
> Thanks
>
>
> [1]
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patches.dpdk.org_project_dpdk_patch_20231208082835.2817601-
> 2D3-2Damitprakashs-
> 40marvell.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgF
> GR69VnJLdSnADun7zLaXG1p5Rs7pXihE&m=b0t1Tduh89vXiJ7GQG0eb_yIMN
> k2aEkmL2losNL5qiqNycAqWUi7kLJRmmOunpvy&s=oy9mqDXsDKhXuVG9iy7
> SLmU_shlYhfjrglvjEoEQ4AM&e=
>
> >
> >> + *
> >> + * @return
> >> + * - rte_dma_dev structure pointer for the given device ID on success,
> NULL
> >> + * otherwise.
> >> + */
> >> +__rte_internal
> >> +struct rte_dma_dev *rte_dma_pmd_get_dev_by_id(const int dev_id);
> > Again, const does not make sense here.
> >
> > Chengwen, please can you comment this patch as you maintain dmadev?
> >
> >
09/02/2024 12:05, Amit Prakash Shukla:
> > On 2024/2/9 0:25, Thomas Monjalon wrote:
> > > 19/12/2023 12:00, Amit Prakash Shukla:
> > >> +struct rte_dma_dev *
> > >> +rte_dma_pmd_get_dev_by_id(const int dev_id)
> > > const does not make sense here for an int parameter.
> >
> >
> > I think it could be OK with const even if the parameter is not pointer.
> >
> > However, most DPDK APIs do not have const for simple types (e.g.
> > int/uin16_t).
> >
> > In this aspect, I think it's also OK to remove const of here for consistency.
> >
> >
> > >
> > >> +{
> > >> + if (!rte_dma_is_valid(dev_id))
> > >> + return NULL;
> > >> +
> > >> + return &rte_dma_devices[dev_id];
> > >> +}
> > > [...]
> > >> +/**
> > >> + * @internal
> > >> + * Get the rte_dma_dev structure device pointer for the device id.
> > >> + *
> > >> + * @param dev_id
> > >> + * Device ID value to select the device structure.
> > > This comment is not explanatory.
> > > What is an ID? Where does it come from?
> > > Where can we see such ID for DMA device?
> >
> > This new API is used in the event-dma driver of cnxk [1]:
> >
> > The rte_event_dma_adapter_vchan_add has parameter of dma_dev_id, and it
> > then
> >
> > invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id, vchan,
> > event),
> >
> > at cnxk driver, this ops will check whether the DMA is
> > cnxk_dmadev_pci_driver.
> >
> > I think this is because the cnxk's event-and-dma implement has deep coupling
> >
> > (because the cnxk's event device could interact with another vendor's
> > dma device).
> >
> >
> > Maybe we should think of a better way to solve this kind of coupling
> > problem.
>
> Id, is the DMA dev id which is used in looking up DMA dev. This API is in-line with the other libraries.
> Crypto library has an api rte_cryptodev_pmd_get_dev to get crypto device based on device id.
OK I think I understand.
It is the library ID, the same as returned by
int rte_dma_get_dev_id_by_name(const char *name);
I can remove the const and apply if you are OK.
I would just change this comment:
+ * @param dev_id
+ * Device ID value to select the device structure.
into
+ * DMA device index in dmadev library.
<snip>
> > >
> > > invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id,
> > > vchan, event),
> > >
> > > at cnxk driver, this ops will check whether the DMA is
> > > cnxk_dmadev_pci_driver.
> > >
> > > I think this is because the cnxk's event-and-dma implement has deep
> > > coupling
> > >
> > > (because the cnxk's event device could interact with another
> > > vendor's dma device).
> > >
> > >
> > > Maybe we should think of a better way to solve this kind of coupling
> > > problem.
> >
> > Id, is the DMA dev id which is used in looking up DMA dev. This API is in-line
> with the other libraries.
> > Crypto library has an api rte_cryptodev_pmd_get_dev to get crypto device
> based on device id.
>
> OK I think I understand.
> It is the library ID, the same as returned by int
> rte_dma_get_dev_id_by_name(const char *name);
>
> I can remove the const and apply if you are OK.
Sure, I am okay.
> I would just change this comment:
>
> + * @param dev_id
> + * Device ID value to select the device structure.
>
> into
>
> + * DMA device index in dmadev library.
Sure.
Thanks.
09/02/2024 12:30, Amit Prakash Shukla:
>
> <snip>
> > > >
> > > > invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id,
> > > > vchan, event),
> > > >
> > > > at cnxk driver, this ops will check whether the DMA is
> > > > cnxk_dmadev_pci_driver.
> > > >
> > > > I think this is because the cnxk's event-and-dma implement has deep
> > > > coupling
> > > >
> > > > (because the cnxk's event device could interact with another
> > > > vendor's dma device).
> > > >
> > > >
> > > > Maybe we should think of a better way to solve this kind of coupling
> > > > problem.
> > >
> > > Id, is the DMA dev id which is used in looking up DMA dev. This API is in-line
> > with the other libraries.
> > > Crypto library has an api rte_cryptodev_pmd_get_dev to get crypto device
> > based on device id.
> >
> > OK I think I understand.
> > It is the library ID, the same as returned by int
> > rte_dma_get_dev_id_by_name(const char *name);
> >
> > I can remove the const and apply if you are OK.
>
> Sure, I am okay.
>
> > I would just change this comment:
> >
> > + * @param dev_id
> > + * Device ID value to select the device structure.
> >
> > into
> >
> > + * DMA device index in dmadev library.
>
> Sure.
> Thanks.
I've also fixed the ID type to be int16_t.
Applied
@@ -397,6 +397,15 @@ rte_dma_is_valid(int16_t dev_id)
rte_dma_devices[dev_id].state != RTE_DMA_DEV_UNUSED;
}
+struct rte_dma_dev *
+rte_dma_pmd_get_dev_by_id(const int dev_id)
+{
+ if (!rte_dma_is_valid(dev_id))
+ return NULL;
+
+ return &rte_dma_devices[dev_id];
+}
+
uint16_t
rte_dma_count_avail(void)
{
@@ -167,6 +167,20 @@ struct rte_dma_dev *rte_dma_pmd_allocate(const char *name, int numa_node,
__rte_internal
int rte_dma_pmd_release(const char *name);
+/**
+ * @internal
+ * Get the rte_dma_dev structure device pointer for the device id.
+ *
+ * @param dev_id
+ * Device ID value to select the device structure.
+ *
+ * @return
+ * - rte_dma_dev structure pointer for the given device ID on success, NULL
+ * otherwise.
+ */
+__rte_internal
+struct rte_dma_dev *rte_dma_pmd_get_dev_by_id(const int dev_id);
+
#ifdef __cplusplus
}
#endif
@@ -25,6 +25,7 @@ INTERNAL {
rte_dma_fp_objs;
rte_dma_pmd_allocate;
+ rte_dma_pmd_get_dev_by_id;
rte_dma_pmd_release;
local: *;