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; }
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
On Fri, Oct 28, 2011 at 15:56, 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
Reviewed-By: Eugeni Dodonov eugeni.dodonov@intel.com
We have no need for the workaround when we don't have IOMMU.
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.
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) {
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
dri-devel@lists.freedesktop.org
-
Ben Widawsky -
Dave Airlie -
Eugeni Dodonov -
Keith Packard