[PATCH] drm/i915: Restore marking context objects as dirty on pinning

[PATCH] drm/i915: Restore marking context objects as dirty on pinning

[Chris Wilson]

Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between
legacy/execlists/guc") converted the legacy intel_ringbuffer submission
to the same context pinning mechanism as execlists - that is to pin the
context until the subsequent request is retired. Previously it used the
vma retirement of the context object to keep itself pinned until the
next request (after i915_vma_move_to_active()). In the conversion, I
missed that the vma retirement was also responsible for marking the
object as dirty. Mark the context object as dirty when pinning
(equivalent to execlists) which ensures that if the context is swapped
out due to mempressure or suspend/hibernation, when it is loaded back in
it does so with the previous state (and not all zero).

Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc")
Reported-by: Dennis Gilmore <dennis@ausil.us>
Reported-by: Mathieu Marquer <mathieu.marquer@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99993
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.11-rc1
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0ca5ea7a9407..62756eb2bd4a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine,
 		ret = context_pin(ctx);
 		if (ret)
 			goto error;
+
+		ce->state->obj->mm.dirty = true;
 	}
 
 	/* The kernel context is only used as a placeholder for flushing the
-- 
2.11.0

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

✗ Fi.CI.BAT: failure for drm/i915: Restore marking context objects as dirty on pinning

[Patchwork]

== Series Details ==

Series: drm/i915: Restore marking context objects as dirty on pinning
URL   : https://patchwork.freedesktop.org/series/21708/
State : failure

== Summary ==

Series 21708v1 drm/i915: Restore marking context objects as dirty on pinning
https://patchwork.freedesktop.org/api/1.0/series/21708/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> INCOMPLETE (fi-byt-j1900)
        Subgroup suspend-read-crc-pipe-a:
                fail       -> PASS       (fi-skl-6700k)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 457s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 452s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 582s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 536s
fi-byt-j1900     total:226  pass:206  dwarn:0   dfail:0   fail:0   skip:19  time: 0s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 501s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 433s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 436s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 514s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 496s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 483s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 492s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 600s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 490s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 524s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 459s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 419s

b1ecc49ac71e484430083e5b3b7cd75d4976a465 drm-tip: 2017y-03m-23d-07h-40m-49s UTC integration manifest
de7a9a9 drm/i915: Restore marking context objects as dirty on pinning

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4266/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Restore marking context objects as dirty on pinning

[Tvrtko Ursulin]

On 22/03/2017 20:59, Chris Wilson wrote:
> Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between
> legacy/execlists/guc") converted the legacy intel_ringbuffer submission
> to the same context pinning mechanism as execlists - that is to pin the
> context until the subsequent request is retired. Previously it used the
> vma retirement of the context object to keep itself pinned until the
> next request (after i915_vma_move_to_active()). In the conversion, I
> missed that the vma retirement was also responsible for marking the
> object as dirty. Mark the context object as dirty when pinning
> (equivalent to execlists) which ensures that if the context is swapped
> out due to mempressure or suspend/hibernation, when it is loaded back in
> it does so with the previous state (and not all zero).
>
> Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc")
> Reported-by: Dennis Gilmore <dennis@ausil.us>
> Reported-by: Mathieu Marquer <mathieu.marquer@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99993
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.11-rc1
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 0ca5ea7a9407..62756eb2bd4a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine,
>  		ret = context_pin(ctx);
>  		if (ret)
>  			goto error;
> +
> +		ce->state->obj->mm.dirty = true;
>  	}
>
>  	/* The kernel context is only used as a placeholder for flushing the
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Could it be related to 99671, it also mentions suspend?

Regards,

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

Re: [PATCH] drm/i915: Restore marking context objects as dirty on pinning

[Chris Wilson]

On Thu, Mar 23, 2017 at 09:41:30AM +0000, Tvrtko Ursulin wrote:
> 
> On 22/03/2017 20:59, Chris Wilson wrote:
> >Commit e8a9c58fcd9a ("drm/i915: Unify active context tracking between
> >legacy/execlists/guc") converted the legacy intel_ringbuffer submission
> >to the same context pinning mechanism as execlists - that is to pin the
> >context until the subsequent request is retired. Previously it used the
> >vma retirement of the context object to keep itself pinned until the
> >next request (after i915_vma_move_to_active()). In the conversion, I
> >missed that the vma retirement was also responsible for marking the
> >object as dirty. Mark the context object as dirty when pinning
> >(equivalent to execlists) which ensures that if the context is swapped
> >out due to mempressure or suspend/hibernation, when it is loaded back in
> >it does so with the previous state (and not all zero).
> >
> >Fixes: e8a9c58fcd9a ("drm/i915: Unify active context tracking between legacy/execlists/guc")
> >Reported-by: Dennis Gilmore <dennis@ausil.us>
> >Reported-by: Mathieu Marquer <mathieu.marquer@gmail.com>
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99993
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100181
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.11-rc1
> >---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >index 0ca5ea7a9407..62756eb2bd4a 100644
> >--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >@@ -1440,6 +1440,8 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine,
> > 		ret = context_pin(ctx);
> > 		if (ret)
> > 			goto error;
> >+
> >+		ce->state->obj->mm.dirty = true;
> > 	}
> >
> > 	/* The kernel context is only used as a placeholder for flushing the
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Could it be related to 99671, it also mentions suspend?

No, I do not believe so. The symptoms do not match (but that may just be
snb handling the failure differently) but more tellingly it started (or
at least based on the bug reports became more prevalent) before the
breakage was introduced.
-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

Re: ✗ Fi.CI.BAT: failure for drm/i915: Restore marking context objects as dirty on pinning

[Chris Wilson]

On Thu, Mar 23, 2017 at 09:03:30AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Restore marking context objects as dirty on pinning
> URL   : https://patchwork.freedesktop.org/series/21708/
> State : failure
> 
> == Summary ==
> 
> Series 21708v1 drm/i915: Restore marking context objects as dirty on pinning
> https://patchwork.freedesktop.org/api/1.0/series/21708/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                 pass       -> INCOMPLETE (fi-byt-j1900)
>         Subgroup suspend-read-crc-pipe-a:
>                 fail       -> PASS       (fi-skl-6700k)

Lalalala, pushed anyway. Thanks for the review.
-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