* [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit
@ 2016-11-17 9:17 Tvrtko Ursulin
2016-11-17 9:28 ` Chris Wilson
2016-11-17 10:46 ` ✓ Fi.CI.BAT: success for " Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-11-17 9:17 UTC (permalink / raw)
To: Intel-gfx; +Cc: Akash Goel
From: Tvrtko Ursulin <tvrtko.ursulin@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.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: ed4596ea992d ("drm/i915/guc: WA to address the Ringbuffer coherency issue")
Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Akash Goel <akash.goel@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4462112725ef..c603837324a0 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -651,7 +651,7 @@ static void i915_guc_submit(struct drm_i915_gem_request *rq)
/* WA to flush out the pending GMADR writes to ring buffer. */
if (i915_vma_is_map_and_fenceable(rq->ring->vma))
- POSTING_READ_FW(GUC_STATUS);
+ POSTING_READ(GUC_STATUS);
b_ret = guc_ring_doorbell(client);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit 2016-11-17 9:17 [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit Tvrtko Ursulin @ 2016-11-17 9:28 ` Chris Wilson 2016-11-17 9:36 ` Tvrtko Ursulin 2016-11-17 10:46 ` ✓ Fi.CI.BAT: success for " Patchwork 1 sibling, 1 reply; 6+ messages in thread From: Chris Wilson @ 2016-11-17 9:28 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx, Akash Goel On Thu, Nov 17, 2016 at 09:17:35AM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit 2016-11-17 9:28 ` Chris Wilson @ 2016-11-17 9:36 ` Tvrtko Ursulin 2016-12-05 10:17 ` Kamble, Sagar A 0 siblings, 1 reply; 6+ messages in thread From: Tvrtko Ursulin @ 2016-11-17 9:36 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Daniele Ceraolo Spurio, Akash Goel, Sagar Arun Kamble 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@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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit 2016-11-17 9:36 ` Tvrtko Ursulin @ 2016-12-05 10:17 ` Kamble, Sagar A 2016-12-05 10:34 ` Chris Wilson 0 siblings, 1 reply; 6+ messages in thread From: Kamble, Sagar A @ 2016-12-05 10:17 UTC (permalink / raw) To: Tvrtko Ursulin, Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Daniele Ceraolo Spurio, Akash Goel, akash.goel 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@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? 2. how does read from forcewake mmio range work if well is down? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit 2016-12-05 10:17 ` Kamble, Sagar A @ 2016-12-05 10:34 ` Chris Wilson 0 siblings, 0 replies; 6+ messages in thread From: Chris Wilson @ 2016-12-05 10:34 UTC (permalink / raw) To: Kamble, Sagar A; +Cc: akash.goel, Intel-gfx, Akash Goel 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@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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/guc: Do not bypass forcewakes in i915_guc_submit 2016-11-17 9:17 [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit Tvrtko Ursulin 2016-11-17 9:28 ` Chris Wilson @ 2016-11-17 10:46 ` Patchwork 1 sibling, 0 replies; 6+ messages in thread From: Patchwork @ 2016-11-17 10:46 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: drm/i915/guc: Do not bypass forcewakes in i915_guc_submit URL : https://patchwork.freedesktop.org/series/15476/ State : success == Summary == Series 15476v1 drm/i915/guc: Do not bypass forcewakes in i915_guc_submit https://patchwork.freedesktop.org/api/1.0/series/15476/revisions/1/mbox/ Test drv_module_reload_basic: dmesg-warn -> PASS (fi-skl-6770hq) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-ivb-3770) fi-bdw-5557u total:244 pass:229 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:244 pass:204 dwarn:0 dfail:0 fail:0 skip:40 fi-bxt-t5700 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-j1900 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20 fi-ilk-650 total:244 pass:191 dwarn:0 dfail:0 fail:0 skip:53 fi-ivb-3520m total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 fi-ivb-3770 total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 fi-kbl-7200u total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:244 pass:223 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6700k total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32 fi-snb-2600 total:244 pass:211 dwarn:0 dfail:0 fail:0 skip:33 ca255285dca0f265f25214078650b7948b03fe21 drm-intel-nightly: 2016y-11m-17d-08h-59m-51s UTC integration manifest c0332fc drm/i915/guc: Do not bypass forcewakes in i915_guc_submit == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3031/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-05 10:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-17 9:17 [PATCH] drm/i915/guc: Do not bypass forcewakes in i915_guc_submit Tvrtko Ursulin 2016-11-17 9:28 ` Chris Wilson 2016-11-17 9:36 ` Tvrtko Ursulin 2016-12-05 10:17 ` Kamble, Sagar A 2016-12-05 10:34 ` Chris Wilson 2016-11-17 10:46 ` ✓ Fi.CI.BAT: success for " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).