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.
Signed-off-by: Arnd Bergmann arnd@arndb.de Fixes: 58fd9375c2c5 ("drm/nouveau/platform: probe IOMMU if present")
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 5ab13e7939db..18dfa4af60ea 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -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
On Tue, May 19, 2015 at 02:53:26PM +0200, Arnd Bergmann wrote:
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.
Signed-off-by: Arnd Bergmann arnd@arndb.de Fixes: 58fd9375c2c5 ("drm/nouveau/platform: probe IOMMU if present")
Acked-by: Thierry Reding treding@nvidia.com
On 05/19/2015 09:53 PM, Arnd Bergmann wrote:
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?
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.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- 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(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c b/drivers/gpu/drm/nouveau/nouveau_platform.c index 775277f1edb0..dcfbbfaf1739 100644 --- a/drivers/gpu/drm/nouveau/nouveau_platform.c +++ b/drivers/gpu/drm/nouveau/nouveau_platform.c @@ -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;
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.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com
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.
Acked-by: Arnd Bergmann arnd@arndb.de
On Wed, May 20, 2015 at 4:07 PM, Arnd Bergmann arnd@arndb.de wrote:
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.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com
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.
Acked-by: Arnd Bergmann arnd@arndb.de
Thanks. Dave, are you ok with this patch? If so, can you take it?
On Wed, May 20, 2015 at 03:10:24PM +0900, 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.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com
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(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c b/drivers/gpu/drm/nouveau/nouveau_platform.c index 775277f1edb0..dcfbbfaf1739 100644 --- a/drivers/gpu/drm/nouveau/nouveau_platform.c +++ b/drivers/gpu/drm/nouveau/nouveau_platform.c @@ -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
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
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.
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
On Wed, May 20, 2015 at 9:01 PM, Arnd Bergmann arnd@arndb.de wrote:
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.
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.
dri-devel@lists.freedesktop.org
-
Alexandre Courbot -
Alexandre Courbot -
Arnd Bergmann -
Thierry Reding