All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-tegra@vger.kernel.org, Sachin Nikam <snikam@nvidia.com>,
	dri-devel@lists.freedesktop.org,
	Puneet Saxena <puneets@nvidia.com>
Subject: [PATCH 09/12] drm/tegra: Remove memory allocation from Falcon library
Date: Mon, 28 Oct 2019 13:37:15 +0100	[thread overview]
Message-ID: <20191028123718.3890217-10-thierry.reding@gmail.com> (raw)
In-Reply-To: <20191028123718.3890217-1-thierry.reding@gmail.com>

From: Thierry Reding <treding@nvidia.com>

Having to provide allocator hooks to the Falcon library is somewhat
cumbersome and it doesn't give the users of the library a lot of
flexibility to deal with allocations. Instead, remove the notion of
Falcon "operations" and let drivers deal with the memory allocations
themselves.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/falcon.c | 50 ++-----------------
 drivers/gpu/drm/tegra/falcon.h | 11 ----
 drivers/gpu/drm/tegra/vic.c    | 91 ++++++++++++++++++++++++----------
 3 files changed, 68 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
index f49ad36e24db..a5b25e4ecbd2 100644
--- a/drivers/gpu/drm/tegra/falcon.c
+++ b/drivers/gpu/drm/tegra/falcon.c
@@ -59,26 +59,11 @@ static void falcon_copy_firmware_image(struct falcon *falcon,
 				       const struct firmware *firmware)
 {
 	u32 *firmware_vaddr = falcon->firmware.vaddr;
-	dma_addr_t daddr;
 	size_t i;
-	int err;
 
 	/* copy the whole thing taking into account endianness */
 	for (i = 0; i < firmware->size / sizeof(u32); i++)
 		firmware_vaddr[i] = le32_to_cpu(((u32 *)firmware->data)[i]);
-
-	/* ensure that caches are flushed and falcon can see the firmware */
-	daddr = dma_map_single(falcon->dev, firmware_vaddr,
-			       falcon->firmware.size, DMA_TO_DEVICE);
-	err = dma_mapping_error(falcon->dev, daddr);
-	if (err) {
-		dev_err(falcon->dev, "failed to map firmware: %d\n", err);
-		return;
-	}
-	dma_sync_single_for_device(falcon->dev, daddr,
-				   falcon->firmware.size, DMA_TO_DEVICE);
-	dma_unmap_single(falcon->dev, daddr, falcon->firmware.size,
-			 DMA_TO_DEVICE);
 }
 
 static int falcon_parse_firmware_image(struct falcon *falcon)
@@ -125,6 +110,8 @@ int falcon_read_firmware(struct falcon *falcon, const char *name)
 	if (err < 0)
 		return err;
 
+	falcon->firmware.size = falcon->firmware.firmware->size;
+
 	return 0;
 }
 
@@ -133,16 +120,6 @@ int falcon_load_firmware(struct falcon *falcon)
 	const struct firmware *firmware = falcon->firmware.firmware;
 	int err;
 
-	falcon->firmware.size = firmware->size;
-
-	/* allocate iova space for the firmware */
-	falcon->firmware.vaddr = falcon->ops->alloc(falcon, firmware->size,
-						    &falcon->firmware.paddr);
-	if (IS_ERR(falcon->firmware.vaddr)) {
-		dev_err(falcon->dev, "DMA memory mapping failed\n");
-		return PTR_ERR(falcon->firmware.vaddr);
-	}
-
 	/* copy firmware image into local area. this also ensures endianness */
 	falcon_copy_firmware_image(falcon, firmware);
 
@@ -150,27 +127,17 @@ int falcon_load_firmware(struct falcon *falcon)
 	err = falcon_parse_firmware_image(falcon);
 	if (err < 0) {
 		dev_err(falcon->dev, "failed to parse firmware image\n");
-		goto err_setup_firmware_image;
+		return err;
 	}
 
 	release_firmware(firmware);
 	falcon->firmware.firmware = NULL;
 
 	return 0;
-
-err_setup_firmware_image:
-	falcon->ops->free(falcon, falcon->firmware.size,
-			  falcon->firmware.paddr, falcon->firmware.vaddr);
-
-	return err;
 }
 
 int falcon_init(struct falcon *falcon)
 {
-	/* check mandatory ops */
-	if (!falcon->ops || !falcon->ops->alloc || !falcon->ops->free)
-		return -EINVAL;
-
 	falcon->firmware.vaddr = NULL;
 
 	return 0;
@@ -178,17 +145,8 @@ int falcon_init(struct falcon *falcon)
 
 void falcon_exit(struct falcon *falcon)
 {
-	if (falcon->firmware.firmware) {
+	if (falcon->firmware.firmware)
 		release_firmware(falcon->firmware.firmware);
-		falcon->firmware.firmware = NULL;
-	}
-
-	if (falcon->firmware.vaddr) {
-		falcon->ops->free(falcon, falcon->firmware.size,
-				  falcon->firmware.paddr,
-				  falcon->firmware.vaddr);
-		falcon->firmware.vaddr = NULL;
-	}
 }
 
 int falcon_boot(struct falcon *falcon)
diff --git a/drivers/gpu/drm/tegra/falcon.h b/drivers/gpu/drm/tegra/falcon.h
index 3d1243217410..92491a1e90df 100644
--- a/drivers/gpu/drm/tegra/falcon.h
+++ b/drivers/gpu/drm/tegra/falcon.h
@@ -74,15 +74,6 @@ struct falcon_fw_os_header_v1 {
 	u32 data_size;
 };
 
-struct falcon;
-
-struct falcon_ops {
-	void *(*alloc)(struct falcon *falcon, size_t size,
-		       dma_addr_t *paddr);
-	void (*free)(struct falcon *falcon, size_t size,
-		     dma_addr_t paddr, void *vaddr);
-};
-
 struct falcon_firmware_section {
 	unsigned long offset;
 	size_t size;
@@ -107,8 +98,6 @@ struct falcon {
 	/* Set by falcon client */
 	struct device *dev;
 	void __iomem *regs;
-	const struct falcon_ops *ops;
-	void *data;
 
 	struct falcon_firmware firmware;
 };
diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index 603f41ed4b81..4345b8054617 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -158,27 +158,6 @@ static int vic_boot(struct vic *vic)
 	return 0;
 }
 
-static void *vic_falcon_alloc(struct falcon *falcon, size_t size,
-			      dma_addr_t *iova)
-{
-	struct tegra_drm *tegra = falcon->data;
-
-	return tegra_drm_alloc(tegra, size, iova);
-}
-
-static void vic_falcon_free(struct falcon *falcon, size_t size,
-			    dma_addr_t iova, void *va)
-{
-	struct tegra_drm *tegra = falcon->data;
-
-	return tegra_drm_free(tegra, size, va, iova);
-}
-
-static const struct falcon_ops vic_falcon_ops = {
-	.alloc = vic_falcon_alloc,
-	.free = vic_falcon_free
-};
-
 static int vic_init(struct host1x_client *client)
 {
 	struct tegra_drm_client *drm = host1x_to_drm_client(client);
@@ -246,6 +225,15 @@ static int vic_exit(struct host1x_client *client)
 	host1x_channel_put(vic->channel);
 	host1x_client_iommu_detach(client);
 
+	if (client->group)
+		tegra_drm_free(tegra, vic->falcon.firmware.size,
+			       vic->falcon.firmware.vaddr,
+			       vic->falcon.firmware.paddr);
+	else
+		dma_free_coherent(vic->dev, vic->falcon.firmware.size,
+				  vic->falcon.firmware.vaddr,
+				  vic->falcon.firmware.paddr);
+
 	return 0;
 }
 
@@ -256,25 +244,75 @@ static const struct host1x_client_ops vic_client_ops = {
 
 static int vic_load_firmware(struct vic *vic)
 {
+	struct host1x_client *client = &vic->client.base;
+	struct tegra_drm *tegra = vic->client.drm;
+	dma_addr_t phys;
+	size_t size;
+	void *virt;
 	int err;
 
-	if (vic->falcon.data)
+	if (vic->falcon.firmware.vaddr)
 		return 0;
 
-	vic->falcon.data = vic->client.drm;
-
 	err = falcon_read_firmware(&vic->falcon, vic->config->firmware);
 	if (err < 0)
-		goto cleanup;
+		return err;
+
+	size = vic->falcon.firmware.size;
+
+	if (!client->group) {
+		virt = dma_alloc_coherent(vic->dev, size, &phys, GFP_KERNEL);
+
+		err = dma_mapping_error(vic->dev, phys);
+		if (err < 0)
+			return err;
+	} else {
+		virt = tegra_drm_alloc(tegra, size, &phys);
+	}
+
+	vic->falcon.firmware.vaddr = virt;
+	vic->falcon.firmware.paddr = phys;
 
 	err = falcon_load_firmware(&vic->falcon);
 	if (err < 0)
 		goto cleanup;
 
+	/*
+	 * In this case we have received an IOVA from the shared domain, so we
+	 * need to make sure to get the physical address so that the DMA API
+	 * knows what memory pages to flush the cache for.
+	 */
+	if (client->group) {
+		phys = dma_map_single(vic->dev, virt, size, DMA_TO_DEVICE);
+
+		err = dma_mapping_error(vic->dev, phys);
+		if (err < 0)
+			goto cleanup;
+
+		/*
+		 * If the DMA API mapped this through a bounce buffer, the
+		 * dma_sync_single_for_device() call below will not be able
+		 * to flush the caches for the right memory pages. Output a
+		 * big warning in that case so that the DMA mask can be set
+		 * properly and the bounce buffer avoided.
+		 */
+		WARN(phys != vic->falcon.firmware.paddr,
+		     "check DMA mask setting for %s\n", dev_name(vic->dev));
+	}
+
+	dma_sync_single_for_device(vic->dev, phys, size, DMA_TO_DEVICE);
+
+	if (client->group)
+		dma_unmap_single(vic->dev, phys, size, DMA_TO_DEVICE);
+
 	return 0;
 
 cleanup:
-	vic->falcon.data = NULL;
+	if (!client->group)
+		dma_free_coherent(vic->dev, size, virt, phys);
+	else
+		tegra_drm_free(tegra, size, virt, phys);
+
 	return err;
 }
 
@@ -415,7 +453,6 @@ static int vic_probe(struct platform_device *pdev)
 
 	vic->falcon.dev = dev;
 	vic->falcon.regs = vic->regs;
-	vic->falcon.ops = &vic_falcon_ops;
 
 	err = falcon_init(&vic->falcon);
 	if (err < 0)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2019-10-28 12:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 12:37 [PATCH 00/12] drm/tegra: Support IOMMU-backed DMA API Thierry Reding
2019-10-28 12:37 ` [PATCH 01/12] memory: tegra: Add gr2d and gr3d to DRM IOMMU group Thierry Reding
2019-10-30 15:05   ` Dmitry Osipenko
2019-11-01  9:56     ` Thierry Reding
2019-10-28 12:37 ` [PATCH 02/12] drm/tegra: Simplify IOMMU group selection Thierry Reding
2019-10-28 12:37 ` [PATCH 03/12] gpu: host1x: Overhaul host1x_bo_{pin,unpin}() API Thierry Reding
2019-10-28 12:37 ` [PATCH 04/12] gpu: host1x: Clean up debugfs on removal Thierry Reding
2019-10-28 12:37 ` [PATCH 05/12] gpu: host1x: Add direction flags to relocations Thierry Reding
2019-10-28 12:37 ` [PATCH 06/12] gpu: host1x: Allocate gather copy for host1x Thierry Reding
2019-10-28 12:37 ` [PATCH 07/12] gpu: host1x: Support DMA mapping of buffers Thierry Reding
2019-10-28 12:37 ` [PATCH 08/12] gpu: host1x: Set DMA mask based on IOMMU setup Thierry Reding
2019-10-28 12:37 ` Thierry Reding [this message]
2019-10-28 12:37 ` [PATCH 10/12] drm/tegra: falcon: Clarify address usage Thierry Reding
2019-10-28 12:37 ` [PATCH 11/12] drm/tegra: Support DMA API for display controllers Thierry Reding
2019-10-28 12:37 ` [PATCH 12/12] drm/tegra: Optionally attach clients to the IOMMU Thierry Reding

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=20191028123718.3890217-10-thierry.reding@gmail.com \
    --to=thierry.reding@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=puneets@nvidia.com \
    --cc=snikam@nvidia.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.