[PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Rodrigo Vivi]

WC is apparently not an option for CNL+ on GTT here.
Trying to use it we get hard hangs.

Credits-to: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 10aa7762d9a6..3019bf509e3d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2717,7 +2717,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
 	 * resort to an uncached mapping. The WC issue is easily caught by the
 	 * readback check when writing GTT PTE entries.
 	 */
-	if (IS_GEN9_LP(dev_priv))
+	if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
 		ggtt->gsm = ioremap_nocache(phys_addr, size);
 	else
 		ggtt->gsm = ioremap_wc(phys_addr, size);
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.
URL   : https://patchwork.freedesktop.org/series/28702/
State : success

== Summary ==

Series 28702v1 drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.
https://patchwork.freedesktop.org/api/1.0/series/28702/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:446s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:428s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:355s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:548s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:522s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:521s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:515s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:603s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:437s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:419s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:507s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:590s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:590s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:542s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:464s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:501s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:555s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:408s

0689b6b1aa3edec5d99f35902c9b38c0e6b701b9 drm-tip: 2017y-08m-11d-18h-55m-01s UTC integration manifest
910245f5e595 drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5387/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Rodrigo Vivi]

On Fri, Aug 11, 2017 at 4:39 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> WC is apparently not an option for CNL+ on GTT here.
> Trying to use it we get hard hangs.
>
> Credits-to: Ben Widawsky <benjamin.widawsky@intel.com>

forgot to CC relavant people for possible reviews:
Cc: Joonas
Cc: Imre
Cc: Ben

> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 10aa7762d9a6..3019bf509e3d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2717,7 +2717,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>          * resort to an uncached mapping. The WC issue is easily caught by the
>          * readback check when writing GTT PTE entries.
>          */
> -       if (IS_GEN9_LP(dev_priv))
> +       if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>                 ggtt->gsm = ioremap_nocache(phys_addr, size);
>         else
>                 ggtt->gsm = ioremap_wc(phys_addr, size);
> --
> 2.13.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Joonas Lahtinen]

On Fri, 2017-08-11 at 17:36 -0700, Rodrigo Vivi wrote:
> On Fri, Aug 11, 2017 at 4:39 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > WC is apparently not an option for CNL+ on GTT here.
> > Trying to use it we get hard hangs.
> > 
> > Credits-to: Ben Widawsky <benjamin.widawsky@intel.com>

I guess "Suggested-by:"/"Reported-by:" would be the standard one :)

Do update the comment above the code, too.

Do we have a "Bspec:" one? Would make it easier to review.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Vivi, Rodrigo]

Cc: chris as well...

On Mon, 2017-08-14 at 15:13 +0300, Joonas Lahtinen wrote:
> On Fri, 2017-08-11 at 17:36 -0700, Rodrigo Vivi wrote:
> > On Fri, Aug 11, 2017 at 4:39 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > WC is apparently not an option for CNL+ on GTT here.
> > > Trying to use it we get hard hangs.
> > > 
> > > Credits-to: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> I guess "Suggested-by:"/"Reported-by:" would be the standard one :)


indeed.
consider:

Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>

> 
> Do update the comment above the code, too.

I will.

> 
> Do we have a "Bspec:" one? Would make it easier to review.

No... for me that was purely experimental. So I was hoping that Ben or
Imre who caught that on BXT would be able to provide a bit of more info.

Without this patch CNL hang on the very first submission to render
engine.

> 
> Regards, Joonas

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Rodrigo Vivi]

Driver’s CPU access to GTT is via the GTTMMADR BAR.

The current HW implementation of that BAR is to only
support <= DW (and maybe QW) writes—not 16/32/64B writes
that could occur with WC and/or SSE/AVX moves.

GTTMMADR must be marked uncacheable (UC).
Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).

v2: Get clarification on the reasons and spec is getting
    updated to reflect it now.

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 708b95cd8c30..7da9621d2c60 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2790,13 +2790,13 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
 	phys_addr = pci_resource_start(pdev, 0) + pci_resource_len(pdev, 0) / 2;
 
 	/*
-	 * On BXT writes larger than 64 bit to the GTT pagetable range will be
-	 * dropped. For WC mappings in general we have 64 byte burst writes
-	 * when the WC buffer is flushed, so we can't use it, but have to
+	 * On BXT+/CNL+ writes larger than 64 bit to the GTT pagetable range
+	 * will be dropped. For WC mappings in general we have 64 byte burst
+	 * writes when the WC buffer is flushed, so we can't use it, but have to
 	 * resort to an uncached mapping. The WC issue is easily caught by the
 	 * readback check when writing GTT PTE entries.
 	 */
-	if (IS_GEN9_LP(dev_priv))
+	if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
 		ggtt->gsm = ioremap_nocache(phys_addr, size);
 	else
 		ggtt->gsm = ioremap_wc(phys_addr, size);
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well. (rev2)
URL   : https://patchwork.freedesktop.org/series/28702/
State : success

== Summary ==

Series 28702v2 drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.
https://patchwork.freedesktop.org/api/1.0/series/28702/revisions/2/mbox/

Test gem_ringfill:
        Subgroup basic-default-hang:
                incomplete -> DMESG-WARN (fi-pnv-d510) fdo#101600
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781
Test kms_frontbuffer_tracking:
        Subgroup basic:
                dmesg-warn -> PASS       (fi-bdw-5557u)

fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:455s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:444s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:358s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:553s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:252s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:519s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:516s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:515s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:435s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:610s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:448s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:421s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:482s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:600s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:598s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:527s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:467s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:486s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:451s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:494s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:550s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:402s
fi-skl-6700k failed to connect after reboot

428ed27345fbf9be530d01ca6dc862eb5895db81 drm-tip: 2017y-08m-29d-17h-43m-11s UTC integration manifest
d2d16811f053 drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5533/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well. (rev2)
URL   : https://patchwork.freedesktop.org/series/28702/
State : warning

== Summary ==

Test kms_cursor_legacy:
        Subgroup short-flip-before-cursor-atomic-transitions-varying-size:
                skip       -> PASS       (shard-hsw)
Test kms_fbcon_fbt:
        Subgroup fbc:
                pass       -> SKIP       (shard-hsw)
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252 +1

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2230 pass:1229 dwarn:0   dfail:0   fail:18  skip:983 time:9595s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5533/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Joonas Lahtinen]

On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> Driver’s CPU access to GTT is via the GTTMMADR BAR.
> 
> The current HW implementation of that BAR is to only
> support <= DW (and maybe QW) writes—not 16/32/64B writes
> that could occur with WC and/or SSE/AVX moves.
> 
> GTTMMADR must be marked uncacheable (UC).
> Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> 
> v2: Get clarification on the reasons and spec is getting
>     updated to reflect it now.
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Rodrigo, can you double-check how this interacts with the patch from
Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.

If that doesn't help with the problem, this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Chris Wilson]

Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > 
> > The current HW implementation of that BAR is to only
> > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > that could occur with WC and/or SSE/AVX moves.
> > 
> > GTTMMADR must be marked uncacheable (UC).
> > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > 
> > v2: Get clarification on the reasons and spec is getting
> >     updated to reflect it now.
> > 
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Rodrigo, can you double-check how this interacts with the patch from
> Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.

Different issue (or should be). The ioremap concerns access through the
PCI BAR, affecting how fast we insert entries into the GGTT (so
establishing new mmaps following frequent runtime pm, loading of new
contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
how the device accesses the physical pages, not the PTE themselves.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Joonas Lahtinen]

On Wed, 2017-08-30 at 12:26 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> > On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > > 
> > > The current HW implementation of that BAR is to only
> > > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > > that could occur with WC and/or SSE/AVX moves.
> > > 
> > > GTTMMADR must be marked uncacheable (UC).
> > > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > > 
> > > v2: Get clarification on the reasons and spec is getting
> > >     updated to reflect it now.
> > > 
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > Rodrigo, can you double-check how this interacts with the patch from
> > Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.
> 
> Different issue (or should be). The ioremap concerns access through the
> PCI BAR, affecting how fast we insert entries into the GGTT (so
> establishing new mmaps following frequent runtime pm, loading of new
> contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
> how the device accesses the physical pages, not the PTE themselves.

Yes, I know it should be :) But Rodrigo also described pretty random
hangs, IIRC not much was pinpointing to either of the issues. With
these two bugs present, device could be operating without write-back on
certain pages, or could be operating on wrong pages altogether.

I'd just like one round of testing to try to avoid this change if we
can.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Vivi, Rodrigo]

On Wed, 2017-08-30 at 14:38 +0300, Joonas Lahtinen wrote:
> On Wed, 2017-08-30 at 12:26 +0100, Chris Wilson wrote:
> > Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> > > On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > > > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > > > 
> > > > The current HW implementation of that BAR is to only
> > > > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > > > that could occur with WC and/or SSE/AVX moves.
> > > > 
> > > > GTTMMADR must be marked uncacheable (UC).
> > > > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > > > 
> > > > v2: Get clarification on the reasons and spec is getting
> > > >     updated to reflect it now.
> > > > 
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > Rodrigo, can you double-check how this interacts with the patch from
> > > Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.
> > 
> > Different issue (or should be). The ioremap concerns access through the
> > PCI BAR, affecting how fast we insert entries into the GGTT (so
> > establishing new mmaps following frequent runtime pm, loading of new
> > contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
> > how the device accesses the physical pages, not the PTE themselves.
> 
> Yes, I know it should be :) But Rodrigo also described pretty random
> hangs, IIRC not much was pinpointing to either of the issues. With
> these two bugs present, device could be operating without write-back on
> certain pages, or could be operating on wrong pages altogether.
> 
> I'd just like one round of testing to try to avoid this change if we
> can.

I had tried already put PAT to non-cached, but I will double check Zhi's
work just in case.

I wish we could avoid this patch here, but it seems by definition this
BAR should be uncached. By BAR's non-Prefetchable attribute.

So probably the ioremap_wc should check that attribute and fail to
allocate that with wc so we would try wc and fallback to uncached.

But since we know this is uncached only for this case and this handle
don't exist yet the best is to move along with this patch.

> 
> Regards, Joonas

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Joonas Lahtinen]

On Wed, 2017-08-30 at 17:58 +0000, Vivi, Rodrigo wrote:
> On Wed, 2017-08-30 at 14:38 +0300, Joonas Lahtinen wrote:
> > On Wed, 2017-08-30 at 12:26 +0100, Chris Wilson wrote:
> > > Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> > > > On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > > > > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > > > > 
> > > > > The current HW implementation of that BAR is to only
> > > > > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > > > > that could occur with WC and/or SSE/AVX moves.
> > > > > 
> > > > > GTTMMADR must be marked uncacheable (UC).
> > > > > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > > > > 
> > > > > v2: Get clarification on the reasons and spec is getting
> > > > >     updated to reflect it now.
> > > > > 
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > 
> > > > Rodrigo, can you double-check how this interacts with the patch from
> > > > Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.
> > > 
> > > Different issue (or should be). The ioremap concerns access through the
> > > PCI BAR, affecting how fast we insert entries into the GGTT (so
> > > establishing new mmaps following frequent runtime pm, loading of new
> > > contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
> > > how the device accesses the physical pages, not the PTE themselves.
> > 
> > Yes, I know it should be :) But Rodrigo also described pretty random
> > hangs, IIRC not much was pinpointing to either of the issues. With
> > these two bugs present, device could be operating without write-back on
> > certain pages, or could be operating on wrong pages altogether.
> > 
> > I'd just like one round of testing to try to avoid this change if we
> > can.
> 
> I had tried already put PAT to non-cached, but I will double check Zhi's
> work just in case.
> 
> I wish we could avoid this patch here, but it seems by definition this
> BAR should be uncached. By BAR's non-Prefetchable attribute.
> 
> So probably the ioremap_wc should check that attribute and fail to
> allocate that with wc so we would try wc and fallback to uncached.
> 
> But since we know this is uncached only for this case and this handle
> don't exist yet the best is to move along with this patch.

Right, you can have the R-b.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Vivi, Rodrigo]

On Thu, 2017-08-31 at 11:27 +0300, Joonas Lahtinen wrote:
> On Wed, 2017-08-30 at 17:58 +0000, Vivi, Rodrigo wrote:
> > On Wed, 2017-08-30 at 14:38 +0300, Joonas Lahtinen wrote:
> > > On Wed, 2017-08-30 at 12:26 +0100, Chris Wilson wrote:
> > > > Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> > > > > On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > > > > > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > > > > > 
> > > > > > The current HW implementation of that BAR is to only
> > > > > > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > > > > > that could occur with WC and/or SSE/AVX moves.
> > > > > > 
> > > > > > GTTMMADR must be marked uncacheable (UC).
> > > > > > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > > > > > 
> > > > > > v2: Get clarification on the reasons and spec is getting
> > > > > >     updated to reflect it now.
> > > > > > 
> > > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > > Suggested-by: Ben Widawsky <benjamin.widawsky@intel.com>
> > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > 
> > > > > Rodrigo, can you double-check how this interacts with the patch from
> > > > > Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.
> > > > 
> > > > Different issue (or should be). The ioremap concerns access through the
> > > > PCI BAR, affecting how fast we insert entries into the GGTT (so
> > > > establishing new mmaps following frequent runtime pm, loading of new
> > > > contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
> > > > how the device accesses the physical pages, not the PTE themselves.
> > > 
> > > Yes, I know it should be :) But Rodrigo also described pretty random
> > > hangs, IIRC not much was pinpointing to either of the issues. With
> > > these two bugs present, device could be operating without write-back on
> > > certain pages, or could be operating on wrong pages altogether.
> > > 
> > > I'd just like one round of testing to try to avoid this change if we
> > > can.
> > 
> > I had tried already put PAT to non-cached, but I will double check Zhi's
> > work just in case.
> > 
> > I wish we could avoid this patch here, but it seems by definition this
> > BAR should be uncached. By BAR's non-Prefetchable attribute.
> > 
> > So probably the ioremap_wc should check that attribute and fail to
> > allocate that with wc so we would try wc and fallback to uncached.
> > 
> > But since we know this is uncached only for this case and this handle
> > don't exist yet the best is to move along with this patch.
> 
> Right, you can have the R-b.

thanks. merged to dinq.

> 
> Regards, Joonas

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx