[Intel-gfx] [PATCH v2] drm/i915: fix FORCEWAKE posting reads

[Chris Wilson]

On Thu, 17 Jan 2013 10:24:09 +0200, Jani Nikula <jani.nikula at intel.com> wrote:
> We stopped reading FORCEWAKE for posting reads in
> 
> commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
> Author: Ben Widawsky <ben at bwidawsk.net>
> Date:   Sat Sep 1 22:59:50 2012 -0700
> 
>     drm/i915: Never read FORCEWAKE
> 
> and started using something from the same cacheline instead. It turns out
> reading ECOBUS as posting read worked fine, while GTFIFODBG did not,
> preventing RC6 states after suspend/resume per the bug report referenced
> below. It's not entirely clear why, but clearly GTFIFODBG was nowhere near
> the same cacheline or address range as FORCEWAKE.
> 
> Trying out various registers for posting reads showed that all tested
> registers for which NEEDS_FORCE_WAKE() (in i915_drv.c) returns true
> work. Conversely, most (but not quite all) registers for which
> NEEDS_FORCE_WAKE() returns false do not work. Details in the referenced
> bug.
> 
> Based on the above, add posting reads on ECOBUS where GTFIFODBG was
> previously relied on.
> 
> In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as
> well, but instead of ECOBUS, use FORCEWAKE_ACK_VLV which is in the same
> address range as FORCEWAKE_VLV.
> 
> v2: Add more details to the commit message. No functional changes.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411
> Reported-and-tested-by: Alexander Bersenev <bay at hackerdom.ru>
> CC: Ben Widawsky <ben at bwidawsk.net>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>

You don't mention at all why you consider this to be important at all. A
hint that the extra read makes rc6 after resume more stable on one
machine would be useful.

So a missing flush of the write in _put() makes sense for rc6 staying
active. (If it happens once, it can happen everytime we do the forcewake
dance and so stay asserted indefinitely). And I can quite believe that
the hw has separate queues for the GT powerwell that require a read from
within that powerwell to flush, so the magic pixie dust looks consistent.

Ok, I'm happy that I can rationalise that this does indeed fix a genuine
quirk of our hw. The cargo cult survives.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre