drm/i915: Don't do posting reads on getting forcewake
authorMika Kuoppala <mika.kuoppala@linux.intel.com>
Wed, 28 Jan 2015 13:25:05 +0000 (15:25 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 30 Jan 2015 16:16:51 +0000 (17:16 +0100)
The checking for ack and also any subsequent mmio access
will serialize with setting the forcewake bit. Drop the
posting read as superfluous.

Note that in the put side we still want to keep the posting read
as it will ensure that the hw sees our forcewake release in a
timely manner and doesn't keep the hw powered up.

Comment from Chris:

On Wed, Jan 28, 2015 at 05:54:14PM +0200, Mika Kuoppala wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> > IIRC the posting read from same cache line actually fixed real bugs. So
> > I'm a bit worried about dropping them. But I suppose it's possible only
> > the _put side was important for those bugs.
>
> I found these:
>
> commit 6af2d180f82151cf3d58952e35a4f96e45bc453a
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Thu Jul 26 16:24:50 2012 +0200
>
>     drm/i915: fix forcewake related hangs on snb
>
> commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Sat Sep 1 22:59:50 2012 -0700
>
>     drm/i915: Never read FORCEWAKE
>
> https://bugs.freedesktop.org/show_bug.cgi?id=51738
> https://bugs.freedesktop.org/show_bug.cgi?id=52424
>
> The snb here seems to survive gem_dummy_reloc_loop and
> gem_ring_sync_loop in here with the get side posting removed.

Note that we kept the once associated with #52424, but judging by my
comments in #51738 the posting read is just a band aid anyway as a full
mb() itself was not adequate.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
[danvet: paste relevant review discussion in.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/intel_uncore.c

index be2c7fcf5f1f672d26fcd73d2a8846086d889fed..76b60a3538b28d97e659dddbb537c20984a66933 100644 (file)
@@ -131,7 +131,6 @@ fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_doma
        for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
                fw_domain_wait_ack_clear(d);
                fw_domain_get(d);
-               fw_domain_posting_read(d);
                fw_domain_wait_ack(d);
        }
 }