[Intel-gfx] [PATCH 4/4] drm/i915: ILK + VT-d workaround
Chris Wilson
chris at chris-wilson.co.uk
Sun Oct 16 00:09:28 CEST 2011
On Sat, 15 Oct 2011 13:47:16 -0700, Ben Widawsky <ben at bwidawsk.net> wrote:
> Idle the GPU before doing any unmaps. We know if VT-d is in use through
> an exported variable from iommu code.
>
> This should avoid a known HW issue.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
Just one minor comment below, but
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/char/agp/intel-gtt.c | 28 ++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 4 +++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++++++++++++++++++-
> include/drm/intel-gtt.h | 2 ++
> 5 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 8515101..80a7ed0 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -923,6 +923,9 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem,
> {
> int ret = -EINVAL;
>
> + if (intel_private.base.do_idle_maps)
> + return -ENODEV;
> +
> if (intel_private.clear_fake_agp) {
> int start = intel_private.base.stolen_size / PAGE_SIZE;
> int end = intel_private.base.gtt_mappable_entries;
> @@ -985,6 +988,9 @@ static int intel_fake_agp_remove_entries(struct agp_memory *mem,
> if (mem->page_count == 0)
> return 0;
>
> + if (intel_private.base.do_idle_maps)
> + return -ENODEV;
> +
> intel_gtt_clear_range(pg_start, mem->page_count);
>
> if (intel_private.base.needs_dmar) {
> @@ -1177,6 +1183,25 @@ 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)
> +{
> + const unsigned short gpu_devid = intel_private.pcidev->device;
> +
> + /* Query intel_iommu to see if we need the workaround. Presumably that
> + * was loaded first.
> + */
> + if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB ||
> + gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) &&
> + intel_iommu_gfx_mapped)
> + return 1;
> +
> + return 0;
> +}
> +
> static int i9xx_setup(void)
> {
> u32 reg_addr;
> @@ -1211,6 +1236,9 @@ static int i9xx_setup(void)
> intel_private.gtt_bus_addr = reg_addr + gtt_offset;
> }
>
> + if (needs_idle_maps());
> + intel_private.base.do_idle_maps = 1;
> +
> intel_i9xx_setup_flush();
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15c0ca5..3aa8612 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1219,7 +1219,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev);
> int __must_check i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj);
> void i915_gem_gtt_rebind_object(struct drm_i915_gem_object *obj,
> enum i915_cache_level cache_level);
> -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
> +int i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
Make this a __must_check, if we forget to handle the error from unbind,
it will undoubtably lead to great confusion and an upset chip/kernel.
Admittedly today there is only callsite, but useful defence against
future callers making a mistake.
>
> /* i915_gem_evict.c */
> int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a925141..89d22eb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2130,7 +2130,9 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
>
> trace_i915_gem_object_unbind(obj);
>
> - i915_gem_gtt_unbind_object(obj);
> + ret = i915_gem_gtt_unbind_object(obj);
> + if (ret == -ERESTARTSYS)
> + return ret;
> i915_gem_object_put_pages_gtt(obj);
>
> list_del_init(&obj->gtt_list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7a709cd..1df6395 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -49,6 +49,15 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> }
> }
>
> +static bool do_idling(struct drm_i915_private *dev_priv)
> +{
> + if (dev_priv->mm.gtt->do_idle_maps)
> + if (i915_gpu_idle(dev_priv->dev))
> + return false;
> +
> + return true;
> +}
> +
> void i915_gem_restore_gtt_mappings(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -115,8 +124,14 @@ void i915_gem_gtt_rebind_object(struct drm_i915_gem_object *obj,
> agp_type);
> }
>
> -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
> +int i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
> {
> + struct drm_device *dev = obj->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (do_idling(dev_priv) == false)
> + return -ERESTARTSYS;
> +
> intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
> obj->base.size >> PAGE_SHIFT);
>
> @@ -124,4 +139,6 @@ void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
> intel_gtt_unmap_memory(obj->sg_list, obj->num_sg);
> obj->sg_list = NULL;
> }
> +
> + return 0;
> }
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index 9e343c0..b174620 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -13,6 +13,8 @@ const struct intel_gtt {
> unsigned int gtt_mappable_entries;
> /* Whether i915 needs to use the dmar apis or not. */
> unsigned int needs_dmar : 1;
> + /* Whether we idle the gpu before mapping/unmapping */
> + unsigned int do_idle_maps : 1;
> } *intel_gtt_get(void);
>
> void intel_gtt_chipset_flush(void);
> --
> 1.7.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list