* [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb
@ 2018-05-08 12:03 Mika Kuoppala
2018-05-08 12:06 ` Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Mika Kuoppala @ 2018-05-08 12:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Not all architectures guarantee that uncached read will
flush the write combining buffer. So marking it explicitly
is recommended [1].
However we know the architecture we are operating on
and can avoid wmb as the UC store will flush the wcb [2].
Omit the wmb() before invalidate as redudant.
v2: squash combining and removal (Chris)
References: http://yarchive.net/comp/linux/write_combining.html [1]
References: http://download.intel.com/design/PentiumII/applnots/24442201.pdf [2]
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c879bfd9294f..2126358761a5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -110,7 +110,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
{
- /* Note that as an uncached mmio write, this should flush the
+ /*
+ * Note that as an uncached mmio write, this will flush the
* WCB of the writes into the GGTT before it triggers the invalidate.
*/
I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
@@ -2418,8 +2419,6 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
for_each_sgt_dma(addr, sgt_iter, vma->pages)
gen8_set_pte(gtt_entries++, pte_encode | addr);
- wmb();
-
/* This next bit makes the above posting read even more important. We
* want to flush the TLBs only after we're certain all the PTE updates
* have finished.
@@ -2460,7 +2459,6 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
dma_addr_t addr;
for_each_sgt_dma(addr, iter, vma->pages)
iowrite32(vm->pte_encode(addr, level, flags), &entries[i++]);
- wmb();
/* This next bit makes the above posting read even more important. We
* want to flush the TLBs only after we're certain all the PTE updates
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb
2018-05-08 12:03 [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb Mika Kuoppala
@ 2018-05-08 12:06 ` Chris Wilson
2018-05-08 12:41 ` Mika Kuoppala
2018-05-11 13:23 ` Mika Kuoppala
2018-05-08 14:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gtt: Trust the uncached store to flush wcb (rev2) Patchwork
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Chris Wilson @ 2018-05-08 12:06 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Matthew Auld
Quoting Mika Kuoppala (2018-05-08 13:03:13)
> Not all architectures guarantee that uncached read will
> flush the write combining buffer. So marking it explicitly
> is recommended [1].
>
> However we know the architecture we are operating on
> and can avoid wmb as the UC store will flush the wcb [2].
>
> Omit the wmb() before invalidate as redudant.
>
> v2: squash combining and removal (Chris)
>
> References: http://yarchive.net/comp/linux/write_combining.html [1]
> References: http://download.intel.com/design/PentiumII/applnots/24442201.pdf [2]
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index c879bfd9294f..2126358761a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -110,7 +110,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
>
> static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
> {
> - /* Note that as an uncached mmio write, this should flush the
> + /*
> + * Note that as an uncached mmio write, this will flush the
> * WCB of the writes into the GGTT before it triggers the invalidate.
> */
> I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> @@ -2418,8 +2419,6 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> for_each_sgt_dma(addr, sgt_iter, vma->pages)
> gen8_set_pte(gtt_entries++, pte_encode | addr);
>
> - wmb();
> -
> /* This next bit makes the above posting read even more important. We
Yeah, we should rewrite this comment as well; just skip the first
obsolete sentence.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb
2018-05-08 12:06 ` Chris Wilson
@ 2018-05-08 12:41 ` Mika Kuoppala
2018-05-11 13:23 ` Mika Kuoppala
1 sibling, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2018-05-08 12:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Not all architectures guarantee that uncached read will
flush the write combining buffer. So marking it explicitly
is recommended [1].
However we know the architecture we are operating on
and can avoid wmb as the UC store will flush the wcb [2].
Omit the wmb() before invalidate as redudant.
v2: squash combining and removal (Chris)
v3: remove obsolete comments about posting reads (Chris)
References: http://yarchive.net/comp/linux/write_combining.html [1]
References: http://download.intel.com/design/PentiumII/applnots/24442201.pdf [2]
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c879bfd9294f..6eae9e1ed8be 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -110,7 +110,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
{
- /* Note that as an uncached mmio write, this should flush the
+ /*
+ * Note that as an uncached mmio write, this will flush the
* WCB of the writes into the GGTT before it triggers the invalidate.
*/
I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
@@ -2418,11 +2419,9 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
for_each_sgt_dma(addr, sgt_iter, vma->pages)
gen8_set_pte(gtt_entries++, pte_encode | addr);
- wmb();
-
- /* This next bit makes the above posting read even more important. We
- * want to flush the TLBs only after we're certain all the PTE updates
- * have finished.
+ /*
+ * We want to flush the TLBs only after we're certain all the PTE
+ * updates have finished.
*/
ggtt->invalidate(vm->i915);
}
@@ -2460,11 +2459,10 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
dma_addr_t addr;
for_each_sgt_dma(addr, iter, vma->pages)
iowrite32(vm->pte_encode(addr, level, flags), &entries[i++]);
- wmb();
- /* This next bit makes the above posting read even more important. We
- * want to flush the TLBs only after we're certain all the PTE updates
- * have finished.
+ /*
+ * We want to flush the TLBs only after we're certain all the PTE
+ * updates have finished.
*/
ggtt->invalidate(vm->i915);
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb
2018-05-08 12:06 ` Chris Wilson
2018-05-08 12:41 ` Mika Kuoppala
@ 2018-05-11 13:23 ` Mika Kuoppala
1 sibling, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2018-05-11 13:23 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Matthew Auld
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2018-05-08 13:03:13)
>> Not all architectures guarantee that uncached read will
>> flush the write combining buffer. So marking it explicitly
>> is recommended [1].
>>
>> However we know the architecture we are operating on
>> and can avoid wmb as the UC store will flush the wcb [2].
>>
>> Omit the wmb() before invalidate as redudant.
>>
>> v2: squash combining and removal (Chris)
>>
>> References: http://yarchive.net/comp/linux/write_combining.html [1]
>> References: http://download.intel.com/design/PentiumII/applnots/24442201.pdf [2]
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index c879bfd9294f..2126358761a5 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -110,7 +110,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
>>
>> static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
>> {
>> - /* Note that as an uncached mmio write, this should flush the
>> + /*
>> + * Note that as an uncached mmio write, this will flush the
>> * WCB of the writes into the GGTT before it triggers the invalidate.
>> */
>> I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>> @@ -2418,8 +2419,6 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
>> for_each_sgt_dma(addr, sgt_iter, vma->pages)
>> gen8_set_pte(gtt_entries++, pte_encode | addr);
>>
>> - wmb();
>> -
>> /* This next bit makes the above posting read even more important. We
>
> Yeah, we should rewrite this comment as well; just skip the first
> obsolete sentence.
v3 pushed. Thanks for review.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
2018-05-08 12:03 [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb Mika Kuoppala
2018-05-08 12:06 ` Chris Wilson
@ 2018-05-08 14:56 ` Patchwork
2018-05-08 15:11 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-08 17:33 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-08 14:56 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
URL : https://patchwork.freedesktop.org/series/42873/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b634b2d79259 drm/i915/gtt: Trust the uncached store to flush wcb
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19:
References: http://download.intel.com/design/PentiumII/applnots/24442201.pdf [2]
total: 0 errors, 1 warnings, 0 checks, 37 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
2018-05-08 12:03 [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb Mika Kuoppala
2018-05-08 12:06 ` Chris Wilson
2018-05-08 14:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gtt: Trust the uncached store to flush wcb (rev2) Patchwork
@ 2018-05-08 15:11 ` Patchwork
2018-05-08 17:33 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-08 15:11 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
URL : https://patchwork.freedesktop.org/series/42873/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4157 -> Patchwork_8942 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8942 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8942, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42873/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8942:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_8942 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_frontbuffer_tracking@basic:
{fi-hsw-peppy}: PASS -> DMESG-FAIL (fdo#102614, fdo#106103)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: NOTRUN -> INCOMPLETE (fdo#103927)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
==== Possible fixes ====
igt@gem_mmap_gtt@basic-small-bo-tiledx:
fi-gdg-551: FAIL (fdo#102575) -> PASS
igt@kms_flip@basic-flip-vs-dpms:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
== Participating hosts (41 -> 37) ==
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4157 -> Patchwork_8942
CI_DRM_4157: 580a7e5eafe15dd21f0dfc92d860a57a404e622d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4464: 1bb318b32db003a377da14715c7b80675a712b6b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8942: b634b2d7925954bfb450932953aa26756dc830ea @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4464: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit
== Linux commits ==
b634b2d79259 drm/i915/gtt: Trust the uncached store to flush wcb
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8942/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* ✓ Fi.CI.IGT: success for drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
2018-05-08 12:03 [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb Mika Kuoppala
` (2 preceding siblings ...)
2018-05-08 15:11 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-08 17:33 ` Patchwork
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-08 17:33 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gtt: Trust the uncached store to flush wcb (rev2)
URL : https://patchwork.freedesktop.org/series/42873/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4157_full -> Patchwork_8942_full =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42873/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_8942_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@modeset-vs-vblank-race-interruptible:
shard-hsw: PASS -> FAIL (fdo#103060)
==== Possible fixes ====
igt@kms_atomic_transition@2x-modeset-transitions-nonblocking:
shard-hsw: DMESG-FAIL (fdo#104724) -> PASS
igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
shard-hsw: FAIL (fdo#103928) -> PASS
igt@kms_flip@2x-plain-flip-ts-check:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@busy-flip-interruptible:
shard-apl: FAIL -> PASS
igt@kms_flip@dpms-vs-vblank-race:
shard-hsw: FAIL (fdo#103060) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4157 -> Patchwork_8942
CI_DRM_4157: 580a7e5eafe15dd21f0dfc92d860a57a404e622d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4464: 1bb318b32db003a377da14715c7b80675a712b6b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8942: b634b2d7925954bfb450932953aa26756dc830ea @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4464: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8942/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-11 13:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-08 12:03 [PATCH] drm/i915/gtt: Trust the uncached store to flush wcb Mika Kuoppala
2018-05-08 12:06 ` Chris Wilson
2018-05-08 12:41 ` Mika Kuoppala
2018-05-11 13:23 ` Mika Kuoppala
2018-05-08 14:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gtt: Trust the uncached store to flush wcb (rev2) Patchwork
2018-05-08 15:11 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-08 17:33 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.