[Intel-gfx] [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit

[Chris Wilson]

On Mon, Dec 05, 2016 at 03:47:24PM +0530, Kamble, Sagar A wrote:
> 
> 
> On 11/17/2016 3:06 PM, Tvrtko Ursulin wrote:
> >
> >On 17/11/2016 09:28, Chris Wilson wrote:
> >>On Thu, Nov 17, 2016 at 09:17:35AM +0000, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>
> >>>Commit ed4596ea992d ("drm/i915/guc: WA to address the Ringbuffer
> >>>coherency issue"), based on incorrect assumptions from a partialy
> >>>broken commit 0dd356bb6ff5 ("drm/i915: Eliminate Gen9 special
> >>>case") used POSTING_READ_FW instead of the POSTING_READ. With the
> >>>latter buggy commit fixed this call site needs fixing as well.
> >>
> >>The theory here is that we don't need the powerwell to force the write
> >>from CPU to be visible before another agent.
> >>
> >>I missed the report, so I am genuinely interested in knowing whether the
> >>theory about the write being posted without the powerwll.
> >
> >Just that the commit message for the patch used "guc registers are
> >not in any forcewake domain" reasoning, which was false - based on
> >a partially broken patch. See "drm/i915: Fix gen9 forcewake range
> >table".
> >
> >Regards,
> >
> >Tvrtko
> >
> Verified this fix without forcewake, so this patch will not be needed.
> Have couple of queries. Chris, could you please clarify:
> 1. why POSTING_READ is done in flush_gtt_write_domain and not
> POSTING_READ_FW like this case?

There is a risk that we read from the register at the same time as a
write to another register in the same cacheline (gen7 issue). It should
be safe, but having erred on the side of skipping the uncore.lock in a 
imilar posting read for gen6_seqno_barrier() this time I played safe.
Along the guc submit path, we know we won't hit the same problematic hw.

> 2. how does read from forcewake mmio range work if well is down?

Returns garbage, usually zero.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre