[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>
---

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: [Intel-gfx] [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.

[PATCH] drm/i915: iommu workaround related cleanups

[Ben Widawsky]

Cleanups recommended by Dave Airlie.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/char/agp/intel-agp.h |    6 ++++++
 drivers/char/agp/intel-gtt.c |    7 ++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
index 5da67f1..d8fa7d6 100644
--- a/drivers/char/agp/intel-agp.h
+++ b/drivers/char/agp/intel-agp.h
@@ -238,4 +238,10 @@
 int intel_gmch_probe(struct pci_dev *pdev,
 			       struct agp_bridge_data *bridge);
 void intel_gmch_remove(struct pci_dev *pdev);
+
+#ifdef CONFIG_INTEL_IOMMU
+/* This is a special note from the iommu driver that we are mapped through it */
+extern int intel_iommu_gfx_mapped;
+#endif
+
 #endif
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index c92424c..11985fb 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1183,14 +1183,13 @@ 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.
  */
 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.
@@ -1199,9 +1198,11 @@ 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;
 }
+#else
+static inline bool needs_idle_maps(void) { return false; }
+#endif
 
 static int i9xx_setup(void)
 {
-- 
1.7.7.1

Re: [PATCH] drm/i915: iommu workaround related cleanups

[Ben Widawsky]

On Tue,  1 Nov 2011 11:29:49 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> Cleanups recommended by Dave Airlie.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/char/agp/intel-agp.h |    6 ++++++
>  drivers/char/agp/intel-gtt.c |    7 ++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
> index 5da67f1..d8fa7d6 100644
> --- a/drivers/char/agp/intel-agp.h
> +++ b/drivers/char/agp/intel-agp.h
> @@ -238,4 +238,10 @@
>  int intel_gmch_probe(struct pci_dev *pdev,
>  			       struct agp_bridge_data *bridge);
>  void intel_gmch_remove(struct pci_dev *pdev);
> +
> +#ifdef CONFIG_INTEL_IOMMU
> +/* This is a special note from the iommu driver that we are mapped through it */
> +extern int intel_iommu_gfx_mapped;
> +#endif
> +
>  #endif
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index c92424c..11985fb 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -1183,14 +1183,13 @@ 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.
>   */
>  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.
> @@ -1199,9 +1198,11 @@ 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;
>  }
> +#else
> +static inline bool needs_idle_maps(void) { return false; }
> +#endif
>  
>  static int i9xx_setup(void)
>  {


I should have kept the ints and bools consistent. So if this patch looks
interesting to anyone, let's just say:

+#else
+static inline int needs_idle_maps(void) { return 0; }
+#endif