public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Flush the PTEs after updating them before suspend
@ 2014-09-18  6:03 Chris Wilson
  2014-09-18 11:52 ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-09-18  6:03 UTC (permalink / raw)
  To: intel-gfx

As we use WC updates of the PTE, we are responsible for notifying the
hardware when to flush its TLBs. Do so after we zap all the PTEs before
suspend (and the BIOS tries to read our GTT).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
Tested-by: ming.yao@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 97ba18846761..aa81b217fac0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1234,6 +1234,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
 	POSTING_READ(RING_FAULT_REG(RCS_ENGINE(dev_priv)));
 }
 
+static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen < 6) {
+		intel_gtt_chipset_flush();
+	} else {
+		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+		POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	}
+}
+
 void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1250,6 +1260,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 				       dev_priv->gtt.base.start,
 				       dev_priv->gtt.base.total,
 				       true);
+
+	i915_ggtt_flush(dev_priv);
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev)
@@ -1302,7 +1314,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
 	}
 
-	i915_gem_chipset_flush(dev);
+	i915_ggtt_flush(dev_priv);
 }
 
 int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-18  6:03 [PATCH] drm/i915: Flush the PTEs after updating them before suspend Chris Wilson
@ 2014-09-18 11:52 ` Daniel Vetter
  2014-09-23 11:55   ` Jani Nikula
  2014-09-25  8:40   ` Chris Wilson
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel Vetter @ 2014-09-18 11:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Takashi Iwai, intel-gfx, Paulo Zanoni

On Thu, Sep 18, 2014 at 07:03:32AM +0100, Chris Wilson wrote:
> As we use WC updates of the PTE, we are responsible for notifying the
> hardware when to flush its TLBs. Do so after we zap all the PTEs before
> suspend (and the BIOS tries to read our GTT).
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
> Tested-by: ming.yao@intel.com
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

This fixes a regression from the (functional) revert

    drm/i915: Undo gtt scratch pte unmapping again

    It apparently blows up on some machines. This functionally reverts

    commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
    Author: Ben Widawsky <benjamin.widawsky@intel.com>
    Date:   Wed Oct 16 09:21:30 2013 -0700

        drm/i915: Disable GGTT PTEs on GEN6+ suspend

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64841
    Reported-and-Tested-by: Brad  Jackson <bjackson0971@gmail.com>
    Cc: stable@vger.kernel.org
    Cc: Takashi Iwai <tiwai@suse.de>
    Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Cc: Todd Previte <tprevite@gmail.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Dave Airlie <airlied@redhat.com>

Cc: stable@vger.kernel.org
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Todd Previte <tprevite@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

When fixing regressions pls don't forget to cite the offending commit and
cc all relevant people. Jani, please amend the commit with the above when
merging.

Aside: This means that the bios writes to various ranges in the gtt, so I
still think we need to insert ptes pointing at stolen, too. Otherwise
we've simply reduced the chances for this bug to destroy important
something I think.
-Daniel


> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 97ba18846761..aa81b217fac0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1234,6 +1234,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
>  	POSTING_READ(RING_FAULT_REG(RCS_ENGINE(dev_priv)));
>  }
>  
> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_INFO(dev_priv)->gen < 6) {
> +		intel_gtt_chipset_flush();
> +	} else {
> +		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> +		POSTING_READ(GFX_FLSH_CNTL_GEN6);
> +	}
> +}
> +
>  void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1250,6 +1260,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>  				       dev_priv->gtt.base.start,
>  				       dev_priv->gtt.base.total,
>  				       true);
> +
> +	i915_ggtt_flush(dev_priv);
>  }
>  
>  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
> @@ -1302,7 +1314,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>  	}
>  
> -	i915_gem_chipset_flush(dev);
> +	i915_ggtt_flush(dev_priv);
>  }
>  
>  int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-18 11:52 ` Daniel Vetter
@ 2014-09-23 11:55   ` Jani Nikula
  2014-09-25  8:44     ` Chris Wilson
  2014-09-25  8:40   ` Chris Wilson
  1 sibling, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2014-09-23 11:55 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson; +Cc: Takashi Iwai, intel-gfx, Paulo Zanoni

On Thu, 18 Sep 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Sep 18, 2014 at 07:03:32AM +0100, Chris Wilson wrote:
>> As we use WC updates of the PTE, we are responsible for notifying the
>> hardware when to flush its TLBs. Do so after we zap all the PTEs before
>> suspend (and the BIOS tries to read our GTT).
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
>> Tested-by: ming.yao@intel.com
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> This fixes a regression from the (functional) revert
>
>     drm/i915: Undo gtt scratch pte unmapping again
>
>     It apparently blows up on some machines. This functionally reverts
>
>     commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
>     Date:   Wed Oct 16 09:21:30 2013 -0700
>
>         drm/i915: Disable GGTT PTEs on GEN6+ suspend
>
>     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64841
>     Reported-and-Tested-by: Brad  Jackson <bjackson0971@gmail.com>
>     Cc: stable@vger.kernel.org
>     Cc: Takashi Iwai <tiwai@suse.de>
>     Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>     Cc: Todd Previte <tprevite@gmail.com>
>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> Cc: stable@vger.kernel.org
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Todd Previte <tprevite@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> When fixing regressions pls don't forget to cite the offending commit and
> cc all relevant people. Jani, please amend the commit with the above when
> merging.

The patch fails to apply on any branch. Please refresh.

BR,
Jani.


>
> Aside: This means that the bios writes to various ranges in the gtt, so I
> still think we need to insert ptes pointing at stolen, too. Otherwise
> we've simply reduced the chances for this bug to destroy important
> something I think.
> -Daniel
>
>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 97ba18846761..aa81b217fac0 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1234,6 +1234,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
>>  	POSTING_READ(RING_FAULT_REG(RCS_ENGINE(dev_priv)));
>>  }
>>  
>> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
>> +{
>> +	if (INTEL_INFO(dev_priv)->gen < 6) {
>> +		intel_gtt_chipset_flush();
>> +	} else {
>> +		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> +		POSTING_READ(GFX_FLSH_CNTL_GEN6);
>> +	}
>> +}
>> +
>>  void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -1250,6 +1260,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  				       dev_priv->gtt.base.start,
>>  				       dev_priv->gtt.base.total,
>>  				       true);
>> +
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>> @@ -1302,7 +1314,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>>  		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>>  	}
>>  
>> -	i915_gem_chipset_flush(dev);
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
>> -- 
>> 2.1.0
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-18 11:52 ` Daniel Vetter
  2014-09-23 11:55   ` Jani Nikula
@ 2014-09-25  8:40   ` Chris Wilson
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2014-09-25  8:40 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Takashi Iwai, intel-gfx, Paulo Zanoni

On Thu, Sep 18, 2014 at 01:52:15PM +0200, Daniel Vetter wrote:
> On Thu, Sep 18, 2014 at 07:03:32AM +0100, Chris Wilson wrote:
> > As we use WC updates of the PTE, we are responsible for notifying the
> > hardware when to flush its TLBs. Do so after we zap all the PTEs before
> > suspend (and the BIOS tries to read our GTT).
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
> > Tested-by: ming.yao@intel.com
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> This fixes a regression from the (functional) revert
> 
>     drm/i915: Undo gtt scratch pte unmapping again
> 
>     It apparently blows up on some machines. This functionally reverts
> 
>     commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
>     Date:   Wed Oct 16 09:21:30 2013 -0700
> 
>         drm/i915: Disable GGTT PTEs on GEN6+ suspend
> 
>     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64841
>     Reported-and-Tested-by: Brad  Jackson <bjackson0971@gmail.com>
>     Cc: stable@vger.kernel.org
>     Cc: Takashi Iwai <tiwai@suse.de>
>     Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>     Cc: Todd Previte <tprevite@gmail.com>
>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> Cc: stable@vger.kernel.org
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Todd Previte <tprevite@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> When fixing regressions pls don't forget to cite the offending commit and
> cc all relevant people. Jani, please amend the commit with the above when
> merging.

I disagree that is the regression it is fixing, it is just band-aids all
the way down. This patch fixes a bug present in the earlier patch.
 
> Aside: This means that the bios writes to various ranges in the gtt, so I
> still think we need to insert ptes pointing at stolen, too. Otherwise
> we've simply reduced the chances for this bug to destroy important
> something I think.

Yup, the BIOS touching hardware it no longer has exclusive access to is
fundamentally broken.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-23 11:55   ` Jani Nikula
@ 2014-09-25  8:44     ` Chris Wilson
  2014-09-25  9:13       ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-09-25  8:44 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, stable, Takashi Iwai, Paulo Zanoni, Todd Previte,
	Daniel Vetter

As we use WC updates of the PTE, we are responsible for notifying the
hardware when to flush its TLBs. Do so after we zap all the PTEs before
suspend (and the BIOS tries to read our GTT).

Fixes a regression from

commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date:   Wed Oct 16 09:21:30 2013 -0700

    drm/i915: Disable GGTT PTEs on GEN6+ suspend

that survived and continue to cause harm even after

commit e568af1c626031925465a5caaab7cca1303d55c7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Mar 26 20:08:20 2014 +0100

    drm/i915: Undo gtt scratch pte unmapping again

v2: Trivial rebase.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
Tested-by: ming.yao@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Todd Previte <tprevite@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1411613f2174..417a33202ba0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
 	POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
 }
 
+static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen < 6) {
+		intel_gtt_chipset_flush();
+	} else {
+		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+		POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	}
+}
+
 void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 				       dev_priv->gtt.base.start,
 				       dev_priv->gtt.base.total,
 				       true);
+
+	i915_ggtt_flush(dev_priv);
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev)
@@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
 	}
 
-	i915_gem_chipset_flush(dev);
+	i915_ggtt_flush(dev_priv);
 }
 
 int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-25  8:44     ` Chris Wilson
@ 2014-09-25  9:13       ` Chris Wilson
  2014-09-29 12:26         ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-09-25  9:13 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, stable, Takashi Iwai, Paulo Zanoni, Todd Previte,
	Daniel Vetter

As we use WC updates of the PTE, we are responsible for notifying the
hardware when to flush its TLBs. Do so after we zap all the PTEs before
suspend (and the BIOS tries to read our GTT).

Fixes a regression from

commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date:   Wed Oct 16 09:21:30 2013 -0700

    drm/i915: Disable GGTT PTEs on GEN6+ suspend

that survived and continue to cause harm even after

commit e568af1c626031925465a5caaab7cca1303d55c7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Mar 26 20:08:20 2014 +0100

    drm/i915: Undo gtt scratch pte unmapping again

v2: Trivial rebase.
v3: Fixes requires pointer dances.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
Tested-by: ming.yao@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Todd Previte <tprevite@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1411613f2174..e42925f76b4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
 	POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
 }
 
+static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv->dev)->gen < 6) {
+		intel_gtt_chipset_flush();
+	} else {
+		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+		POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	}
+}
+
 void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 				       dev_priv->gtt.base.start,
 				       dev_priv->gtt.base.total,
 				       true);
+
+	i915_ggtt_flush(dev_priv);
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev)
@@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
 	}
 
-	i915_gem_chipset_flush(dev);
+	i915_ggtt_flush(dev_priv);
 }
 
 int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-25  9:13       ` Chris Wilson
@ 2014-09-29 12:26         ` Daniel Vetter
  2014-09-29 13:20           ` Jani Nikula
  2014-09-29 13:20           ` Jani Nikula
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel Vetter @ 2014-09-29 12:26 UTC (permalink / raw)
  To: Chris Wilson
  Cc: intel-gfx, stable, Takashi Iwai, Paulo Zanoni, Todd Previte,
	Daniel Vetter, Jani Nikula

On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
> As we use WC updates of the PTE, we are responsible for notifying the
> hardware when to flush its TLBs. Do so after we zap all the PTEs before
> suspend (and the BIOS tries to read our GTT).
> 
> Fixes a regression from
> 
> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
> Author: Ben Widawsky <benjamin.widawsky@intel.com>
> Date:   Wed Oct 16 09:21:30 2013 -0700
> 
>     drm/i915: Disable GGTT PTEs on GEN6+ suspend
> 
> that survived and continue to cause harm even after
> 
> commit e568af1c626031925465a5caaab7cca1303d55c7
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Mar 26 20:08:20 2014 +0100
> 
>     drm/i915: Undo gtt scratch pte unmapping again
> 
> v2: Trivial rebase.
> v3: Fixes requires pointer dances.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
> Tested-by: ming.yao@intel.com
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Todd Previte <tprevite@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1411613f2174..e42925f76b4b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
>  	POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
>  }
>  
> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
> +{
> +	if (INTEL_INFO(dev_priv->dev)->gen < 6) {
> +		intel_gtt_chipset_flush();
> +	} else {
> +		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> +		POSTING_READ(GFX_FLSH_CNTL_GEN6);
> +	}
> +}
> +
>  void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>  				       dev_priv->gtt.base.start,
>  				       dev_priv->gtt.base.total,
>  				       true);
> +
> +	i915_ggtt_flush(dev_priv);
>  }
>  
>  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>  	}
>  
> -	i915_gem_chipset_flush(dev);
> +	i915_ggtt_flush(dev_priv);
>  }
>  
>  int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-29 12:26         ` Daniel Vetter
@ 2014-09-29 13:20           ` Jani Nikula
  2014-09-29 13:20           ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2014-09-29 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson
  Cc: intel-gfx, stable, Takashi Iwai, Paulo Zanoni, Todd Previte,
	Daniel Vetter

On Mon, 29 Sep 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
>> As we use WC updates of the PTE, we are responsible for notifying the
>> hardware when to flush its TLBs. Do so after we zap all the PTEs before
>> suspend (and the BIOS tries to read our GTT).
>> 
>> Fixes a regression from
>> 
>> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>> Author: Ben Widawsky <benjamin.widawsky@intel.com>
>> Date:   Wed Oct 16 09:21:30 2013 -0700
>> 
>>     drm/i915: Disable GGTT PTEs on GEN6+ suspend
>> 
>> that survived and continue to cause harm even after
>> 
>> commit e568af1c626031925465a5caaab7cca1303d55c7
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Wed Mar 26 20:08:20 2014 +0100
>> 
>>     drm/i915: Undo gtt scratch pte unmapping again
>> 
>> v2: Trivial rebase.
>> v3: Fixes requires pointer dances.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
>> Tested-by: ming.yao@intel.com
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: stable@vger.kernel.org
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Todd Previte <tprevite@gmail.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 1411613f2174..e42925f76b4b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
>>  	POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
>>  }
>>  
>> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
>> +{
>> +	if (INTEL_INFO(dev_priv->dev)->gen < 6) {
>> +		intel_gtt_chipset_flush();
>> +	} else {
>> +		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> +		POSTING_READ(GFX_FLSH_CNTL_GEN6);
>> +	}
>> +}
>> +
>>  void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  				       dev_priv->gtt.base.start,
>>  				       dev_priv->gtt.base.total,
>>  				       true);
>> +
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>>  		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>>  	}
>>  
>> -	i915_gem_chipset_flush(dev);
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
>> -- 
>> 2.1.0
>> 
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Flush the PTEs after updating them before suspend
  2014-09-29 12:26         ` Daniel Vetter
  2014-09-29 13:20           ` Jani Nikula
@ 2014-09-29 13:20           ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2014-09-29 13:20 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson
  Cc: intel-gfx, stable, Takashi Iwai, Paulo Zanoni, Todd Previte,
	Daniel Vetter

On Mon, 29 Sep 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Sep 25, 2014 at 10:13:12AM +0100, Chris Wilson wrote:
>> As we use WC updates of the PTE, we are responsible for notifying the
>> hardware when to flush its TLBs. Do so after we zap all the PTEs before
>> suspend (and the BIOS tries to read our GTT).
>> 
>> Fixes a regression from
>> 
>> commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
>> Author: Ben Widawsky <benjamin.widawsky@intel.com>
>> Date:   Wed Oct 16 09:21:30 2013 -0700
>> 
>>     drm/i915: Disable GGTT PTEs on GEN6+ suspend
>> 
>> that survived and continue to cause harm even after
>> 
>> commit e568af1c626031925465a5caaab7cca1303d55c7
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Wed Mar 26 20:08:20 2014 +0100
>> 
>>     drm/i915: Undo gtt scratch pte unmapping again
>> 
>> v2: Trivial rebase.
>> v3: Fixes requires pointer dances.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82340
>> Tested-by: ming.yao@intel.com
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: stable@vger.kernel.org
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Todd Previte <tprevite@gmail.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 1411613f2174..e42925f76b4b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1310,6 +1310,16 @@ void i915_check_and_clear_faults(struct drm_device *dev)
>>  	POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
>>  }
>>  
>> +static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
>> +{
>> +	if (INTEL_INFO(dev_priv->dev)->gen < 6) {
>> +		intel_gtt_chipset_flush();
>> +	} else {
>> +		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> +		POSTING_READ(GFX_FLSH_CNTL_GEN6);
>> +	}
>> +}
>> +
>>  void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -1326,6 +1336,8 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
>>  				       dev_priv->gtt.base.start,
>>  				       dev_priv->gtt.base.total,
>>  				       true);
>> +
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>> @@ -1378,7 +1390,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>>  		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
>>  	}
>>  
>> -	i915_gem_chipset_flush(dev);
>> +	i915_ggtt_flush(dev_priv);
>>  }
>>  
>>  int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
>> -- 
>> 2.1.0
>> 
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-09-29 13:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-18  6:03 [PATCH] drm/i915: Flush the PTEs after updating them before suspend Chris Wilson
2014-09-18 11:52 ` Daniel Vetter
2014-09-23 11:55   ` Jani Nikula
2014-09-25  8:44     ` Chris Wilson
2014-09-25  9:13       ` Chris Wilson
2014-09-29 12:26         ` Daniel Vetter
2014-09-29 13:20           ` Jani Nikula
2014-09-29 13:20           ` Jani Nikula
2014-09-25  8:40   ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox