[PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[Daniel Vetter]

Include depency hell ftw! So need to move this into a real function.

Also fix up the header include order in i915_drv.h: The rule is to
always include core headers first, then local stuff.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     | 7 -------
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++++
 drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a89eb87e4af6..c783cfe5515d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2541,13 +2541,6 @@ int __must_check i915_gem_evict_something(struct drm_device *dev,
 int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 int i915_gem_evict_everything(struct drm_device *dev);
 
-/* belongs in i915_gem_gtt.h */
-static inline void i915_gem_chipset_flush(struct drm_device *dev)
-{
-	if (INTEL_INFO(dev)->gen < 6)
-		intel_gtt_chipset_flush();
-}
-
 /* i915_gem_stolen.c */
 int i915_gem_init_stolen(struct drm_device *dev);
 int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 58c8f99680a6..9c9fac190ad4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2031,6 +2031,12 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
 	teardown_scratch_page(vm->dev);
 }
 
+void i915_gem_chipset_flush(struct drm_device *dev)
+{
+	if (INTEL_INFO(dev)->gen < 6)
+		intel_gtt_chipset_flush();
+}
+
 static int i915_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c2ff69e1f781..00148266ee6f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -292,4 +292,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev);
 int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 
+void i915_gem_chipset_flush(struct drm_device *dev);
+
 #endif
-- 
2.0.1

[PATCH 1/2] drm/i915: Flush the PTEs after updating them before suspend

[Chris Wilson]

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).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h     |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1513e6ea50d..d0222cb9b7a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2082,7 +2082,7 @@ struct drm_i915_cmd_table {
 
 /* Note that the (struct drm_i915_private *) cast is just to shut up gcc. */
 #define __I915__(p)	((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? \
-			 (struct drm_i915_private *)(p) : to_i915(p))
+			 (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p))
 #define INTEL_INFO(p) 	(&__I915__(p)->info)
 #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f3fd448505f1..b8d5a5d67a73 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1316,6 +1316,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;
@@ -1332,6 +1342,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)
@@ -1384,7 +1396,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.rc1

[PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

That's right, back to where it was!

This effectively reverts

commit 9aa2062bb850e665b3b673b53bd90127d1215490
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Aug 6 15:04:46 2014 +0200

    drm/i915: Move i915_gem_chipset_flush to where it belongs

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     | 6 ++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ------
 drivers/gpu/drm/i915/i915_gem_gtt.h | 2 --
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d0222cb9b7a5..8dbfa18c265b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3007,4 +3007,10 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 	}
 }
 
+static inline void i915_gem_chipset_flush(struct drm_device *dev)
+{
+	if (INTEL_INFO(dev)->gen < 6)
+		intel_gtt_chipset_flush();
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b8d5a5d67a73..bbab8156b7ca 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2070,12 +2070,6 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
 	teardown_scratch_page(vm->dev);
 }
 
-void i915_gem_chipset_flush(struct drm_device *dev)
-{
-	if (INTEL_INFO(dev)->gen < 6)
-		intel_gtt_chipset_flush();
-}
-
 static int i915_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 55de8aec0a33..6280648d4805 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -299,6 +299,4 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev);
 int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 
-void i915_gem_chipset_flush(struct drm_device *dev);
-
 #endif
-- 
2.1.0.rc1

Re: [PATCH 1/2] drm/i915: Flush the PTEs after updating them before suspend

[Daniel Vetter]

On Wed, Aug 13, 2014 at 11:51:54AM +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).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Imo would make more sense to add them to the clear_range functions.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d1513e6ea50d..d0222cb9b7a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2082,7 +2082,7 @@ struct drm_i915_cmd_table {
>  
>  /* Note that the (struct drm_i915_private *) cast is just to shut up gcc. */
>  #define __I915__(p)	((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? \
> -			 (struct drm_i915_private *)(p) : to_i915(p))
> +			 (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p))
>  #define INTEL_INFO(p) 	(&__I915__(p)->info)
>  #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f3fd448505f1..b8d5a5d67a73 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1316,6 +1316,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;
> @@ -1332,6 +1342,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)
> @@ -1384,7 +1396,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.rc1
> 
> _______________________________________________
> 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

Re: [PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[Daniel Vetter]

On Wed, Aug 13, 2014 at 11:51:55AM +0100, Chris Wilson wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> That's right, back to where it was!
> 
> This effectively reverts

Hm, why?
-Daniel

> 
> commit 9aa2062bb850e665b3b673b53bd90127d1215490
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Aug 6 15:04:46 2014 +0200
> 
>     drm/i915: Move i915_gem_chipset_flush to where it belongs
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 6 ++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ------
>  drivers/gpu/drm/i915/i915_gem_gtt.h | 2 --
>  3 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d0222cb9b7a5..8dbfa18c265b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3007,4 +3007,10 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
>  	}
>  }
>  
> +static inline void i915_gem_chipset_flush(struct drm_device *dev)
> +{
> +	if (INTEL_INFO(dev)->gen < 6)
> +		intel_gtt_chipset_flush();
> +}
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b8d5a5d67a73..bbab8156b7ca 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2070,12 +2070,6 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
>  	teardown_scratch_page(vm->dev);
>  }
>  
> -void i915_gem_chipset_flush(struct drm_device *dev)
> -{
> -	if (INTEL_INFO(dev)->gen < 6)
> -		intel_gtt_chipset_flush();
> -}
> -
>  static int i915_gmch_probe(struct drm_device *dev,
>  			   size_t *gtt_total,
>  			   size_t *stolen,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 55de8aec0a33..6280648d4805 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -299,6 +299,4 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev);
>  int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
>  void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
>  
> -void i915_gem_chipset_flush(struct drm_device *dev);
> -
>  #endif
> -- 
> 2.1.0.rc1
> 

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

Re: [PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[Chris Wilson]

On Wed, Aug 13, 2014 at 01:32:31PM +0200, Daniel Vetter wrote:
> On Wed, Aug 13, 2014 at 11:51:55AM +0100, Chris Wilson wrote:
> > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > That's right, back to where it was!
> > 
> > This effectively reverts
> 
> Hm, why?

Becuase it doesn't belong in i915_gem_gtt.c since the flush is not about
flushing the GGTT and it is not used in i915_gem_gtt.c
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/2] drm/i915: Flush the PTEs after updating them before suspend

[Chris Wilson]

On Wed, Aug 13, 2014 at 01:32:08PM +0200, Daniel Vetter wrote:
> On Wed, Aug 13, 2014 at 11:51:54AM +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).
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Imo would make more sense to add them to the clear_range functions.

They are not required in the clear_range function. Here it is only
required in the i915_gem_suspend_gtt_mapping() because a third party
is involved.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] drm/i915: Move i915_gem_chipset_flush to where it belongs

[Daniel Vetter]

On Wed, Aug 13, 2014 at 12:45:11PM +0100, Chris Wilson wrote:
> On Wed, Aug 13, 2014 at 01:32:31PM +0200, Daniel Vetter wrote:
> > On Wed, Aug 13, 2014 at 11:51:55AM +0100, Chris Wilson wrote:
> > > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > 
> > > That's right, back to where it was!
> > > 
> > > This effectively reverts
> > 
> > Hm, why?
> 
> Becuase it doesn't belong in i915_gem_gtt.c since the flush is not about
> flushing the GGTT and it is not used in i915_gem_gtt.c

Indeed, makes sense. Dropped my original patch from dinq.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch