Re: [PATCH] dma/skeleton: support multiple instances

[fengchengwen <fengchengwen@huawei.com>]

This patch LGTM, but it forget remove skeldma_count variable.

Also I find the skeleton_dma has the same bug with [1], So I will
send a patchset which contains this patch (add my reviewed-by).

Sivaprasad, Vipin, thanks for your work.

[1] dma/hisilicon: fix return last-idx when no DMA completed

On 2022/6/7 18:49, Thomas Monjalon wrote:
> Chengwen, as the maintainer of this driver, what do you think?
> 
> 
> 28/03/2022 16:40, Varghese, Vipin:
>> [AMD Official Use Only]
>>
>> Verified on ` AMD EPYC 7713 64-Core Processor` platform with argument ` --vdev=dma_skeleton0,lcore=4 --vdev=dma_skeleton1,lcore=5`. 
>>
>> Tested-by: Vipin Varghese <Vipin.Varghese@amd.com>
>>
>> -----Original Message-----
>> From: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com> 
>> Sent: Monday, March 28, 2022 7:53 PM
>> To: fengchengwen@huawei.com
>> Cc: dev@dpdk.org; Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>
>> Subject: [PATCH] dma/skeleton: support multiple instances
>>
>> [CAUTION: External Email]
>>
>> dpdk app can support multiple hardware dma instances.
>> with dma skeleton, only a single instance can be configured.
>>
>> This patch supports multiple driver instances per device.
>>
>> Signed-off-by: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
>> ---
>>  drivers/dma/skeleton/skeleton_dmadev.c | 37 +++++++++++++++-----------
>>  1 file changed, 22 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/dma/skeleton/skeleton_dmadev.c b/drivers/dma/skeleton/skeleton_dmadev.c
>> index 81cbdd286e..6b1c01d89c 100644
>> --- a/drivers/dma/skeleton/skeleton_dmadev.c
>> +++ b/drivers/dma/skeleton/skeleton_dmadev.c
>> @@ -100,6 +100,7 @@ static int
>>  skeldma_start(struct rte_dma_dev *dev)
>>  {
>>         struct skeldma_hw *hw = dev->data->dev_private;
>> +       char name[RTE_MAX_THREAD_NAME_LEN];
>>         rte_cpuset_t cpuset;
>>         int ret;
>>
>> @@ -125,7 +126,8 @@ skeldma_start(struct rte_dma_dev *dev)
>>
>>         rte_mb();
>>
>> -       ret = rte_ctrl_thread_create(&hw->thread, "dma_skeleton", NULL,
>> +       snprintf(name, sizeof(name), "dma_skel_%d", dev->data->dev_id);
>> +       ret = rte_ctrl_thread_create(&hw->thread, name, NULL,
>>                                      cpucopy_thread, dev);
>>         if (ret) {
>>                 SKELDMA_LOG(ERR, "Start cpucopy thread fail!"); @@ -160,7 +162,7 @@ skeldma_stop(struct rte_dma_dev *dev)  }
>>
>>  static int
>> -vchan_setup(struct skeldma_hw *hw, uint16_t nb_desc)
>> +vchan_setup(struct skeldma_hw *hw, int16_t dev_id, uint16_t nb_desc)
>>  {
>>         struct skeldma_desc *desc;
>>         struct rte_ring *empty;
>> @@ -168,8 +170,12 @@ vchan_setup(struct skeldma_hw *hw, uint16_t nb_desc)
>>         struct rte_ring *running;
>>         struct rte_ring *completed;
>>         uint16_t i;
>> +       char pool_name[RTE_RING_NAMESIZE];
>> +       char ring_name[RTE_RING_NAMESIZE];
>>
>> -       desc = rte_zmalloc_socket("dma_skeleton_desc",
>> +       snprintf(pool_name, RTE_RING_NAMESIZE, "dma_skel_desc_pool_%d",
>> +                       dev_id);
>> +       desc = rte_zmalloc_socket(pool_name,
>>                                   nb_desc * sizeof(struct skeldma_desc),
>>                                   RTE_CACHE_LINE_SIZE, hw->socket_id);
>>         if (desc == NULL) {
>> @@ -177,13 +183,21 @@ vchan_setup(struct skeldma_hw *hw, uint16_t nb_desc)
>>                 return -ENOMEM;
>>         }
>>
>> -       empty = rte_ring_create("dma_skeleton_desc_empty", nb_desc,
>> +       snprintf(ring_name, RTE_RING_NAMESIZE, "dma_skel_desc_empty_%d",
>> +                       dev_id);
>> +       empty = rte_ring_create(ring_name, nb_desc,
>>                                 hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ);
>> -       pending = rte_ring_create("dma_skeleton_desc_pending", nb_desc,
>> +       snprintf(ring_name, RTE_RING_NAMESIZE, "dma_skel_desc_pend_%d",
>> +                       dev_id);
>> +       pending = rte_ring_create(ring_name, nb_desc,
>>                                   hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ);
>> -       running = rte_ring_create("dma_skeleton_desc_running", nb_desc,
>> +       snprintf(ring_name, RTE_RING_NAMESIZE, "dma_skel_desc_run_%d",
>> +                       dev_id);
>> +       running = rte_ring_create(ring_name, nb_desc,
>>                                   hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ);
>> -       completed = rte_ring_create("dma_skeleton_desc_completed", nb_desc,
>> +       snprintf(ring_name, RTE_RING_NAMESIZE, "dma_skel_desc_comp_%d",
>> +                       dev_id);
>> +       completed = rte_ring_create(ring_name, nb_desc,
>>                                   hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ);
>>         if (empty == NULL || pending == NULL || running == NULL ||
>>             completed == NULL) {
>> @@ -254,7 +268,7 @@ skeldma_vchan_setup(struct rte_dma_dev *dev, uint16_t vchan,
>>         }
>>
>>         vchan_release(hw);
>> -       return vchan_setup(hw, conf->nb_desc);
>> +       return vchan_setup(hw, dev->data->dev_id, conf->nb_desc);
>>  }
>>
>>  static int
>> @@ -548,13 +562,6 @@ skeldma_probe(struct rte_vdev_device *vdev)
>>                 return -EINVAL;
>>         }
>>
>> -       /* More than one instance is not supported */
>> -       if (skeldma_count > 0) {
>> -               SKELDMA_LOG(ERR, "Multiple instance not supported for %s",
>> -                       name);
>> -               return -EINVAL;
>> -       }
>> -
>>         skeldma_parse_vdev_args(vdev, &lcore_id);
>>
>>         ret = skeldma_create(name, vdev, lcore_id);
>> --
>> 2.17.1
>>
> 
> 
> 
> 
> 
> 
> .
>