Re: [PATCH] drm/msm: Check for the GPU IOMMU during bind

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

 



On 10/03/2023 00:20, Jordan Crouse wrote:
While booting with amd,imageon on a headless target the GPU probe was
failing with -ENOSPC in get_pages() from msm_gem.c.

Investigation showed that the driver was using the default 16MB VRAM
carveout because msm_use_mmu() was returning false since headless devices
use a dummy parent device. Avoid this by extending the existing is_a2xx
priv member to check the GPU IOMMU state on all platforms and use that
check in msm_use_mmu().

I wonder if we can fix this by setting 'dummy_dev'->of_node to adreno's of_node. Did you check that possibility?


This works for memory allocations but it doesn't prevent the VRAM carveout
from being created because that happens before we have a chance to check
the GPU IOMMU state in adreno_bind.

There are a number of possible options to resolve this but none of them are
very clean. The easiest way is to likely specify vram=0 as module parameter
on headless devices so that the memory doesn't get wasted.

Signed-off-by: Jordan Crouse <jorcrous@xxxxxxxxxx>
---

  drivers/gpu/drm/msm/adreno/adreno_device.c | 6 +++++-
  drivers/gpu/drm/msm/msm_drv.c              | 7 +++----
  drivers/gpu/drm/msm/msm_drv.h              | 2 +-
  3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 36f062c7582f..4f19da28f80f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -539,7 +539,11 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
  	DBG("Found GPU: %u.%u.%u.%u", config.rev.core, config.rev.major,
  		config.rev.minor, config.rev.patchid);
- priv->is_a2xx = config.rev.core == 2;
+	/*
+	 * A2xx has a built in IOMMU and all other IOMMU enabled targets will
+	 * have an ARM IOMMU attached
+	 */
+	priv->has_gpu_iommu = config.rev.core == 2 || device_iommu_mapped(dev);
  	priv->has_cached_coherent = config.rev.core >= 6;
gpu = info->init(drm);
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index aca48c868c14..a125a351ec90 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -318,11 +318,10 @@ bool msm_use_mmu(struct drm_device *dev)
  	struct msm_drm_private *priv = dev->dev_private;
/*
-	 * a2xx comes with its own MMU
-	 * On other platforms IOMMU can be declared specified either for the
-	 * MDP/DPU device or for its parent, MDSS device.
+	 * Return true if the GPU or the MDP/DPU or parent MDSS device has an
+	 * IOMMU
  	 */
-	return priv->is_a2xx ||
+	return priv->has_gpu_iommu ||
  		device_iommu_mapped(dev->dev) ||
  		device_iommu_mapped(dev->dev->parent);

It is not a problem of you patch, of course, but this check now looks strange to me. We mix the GPU check and MDP/DPU checks. Consider msm8x60 (a220, mdp4) and, for example, no system level MMU.

  }
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 9f0c184b02a0..f33f94acd1b9 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -126,7 +126,7 @@ struct msm_drm_private {
  	struct msm_gpu *gpu;
/* gpu is only set on open(), but we need this info earlier */
-	bool is_a2xx;
+	bool has_gpu_iommu;
  	bool has_cached_coherent;
struct drm_fb_helper *fbdev;

--
With best wishes
Dmitry




[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