Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2017-11-03 14:03:25)
>> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>>
>> > In the next few patches, we will have a hard requirement that we emit a
>> > context-switch to the perma-pinned i915->kernel_context (so that we can
>> > save the HW state using that context-switch). As the first context
>> > itself may be classed as a kernel context, we want to be explicit in our
>> > comparison. For an extra-layer of finesse, we can check the last
>> > unretired context on the engine; as well as the last retired context
>> > when idle.
>> >
>> > v2: verbose verbosity
>> > v3: Always force the switch, even when the engine is idle, and update
>> > the assert that this happens before suspend.
>> >
>> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
>> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> #v1
>> > ---
>> > drivers/gpu/drm/i915/i915_gem.c | 10 ++++++----
>> > drivers/gpu/drm/i915/intel_engine_cs.c | 26 ++++++++++++++++++++++++--
>> > 2 files changed, 30 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> > index 9470ba0c1930..fe312ab3b664 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > @@ -4687,14 +4687,16 @@ void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj)
>> > i915_gem_object_put(obj);
>> > }
>> >
>> > -static void assert_kernel_context_is_current(struct drm_i915_private *dev_priv)
>> > +static void assert_kernel_context_is_current(struct drm_i915_private *i915)
>> > {
>> > + struct i915_gem_context *kernel_context = i915->kernel_context;
>> > struct intel_engine_cs *engine;
>> > enum intel_engine_id id;
>> >
>> > - for_each_engine(engine, dev_priv, id)
>> > - GEM_BUG_ON(engine->last_retired_context &&
>> > - !i915_gem_context_is_kernel(engine->last_retired_context));
>> > + for_each_engine(engine, i915, id) {
>> > + GEM_BUG_ON(__i915_gem_active_peek(&engine->timeline->last_request));
>>
>> It could be that we have a active request for kernel context and we
>> still bail out with assert. Why?
>
> Because we waited for the system to be idle, so we expect the timeline
> to have been completely retired, which implies that
> timline->last_request is now NULL.
Ok, the callsite did reveal this. And I did reveal being lazy for not
looking.
assert_idle_on_kernel_context()?
Regardless,
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
