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 <[email protected]>
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 <[email protected]>
> Fixes: 58fd9375c2c5 ("drm/nouveau/platform: probe IOMMU if present")
Acked-by: Thierry Reding <[email protected]>
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 <[email protected]>
---
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;
--
2.4.0
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> ---
> 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 <[email protected]> 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.
On Wed, May 20, 2015 at 4:07 PM, Arnd Bergmann <[email protected]> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
Thanks. Dave, are you ok with this patch? If so, can you take it?