Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1586916464-27727-1-git-send-email-alan.mikhak@sifive.com>
 <DM5PR12MB1276CB8FA4457D4CDCE3137EDADB0@DM5PR12MB1276.namprd12.prod.outlook.com>
 <CABEDWGwYmO52g6cqvQdWb6HXWEHaMA1rcf96aUqv0f32tJZT-g@mail.gmail.com>
 <DM5PR12MB1276E09460BD4DB7E70EAF91DADB0@DM5PR12MB1276.namprd12.prod.outlook.com>
 <CABEDWGw0OyQNppLpDaNgMedfB0Ci=kZVKm+h4T-LJoZYmbSgqA@mail.gmail.com>
 <DM5PR12MB127673CFDCA38A47E6F6F4DBDADB0@DM5PR12MB1276.namprd12.prod.outlook.com>
 <CABEDWGyUfq4c65K+btmKBcGLv59h6PFVUkSD_52kOw9R0Rtynw@mail.gmail.com>
In-Reply-To: <CABEDWGyUfq4c65K+btmKBcGLv59h6PFVUkSD_52kOw9R0Rtynw@mail.gmail.com>
From:   Alan Mikhak <alan.mikhak@sifive.com>
Date:   Wed, 15 Apr 2020 16:26:30 -0700
Message-ID: <CABEDWGz1KLGmQBWU_kcKF7zxjnP02x=0vbXghsZMKNWeExF+rA@mail.gmail.com>
Subject: Re: [PATCH RFC] dmaengine: dw-edma: Decouple dw-edma-core.c from
 struct pci_dev
To:     Gustavo Pimentel <Gustavo.Pimentel@synopsys.com>
Cc:     "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        "vkoul@kernel.org" <vkoul@kernel.org>,
        "kishon@ti.com" <kishon@ti.com>,
        "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 15, 2020 at 2:23 PM Alan Mikhak <alan.mikhak@sifive.com> wrote:
>
> On Wed, Apr 15, 2020 at 1:58 PM Gustavo Pimentel
> <Gustavo.Pimentel@synopsys.com> wrote:
> >
> > Hi Alan,
> >
> > > > > 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.
> > >
> > > I believe this behavior exists in the kernel tree because the call to
> > > dma_request_chan_by_mask() always specifies channel zero. The user
> > > of pcitest has no way of specifying which one of the available dma channels
> > > to use.
> >
> > I think we were discussing different things. I was referring to the
> > pci-epf-test code, that I wasn't being able to find any instruction to
> > call the DMA driver which had the described behavior.
> >
> > I think you can do it by doing this:
> >
> > Pseudo code:
> >
> > #define EDMA_TEST_CHANNEL_NAME                  "dma%uchan%u"
> >
> > static bool dw_edma_test_filter(struct dma_chan *chan, void *filter)
> > {
> >         if (strcmp(dev_name(chan->device->dev), EDMA_TEST_DEVICE_NAME) ||
> > strcmp(dma_chan_name(chan), filter))
> >                 return false;
> >
> >         return true;
> > }
> >
> > static void dw_edma_test_thread_create(int id, int channel)
> > {
> >         struct dma_chan *chan;
> >         dma_cap_mask_t mask;
> >         char filter[20];
> >
> >         dma_cap_zero(mask);
> >         dma_cap_set(DMA_SLAVE, mask);
> >         dma_cap_set(DMA_CYCLIC, mask);
> >
> >         snprintf(filter, sizeof(filter), EDMA_TEST_CHANNEL_NAME, id,
> > channel);
> >         chan = dma_request_channel(mask, dw_edma_test_filter, filter);
> >
> >         [..]
> > }
>
> Thanks Gustavo, This pseudo code is very useful. Now I know how to do
> that part of the change.
>
> What I have further in mind is to enable the pcitest user to specify some
> arbitrary string with -D option to select one or more of the dma channels
> that are available on the endpoint side. Since the user executes pcitest
> from host-side command prompt and pci-epf-test executes in kernel on the
> endpoint side, the messaging between userspace pcitest and kernel-space
> pci_endpoint_test as well as the messaging across the bus between
> pci_endpoint_test and pci-epf-test needs to be expanded to pass the user
> string from the host to the endpoint. Upon receiving each read, write, or
> copy message, pci-epf-test could then try to acquire the specified dma
> channel and execute the user command or fail it if no such channel is
> available at that moment.
>
> >
> > > I believe this behavior exists in the kernel tree because the call to
> > > dma_request_chan_by_mask() happens during the execution of
> > > pci_epf_test_bind() and the call to dma_release_channel() happens
> > > during the execution of pci_epf_test_unbind(). As long as pci-epf-test
> > > is bound, I cannot use another program such as dmatest from the
> > > endpoint-side command prompt to exercise the same channel.
> >
> > Ok, I understood it now. Right, you can't use the dmatest here, even
> > because, as far as I know, it is only MEM TO MEM operations and we need
> > DEVICE_TO_MEM and vice-versa.
> >

On the platforms that I have this in mind for, I may not only have embedded
dma channels inside the PCIe controller but also platform dma channels. Either
type of dma channel can be used by pcitest whereas dmatest can only use
the type that can do MEM to MEM as you said. Either of these types can
be used to transfer data to or from the PCIe bus. I need to use both kinds
with pcitest to make sure the PCIe subsystem is ok.

> > >
> > > What I was suggesting is perhaps pci-epf-test can be modified to
> > > acquire and release the channel on each call to pci_epf_test_read(),
> > > ...write(), or ...copy() when the pcitest user specifies -D option.
> >
> > Right, you are on the right track.
> > Perhaps you could take a look at patch [1] that I have done some time ago
> > for testing the eDMA, I think you have all the tools/guideline there to
> > do this adaption.
> > Another thing,
> >
> > [1] https://patchwork.kernel.org/patch/10760521/
>
> Thanks for the guidance and reference code patch [1]. I will definitely
> take a close look at [1].
>
> >
> >
> >