On Mon, Jun 04, 2012 at 11:18:15AM +0200, Daniel Vetter wrote: > Empirical evidence suggests that we need to: On at least one ivb > machine when running the hangman i-g-t test, the rings don't properly > initialize properly - the RING_START registers seems to be stuck at > all zeros. > > Holding forcewake around this register init sequences makes chip reset > reliable again. Note that this is not the first such issue: > > commit f01db988ef6f6c70a6cc36ee71e4a98a68901229 > Author: Sean Paul <seanpaul at chromium.org> > Date: Fri Mar 16 12:43:22 2012 -0400 > > drm/i915: Add wait_for in init_ring_common > > added delay loops to make RING_START and RING_CTL initialization > reliable on the blt ring at boot-up. So I guess it won't hurt if we do > this unconditionally for all force_wake needing gpus. > > To avoid copy&pasting of the HAS_FORCE_WAKE check I've added a new > intel_info bit for that. > > v2: Fixup missing commas in static struct and properly handling the > error case in init_ring_common, both noticed by Jani Nikula. > > Cc: stable at vger.kernel.org > Reported-by: Yang Guang <guang.a.yang at intel.com> Now also tested-by, Yang confirmed that the new patch still works. -Daniel > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522 > Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++---- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_ringbuffer.c | 16 +++++++++++++--- > 3 files changed, 25 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 238a521..9fe9ebe 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -233,6 +233,7 @@ static const struct intel_device_info intel_sandybridge_d_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct intel_device_info intel_sandybridge_m_info = { > @@ -243,6 +244,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct intel_device_info intel_ivybridge_d_info = { > @@ -252,6 +254,7 @@ static const struct intel_device_info intel_ivybridge_d_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct intel_device_info intel_ivybridge_m_info = { > @@ -262,6 +265,7 @@ static const struct intel_device_info intel_ivybridge_m_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct intel_device_info intel_valleyview_m_info = { > @@ -289,6 +293,7 @@ static const struct intel_device_info intel_haswell_d_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct intel_device_info intel_haswell_m_info = { > @@ -298,6 +303,7 @@ static const struct intel_device_info intel_haswell_m_info = { > .has_blt_ring = 1, > .has_llc = 1, > .has_pch_split = 1, > + .has_force_wake = 1, > }; > > static const struct pci_device_id pciidlist[] = { /* aka */ > @@ -1139,10 +1145,9 @@ MODULE_LICENSE("GPL and additional rights"); > > /* We give fast paths for the really cool registers */ > #define NEEDS_FORCE_WAKE(dev_priv, reg) \ > - (((dev_priv)->info->gen >= 6) && \ > - ((reg) < 0x40000) && \ > - ((reg) != FORCEWAKE)) && \ > - (!IS_VALLEYVIEW((dev_priv)->dev)) > + ((HAS_FORCE_WAKE((dev_priv)->dev)) && \ > + ((reg) < 0x40000) && \ > + ((reg) != FORCEWAKE)) > > #define __i915_read(x, y) \ > u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ccabadd..7cc36db 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -285,6 +285,7 @@ struct intel_device_info { > u8 is_ivybridge:1; > u8 is_valleyview:1; > u8 has_pch_split:1; > + u8 has_force_wake:1; > u8 is_haswell:1; > u8 has_fbc:1; > u8 has_pipe_cxsr:1; > @@ -1105,6 +1106,8 @@ struct drm_i915_file_private { > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) > > +#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) > + > #include "i915_trace.h" > > /** > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 89a5e7f..0b172ee 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -266,10 +266,15 @@ u32 intel_ring_get_active_head(struct intel_ring_buffer *ring) > > static int init_ring_common(struct intel_ring_buffer *ring) > { > - drm_i915_private_t *dev_priv = ring->dev->dev_private; > + struct drm_device *dev = ring->dev; > + drm_i915_private_t *dev_priv = dev->dev_private; > struct drm_i915_gem_object *obj = ring->obj; > + int ret = 0; > u32 head; > > + if (HAS_FORCE_WAKE(dev)) > + gen6_gt_force_wake_get(dev_priv); > + > /* Stop the ring if it's running. */ > I915_WRITE_CTL(ring, 0); > I915_WRITE_HEAD(ring, 0); > @@ -317,7 +322,8 @@ static int init_ring_common(struct intel_ring_buffer *ring) > I915_READ_HEAD(ring), > I915_READ_TAIL(ring), > I915_READ_START(ring)); > - return -EIO; > + ret = -EIO; > + goto out; > } > > if (!drm_core_check_feature(ring->dev, DRIVER_MODESET)) > @@ -328,7 +334,11 @@ static int init_ring_common(struct intel_ring_buffer *ring) > ring->space = ring_space(ring); > } > > - return 0; > +out: > + if (HAS_FORCE_WAKE(dev)) > + gen6_gt_force_wake_put(dev_priv); > + > + return ret; > } > > static int > -- > 1.7.10 > -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
