[PATCH] agp/intel: Flush all chipset writes after updating the GGTT

[PATCH] agp/intel: Flush all chipset writes after updating the GGTT

[Chris Wilson]

Before accessing the GGTT we must flush the PTE writes and make them
visible to the chipset, or else the indirect access may end up in the
wrong page. In commit 3497971a71d8 ("agp/intel: Flush chipset writes
after updating a single PTE"), we noticed corruption of the uploads for
pwrite and for capturing GPU error states, but it was presumed that the
explicit calls to intel_gtt_chipset_flush() were sufficient for the
execbuffer path. However, we have not been flushing the chipset between
the PTE writes and access via the GTT itself.

For simplicity, do the flush after any PTE update rather than try and
batch the flushes on a just-in-time basis.

References: 3497971a71d8 ("agp/intel: Flush chipset writes after updating a single PTE")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
---
 drivers/char/agp/intel-gtt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 9b6b6023193b..dde7caac7f9f 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -872,6 +872,8 @@ void intel_gtt_insert_sg_entries(struct sg_table *st,
 		}
 	}
 	wmb();
+	if (intel_private.driver->chipset_flush)
+		intel_private.driver->chipset_flush();
 }
 EXPORT_SYMBOL(intel_gtt_insert_sg_entries);
 
-- 
2.15.1

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

✓ Fi.CI.BAT: success for agp/intel: Flush all chipset writes after updating the GGTT

[Patchwork]

== Series Details ==

Series: agp/intel: Flush all chipset writes after updating the GGTT
URL   : https://patchwork.freedesktop.org/series/35119/
State : success

== Summary ==

Series 35119v1 agp/intel: Flush all chipset writes after updating the GGTT
https://patchwork.freedesktop.org/api/1.0/series/35119/revisions/1/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:437s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:509s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:510s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:486s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:474s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:269s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:359s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:259s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:394s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:448s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:479s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:528s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:532s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:596s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:545s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:444s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:544s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:416s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:615s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:648s
fi-glk-dsi       total:288  pass:179  dwarn:1   dfail:4   fail:0   skip:104 time:354s

06dd422e3209a968c420e10504f75fbbe897f06c drm-tip: 2017y-12m-08d-21h-06m-35s UTC integration manifest
007096e34b96 agp/intel: Flush all chipset writes after updating the GGTT

== Logs ==

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

✗ Fi.CI.IGT: warning for agp/intel: Flush all chipset writes after updating the GGTT

[Patchwork]

== Series Details ==

Series: agp/intel: Flush all chipset writes after updating the GGTT
URL   : https://patchwork.freedesktop.org/series/35119/
State : warning

== Summary ==

Test kms_draw_crc:
        Subgroup draw-method-rgb565-mmap-wc-xtiled:
                pass       -> SKIP       (shard-snb)
Test gem_softpin:
        Subgroup noreloc-s3:
                pass       -> SKIP       (shard-snb) fdo#102365
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-snb) fdo#104009
Test drv_selftest:
        Subgroup live_hangcheck:
                incomplete -> PASS       (shard-snb) fdo#103880
Test gem_eio:
        Subgroup in-flight-contexts:
                pass       -> DMESG-WARN (shard-snb) fdo#104058
Test gem_userptr_blits:
        Subgroup map-fixed-invalidate-gup:
                pass       -> DMESG-WARN (shard-hsw)

fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#103880 https://bugs.freedesktop.org/show_bug.cgi?id=103880
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058

shard-hsw        total:2680 pass:1536 dwarn:2   dfail:0   fail:10  skip:1132 time:9417s
shard-snb        total:2680 pass:1306 dwarn:2   dfail:0   fail:12  skip:1360 time:8012s
Blacklisted hosts:
shard-apl        total:2680 pass:1638 dwarn:1   dfail:1   fail:25  skip:1015 time:13287s
shard-kbl        total:2631 pass:1764 dwarn:1   dfail:0   fail:21  skip:844 time:10469s

== Logs ==

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

Re: [PATCH] agp/intel: Flush all chipset writes after updating the GGTT

[Joonas Lahtinen]

On Fri, 2017-12-08 at 21:46 +0000, Chris Wilson wrote:
> Before accessing the GGTT we must flush the PTE writes and make them
> visible to the chipset, or else the indirect access may end up in the
> wrong page. In commit 3497971a71d8 ("agp/intel: Flush chipset writes
> after updating a single PTE"), we noticed corruption of the uploads for
> pwrite and for capturing GPU error states, but it was presumed that the
> explicit calls to intel_gtt_chipset_flush() were sufficient for the
> execbuffer path. However, we have not been flushing the chipset between
> the PTE writes and access via the GTT itself.
> 
> For simplicity, do the flush after any PTE update rather than try and
> batch the flushes on a just-in-time basis.
> 
> References: 3497971a71d8 ("agp/intel: Flush chipset writes after updating a single PTE")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org

I don't think this is being used so much anymore? (+ Jani for this)

Why not Cc: stable? My DIM says # v4.9+

> +++ b/drivers/char/agp/intel-gtt.c
> @@ -872,6 +872,8 @@ void intel_gtt_insert_sg_entries(struct sg_table *st,
>  		}
>  	}
>  	wmb();
> +	if (intel_private.driver->chipset_flush)
> +		intel_private.driver->chipset_flush();

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] agp/intel: Flush all chipset writes after updating the GGTT

[Chris Wilson]

Quoting Joonas Lahtinen (2017-12-11 10:55:40)
> On Fri, 2017-12-08 at 21:46 +0000, Chris Wilson wrote:
> > Before accessing the GGTT we must flush the PTE writes and make them
> > visible to the chipset, or else the indirect access may end up in the
> > wrong page. In commit 3497971a71d8 ("agp/intel: Flush chipset writes
> > after updating a single PTE"), we noticed corruption of the uploads for
> > pwrite and for capturing GPU error states, but it was presumed that the
> > explicit calls to intel_gtt_chipset_flush() were sufficient for the
> > execbuffer path. However, we have not been flushing the chipset between
> > the PTE writes and access via the GTT itself.
> > 
> > For simplicity, do the flush after any PTE update rather than try and
> > batch the flushes on a just-in-time basis.
> > 
> > References: 3497971a71d8 ("agp/intel: Flush chipset writes after updating a single PTE")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> 
> I don't think this is being used so much anymore? (+ Jani for this)
> 
> Why not Cc: stable? My DIM says # v4.9+

I don't use stable@ anymore since Greg doesn't like our patches and
would much prefer to pick randomly instead. /slightly-s

So I leave that management to you guys.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.IGT: warning for agp/intel: Flush all chipset writes after updating the GGTT

[Chris Wilson]

Quoting Patchwork (2017-12-09 00:25:53)
> == Series Details ==
> 
> Series: agp/intel: Flush all chipset writes after updating the GGTT
> URL   : https://patchwork.freedesktop.org/series/35119/
> State : warning
> 
> == Summary ==
> 
> Test kms_draw_crc:
>         Subgroup draw-method-rgb565-mmap-wc-xtiled:
>                 pass       -> SKIP       (shard-snb)
> Test gem_softpin:
>         Subgroup noreloc-s3:
>                 pass       -> SKIP       (shard-snb) fdo#102365
> Test gem_tiled_swapping:
>         Subgroup non-threaded:
>                 incomplete -> PASS       (shard-snb) fdo#104009
> Test drv_selftest:
>         Subgroup live_hangcheck:
>                 incomplete -> PASS       (shard-snb) fdo#103880
> Test gem_eio:
>         Subgroup in-flight-contexts:
>                 pass       -> DMESG-WARN (shard-snb) fdo#104058
> Test gem_userptr_blits:
>         Subgroup map-fixed-invalidate-gup:
>                 pass       -> DMESG-WARN (shard-hsw)

Thanks for the review. Empirically, based on the evidence from
insert-page, this should fix a corner case or two for us, so fingers
crossed it's the same one CI is tripping over.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx