From: "Krzysztof Wilczyński" <kw@linux.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: lpieralisi@kernel.org, kishon@kernel.org, bhelgaas@google.com,
mhi@lists.linux.dev, linux-arm-msm@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/9] PCI: epf-mhi: Add support for DMA async read/write operation
Date: Thu, 14 Dec 2023 03:50:34 +0900 [thread overview]
Message-ID: <20231213185034.GC924726@rocinante> (raw)
In-Reply-To: <20231127124529.78203-6-manivannan.sadhasivam@linaro.org>
Hello,
> The driver currently supports only the sync read/write operation i.e., it
> waits for the DMA transfer to complete before returning to the caller
> (MHI stack). But it is sub-optimal and defeats the actual purpose of using
> DMA.
>
> So let's add support for DMA async read/write operation by skipping the DMA
> transfer completion and returning to the caller immediately. When the
> completion actually happens later, the driver will be notified using the
> DMA completion handler and in turn it will notify the caller using the
> newly introduced callback in "struct mhi_ep_buf_info".
>
> Since the DMA completion handler is invoked from the interrupt context, a
> separate workqueue (epf_mhi->dma_wq) is used to notify the caller about the
> completion of the transfer.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/pci/endpoint/functions/pci-epf-mhi.c | 231 ++++++++++++++++++-
> 1 file changed, 228 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 7214f4da733b..3d09a37e5f7c 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -21,6 +21,15 @@
> /* Platform specific flags */
> #define MHI_EPF_USE_DMA BIT(0)
>
> +struct pci_epf_mhi_dma_transfer {
> + struct pci_epf_mhi *epf_mhi;
> + struct mhi_ep_buf_info buf_info;
> + struct list_head node;
> + dma_addr_t paddr;
> + enum dma_data_direction dir;
> + size_t size;
> +};
> +
> struct pci_epf_mhi_ep_info {
> const struct mhi_ep_cntrl_config *config;
> struct pci_epf_header *epf_header;
> @@ -124,6 +133,10 @@ struct pci_epf_mhi {
> resource_size_t mmio_phys;
> struct dma_chan *dma_chan_tx;
> struct dma_chan *dma_chan_rx;
> + struct workqueue_struct *dma_wq;
> + struct work_struct dma_work;
> + struct list_head dma_list;
> + spinlock_t list_lock;
> u32 mmio_size;
> int irq;
> };
> @@ -418,6 +431,198 @@ static int pci_epf_mhi_edma_write(struct mhi_ep_cntrl *mhi_cntrl,
> return ret;
> }
>
> +static void pci_epf_mhi_dma_worker(struct work_struct *work)
> +{
> + struct pci_epf_mhi *epf_mhi = container_of(work, struct pci_epf_mhi, dma_work);
> + struct device *dma_dev = epf_mhi->epf->epc->dev.parent;
> + struct pci_epf_mhi_dma_transfer *itr, *tmp;
> + struct mhi_ep_buf_info *buf_info;
> + unsigned long flags;
> + LIST_HEAD(head);
> +
> + spin_lock_irqsave(&epf_mhi->list_lock, flags);
> + list_splice_tail_init(&epf_mhi->dma_list, &head);
> + spin_unlock_irqrestore(&epf_mhi->list_lock, flags);
> +
> + list_for_each_entry_safe(itr, tmp, &head, node) {
> + list_del(&itr->node);
> + dma_unmap_single(dma_dev, itr->paddr, itr->size, itr->dir);
> + buf_info = &itr->buf_info;
> + buf_info->cb(buf_info);
> + kfree(itr);
> + }
> +}
> +
> +static void pci_epf_mhi_dma_async_callback(void *param)
> +{
> + struct pci_epf_mhi_dma_transfer *transfer = param;
> + struct pci_epf_mhi *epf_mhi = transfer->epf_mhi;
> +
> + spin_lock(&epf_mhi->list_lock);
> + list_add_tail(&transfer->node, &epf_mhi->dma_list);
> + spin_unlock(&epf_mhi->list_lock);
> +
> + queue_work(epf_mhi->dma_wq, &epf_mhi->dma_work);
> +}
> +
> +static int pci_epf_mhi_edma_read_async(struct mhi_ep_cntrl *mhi_cntrl,
> + struct mhi_ep_buf_info *buf_info)
> +{
> + struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> + struct device *dma_dev = epf_mhi->epf->epc->dev.parent;
> + struct pci_epf_mhi_dma_transfer *transfer = NULL;
> + struct dma_chan *chan = epf_mhi->dma_chan_rx;
> + struct device *dev = &epf_mhi->epf->dev;
> + DECLARE_COMPLETION_ONSTACK(complete);
> + struct dma_async_tx_descriptor *desc;
> + struct dma_slave_config config = {};
> + dma_cookie_t cookie;
> + dma_addr_t dst_addr;
> + int ret;
> +
> + mutex_lock(&epf_mhi->lock);
> +
> + config.direction = DMA_DEV_TO_MEM;
> + config.src_addr = buf_info->host_addr;
> +
> + ret = dmaengine_slave_config(chan, &config);
> + if (ret) {
> + dev_err(dev, "Failed to configure DMA channel\n");
> + goto err_unlock;
> + }
> +
> + dst_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
> + DMA_FROM_DEVICE);
> + ret = dma_mapping_error(dma_dev, dst_addr);
> + if (ret) {
> + dev_err(dev, "Failed to map remote memory\n");
> + goto err_unlock;
> + }
> +
> + desc = dmaengine_prep_slave_single(chan, dst_addr, buf_info->size,
> + DMA_DEV_TO_MEM,
> + DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(dev, "Failed to prepare DMA\n");
> + ret = -EIO;
> + goto err_unmap;
> + }
> +
> + transfer = kzalloc(sizeof(*transfer), GFP_KERNEL);
> + if (!transfer) {
> + ret = -ENOMEM;
> + goto err_unmap;
> + }
> +
> + transfer->epf_mhi = epf_mhi;
> + transfer->paddr = dst_addr;
> + transfer->size = buf_info->size;
> + transfer->dir = DMA_FROM_DEVICE;
> + memcpy(&transfer->buf_info, buf_info, sizeof(*buf_info));
> +
> + desc->callback = pci_epf_mhi_dma_async_callback;
> + desc->callback_param = transfer;
> +
> + cookie = dmaengine_submit(desc);
> + ret = dma_submit_error(cookie);
> + if (ret) {
> + dev_err(dev, "Failed to do DMA submit\n");
> + goto err_free_transfer;
> + }
> +
> + dma_async_issue_pending(chan);
> +
> + goto err_unlock;
> +
> +err_free_transfer:
> + kfree(transfer);
> +err_unmap:
> + dma_unmap_single(dma_dev, dst_addr, buf_info->size, DMA_FROM_DEVICE);
> +err_unlock:
> + mutex_unlock(&epf_mhi->lock);
> +
> + return ret;
> +}
> +
> +static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
> + struct mhi_ep_buf_info *buf_info)
> +{
> + struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> + struct device *dma_dev = epf_mhi->epf->epc->dev.parent;
> + struct pci_epf_mhi_dma_transfer *transfer = NULL;
> + struct dma_chan *chan = epf_mhi->dma_chan_tx;
> + struct device *dev = &epf_mhi->epf->dev;
> + DECLARE_COMPLETION_ONSTACK(complete);
> + struct dma_async_tx_descriptor *desc;
> + struct dma_slave_config config = {};
> + dma_cookie_t cookie;
> + dma_addr_t src_addr;
> + int ret;
> +
> + mutex_lock(&epf_mhi->lock);
> +
> + config.direction = DMA_MEM_TO_DEV;
> + config.dst_addr = buf_info->host_addr;
> +
> + ret = dmaengine_slave_config(chan, &config);
> + if (ret) {
> + dev_err(dev, "Failed to configure DMA channel\n");
> + goto err_unlock;
> + }
> +
> + src_addr = dma_map_single(dma_dev, buf_info->dev_addr, buf_info->size,
> + DMA_TO_DEVICE);
> + ret = dma_mapping_error(dma_dev, src_addr);
> + if (ret) {
> + dev_err(dev, "Failed to map remote memory\n");
> + goto err_unlock;
> + }
> +
> + desc = dmaengine_prep_slave_single(chan, src_addr, buf_info->size,
> + DMA_MEM_TO_DEV,
> + DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(dev, "Failed to prepare DMA\n");
> + ret = -EIO;
> + goto err_unmap;
> + }
> +
> + transfer = kzalloc(sizeof(*transfer), GFP_KERNEL);
> + if (!transfer) {
> + ret = -ENOMEM;
> + goto err_unmap;
> + }
> +
> + transfer->epf_mhi = epf_mhi;
> + transfer->paddr = src_addr;
> + transfer->size = buf_info->size;
> + transfer->dir = DMA_TO_DEVICE;
> + memcpy(&transfer->buf_info, buf_info, sizeof(*buf_info));
> +
> + desc->callback = pci_epf_mhi_dma_async_callback;
> + desc->callback_param = transfer;
> +
> + cookie = dmaengine_submit(desc);
> + ret = dma_submit_error(cookie);
> + if (ret) {
> + dev_err(dev, "Failed to do DMA submit\n");
> + goto err_free_transfer;
> + }
> +
> + dma_async_issue_pending(chan);
> +
> + goto err_unlock;
> +
> +err_free_transfer:
> + kfree(transfer);
> +err_unmap:
> + dma_unmap_single(dma_dev, src_addr, buf_info->size, DMA_TO_DEVICE);
> +err_unlock:
> + mutex_unlock(&epf_mhi->lock);
> +
> + return ret;
> +}
> +
> struct epf_dma_filter {
> struct device *dev;
> u32 dma_mask;
> @@ -441,6 +646,7 @@ static int pci_epf_mhi_dma_init(struct pci_epf_mhi *epf_mhi)
> struct device *dev = &epf_mhi->epf->dev;
> struct epf_dma_filter filter;
> dma_cap_mask_t mask;
> + int ret;
>
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
> @@ -459,16 +665,35 @@ static int pci_epf_mhi_dma_init(struct pci_epf_mhi *epf_mhi)
> &filter);
> if (IS_ERR_OR_NULL(epf_mhi->dma_chan_rx)) {
> dev_err(dev, "Failed to request rx channel\n");
> - dma_release_channel(epf_mhi->dma_chan_tx);
> - epf_mhi->dma_chan_tx = NULL;
> - return -ENODEV;
> + ret = -ENODEV;
> + goto err_release_tx;
> + }
> +
> + epf_mhi->dma_wq = alloc_workqueue("pci_epf_mhi_dma_wq", 0, 0);
> + if (!epf_mhi->dma_wq) {
> + ret = -ENOMEM;
> + goto err_release_rx;
> }
>
> + INIT_LIST_HEAD(&epf_mhi->dma_list);
> + INIT_WORK(&epf_mhi->dma_work, pci_epf_mhi_dma_worker);
> + spin_lock_init(&epf_mhi->list_lock);
> +
> return 0;
> +
> +err_release_rx:
> + dma_release_channel(epf_mhi->dma_chan_rx);
> + epf_mhi->dma_chan_rx = NULL;
> +err_release_tx:
> + dma_release_channel(epf_mhi->dma_chan_tx);
> + epf_mhi->dma_chan_tx = NULL;
> +
> + return ret;
> }
>
> static void pci_epf_mhi_dma_deinit(struct pci_epf_mhi *epf_mhi)
> {
> + destroy_workqueue(epf_mhi->dma_wq);
> dma_release_channel(epf_mhi->dma_chan_tx);
> dma_release_channel(epf_mhi->dma_chan_rx);
> epf_mhi->dma_chan_tx = NULL;
Looks good!
Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
Krzysztof
next prev parent reply other threads:[~2023-12-13 18:50 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-27 12:45 [PATCH 0/9] bus: mhi: ep: Add async read/write support Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 1/9] bus: mhi: ep: Pass mhi_ep_buf_info struct to read/write APIs Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 2/9] bus: mhi: ep: Rename read_from_host() and write_to_host() APIs Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 3/9] bus: mhi: ep: Introduce async read/write callbacks Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 4/9] PCI: epf-mhi: Simulate async read/write using iATU Manivannan Sadhasivam
2023-12-13 18:49 ` Krzysztof Wilczyński
2023-11-27 12:45 ` [PATCH 5/9] PCI: epf-mhi: Add support for DMA async read/write operation Manivannan Sadhasivam
2023-12-13 18:50 ` Krzysztof Wilczyński [this message]
2023-11-27 12:45 ` [PATCH 6/9] PCI: epf-mhi: Enable MHI async read/write support Manivannan Sadhasivam
2023-12-13 18:48 ` Krzysztof Wilczyński
2023-12-14 5:19 ` Manivannan Sadhasivam
2023-12-14 9:40 ` Krishna Chaitanya Chundru
2023-12-14 10:09 ` Manivannan Sadhasivam
2023-12-14 10:14 ` Krishna Chaitanya Chundru
2023-12-14 10:47 ` Manivannan Sadhasivam
2023-12-14 10:54 ` Krishna Chaitanya Chundru
2023-11-27 12:45 ` [PATCH 7/9] bus: mhi: ep: Add support for async DMA write operation Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 8/9] bus: mhi: ep: Add support for async DMA read operation Manivannan Sadhasivam
2023-11-27 12:45 ` [PATCH 9/9] bus: mhi: ep: Add checks for read/write callbacks while registering controllers Manivannan Sadhasivam
2023-12-13 19:31 ` [PATCH 0/9] bus: mhi: ep: Add async read/write support Bjorn Helgaas
2023-12-14 5:21 ` Manivannan Sadhasivam
2023-12-14 10:55 ` Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231213185034.GC924726@rocinante \
--to=kw@linux.com \
--cc=bhelgaas@google.com \
--cc=kishon@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=mhi@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox