On 2021/10/13 3:09, Thomas Monjalon wrote:
> 11/10/2021 09:33, Chengwen Feng:
>> --- /dev/null
>> +++ b/doc/guides/prog_guide/dmadev.rst
>> @@ -0,0 +1,60 @@
>> +.. SPDX-License-Identifier: BSD-3-Clause
>> + Copyright 2021 HiSilicon Limited
>> +
>> +DMA Device Library
>> +==================
>> +
>> +The DMA library provides a DMA device framework for management and
>> provisioning
>> +of hardware and software DMA poll mode drivers, defining generic API which
>> +support a number of different DMA operations.
>> +
>> +
>> +Design Principles
>> +-----------------
>> +
>> +The DMA framework provides a generic DMA device framework which supports
>> both
>> +physical (hardware) and virtual (software) DMA devices, as well as a
>> generic DMA
>> +API which allows DMA devices to be managed and configured, and supports DMA
>> +operations to be provisioned on DMA poll mode driver.
>> +
>> +.. _figure_dmadev:
>> +
>> +.. figure:: img/dmadev.*
>> +
>> +The above figure shows the model on which the DMA framework is built on:
>> +
>> + * The DMA controller could have multiple hardware DMA channels (aka.
>> hardware
>> + DMA queues), each hardware DMA channel should be represented by a dmadev.
>> + * The dmadev could create multiple virtual DMA channels, each virtual DMA
>> + channel represents a different transfer context. The DMA operation
>> request
>> + must be submitted to the virtual DMA channel. e.g. Application could
>> create
>> + virtual DMA channel 0 for memory-to-memory transfer scenario, and create
>> + virtual DMA channel 1 for memory-to-device transfer scenario.
>
> When updating the doc, we would like to change a minimum of lines,
> so it's better to split lines logically: after a comma, a point,
> or before the next part of the sentence.
> Do not hesitate to make short lines if needed.
> Such change is quite fast to do, thanks.
>
> [...]
>> +* **Introduced dmadev library with:**
>> +
>> + * Device allocation functions.
>
> You can drop this line, it is not a feature.
I'm going to try another description.
>
> [...]
>> +static int
>> +dma_dev_data_prepare(void)
>> +{
>> + size_t size;
>> +
>> + if (rte_dma_devices != NULL)
>> + return 0;
>> +
>> + size = dma_devices_max * sizeof(struct rte_dma_dev);
>> + rte_dma_devices = malloc(size);
>> + if (rte_dma_devices == NULL)
>> + return -ENOMEM;
>> + memset(rte_dma_devices, 0, size);
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +dma_data_prepare(void)
>> +{
>> + if (dma_devices_max == 0)
>> + dma_devices_max = RTE_DMADEV_DEFAULT_MAX;
>> + return dma_dev_data_prepare();
>> +}
>> +
>> +static struct rte_dma_dev *
>> +dma_allocate(const char *name, int numa_node, size_t private_data_size)
>> +{
>> + struct rte_dma_dev *dev;
>> + void *dev_private;
>> + int16_t dev_id;
>> + int ret;
>> +
>> + ret = dma_data_prepare();
>> + if (ret < 0) {
>> + RTE_DMA_LOG(ERR, "Cannot initialize dmadevs data");
>> + return NULL;
>> + }
>> +
>> + dev = dma_find_by_name(name);
>> + if (dev != NULL) {
>> + RTE_DMA_LOG(ERR, "DMA device already allocated");
>> + return NULL;
>> + }
>> +
>> + dev_private = rte_zmalloc_socket(name, private_data_size,
>> + RTE_CACHE_LINE_SIZE, numa_node);
>> + if (dev_private == NULL) {
>> + RTE_DMA_LOG(ERR, "Cannot allocate private data");
>> + return NULL;
>> + }
>> +
>> + dev_id = dma_find_free_id();
>> + if (dev_id < 0) {
>> + RTE_DMA_LOG(ERR, "Reached maximum number of DMA devices");
>> + rte_free(dev_private);
>> + return NULL;
>> + }
>> +
>> + dev = &rte_dma_devices[dev_id];
>> + rte_strscpy(dev->dev_name, name, sizeof(dev->dev_name));
>> + dev->dev_id = dev_id;
>> + dev->numa_node = numa_node;
>> + dev->dev_private = dev_private;
>> +
>> + return dev;
>> +}
>> +
>> +static void
>> +dma_release(struct rte_dma_dev *dev)
>> +{
>> + rte_free(dev->dev_private);
>> + memset(dev, 0, sizeof(struct rte_dma_dev));
>> +}
>> +
>> +struct rte_dma_dev *
>> +rte_dma_pmd_allocate(const char *name, int numa_node, size_t
>> private_data_size)
>> +{
>> + struct rte_dma_dev *dev;
>> +
>> + if (dma_check_name(name) != 0 || private_data_size == 0)
>> + return NULL;
>> +
>> + dev = dma_allocate(name, numa_node, private_data_size);
>> + if (dev == NULL)
>> + return NULL;
>> +
>> + dev->state = RTE_DMA_DEV_REGISTERED;
>> +
>> + return dev;
>> +}
>> +
>> +int
>> +rte_dma_pmd_release(const char *name)
>> +{
>> + struct rte_dma_dev *dev;
>> +
>> + if (dma_check_name(name) != 0)
>> + return -EINVAL;
>> +
>> + dev = dma_find_by_name(name);
>> + if (dev == NULL)
>> + return -EINVAL;
>> +
>> + dma_release(dev);
>> + return 0;
>> +}
>
> Trying to understand the logic of creation/destroy.
> skeldma_probe
> \-> skeldma_create
> \-> rte_dma_pmd_allocate
> \-> dma_allocate
> \-> dma_data_prepare
> \-> dma_dev_data_prepare
> skeldma_remove
> \-> skeldma_destroy
> \-> rte_dma_pmd_release
> \-> dma_release
This patch only provide device allocate function, the 2st patch provide extra
logic:
diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 42a4693bd9..a6a5680d2b 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -201,6 +201,9 @@ rte_dma_pmd_release(const char *name)
if (dev == NULL)
return -EINVAL;
+ if (dev->state == RTE_DMA_DEV_READY)
+ return rte_dma_close(dev->dev_id);
+
dma_release(dev);
return 0;
}
So the skeldma remove will be:
skeldma_remove
\-> skeldma_destroy
\-> rte_dma_pmd_release
\-> rte_dma_close
\-> dma_release
> app
> \-> rte_dma_close
> \-> skeldma_close
> \-> dma_release
>
> My only concern is that the PMD remove does not call rte_dma_close.
If the device create success, it will call rte_dma_close in device remove phase.
> The PMD should check which dmadev is open for the rte_device to remove,
> and close the dmadev first.
> This way, no need for the function rte_dma_pmd_release,
> and no need to duplicate the release process in two paths.
> By the way, the function vchan_release is called only in the close function,
> not in the "remove path".
>
>
>
>
> .
>
Thanks
