[Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init

[Chris Wilson]

On Wed, Jan 28, 2015 at 11:45:04AM +0200, Mika Kuoppala wrote:
> intel_uncore_early_sanitize() will reset the forcewake registers. When
> forcewake domains were introduced, the domain init was done after the
> sanitization of the forcewake registers. And as the resetting of
> registers use the domain accessors, we tried to reset the forcewake
> registers with unitialized forcewake domains and failed.
> 
> Fix this by sanitizing after all the domains have been initialized.
> On ivb we need special care as there we need early forcewake access to
> determine the final configuration for the forcewake domain.
> 
> This regression was introduced in
> 
> commit 05a2fb157e44a53c79133805d30eaada43911941
> Author: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Date:   Mon Jan 19 16:20:43 2015 +0200
> 
>     drm/i915: Consolidate forcewake code
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
> Reported-by: Olof Johansson <olof at lixom.net>
> Tested-by: Darren Hart <dvhart at linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index b3951f2..c438ca4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private *dev_priv)
>  static inline void
>  fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
>  {
> +	WARN_ON(d->reg_set == 0);
>  	__raw_i915_write32(d->i915, d->reg_set, d->val_reset);
>  }
>  
> @@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_do
>  	struct intel_uncore_forcewake_domain *d;
>  	enum forcewake_domain_id id;
>  
> +	WARN_ON(dev_priv->uncore.fw_domains == 0);
> +
>  	for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
>  		fw_domain_reset(d);
>  
> @@ -987,8 +990,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
>  void intel_uncore_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	__intel_uncore_early_sanitize(dev, false);
> +	bool sanitize_done = false;

I felt this looks quite clumsy. The only reason why you want to restrict
calling __intel_uncore_early_sanitize() is that it does ellc_size
detection and has a DRM_INFO right?

I think you want to pull that out of __intel_uncore_early_santize() into
intel_uncore_init() itself (or better, it's own
intel_uncore_ellc_detect()). ellc_size detection has nothing to do with
sanitizing register state.

Then it should be simple to enough to sanitize twice, once with a
comment in the code explaining how we verify that FORCEWAKE_MT is
enabled by a manual forcewaked read of ECOBUS.
-Chris


-- 
Chris Wilson, Intel Open Source Technology Centre