Re: [PATCH] drm/tegra: Remove of_dma_configure() from host1x_device_add()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 31/01/2024 15:33, Jason Gunthorpe wrote:
On Tue, Jan 30, 2024 at 09:55:18PM +0000, Jon Hunter wrote:

On 30/01/2024 16:15, Jason Gunthorpe wrote:
This was added in commit c95469aa5a18 ("gpu: host1x: Set DMA ops on device
creation") with the note:

      Currently host1x-instanciated devices have their dma_ops left to NULL,
      which makes any DMA operation (like buffer import) on ARM64 fallback
      to the dummy_dma_ops and fail with an error.

Since commit 14891af3799e ("iommu: Move the iommu driver sysfs setup into
iommu_init/deinit_device()") this call now fails because the struct device
is not fully configured enough to setup the sysfs and we now catch that
error.

This failure means the DMA ops are no longer set during this failing call.

Looking at it more it seems the arch dma ops are setup still, we
ignore the failure on multiple levels :(

It seems this is no longer a problem because
commit 07397df29e57 ("dma-mapping: move dma configuration to bus
infrastructure") added another call to of_dma_configure() inside the
bus_type->dma_configure() callback.

So long as a driver is probed the to the device it will have DMA properly
setup in the ordinary way.

Remove the unnecessary call which also removes the new long print:

[    1.200004] host1x drm: iommu configuration for device failed with -ENOENT

Reported-by: Diogo Ivo <diogo.ivo@xxxxxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/all/bbmhcoghrprmbdibnjum6lefix2eoquxrde7wyqeulm4xabmlm@b6jy32saugqh/
Reported-by: Jon Hunter <jonathanh@xxxxxxxxxx>
Closes: https://lore.kernel.org/all/b0334c5e-3a6c-4b58-b525-e72bed8899b3@xxxxxxxxxx/
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
---
   drivers/gpu/host1x/bus.c | 2 --
   1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index 84d042796d2e66..61214d35cadc34 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -458,8 +458,6 @@ static int host1x_device_add(struct host1x *host1x,
   	device->dev.bus = &host1x_bus_type;
   	device->dev.parent = host1x->dev;
-	of_dma_configure(&device->dev, host1x->dev->of_node, true);
-
   	device->dev.dma_parms = &device->dma_parms;
   	dma_set_max_seg_size(&device->dev, UINT_MAX);


In my case the warning is coming from the of_dma_configure_id() in
drivers/gpu/host1x/context.c. So with the above change I am still seeing the
warning.

You mean this sequence?

		err = device_add(&ctx->dev);
		if (err) {
			dev_err(host1x->dev, "could not add context device %d: %d\n", i, err);
			put_device(&ctx->dev);
			goto unreg_devices;
		}

		err = of_dma_configure_id(&ctx->dev, node, true, &i);
		if (err) {
			dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n",
				i, err);
			device_unregister(&ctx->dev);
			goto unreg_devices;
		}

Yes this sequence.

I didn't seem an obvious place that this would get fixed up later?

device_add() was done before so the iommu_device_link() shouldn't be
failing? Are you hitting a duplicate link (ie remove the nowarn from
iommu_device_link())


Removing the '_nowarn' does appear to fix it, although it is not clear to me why?

Thanks
Jon

--
nvpublic



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux