[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set

[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set

[Keith Packard]

Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.

Might be better to completely remove the work-around from the kernel
in this case?

Signed-off-by: Keith Packard <keithp@keithp.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/char/agp/intel-gtt.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..222190f 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1183,6 +1183,7 @@ static void gen6_cleanup(void)
 {
 }
 
+#ifdef CONFIG_INTEL_IOMMU
 /* Certain Gen5 chipsets require require idling the GPU before
  * unmapping anything from the GTT when VT-d is enabled.
  */
@@ -1201,6 +1202,7 @@ static inline int needs_idle_maps(void)
 
 	return 0;
 }
+#endif
 
 static int i9xx_setup(void)
 {
@@ -1236,8 +1238,10 @@ static int i9xx_setup(void)
 		intel_private.gtt_bus_addr = reg_addr + gtt_offset;
 	}
 
+#ifdef CONFIG_INTEL_IOMMU
 	if (needs_idle_maps())
 		intel_private.base.do_idle_maps = 1;
+#endif
 
 	intel_i9xx_setup_flush();
 
-- 
1.7.7

[PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set

[Keith Packard]

Kernels with no iommu support cannot ever need the Ironlake
work-around, so never enable it in that case.

Might be better to completely remove the work-around from the kernel
in this case?

Signed-off-by: Keith Packard <keithp@keithp.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
---

Here's a shorter patch which just elides the body of the needs_idle_maps functions

 drivers/char/agp/intel-gtt.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3a8d448..c92424c 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1186,10 +1186,11 @@ static void gen6_cleanup(void)
 /* Certain Gen5 chipsets require require idling the GPU before
  * unmapping anything from the GTT when VT-d is enabled.
  */
-extern int intel_iommu_gfx_mapped;
 static inline int needs_idle_maps(void)
 {
+#ifdef CONFIG_INTEL_IOMMU
 	const unsigned short gpu_devid = intel_private.pcidev->device;
+	extern int intel_iommu_gfx_mapped;
 
 	/* Query intel_iommu to see if we need the workaround. Presumably that
 	 * was loaded first.
@@ -1198,7 +1199,7 @@ static inline int needs_idle_maps(void)
 	     gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
 	     intel_iommu_gfx_mapped)
 		return 1;
-
+#endif
 	return 0;
 }
 
-- 
1.7.7

Re: [PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set

[Ben Widawsky]

On Fri, 28 Oct 2011 10:56:29 -0700
Keith Packard <keithp@keithp.com> wrote:

> Kernels with no iommu support cannot ever need the Ironlake
> work-around, so never enable it in that case.
> 
> Might be better to completely remove the work-around from the kernel
> in this case?
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> ---
wfm Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH] agp: iommu_gfx_mapped only available if CONFIG_INTEL_IOMMU is set

[Dave Airlie]

On Fri, Oct 28, 2011 at 6:56 PM, Keith Packard <keithp@keithp.com> wrote:
> Kernels with no iommu support cannot ever need the Ironlake
> work-around, so never enable it in that case.
>
> Might be better to completely remove the work-around from the kernel
> in this case?
>

While I'm not offended by this patch, I am offended by the

extern int intel_iommu_gfx_mapped;

introduced in the previous patches, I dislike having extern
declarations in a C file, a header file is the place for that sort of
thing.

Also have a look around some other places that do CONFIG_ stuff, I
think there may be examples of both in which case this approach is
fine, but I do remember there being a fair few,

#ifdef CONFIG_BLAH
function
#else
#define function do while(0)
#endif

Dave.