Skip to contentSkip to conversation

drm/nouveau/platform: add IOMMU dependency

PATCH In review Arnd Bergmann 4 participants 9 messages May 19, 2015 – May 26, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/nouveau/platform: add IOMMU dependency

The recently added iommu code in the nouveau driver fails to build when the IOMMU support is disabled:

drivers/gpu/drm/nouveau/nouveau_platform.c: In function 'nouveau_platform_probe_iommu':
drivers/gpu/drm/nouveau/nouveau_platform.c:113:41: error: 'const struct iommu_ops' has no mem

To avoid the build error, this now adds an explicit dependency on the IOMMU implementation.

Fixes 58fd9375c2c5 ("drm/nouveau/platform: probe IOMMU if present")
drivers/gpu/drm/nouveau/Kconfig +1 -0
index 5ab13e7939db..18dfa4af60ea 100644
@@ -28,6 +28,7 @@ config DRM_NOUVEAU
 config NOUVEAU_PLATFORM_DRIVER
 	bool "Nouveau (NVIDIA) SoC GPUs"
 	depends on DRM_NOUVEAU && ARCH_TEGRA
+	depends on IOMMU
 	default y
 	help
 	  Support for Nouveau platform driver, used for SoC GPUs as found
Acked-by on v1 1/1
#1 lore raw
Quoted 11 lines · Arnd Bergmann

The recently added iommu code in the nouveau driver fails to build when the IOMMU support is disabled:

drivers/gpu/drm/nouveau/nouveau_platform.c: In function 'nouveau_platform_probe_iommu':
drivers/gpu/drm/nouveau/nouveau_platform.c:113:41: error: 'const struct iommu_ops' has no mem

To avoid the build error, this now adds an explicit dependency on the IOMMU implementation.

Fixes 58fd9375c2c5 ("drm/nouveau/platform: probe IOMMU if present")

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150519/b896fe1c/attachment.sig>

#2 lore raw
In reply to Arnd Bergmann
Quoted 8 lines · Arnd Bergmann

The recently added iommu code in the nouveau driver fails to build when the IOMMU support is disabled:

drivers/gpu/drm/nouveau/nouveau_platform.c: In function 'nouveau_platform_probe_iommu':
drivers/gpu/drm/nouveau/nouveau_platform.c:113:41: error: 'const struct iommu_ops' has no mem

To avoid the build error, this now adds an explicit dependency on the IOMMU implementation.

I have a local patch to nouveau_platform.c that only calls the IOMMU functions if CONFIG_IOMMU is set. Wouldn't this be more suitable as IOMMU support is only used by Tegra and thus not beneficial for desktop GPUs?

#3 lore raw

PATCH 1/1 drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

The lack of IOMMU API support can make nouveau_platform_probe_iommu() fail to compile because struct iommu_ops is then empty. Fix this by skipping IOMMU probe in that case - lack of IOMMU on platform devices is sub-optimal, but is not an error.

---
This is an alternative to https://lkml.org/lkml/2015/5/19/484. Most users of Nouveau do not care about IOMMU support, so we should not impose that option on them.

 drm/nouveau/nouveau_platform.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/gpu/drm/nouveau/nouveau_platform.c +16 -0
index 775277f1edb0..dcfbbfaf1739 100644
@@ -92,6 +92,8 @@ static int nouveau_platform_power_down(struct nouveau_platform_gpu *gpu)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_IOMMU_API)
+
 static void nouveau_platform_probe_iommu(struct device *dev,
 					 struct nouveau_platform_gpu *gpu)
 {
@@ -158,6 +160,20 @@ static void nouveau_platform_remove_iommu(struct device *dev,
 	}
 }
 
+#else
+
+static void nouveau_platform_probe_iommu(struct device *dev,
+					 struct nouveau_platform_gpu *gpu)
+{
+}
+
+static void nouveau_platform_remove_iommu(struct device *dev,
+					  struct nouveau_platform_gpu *gpu)
+{
+}
+
+#endif
+
 static int nouveau_platform_probe(struct platform_device *pdev)
 {
 	struct nouveau_platform_gpu *gpu;
Signature · 1 line
2.4.0
author Acked-by on v1 1/1
#4 lore raw

Re: [PATCH] drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

Quoted 11 lines · On Wednesday 20 May 2015 15:10:24 Alexandre Courbot wrote:

The lack of IOMMU API support can make nouveau_platform_probe_iommu() fail to compile because struct iommu_ops is then empty. Fix this by skipping IOMMU probe in that case - lack of IOMMU on platform devices is sub-optimal, but is not an error.

---
This is an alternative to https://lkml.org/lkml/2015/5/19/484. Most users of Nouveau do not care about IOMMU support, so we should not impose that option on them.

Yes, good idea.

#5 lore raw

Re: [PATCH] drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · On Wednesday 20 May 2015 15:10:24 Alexandre Courbot wrote:

The lack of IOMMU API support can make nouveau_platform_probe_iommu() fail to compile because struct iommu_ops is then empty. Fix this by skipping IOMMU probe in that case - lack of IOMMU on platform devices is sub-optimal, but is not an error.

---
This is an alternative to https://lkml.org/lkml/2015/5/19/484. Most users of Nouveau do not care about IOMMU support, so we should not impose that option on them.

Quoted 3 lines

Yes, good idea.

Thanks. Dave, are you ok with this patch? If so, can you take it?

#6 lore raw

Re: [PATCH] drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

In reply to Alexandre Courbot
Quoted 35 lines · Alexandre Courbot

The lack of IOMMU API support can make nouveau_platform_probe_iommu() fail to compile because struct iommu_ops is then empty. Fix this by skipping IOMMU probe in that case - lack of IOMMU on platform devices is sub-optimal, but is not an error.

---
This is an alternative to https://lkml.org/lkml/2015/5/19/484. Most users of Nouveau do not care about IOMMU support, so we should not impose that option on them.

 drm/nouveau/nouveau_platform.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/gpu/drm/nouveau/nouveau_platform.c +6 -0
index 775277f1edb0..dcfbbfaf1739 100644
@@ -92,6 +92,8 @@ static int nouveau_platform_power_down(struct nouveau_platform_gpu *gpu)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_IOMMU_API)
+
 static void nouveau_platform_probe_iommu(struct device *dev,
 					 struct nouveau_platform_gpu *gpu)
 {
@@ -158,6 +160,20 @@ static void nouveau_platform_remove_iommu(struct device *dev,
 	}
 }
 
+#else
+
+static void nouveau_platform_probe_iommu(struct device *dev,
+					 struct nouveau_platform_gpu *gpu)

drivers/gpu/drm/nouveau/nouveau_platform.c static void nouveau_platform_remove_iommu(struct device *dev, · line 176

Quoted 10 lines
drivers/gpu/drm/nouveau/nouveau_platform.c +10 -0
+{
+}
+
+static void nouveau_platform_remove_iommu(struct device *dev,
+					  struct nouveau_platform_gpu *gpu)
+{
+}
+
+#endif
+

Since these are all static functions, perhaps an "if (IS_ENABLED(...))" would work here? That way you'd get compile coverage of the code in all cases.

But perhaps that doesn't work for IOMMU. I have a vague memory of running across something like this before and IOMMU has this quirk of defining struct iommu_ops as empty if IOMMU_API is deselected so you'll probably get compiler errors unless you actually preprocess the code out.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150520/b2177925/attachment.sig>

#7 lore raw

Re: [PATCH] drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

Quoted 4 lines · On Wednesday 20 May 2015 13:32:33 Thierry Reding wrote:

Since these are all static functions, perhaps an "if (IS_ENABLED(...))" would work here? That way you'd get compile coverage of the code in all cases.

I had the same thought at first.

Quoted 5 lines

But perhaps that doesn't work for IOMMU. I have a vague memory of running across something like this before and IOMMU has this quirk of defining struct iommu_ops as empty if IOMMU_API is deselected so you'll probably get compiler errors unless you actually preprocess the code out.

Exactly.

Arnd

#8 lore raw

Re: [PATCH] drm/nouveau/platform: fix compilation if !CONFIG_IOMMU

Quoted 8 lines · Arnd Bergmann
Quoted 4 lines · On Wednesday 20 May 2015 13:32:33 Thierry Reding wrote:

Since these are all static functions, perhaps an "if (IS_ENABLED(...))" would work here? That way you'd get compile coverage of the code in all cases.

I had the same thought at first.

Quoted 7 lines
Quoted 5 lines

But perhaps that doesn't work for IOMMU. I have a vague memory of running across something like this before and IOMMU has this quirk of defining struct iommu_ops as empty if IOMMU_API is deselected so you'll probably get compiler errors unless you actually preprocess the code out.

Exactly.

That's precisely the issue here, so not covering this code is exactly what we want if !CONFIG_IOMMU.