RE: [PATCH RFC] dmaengine: dw-edma: Decouple dw-edma-core.c from struct pci_dev

[Gustavo Pimentel]

Hi Alan,

> > I like your approach, it separates the PCIe glue logic from the eDMA
> > itself.
> > I would suggest that pcitest would have multiple options that could be
> > triggered, for instance:
> >  1 - Execute Endpoint DMA (read/write) remotely with Linked List feature
> > (from the Root Complex side)
> >  2 - Execute Endpoint DMA (read/write) remotely without Linked List
> > feature (from the Root Complex side)
> >  3 - Execute Endpoint DMA (read/write) locally with Linked List feature
> >  4 - Execute Endpoint DMA (read/write) locally without Linked List
> > feature
> >
> 
> I have all of the above four use cases in mind as well. At the moment,
> only #4 is possible with pcitest.
> 
> Use case #3 would need a new command line option for pcitest such as -L
> to let its user specify linked list operationwhen used with dma in
> conjunction with the existing -D option.
> 
> Use cases #1 and #2 would need another new command line option such as -R
> to specify remotely initiated dma operation in conjunction with -D option.
> 
> New code in pci-epf-test and pci_endpoint_test drivers would be needed
> to support use cases #1, #2, and #3. However, use case #4 should be
> possible without modification to pci-epf-test or pci_endpoint_test as long
> as the dmaengine channels become available on the endpoint side.

I would suggest something like this:

-L option, local DMA triggering
-R option, remote DMA triggering
-W <n> option, to select the DMA write channel n => (0 ... 7) to be 
used
-R <n> option, to select the DMA read channel n => (0 ... 7) to be 
used
-K option, to use or not the linked list feature (K presence enables 
the LL use)
-T <n> option, to select which type of DMA transfer to be used => (n = 0 
- scatter-gather mode, 1 - cyclic mode)
-N <n> option, to define the number of cyclic transfers to perform in 
total
-C <n> option, to define the size of each chunk to be used
-t <time> option, to define a timeout for the DMA operation 

Also, the use of this options (especially when using the remote DMA 
option) should be checked through the pci_epc_get_features(), which means 
probably we need to pass the EP features capabilities to the 
pci_endpoint_test Driver, perhaps using some sets of registers on located 
on BAR0 or other.

> At the moment, pci-epf-test grabs the first available dma channel on the
> endpoint side and uses it for either read, write, or copy operation. it is not
> possible at the moment to specify which dma channel to use on the pcitest
> command line. This may be possible by modifying the command line option
> -D to also specify the name of one or more dma channels.

I'm assuming that behavior is due to your code, right? I'm not seen that 
behavior on the Kernel tree.
Check my previous suggestion, it should be something similar to what is 
been done while you select the MSI/MSI-X interrupt to trigger.

> Also, pci-epf-test grabs the dma channel at bind time and holds on to it
> until unloaded. This denies the use of the dma channel to others on the
> endpoint side. However, it seems possible to grab and release the dma
> channel only for the duration of each read, write, or copy test. These are
> improvements that can come over time. It is great that pci-epf-test was
> recently updated to include support for dma operations which makes such
> improvements possible.

Check my previous suggestion. I think by having a timeout for the DMA 
operation we can provide a way to release the dma channel.
Or we could provide some kind of heart beat, once again through some 
register in a BAR.

> > Relative to the implementation of the options 3 and 4, I wonder if the
> > linked list memory space and size could be set through the DT or by the
> > configfs available on the pci-epf-test driver.
> >
> 
> Although these options could be set through DT or by configfs, another
> option is to enable the user of pcitest to specify such parameters on
> the command line when invoking each test from the host side.

That would be an easy and quick solution, but so far as I know there is a 
movement in the Kernel to avoid any configuration through module 
parameters. So I'm afraid that you have to choose by DT or configfs 
strategy. Kishon can help you on this matter, by telling you what he 
prefers.

Regards,
Gustavo