[PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation

[PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation

[Chris Wilson]

Currently we call gen6_enable_rps() (which writes into the per-ring
register mmio space) from intel_modeset_init_hw() which is called before
we initialise the rings. If we defer intel_modeset_init_hw() until
afterwards (in the intel_modeset_gem_init() phase) all is well.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be59e8f..27eda9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6792,6 +6792,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
 
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_enable_drps(dev);
+		ironlake_enable_rc6(dev);
 		intel_init_emon(dev);
 	}
 
@@ -6849,8 +6850,6 @@ void intel_modeset_init(struct drm_device *dev)
 	i915_disable_vga(dev);
 	intel_setup_outputs(dev);
 
-	intel_modeset_init_hw(dev);
-
 	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
 	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
 		    (unsigned long)dev);
@@ -6861,8 +6860,7 @@ void intel_modeset_init(struct drm_device *dev)
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
-	if (IS_IRONLAKE_M(dev))
-		ironlake_enable_rc6(dev);
+	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);
 }
-- 
1.7.10

Re: [PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation

[Daniel Vetter]

On Fri, Apr 27, 2012 at 06:36:37PM +0100, Chris Wilson wrote:
> Currently we call gen6_enable_rps() (which writes into the per-ring
> register mmio space) from intel_modeset_init_hw() which is called before
> we initialise the rings. If we defer intel_modeset_init_hw() until
> afterwards (in the intel_modeset_gem_init() phase) all is well.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

One thing I've noticed that with this patch we still have conflicting
ordering of gem_init_hw vs. modeset_init_hw. With this patch we have
- gem before modeset init_hw for initial load and after gpu reset
- but gem _after_ modeset init_hw after resume.

Given that we already have another workaround that wants gem (and the
rings) fully running, I guess gem before modeset init_hw is the right
order. Care to amend the patch to fix the resume path?
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index be59e8f..27eda9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6792,6 +6792,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  
>  	if (IS_IRONLAKE_M(dev)) {
>  		ironlake_enable_drps(dev);
> +		ironlake_enable_rc6(dev);
>  		intel_init_emon(dev);
>  	}
>  
> @@ -6849,8 +6850,6 @@ void intel_modeset_init(struct drm_device *dev)
>  	i915_disable_vga(dev);
>  	intel_setup_outputs(dev);
>  
> -	intel_modeset_init_hw(dev);
> -
>  	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
>  	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
>  		    (unsigned long)dev);
> @@ -6861,8 +6860,7 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  void intel_modeset_gem_init(struct drm_device *dev)
>  {
> -	if (IS_IRONLAKE_M(dev))
> -		ironlake_enable_rc6(dev);
> +	intel_modeset_init_hw(dev);
>  
>  	intel_setup_overlay(dev);
>  }
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation

[Chris Wilson]

Currently we call gen6_enable_rps() (which writes into the per-ring
register mmio space) from intel_modeset_init_hw() which is called before
we initialise the rings. If we defer intel_modeset_init_hw() until
afterwards (in the intel_modeset_gem_init() phase) all is well.

v2: Rectify ordering of gem vs display HW init upon resume. (Daniel)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c      |   10 ++++------
 drivers/gpu/drm/i915/i915_suspend.c  |    6 ------
 drivers/gpu/drm/i915/intel_display.c |    6 ++----
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 77b7a50..981f3a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -622,15 +622,16 @@ static int i915_drm_thaw(struct drm_device *dev)
 
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		if (HAS_PCH_SPLIT(dev))
+			ironlake_init_pch_refclk(dev);
+
 		mutex_lock(&dev->struct_mutex);
 		dev_priv->mm.suspended = 0;
 
 		error = i915_gem_init_hw(dev);
+		intel_modeset_init_hw(dev);
 		mutex_unlock(&dev->struct_mutex);
 
-		if (HAS_PCH_SPLIT(dev))
-			ironlake_init_pch_refclk(dev);
-
 		drm_mode_config_reset(dev);
 		drm_irq_install(dev);
 
@@ -638,9 +639,6 @@ static int i915_drm_thaw(struct drm_device *dev)
 		mutex_lock(&dev->mode_config.mutex);
 		drm_helper_resume_force_mode(dev);
 		mutex_unlock(&dev->mode_config.mutex);
-
-		if (IS_IRONLAKE_M(dev))
-			ironlake_enable_rc6(dev);
 	}
 
 	intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 73a5c3c..0ede02a 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -876,12 +876,6 @@ int i915_restore_state(struct drm_device *dev)
 		I915_WRITE(IER, dev_priv->saveIER);
 		I915_WRITE(IMR, dev_priv->saveIMR);
 	}
-	mutex_unlock(&dev->struct_mutex);
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		intel_modeset_init_hw(dev);
-
-	mutex_lock(&dev->struct_mutex);
 
 	/* Cache mode state */
 	I915_WRITE(CACHE_MODE_0, dev_priv->saveCACHE_MODE_0 | 0xffff0000);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1cbe268..42b9e20 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6658,6 +6658,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
 
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_enable_drps(dev);
+		ironlake_enable_rc6(dev);
 		intel_init_emon(dev);
 	}
 
@@ -6719,8 +6720,6 @@ void intel_modeset_init(struct drm_device *dev)
 	i915_disable_vga(dev);
 	intel_setup_outputs(dev);
 
-	intel_modeset_init_hw(dev);
-
 	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
 	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
 		    (unsigned long)dev);
@@ -6728,8 +6727,7 @@ void intel_modeset_init(struct drm_device *dev)
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
-	if (IS_IRONLAKE_M(dev))
-		ironlake_enable_rc6(dev);
+	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);
 }
-- 
1.7.10

Re: [PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation

[Daniel Vetter]

On Wed, May 09, 2012 at 11:56:28AM +0100, Chris Wilson wrote:
> Currently we call gen6_enable_rps() (which writes into the per-ring
> register mmio space) from intel_modeset_init_hw() which is called before
> we initialise the rings. If we defer intel_modeset_init_hw() until
> afterwards (in the intel_modeset_gem_init() phase) all is well.
> 
> v2: Rectify ordering of gem vs display HW init upon resume. (Daniel)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch. We still have a few strange things
in our hw setup paths, but that can be cleanup up later on.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48