* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:48 [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context Chris Wilson
@ 2018-03-27 13:52 ` Chris Wilson
2018-03-27 13:59 ` Chris Wilson
2018-03-27 15:25 ` Tvrtko Ursulin
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2018-03-27 13:52 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
Quoting Chris Wilson (2018-03-27 14:48:43)
> If we inject a reset into the target context, there is a risk that the
> register state is never saved back to memory. The exact interaction
> between reset, the context image and the precise timing of our execution
> are not well defined. Since we cannot ensure that the context image
> remains valid, force a context switch prior to the reset.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
> index d8109aa0..4968e367 100644
> --- a/tests/gem_ctx_isolation.c
> +++ b/tests/gem_ctx_isolation.c
> @@ -522,6 +522,32 @@ static void isolation(int fd,
> #define S4 (4 << 8)
> #define SLEEP_MASK (0xf << 8)
>
> +static void inject_reset_context(int fd, unsigned int engine)
> +{
> + igt_spin_t *spin;
> + uint32_t ctx;
> +
> + /*
> + * Force a context switch before triggering the reset, or else
> + * we risk corrupting the target context and we can't blame the
> + * HW for screwing up if the context was already broken.
> + */
> +
> + ctx = gem_context_create(fd);
> + if (gem_can_store_dword(fd, engine)) {
> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
> + igt_spin_busywait_until_running(spin);
> + } else {
> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> + usleep(1000); /* better than nothing */
> + }
Tvrtko, maybe we want igt_spin_batch_run()? Not sure though, so far we
have an example where you need precise control and a couple of examples
where we just want a running spinner.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:52 ` Chris Wilson
@ 2018-03-27 13:59 ` Chris Wilson
2018-03-27 15:39 ` Tvrtko Ursulin
2018-03-27 15:41 ` [Intel-gfx] " Tvrtko Ursulin
0 siblings, 2 replies; 13+ messages in thread
From: Chris Wilson @ 2018-03-27 13:59 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
Quoting Chris Wilson (2018-03-27 14:52:20)
> Quoting Chris Wilson (2018-03-27 14:48:43)
> > If we inject a reset into the target context, there is a risk that the
> > register state is never saved back to memory. The exact interaction
> > between reset, the context image and the precise timing of our execution
> > are not well defined. Since we cannot ensure that the context image
> > remains valid, force a context switch prior to the reset.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> > tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
> > index d8109aa0..4968e367 100644
> > --- a/tests/gem_ctx_isolation.c
> > +++ b/tests/gem_ctx_isolation.c
> > @@ -522,6 +522,32 @@ static void isolation(int fd,
> > #define S4 (4 << 8)
> > #define SLEEP_MASK (0xf << 8)
> >
> > +static void inject_reset_context(int fd, unsigned int engine)
> > +{
> > + igt_spin_t *spin;
> > + uint32_t ctx;
> > +
> > + /*
> > + * Force a context switch before triggering the reset, or else
> > + * we risk corrupting the target context and we can't blame the
> > + * HW for screwing up if the context was already broken.
> > + */
> > +
> > + ctx = gem_context_create(fd);
> > + if (gem_can_store_dword(fd, engine)) {
> > + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
> > + igt_spin_busywait_until_running(spin);
> > + } else {
> > + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> > + usleep(1000); /* better than nothing */
> > + }
>
> Tvrtko, maybe we want igt_spin_batch_run()? Not sure though, so far we
> have an example where you need precise control and a couple of examples
> where we just want a running spinner.
Also this whole function might want to make its way into lib/i915 as the
basis of all hang/reset injection. Some improvement required to set
context parameters (e.g. disabling error state capturing) and skipping
if no reset is available or disabled (although that's the job for the
fixture, so may not be required for the library function). I'm sure
someone will catch me reusing this chunk over and over again...
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:59 ` Chris Wilson
@ 2018-03-27 15:39 ` Tvrtko Ursulin
2018-03-27 15:41 ` [Intel-gfx] " Tvrtko Ursulin
1 sibling, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
On 27/03/2018 14:59, Chris Wilson wrote:
> Quoting Chris Wilson (2018-03-27 14:52:20)
>> Quoting Chris Wilson (2018-03-27 14:48:43)
>>> If we inject a reset into the target context, there is a risk that the
>>> register state is never saved back to memory. The exact interaction
>>> between reset, the context image and the precise timing of our execution
>>> are not well defined. Since we cannot ensure that the context image
>>> remains valid, force a context switch prior to the reset.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> ---
>>> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
>>> 1 file changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
>>> index d8109aa0..4968e367 100644
>>> --- a/tests/gem_ctx_isolation.c
>>> +++ b/tests/gem_ctx_isolation.c
>>> @@ -522,6 +522,32 @@ static void isolation(int fd,
>>> #define S4 (4 << 8)
>>> #define SLEEP_MASK (0xf << 8)
>>>
>>> +static void inject_reset_context(int fd, unsigned int engine)
>>> +{
>>> + igt_spin_t *spin;
>>> + uint32_t ctx;
>>> +
>>> + /*
>>> + * Force a context switch before triggering the reset, or else
>>> + * we risk corrupting the target context and we can't blame the
>>> + * HW for screwing up if the context was already broken.
>>> + */
>>> +
>>> + ctx = gem_context_create(fd);
>>> + if (gem_can_store_dword(fd, engine)) {
>>> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
>>> + igt_spin_busywait_until_running(spin);
>>> + } else {
>>> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
>>> + usleep(1000); /* better than nothing */
>>> + }
>>
>> Tvrtko, maybe we want igt_spin_batch_run()? Not sure though, so far we
>> have an example where you need precise control and a couple of examples
>> where we just want a running spinner.
I wasn't sure it is in good taste to put a thing with that usleep in
lib/, since it incurs a delay to unsuspecting callers. And I couldn't
decide how to handle it better - skip from lib/? Not so great. Return
value and make callers handle it - even more cumbersome.
It only affects old gens, but do we want tests there suddenly take much
longer because someone spotted a cool new API and went to use it?
But it is a third user now so not great to copy paste it all around. I
don't know. Make the lib functions skip where unsupported and add double
underscore prefix flavour which sleeps where not supported?
> Also this whole function might want to make its way into lib/i915 as the
> basis of all hang/reset injection. Some improvement required to set
> context parameters (e.g. disabling error state capturing) and skipping
> if no reset is available or disabled (although that's the job for the
> fixture, so may not be required for the library function). I'm sure
> someone will catch me reusing this chunk over and over again...
Sounds OK to me.
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:59 ` Chris Wilson
@ 2018-03-27 15:41 ` Tvrtko Ursulin
2018-03-27 15:41 ` [Intel-gfx] " Tvrtko Ursulin
1 sibling, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:41 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
[resend for typo in cc]
On 27/03/2018 14:59, Chris Wilson wrote:
> Quoting Chris Wilson (2018-03-27 14:52:20)
>> Quoting Chris Wilson (2018-03-27 14:48:43)
>>> If we inject a reset into the target context, there is a risk that the
>>> register state is never saved back to memory. The exact interaction
>>> between reset, the context image and the precise timing of our execution
>>> are not well defined. Since we cannot ensure that the context image
>>> remains valid, force a context switch prior to the reset.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> ---
>>> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
>>> 1 file changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
>>> index d8109aa0..4968e367 100644
>>> --- a/tests/gem_ctx_isolation.c
>>> +++ b/tests/gem_ctx_isolation.c
>>> @@ -522,6 +522,32 @@ static void isolation(int fd,
>>> #define S4 (4 << 8)
>>> #define SLEEP_MASK (0xf << 8)
>>>
>>> +static void inject_reset_context(int fd, unsigned int engine)
>>> +{
>>> + igt_spin_t *spin;
>>> + uint32_t ctx;
>>> +
>>> + /*
>>> + * Force a context switch before triggering the reset, or else
>>> + * we risk corrupting the target context and we can't blame the
>>> + * HW for screwing up if the context was already broken.
>>> + */
>>> +
>>> + ctx = gem_context_create(fd);
>>> + if (gem_can_store_dword(fd, engine)) {
>>> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
>>> + igt_spin_busywait_until_running(spin);
>>> + } else {
>>> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
>>> + usleep(1000); /* better than nothing */
>>> + }
>>
>> Tvrtko, maybe we want igt_spin_batch_run()? Not sure though, so far we
>> have an example where you need precise control and a couple of examples
>> where we just want a running spinner.
I wasn't sure it is in good taste to put a thing with that usleep in
lib/, since it incurs a delay to unsuspecting callers. And I couldn't
decide how to handle it better - skip from lib/? Not so great. Return
value and make callers handle it - even more cumbersome.
It only affects old gens, but do we want tests there suddenly take much
longer because someone spotted a cool new API and went to use it?
But it is a third user now so not great to copy paste it all around. I
don't know. Make the lib functions skip where unsupported and add double
underscore prefix flavour which sleeps where not supported?
> Also this whole function might want to make its way into lib/i915 as the
> basis of all hang/reset injection. Some improvement required to set
> context parameters (e.g. disabling error state capturing) and skipping
> if no reset is available or disabled (although that's the job for the
> fixture, so may not be required for the library function). I'm sure
> someone will catch me reusing this chunk over and over again...
Sounds OK to me.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Intel-gfx] [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
@ 2018-03-27 15:41 ` Tvrtko Ursulin
0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:41 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
[resend for typo in cc]
On 27/03/2018 14:59, Chris Wilson wrote:
> Quoting Chris Wilson (2018-03-27 14:52:20)
>> Quoting Chris Wilson (2018-03-27 14:48:43)
>>> If we inject a reset into the target context, there is a risk that the
>>> register state is never saved back to memory. The exact interaction
>>> between reset, the context image and the precise timing of our execution
>>> are not well defined. Since we cannot ensure that the context image
>>> remains valid, force a context switch prior to the reset.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> ---
>>> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
>>> 1 file changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
>>> index d8109aa0..4968e367 100644
>>> --- a/tests/gem_ctx_isolation.c
>>> +++ b/tests/gem_ctx_isolation.c
>>> @@ -522,6 +522,32 @@ static void isolation(int fd,
>>> #define S4 (4 << 8)
>>> #define SLEEP_MASK (0xf << 8)
>>>
>>> +static void inject_reset_context(int fd, unsigned int engine)
>>> +{
>>> + igt_spin_t *spin;
>>> + uint32_t ctx;
>>> +
>>> + /*
>>> + * Force a context switch before triggering the reset, or else
>>> + * we risk corrupting the target context and we can't blame the
>>> + * HW for screwing up if the context was already broken.
>>> + */
>>> +
>>> + ctx = gem_context_create(fd);
>>> + if (gem_can_store_dword(fd, engine)) {
>>> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
>>> + igt_spin_busywait_until_running(spin);
>>> + } else {
>>> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
>>> + usleep(1000); /* better than nothing */
>>> + }
>>
>> Tvrtko, maybe we want igt_spin_batch_run()? Not sure though, so far we
>> have an example where you need precise control and a couple of examples
>> where we just want a running spinner.
I wasn't sure it is in good taste to put a thing with that usleep in
lib/, since it incurs a delay to unsuspecting callers. And I couldn't
decide how to handle it better - skip from lib/? Not so great. Return
value and make callers handle it - even more cumbersome.
It only affects old gens, but do we want tests there suddenly take much
longer because someone spotted a cool new API and went to use it?
But it is a third user now so not great to copy paste it all around. I
don't know. Make the lib functions skip where unsupported and add double
underscore prefix flavour which sleeps where not supported?
> Also this whole function might want to make its way into lib/i915 as the
> basis of all hang/reset injection. Some improvement required to set
> context parameters (e.g. disabling error state capturing) and skipping
> if no reset is available or disabled (although that's the job for the
> fixture, so may not be required for the library function). I'm sure
> someone will catch me reusing this chunk over and over again...
Sounds OK to me.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:48 [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context Chris Wilson
2018-03-27 13:52 ` Chris Wilson
@ 2018-03-27 15:25 ` Tvrtko Ursulin
2018-03-27 15:41 ` Tvrtko Ursulin
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:25 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
On 27/03/2018 14:48, Chris Wilson wrote:
> If we inject a reset into the target context, there is a risk that the
> register state is never saved back to memory. The exact interaction
> between reset, the context image and the precise timing of our execution
> are not well defined. Since we cannot ensure that the context image
> remains valid, force a context switch prior to the reset.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
> index d8109aa0..4968e367 100644
> --- a/tests/gem_ctx_isolation.c
> +++ b/tests/gem_ctx_isolation.c
> @@ -522,6 +522,32 @@ static void isolation(int fd,
> #define S4 (4 << 8)
> #define SLEEP_MASK (0xf << 8)
>
> +static void inject_reset_context(int fd, unsigned int engine)
> +{
> + igt_spin_t *spin;
> + uint32_t ctx;
> +
> + /*
> + * Force a context switch before triggering the reset, or else
> + * we risk corrupting the target context and we can't blame the
> + * HW for screwing up if the context was already broken.
> + */
> +
> + ctx = gem_context_create(fd);
> + if (gem_can_store_dword(fd, engine)) {
> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
> + igt_spin_busywait_until_running(spin);
> + } else {
> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> + usleep(1000); /* better than nothing */
> + }
> +
> + igt_force_gpu_reset(fd);
> +
> + igt_spin_batch_free(fd, spin);
> + gem_context_destroy(fd, ctx);
> +}
> +
> static void preservation(int fd,
> const struct intel_execution_engine2 *e,
> unsigned int flags)
> @@ -558,7 +584,7 @@ static void preservation(int fd,
> igt_spin_batch_free(fd, spin);
>
> if (flags & RESET)
> - igt_force_gpu_reset(fd);
> + inject_reset_context(fd, engine);
>
> switch (flags & SLEEP_MASK) {
> case NOSLEEP:
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:48 [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context Chris Wilson
@ 2018-03-27 15:41 ` Tvrtko Ursulin
2018-03-27 15:25 ` Tvrtko Ursulin
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:41 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
[resend for typo in cc]
On 27/03/2018 14:48, Chris Wilson wrote:
> If we inject a reset into the target context, there is a risk that the
> register state is never saved back to memory. The exact interaction
> between reset, the context image and the precise timing of our execution
> are not well defined. Since we cannot ensure that the context image
> remains valid, force a context switch prior to the reset.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
> index d8109aa0..4968e367 100644
> --- a/tests/gem_ctx_isolation.c
> +++ b/tests/gem_ctx_isolation.c
> @@ -522,6 +522,32 @@ static void isolation(int fd,
> #define S4 (4 << 8)
> #define SLEEP_MASK (0xf << 8)
>
> +static void inject_reset_context(int fd, unsigned int engine)
> +{
> + igt_spin_t *spin;
> + uint32_t ctx;
> +
> + /*
> + * Force a context switch before triggering the reset, or else
> + * we risk corrupting the target context and we can't blame the
> + * HW for screwing up if the context was already broken.
> + */
> +
> + ctx = gem_context_create(fd);
> + if (gem_can_store_dword(fd, engine)) {
> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
> + igt_spin_busywait_until_running(spin);
> + } else {
> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> + usleep(1000); /* better than nothing */
> + }
> +
> + igt_force_gpu_reset(fd);
> +
> + igt_spin_batch_free(fd, spin);
> + gem_context_destroy(fd, ctx);
> +}
> +
> static void preservation(int fd,
> const struct intel_execution_engine2 *e,
> unsigned int flags)
> @@ -558,7 +584,7 @@ static void preservation(int fd,
> igt_spin_batch_free(fd, spin);
>
> if (flags & RESET)
> - igt_force_gpu_reset(fd);
> + inject_reset_context(fd, engine);
>
> switch (flags & SLEEP_MASK) {
> case NOSLEEP:
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
@ 2018-03-27 15:41 ` Tvrtko Ursulin
0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-03-27 15:41 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
[resend for typo in cc]
On 27/03/2018 14:48, Chris Wilson wrote:
> If we inject a reset into the target context, there is a risk that the
> register state is never saved back to memory. The exact interaction
> between reset, the context image and the precise timing of our execution
> are not well defined. Since we cannot ensure that the context image
> remains valid, force a context switch prior to the reset.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105270
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105457
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105545
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> tests/gem_ctx_isolation.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
> index d8109aa0..4968e367 100644
> --- a/tests/gem_ctx_isolation.c
> +++ b/tests/gem_ctx_isolation.c
> @@ -522,6 +522,32 @@ static void isolation(int fd,
> #define S4 (4 << 8)
> #define SLEEP_MASK (0xf << 8)
>
> +static void inject_reset_context(int fd, unsigned int engine)
> +{
> + igt_spin_t *spin;
> + uint32_t ctx;
> +
> + /*
> + * Force a context switch before triggering the reset, or else
> + * we risk corrupting the target context and we can't blame the
> + * HW for screwing up if the context was already broken.
> + */
> +
> + ctx = gem_context_create(fd);
> + if (gem_can_store_dword(fd, engine)) {
> + spin = __igt_spin_batch_new_poll(fd, ctx, engine);
> + igt_spin_busywait_until_running(spin);
> + } else {
> + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> + usleep(1000); /* better than nothing */
> + }
> +
> + igt_force_gpu_reset(fd);
> +
> + igt_spin_batch_free(fd, spin);
> + gem_context_destroy(fd, ctx);
> +}
> +
> static void preservation(int fd,
> const struct intel_execution_engine2 *e,
> unsigned int flags)
> @@ -558,7 +584,7 @@ static void preservation(int fd,
> igt_spin_batch_free(fd, spin);
>
> if (flags & RESET)
> - igt_force_gpu_reset(fd);
> + inject_reset_context(fd, engine);
>
> switch (flags & SLEEP_MASK) {
> case NOSLEEP:
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 15:41 ` Tvrtko Ursulin
@ 2018-03-27 15:58 ` Chris Wilson
-1 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2018-03-27 15:58 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev
Quoting Tvrtko Ursulin (2018-03-27 16:41:14)
>
> [resend for typo in cc]
The small advantage in redundancy, I guess.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:48 [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context Chris Wilson
` (2 preceding siblings ...)
2018-03-27 15:41 ` Tvrtko Ursulin
@ 2018-03-27 16:32 ` Patchwork
2018-03-27 21:47 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-03-27 16:32 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gem_ctx_isolation: Reset a scratch context
URL : https://patchwork.freedesktop.org/series/40740/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
a9741da52ad1963f7632ef1e852cbe1c3bcc601e tests/perf_pmu: Improve accuracy by waiting on spinner to start
with latest DRM-Tip kernel build CI_DRM_3989
ff7820832182 drm-tip: 2018y-03m-27d-14h-31m-00s UTC integration manifest
No testlist changes.
---- Known issues:
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (fi-cfl-s3) fdo#100368
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-cnl-y3) fdo#104951
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:435s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:382s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:546s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:300s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:516s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:515s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:528s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:515s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s
fi-cfl-s3 total:285 pass:258 dwarn:0 dfail:0 fail:1 skip:26 time:552s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:586s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:429s
fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:321s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:538s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:405s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:424s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:475s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:429s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:477s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:471s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:519s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:659s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:534s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:506s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:514s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:430s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:449s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:586s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:405s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:535s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:489s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1196/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* [igt-dev] ✓ Fi.CI.IGT: success for igt/gem_ctx_isolation: Reset a scratch context
2018-03-27 13:48 [igt-dev] [PATCH igt] igt/gem_ctx_isolation: Reset a scratch context Chris Wilson
` (3 preceding siblings ...)
2018-03-27 16:32 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-27 21:47 ` Patchwork
4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-03-27 21:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gem_ctx_isolation: Reset a scratch context
URL : https://patchwork.freedesktop.org/series/40740/
State : success
== Summary ==
---- Known issues:
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-atomic:
fail -> PASS (shard-hsw) fdo#102670
Test kms_flip:
Subgroup 2x-flip-vs-wf_vblank:
fail -> PASS (shard-hsw) fdo#100368 +2
Test kms_rotation_crc:
Subgroup sprite-rotation-180:
pass -> FAIL (shard-snb) fdo#103925
Test kms_sysfs_edid_timing:
pass -> WARN (shard-apl) fdo#100047
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
shard-apl total:3495 pass:1831 dwarn:1 dfail:0 fail:7 skip:1655 time:12938s
shard-hsw total:3495 pass:1781 dwarn:1 dfail:0 fail:3 skip:1709 time:11630s
shard-snb total:3495 pass:1373 dwarn:1 dfail:0 fail:4 skip:2117 time:6987s
Blacklisted hosts:
shard-kbl total:3495 pass:1955 dwarn:1 dfail:1 fail:10 skip:1528 time:9700s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1196/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread