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@xxxxxxxxxxxxxxx> > 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@xxxxxxxxx> > Tested-by: Darren Hart <dvhart@xxxxxxxxxxxxxxx> > Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > --- > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
