The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
Cc: "Vinod Koul" <vkoul@kernel.org>, "Frank Li" <Frank.Li@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Jeff Hugo" <jeff.hugo@oss.qualcomm.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	dmaengine@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, mhi@lists.linux.dev,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 4/5] PCI: epf-mhi: Add batched DMA read support
Date: Thu, 6 Aug 2026 12:11:34 -0500	[thread overview]
Message-ID: <anTARsISvCPUA9_v@SMW015318> (raw)
In-Reply-To: <20260803-dma_multi_sg-v2-4-c12bb05e42d6@oss.qualcomm.com>

On Mon, Aug 03, 2026 at 04:01:46PM +0530, Sumit Kumar wrote:
> [You don't often get email from sumit.kumar@oss.qualcomm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Add support for batched DMA transfers in the PCI EPF MHI driver to
> improve performance when reading multiple buffers from the host.
>
> Implement two variants of the read_batch() callback:
> pci_epf_mhi_edma_read_batch() is a DMA-optimized implementation that uses
> dmaengine_prep_dma_sg() to transfer multiple buffers in a single DMA
> transaction, while pci_epf_mhi_iatu_read_batch() serves as a CPU-copy
> fallback for platforms without DMA support by sequentially processing each
> buffer via IATU mapping. Wire up read_batch() to the eDMA variant only
> when the RX DMA channel advertises the DMA_SG capability, falling back to
> the IATU variant otherwise.
>
> On a successful batch, notify completion for every buffer via its
> caller-supplied buf_info->cb, mirroring the read_sync/read_async
> completion semantics so the MHI stack can free each buffer and raise its
> transfer completion event. Make read_batch() a mandatory callback in
> mhi_ep_register_controller(), alongside read_sync/write_sync/read_async/
> write_async.
>
> This enables the MHI endpoint stack to cache ring data efficiently,
> particularly for wraparound scenarios where ring data spans two
> non-contiguous memory regions.
>
> Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> ---
>  drivers/bus/mhi/ep/main.c                    |   3 +-
>  drivers/pci/endpoint/functions/pci-epf-mhi.c | 159 +++++++++++++++++++++++++++
>  include/linux/mhi_ep.h                       |   7 ++
>  3 files changed, 168 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 21bc2c50170ff7f0181c0d0d4eefd51db30f34c8..c45240fc481dc3260a61b546c223b6be8e105587 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -1459,7 +1459,8 @@ int mhi_ep_register_controller(struct mhi_ep_cntrl *mhi_cntrl,
>                 return -EINVAL;
>
>         if (!mhi_cntrl->read_sync || !mhi_cntrl->write_sync ||
> -           !mhi_cntrl->read_async || !mhi_cntrl->write_async)
> +           !mhi_cntrl->read_async || !mhi_cntrl->write_async ||
> +           !mhi_cntrl->read_batch)
>                 return -EINVAL;
>
>         ret = mhi_ep_chan_init(mhi_cntrl, config);
> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 6bac69fc84b472c5f1abbeede2b441b5db73c784..ad245148de1462cffc950ba9a9e232405514dfb7 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -444,6 +444,162 @@ static int pci_epf_mhi_edma_write(struct mhi_ep_cntrl *mhi_cntrl,
>         return ret;
>  }
>
> +static int pci_epf_mhi_iatu_read_batch(struct mhi_ep_cntrl *mhi_cntrl,
> +                                      struct mhi_ep_buf_info *buf_info_array,
> +                                      u32 num_buffers)
> +{
> +       struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> +       struct device *dev = &epf_mhi->epf->dev;
> +       u32 i;
> +       int ret;
> +
> +       if (num_buffers == 0)
> +               return -EINVAL;
> +
> +       for (i = 0; i < num_buffers; i++) {
> +               ret = pci_epf_mhi_iatu_read(mhi_cntrl, &buf_info_array[i]);
> +               if (ret < 0) {
> +                       dev_err(dev, "Failed to read buffer %u of %u in batch: %d\n",
> +                               i, num_buffers, ret);
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int pci_epf_mhi_edma_read_batch(struct mhi_ep_cntrl *mhi_cntrl,
> +                                      struct mhi_ep_buf_info *buf_info_array,
> +                                      u32 num_buffers)
> +{
> +       struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> +       struct device *dma_dev = epf_mhi->epf->epc->dev.parent;
> +       struct device *dev = &epf_mhi->epf->dev;
> +       struct dma_async_tx_descriptor *desc;
> +       struct dma_slave_config config = {};
> +       DECLARE_COMPLETION_ONSTACK(complete);
> +       struct scatterlist *src_sg;
> +       struct scatterlist *dst_sg;
> +       unsigned long time_left;
> +       struct dma_chan *chan;
> +       dma_cookie_t cookie;
> +       unsigned int i;
> +       int mapped;
> +       void *buf;
> +       int ret;
> +
> +       if (num_buffers == 0)
> +               return -EINVAL;
> +
> +       /*
> +        * Single allocation carved into two arrays: src_sg[], dst_sg[].
> +        * Reduces allocator round-trips on the ring-cache hot path. Done
> +        * before taking the lock so direct reclaim cannot stall other
> +        * transfers waiting on epf_mhi->lock.
> +        */
> +       buf = kcalloc(num_buffers, 2 * sizeof(*src_sg), GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +       src_sg = buf;
> +       dst_sg = src_sg + num_buffers;
> +
> +       mutex_lock(&epf_mhi->lock);
> +
> +       chan = epf_mhi->dma_chan_rx;
> +       if (!chan) {
> +               ret = -ENODEV;
> +               goto err_unlock;
> +       }
> +
> +       sg_init_table(src_sg, num_buffers);
> +       sg_init_table(dst_sg, num_buffers);
> +
> +       for (i = 0; i < num_buffers; i++) {
> +               /*
> +                * src addresses are PCIe host bus addresses already visible to
> +                * the eDMA engine; no dma_map_sg() is needed for the source list.
> +                */
> +               sg_dma_address(&src_sg[i]) = buf_info_array[i].host_addr;
> +               sg_dma_len(&src_sg[i]) = buf_info_array[i].size;
> +
> +               sg_set_buf(&dst_sg[i], buf_info_array[i].dev_addr, buf_info_array[i].size);
> +       }
> +
> +       mapped = dma_map_sg(dma_dev, dst_sg, num_buffers, DMA_FROM_DEVICE);
> +       if (!mapped) {
> +               dev_err(dev, "Failed to map destination buffers for %u-buffer batch read\n",
> +                       num_buffers);
> +               ret = -EIO;
> +               goto err_unlock;
> +       }
> +
> +       config.direction = DMA_DEV_TO_MEM;
> +       ret = dmaengine_slave_config(chan, &config);
> +       if (ret) {
> +               dev_err(dev, "Failed to configure DMA channel for %u-buffer batch read: %d\n",
> +                       num_buffers, ret);
> +               goto err_unmap;
> +       }
> +
> +       desc = dmaengine_prep_dma_sg(chan, dst_sg, num_buffers,
> +                                    src_sg, num_buffers,
> +                                    DMA_CTRL_ACK | DMA_PREP_INTERRUPT);

Although both side memory address, but as previous discussion, which is not
memcpy, still memory to IO space transfer.

Maybe enhence device_prep_peripheral_dma_vec(). Actually eDMA still split
it to small trunk and linked together.

you can submit more than one require before issue_pending().

Koichiro Den is working on dynmatic append request.

https://lore.kernel.org/dmaengine/20260729143036.3087722-1-den@valinux.co.jp/

Frank

> +       if (!desc) {
> +               dev_err(dev, "Failed to prepare batch DMA\n");
> +               ret = -EIO;
> +               goto err_unmap;
> +       }
> +
> +       desc->callback = pci_epf_mhi_dma_callback;
> +       desc->callback_param = &complete;
> +
> +       cookie = dmaengine_submit(desc);
> +       ret = dma_submit_error(cookie);
> +       if (ret) {
> +               dev_err(dev, "Failed to submit DMA\n");
> +               if (dmaengine_terminate_sync(chan))
> +                       dev_err(dev, "Failed to terminate DMA channel after submit failure\n");
> +               goto err_unmap;
> +       }
> +
> +       dma_async_issue_pending(chan);
> +
> +       time_left = wait_for_completion_timeout(&complete,
> +                                               msecs_to_jiffies(PCI_EPF_MHI_DMA_TIMEOUT_MS));
> +       if (!time_left) {
> +               dev_err(dev, "DMA transfer timeout\n");
> +               if (dmaengine_terminate_sync(chan))
> +                       dev_err(dev, "Failed to terminate DMA channel after timeout\n");
> +               ret = -ETIMEDOUT;
> +               goto err_unmap;
> +       }
> +
> +       ret = 0;
> +
> +err_unmap:
> +       /* dma_unmap_sg() must use the nents passed to dma_map_sg(), not its return value */
> +       dma_unmap_sg(dma_dev, dst_sg, num_buffers, DMA_FROM_DEVICE);
> +err_unlock:
> +       mutex_unlock(&epf_mhi->lock);
> +
> +       kfree(buf);
> +
> +       /*
> +        * On a successful batch, notify completion for every buffer via its
> +        * caller-supplied callback so MHI can free the backing buffer and raise
> +        * the transfer completion event per entry, matching the
> +        * read_sync/read_async semantics. Done after dropping the lock as a
> +        * callback may re-enter the driver and epf_mhi->lock is not reentrant.
> +        */
> +       if (!ret) {
> +               for (i = 0; i < num_buffers; i++)
> +                       if (buf_info_array[i].cb)
> +                               buf_info_array[i].cb(&buf_info_array[i]);
> +       }
> +
> +       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);
> @@ -789,11 +945,14 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
>         mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free;
>         mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read;
>         mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write;
> +       mhi_cntrl->read_batch = pci_epf_mhi_iatu_read_batch;
>         if (info->flags & MHI_EPF_USE_DMA) {
>                 mhi_cntrl->read_sync = pci_epf_mhi_edma_read;
>                 mhi_cntrl->write_sync = pci_epf_mhi_edma_write;
>                 mhi_cntrl->read_async = pci_epf_mhi_edma_read_async;
>                 mhi_cntrl->write_async = pci_epf_mhi_edma_write_async;
> +               if (dma_has_cap(DMA_SG, epf_mhi->dma_chan_rx->device->cap_mask))
> +                       mhi_cntrl->read_batch = pci_epf_mhi_edma_read_batch;
>         }
>
>         /* Register the MHI EP controller */
> diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
> index 7b40fc8cbe77ab8419d167e89264b69a817b9fb1..51f66cae937a53a93e9ae76cdcfaac7b1e08283f 100644
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
> @@ -107,6 +107,11 @@ struct mhi_ep_buf_info {
>   * @write_sync: CB function for writing to host memory synchronously
>   * @read_async: CB function for reading from host memory asynchronously
>   * @write_async: CB function for writing to host memory asynchronously
> + * @read_batch: CB function for reading from host memory in batches synchronously.
> + *              Callers must pass at least one buffer (num_buffers > 0); each
> + *              buf_info entry must have host_addr, dev_addr and size set.
> + *              Each buffer's buf_info->cb, if set, is invoked once the batch
> + *              completes successfully, mirroring read_sync/read_async.
>   * @mhi_state: MHI Endpoint state
>   * @max_chan: Maximum channels supported by the endpoint controller
>   * @mru: MRU (Maximum Receive Unit) value of the endpoint controller
> @@ -164,6 +169,8 @@ struct mhi_ep_cntrl {
>         int (*write_sync)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
>         int (*read_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
>         int (*write_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
> +       int (*read_batch)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info_array,
> +                         u32 num_buffers);
>
>         enum mhi_state mhi_state;
>
>
> --
> 2.34.1
>

  reply	other threads:[~2026-08-06 17:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-03 10:31 [PATCH v2 0/5] dmaengine: Add batched scatter-gather DMA support Sumit Kumar
2026-08-03 10:31 ` [PATCH v2 1/5] dmaengine: Add DMA_SG support for multi-buffer scatter-gather transfers Sumit Kumar
2026-08-03 10:31 ` [PATCH v2 2/5] dmaengine: dw-edma: Add DMA_SG support Sumit Kumar
2026-08-06 19:06   ` Frank Li
2026-08-03 10:31 ` [PATCH v2 3/5] PCI: epf-mhi: Use a define for the DMA transfer timeout Sumit Kumar
2026-08-03 10:31 ` [PATCH v2 4/5] PCI: epf-mhi: Add batched DMA read support Sumit Kumar
2026-08-06 17:11   ` Frank Li [this message]
2026-08-03 10:31 ` [PATCH v2 5/5] bus: mhi: ep: Use batched read for ring caching Sumit Kumar

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=anTARsISvCPUA9_v@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=corbet@lwn.net \
    --cc=dmaengine@vger.kernel.org \
    --cc=jeff.hugo@oss.qualcomm.com \
    --cc=kishon@kernel.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=mhi@lists.linux.dev \
    --cc=skhan@linuxfoundation.org \
    --cc=sumit.kumar@oss.qualcomm.com \
    --cc=vkoul@kernel.org \
    /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