From: Marco Pagani <marpagan@redhat.com>
To: Nava kishore Manne <nava.kishore.manne@amd.com>
Cc: mdf@kernel.org, hao.wu@intel.com, yilun.xu@intel.com,
trix@redhat.com, sumit.semwal@linaro.org,
christian.koenig@amd.com, linux-fpga@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org
Subject: Re: [RFC 1/2] fpga: support loading from a pre-allocated buffer
Date: Mon, 15 Jan 2024 18:08:31 +0100 [thread overview]
Message-ID: <12b50394-3d02-4fe2-9b00-97788b2a64ef@redhat.com> (raw)
In-Reply-To: <20231122053035.3758124-2-nava.kishore.manne@amd.com>
On 2023-11-22 06:30, Nava kishore Manne wrote:
> Some systems are memory constrained but they need to load very
> large Configuration files. The FPGA subsystem allows drivers to
> request this Configuration image be loaded from the filesystem,
> but this requires that the entire configuration data be loaded
> into kernel memory first before it's provided to the driver.
> This can lead to a situation where we map the configuration
> data twice, once to load the configuration data into kernel
> memory and once to copy the configuration data into the final
> resting place which is nothing but a dma-able continuous buffer.
>
> This creates needless memory pressure and delays due to multiple
> copies. Let's add a dmabuf handling support to the fpga manager
> framework that allows drivers to load the Configuration data
> directly from a pre-allocated buffer. This skips the intermediate
> step of allocating a buffer in kernel memory to hold the
> Configuration data.
Sharing images/bitstreams using dma-buf to avoid multiple copies
make sense to me to have a fast path for partial reconfiguration.
However, implementing the userspace interface for importing the
buffer at the manager level seems questionable, considering that
the manager should be responsible only for writing images.
Wouldn't it be conceptually cleaner to implement the interface for
importing dma-buf as a separate layer on top of the manager? Such a
layer could then program the FPGA using the standard write_sg
interface exported by the manager. In this way, each component would
have its own responsibility.
>
> Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
> ---
> drivers/fpga/fpga-mgr.c | 113 ++++++++++++++++++++++++++++++++++
> include/linux/fpga/fpga-mgr.h | 10 +++
> 2 files changed, 123 insertions(+)
>
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 06651389c592..23d2b4d45827 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -8,6 +8,8 @@
> * With code from the mailing list:
> * Copyright (C) 2013 Xilinx, Inc.
> */
> +#include <linux/dma-buf.h>
> +#include <linux/dma-map-ops.h>
> #include <linux/firmware.h>
> #include <linux/fpga/fpga-mgr.h>
> #include <linux/idr.h>
> @@ -519,6 +521,39 @@ static int fpga_mgr_buf_load(struct fpga_manager *mgr,
> return rc;
> }
>
> +static int fpga_dmabuf_load(struct fpga_manager *mgr,
> + struct fpga_image_info *info)
> +{
> + struct dma_buf_attachment *attach;
> + struct sg_table *sgt;
> + int ret;
> +
> + /* create attachment for dmabuf with the user device */
> + attach = dma_buf_attach(mgr->dmabuf, &mgr->dev);
> + if (IS_ERR(attach)) {
> + pr_err("failed to attach dmabuf\n");
> + ret = PTR_ERR(attach);
> + goto fail_put;
> + }
> +
> + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> + if (IS_ERR(sgt)) {
> + ret = PTR_ERR(sgt);
> + goto fail_detach;
> + }
> +
> + info->sgt = sgt;
> + ret = fpga_mgr_buf_load_sg(mgr, info, info->sgt);
> + dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> +
> +fail_detach:
> + dma_buf_detach(mgr->dmabuf, attach);
> +fail_put:
> + dma_buf_put(mgr->dmabuf);
> +
> + return ret;
> +}
> +
> /**
> * fpga_mgr_firmware_load - request firmware and load to fpga
> * @mgr: fpga manager
> @@ -573,6 +608,8 @@ int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
> {
> info->header_size = mgr->mops->initial_header_size;
>
> + if (mgr->flags & FPGA_MGR_CONFIG_DMA_BUF)
> + return fpga_dmabuf_load(mgr, info);
I'm not understanding the whole picture. After the dma-buf has been
imported from userspace, who is supposed to call fpga_mgr_load() or
fpga_region_program_fpga()? And who should load and export the dma-buf
containing the image in the first place?
I think it would be interesting to have a system that buffers a set of
alternative configurations for each (reconfigurable) region. Alternative
configurations could be represented and activated through a sysfs
interface. The user could request a specific configuration by writing in
the corresponding sysfs file, and the system would use the preloaded
image and optionally the overlay to configure the region. What do you
think?
> [...]
Thanks,
Marco
next prev parent reply other threads:[~2024-01-15 17:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-22 5:30 [RFC 0/2]fpga: Add fpga configuration support from a pre-allocated dma-able buffer Nava kishore Manne
2023-11-22 5:30 ` [RFC 1/2] fpga: support loading from a pre-allocated buffer Nava kishore Manne
2024-01-15 17:08 ` Marco Pagani [this message]
2023-11-22 5:30 ` [RFC 2/2] fpga: versal: Use the scatterlist interface Nava kishore Manne
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=12b50394-3d02-4fe2-9b00-97788b2a64ef@redhat.com \
--to=marpagan@redhat.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hao.wu@intel.com \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mdf@kernel.org \
--cc=nava.kishore.manne@amd.com \
--cc=sumit.semwal@linaro.org \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).