09/09/2021 15:33, fengchengwen: > On 2021/9/9 18:33, Thomas Monjalon wrote: > > 07/09/2021 14:56, Chengwen Feng: > >> + * The DMA controller could have multiple HW-DMA-channels (aka. > >> HW-DMA-queues), > >> + * each HW-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. > > > > What is the benefit of virtual channels compared to have separate dmadevs > > for the same HW channel? > > This design is from the previous discussion [1]. If a dmadev is created for > each > virtual channel, there are many associations between the dmadevs. For example, > channel operations of some devices need to interact with the kernel, the > corresponding > kernel operation handles need to be shared among multiple dmadevs. It's going > to get > more complicated. > > [1] > https://lore.kernel.org/dpdk-dev/c4a0ee30-f7b8-f8a1-463c-8eedaec82...@huawei.com/
OK thanks for the explanation. [...] > >> + * be released by rte_dmadev_pmd_release() during the PCI/SoC device > >> removing > >> + * phase. > > > > Again what is this phase? > > I think freeing should be done only via the "close" function. > > OK > The allocate/release will be reconstructed with reference to > rte_eth_dev_pci_generic_probe. You shouldn't always mimic ethdev, there can be some misconceptions :) I think you don't need PCI specific helper. [...] > >> + * @note If the dmadev works in silent mode (@see RTE_DMADEV_CAPA_SILENT), > >> + * application does not invoke the above two completed APIs. > >> + * > >> + * About the ring_idx which enqueue APIs (e.g. rte_dmadev_copy() > >> + * rte_dmadev_fill()) returned, the rules are as follows: > > > > I feel a word is missing above. > > Can you point it out specifically ? > PS: I specifically examined by access https://www.nounplus.net/grammarcheck/ > and found > it prompts the 'enqueue' to 'enqueues or enqueued'. After second read, I think it is a tense problem. What about "returned" -> "return" ? [...] > >> +/**< DMA device support memory-to-memory transfer. > >> + * > >> + * @see struct rte_dmadev_info::dev_capa > >> + */ > > > > It is preferred to have documentation before the item. > > Is this particularly strong? > I use postfix comment style for whole doxygen comments. I think it's better > to maintain a unified > style. In general prefix comment is preferred. Postfix comments are OK for short comments on the same line. [...] > > This series add one file per patch. > > Instead it would be better to have groups of features per patch, > > meaning the implementation and the driver interface should be > > in the same patch. > > You can split like this: > > 1/ device allocation > > 2/ configuration and start/stop > > 3/ dataplane functions > > > > I would suggest 2 more patches: > > 4/ event notification > > see > > https://patches.dpdk.org/project/dpdk/patch/20210730135533.417611-3-tho...@monjalon.net/ > > 5/ multi-process > > see > > https://patches.dpdk.org/project/dpdk/patch/20210730135533.417611-5-tho...@monjalon.net/ > > The split mode you recommend is better. > But is this particularly strong ? Yes, that's really better. > Because many acked-by and reviewed-by base on stand-alone file. > Does this division mean that a new acked/reviewed ? You can keep the acks which are commong to the first 4 patches I guess and ask for re-ack to others.
