>> From: Fei Yang <fei.yang@xxxxxxxxx> >> >> On MTL, objects allocated through i915_gem_object_create_internal() are >> mapped as uncached in GPU by default because HAS_LLC is false. However >> in the live_hwsp_read selftest these watcher objects are mapped as WB >> on CPU side. The conseqence is that the updates done by the GPU are not >> immediately visible to CPU, thus the selftest is randomly failing due to >> the stale data in CPU cache. Solution can be either setting WC for CPU + >> UC for GPU, or WB for CPU + 1-way coherent WB for GPU. >> To keep the consistency, let's simply inherit the same cache settings >> from the timeline, which is WB for CPU + 1-way coherent WB for GPU, >> because this test is supposed to emulate the behavior of the timeline >> anyway. >> >> Signed-off-by: Fei Yang <fei.yang@xxxxxxxxx> > > It looks like there might be an indentation mistake on the second line > of the i915_gem_object_pin_map_unlocked() call, but we can fix that up > while applying; no need to re-send. > > Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Thanks for reviewing. > Is there an FDO issue # for the random failures thar were being seen? > If so, we should add a Closes: or References: tag here. I'm not aware of a FDO filed for this failure. That might be because the issue is reproduced on MTL which might not be widely available to the community yet. > Matt >> --- >> drivers/gpu/drm/i915/gt/selftest_timeline.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c >> index 522d0190509c..631aaed9bc3d 100644 >> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c >> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c >> @@ -825,7 +825,8 @@ static bool cmp_gte(u32 a, u32 b) >> return a >= b; >> } >> >> -static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt) >> +static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt, >> + struct intel_timeline *tl) >> { >> struct drm_i915_gem_object *obj; >> struct i915_vma *vma; >> @@ -834,7 +835,10 @@ static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt) >> if (IS_ERR(obj)) >> return PTR_ERR(obj); >> >> - w->map = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); >> + /* keep the same cache settings as timeline */ >> + i915_gem_object_set_cache_coherency(obj, tl->hwsp_ggtt->obj->cache_level); >> + w->map = i915_gem_object_pin_map_unlocked(obj, >> + page_unmask_bits(tl->hwsp_ggtt->obj->mm.mapping)); >> if (IS_ERR(w->map)) { >> i915_gem_object_put(obj); >> return PTR_ERR(w->map); >> @@ -1004,8 +1008,10 @@ static int live_hwsp_read(void *arg) >> if (!tl->has_initial_breadcrumb) >> goto out_free; >> >> + selftest_tl_pin(tl); >> + >> for (i = 0; i < ARRAY_SIZE(watcher); i++) { >> - err = setup_watcher(&watcher[i], gt); >> + err = setup_watcher(&watcher[i], gt, tl); >> if (err) >> goto out; >> } >> @@ -1160,6 +1166,8 @@ static int live_hwsp_read(void *arg) >> for (i = 0; i < ARRAY_SIZE(watcher); i++) >> cleanup_watcher(&watcher[i]); >> >> + intel_timeline_unpin(tl); >> + >> if (igt_flush_test(gt->i915)) >> err = -EIO; >> >> -- >> 2.25.1