Re: [PATCH 07/12] drm/i915: Write mmio workarounds after gpu reset

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 06, 2015 at 05:26:48PM +0300, Mika Kuoppala wrote:
> From: Mika Kuoppala <miku@testikku.fi.intel.com>
> 
> Rewrite everything in mmio workaround list right after
> gpu reset. This ensures that we start the reinitialization
> with proper mmio workarounds in place, before we
> start the rings.
> 
> This commit just adds the mechanism, the list itself
> is still empty. Following commits will add registers.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  8 ++++++++
>  drivers/gpu/drm/i915/i915_irq.c         |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c         | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
>  drivers/gpu/drm/i915/intel_uncore.c     | 12 ++++++++++++
>  5 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ae5b6b3..d41808a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2732,6 +2732,7 @@ void intel_uncore_forcewake_get__locked(struct drm_i915_private *dev_priv,
>  void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
>  					enum forcewake_domains domains);
>  void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> +void assert_forcewakes_active(struct drm_i915_private *dev_priv);
>  static inline bool intel_vgpu_active(struct drm_device *dev)
>  {
>  	return to_i915(dev)->vgpu.active;
> @@ -3545,7 +3546,14 @@ static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
>  #define WA_CLR_BIT(t, addr, mask) \
>  	WA_REG_##t(addr, mask, I915_READ((addr)) & ~(mask))
>  
> +static inline void intel_wa_init(struct i915_workarounds *w)
> +{
> +	w->count = 0;
> +}
> +
>  int intel_wa_add(struct i915_workarounds *w,
>  		 const u32 addr, const u32 mask, const u32 val);
>  
> +void intel_wa_write_mmio(struct drm_i915_private *dev_priv,
> +			 const struct i915_workarounds *w);
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8ca772d..e686f78 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2441,6 +2441,8 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
>  		 */
>  		ret = i915_reset(dev);
>  
> +		intel_wa_write_mmio(dev_priv, &dev_priv->mmio_workarounds);
> +
>  		intel_finish_reset(dev);
>  
>  		intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 60d120c..e97f271 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -52,6 +52,22 @@
>  #define INTEL_RC6p_ENABLE			(1<<1)
>  #define INTEL_RC6pp_ENABLE			(1<<2)
>  
> +void intel_wa_write_mmio(struct drm_i915_private *dev_priv,
> +			 const struct i915_workarounds *w)
> +{
> +	int i;
> +
> +	if (WARN_ON_ONCE(w->count == 0))
> +		return;
> +
> +	assert_forcewakes_active(dev_priv);
> +
> +	for (i = 0; i < w->count; i++)
> +		I915_WRITE_FW(w->reg[i].addr, w->reg[i].value);
> +
> +	DRM_DEBUG_DRIVER("Number of Workarounds written: %d\n", w->count);
> +}
> +
>  static void gen9_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6973,8 +6989,12 @@ void intel_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	intel_wa_init(&dev_priv->mmio_workarounds);
> +
>  	if (dev_priv->display.init_clock_gating)
>  		dev_priv->display.init_clock_gating(dev);
> +
> +	intel_wa_write_mmio(dev_priv, &dev_priv->mmio_workarounds);

init_clock_gating is for display block workarounds, not for anything in
the GT. That should be done by either the various engine/ring init hooks
or by new code. The reason for that is that display and GT can be reset
separately (well GT can be reset without display going down, at least on
modern hw).
-Daniel

>  }
>  
>  void intel_suspend_hw(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index c9d3489e..3667dd9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1074,7 +1074,7 @@ int init_workarounds_ring(struct intel_engine_cs *ring)
>  
>  	WARN_ON(ring->id != RCS);
>  
> -	dev_priv->lri_workarounds.count = 0;
> +	intel_wa_init(&dev_priv->lri_workarounds);
>  
>  	if (IS_BROADWELL(dev))
>  		return bdw_init_workarounds(ring);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index b43c6d0..3f8d1f6 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -524,6 +524,18 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
>  		WARN_ON(domain->wake_count);
>  }
>  
> +void assert_forcewakes_active(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_uncore_forcewake_domain *domain;
> +	enum forcewake_domain_id id;
> +
> +	if (!dev_priv->uncore.funcs.force_wake_get)
> +		return;
> +
> +	for_each_fw_domain(domain, dev_priv, id)
> +		WARN_ON(domain->wake_count == 0);
> +}
> +
>  /* We give fast paths for the really cool registers */
>  #define NEEDS_FORCE_WAKE(dev_priv, reg) \
>  	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx