Re: [Intel-gfx] [PATCH 5/6] drm/i915/gt: Discard stale context state from across idling
Quoting Mika Kuoppala (2019-12-30 16:30:21)
> Chris Wilson writes:
>
> > Before we idle, on parking, we switch to the kernel context such that we
> > have a scratch context loaded while the GPU idle, protecting any
> > precious user state. Be paranoid and assume that the idle state may have
> > been trashed, and reset the kernel_context image after idling.
> >
> > Signed-off-by: Chris Wilson
> > Cc: Imre Deak
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 6 ++
> > drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8
> > drivers/gpu/drm/i915/gt/mock_engine.c | 5 +
> > 3 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > index cd82f0baef49..1b9f73948f22 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > @@ -20,6 +20,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
> > {
> > struct intel_engine_cs *engine =
> > container_of(wf, typeof(*engine), wakeref);
> > + struct intel_context *ce;
> > void *map;
> >
> > ENGINE_TRACE(engine, "\n");
> > @@ -34,6 +35,11 @@ static int __engine_unpark(struct intel_wakeref *wf)
> > if (!IS_ERR_OR_NULL(map))
> > engine->pinned_default_state = map;
> >
> > + /* Discard stale context state from across idling */
> > + ce = engine->kernel_context;
> > + if (ce)
> > + ce->ops->reset(ce);
> > +
>
> Expect the worst, sure.
> Checksum would get us datapoints tho.
Inject yet-another request to the kernel_context on parking to force a
context switch [to itself] and so ensure the image is saved to HW. Wait
for parking, compute the CRC. Then on unpark compute the CRC again...
The design is all based around that we don't trust the HW and try to
avoid caring about what's in the scratch kernel_context, so other than
validating HW... Not a terrible idea, but we can probably cook up
something more targeted if we thought about it.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915/gt: Discard stale context state from across idling
Chris Wilson writes:
> Before we idle, on parking, we switch to the kernel context such that we
> have a scratch context loaded while the GPU idle, protecting any
> precious user state. Be paranoid and assume that the idle state may have
> been trashed, and reset the kernel_context image after idling.
>
> Signed-off-by: Chris Wilson
> Cc: Imre Deak
> ---
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 6 ++
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8
> drivers/gpu/drm/i915/gt/mock_engine.c | 5 +
> 3 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index cd82f0baef49..1b9f73948f22 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -20,6 +20,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
> {
> struct intel_engine_cs *engine =
> container_of(wf, typeof(*engine), wakeref);
> + struct intel_context *ce;
> void *map;
>
> ENGINE_TRACE(engine, "\n");
> @@ -34,6 +35,11 @@ static int __engine_unpark(struct intel_wakeref *wf)
> if (!IS_ERR_OR_NULL(map))
> engine->pinned_default_state = map;
>
> + /* Discard stale context state from across idling */
> + ce = engine->kernel_context;
> + if (ce)
> + ce->ops->reset(ce);
> +
Expect the worst, sure.
Checksum would get us datapoints tho.
Reviewed-by: Mika Kuoppala
> if (engine->unpark)
> engine->unpark(engine);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 9b220c930ebc..d1c2f034296a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -213,16 +213,8 @@ int intel_gt_resume(struct intel_gt *gt)
> intel_llc_enable(>llc);
>
> for_each_engine(engine, gt, id) {
> - struct intel_context *ce;
> -
> intel_engine_pm_get(engine);
>
> - ce = engine->kernel_context;
> - if (ce) {
> - GEM_BUG_ON(!intel_context_is_pinned(ce));
> - ce->ops->reset(ce);
> - }
> -
> engine->serial++; /* kernel context lost */
> err = engine->resume(engine);
>
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c
> b/drivers/gpu/drm/i915/gt/mock_engine.c
> index 4e1eafa94be9..d0e68ce9aa51 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -152,6 +152,10 @@ static int mock_context_pin(struct intel_context *ce)
> return intel_context_active_acquire(ce);
> }
>
> +static void mock_context_reset(struct intel_context *ce)
> +{
> +}
> +
> static const struct intel_context_ops mock_context_ops = {
> .alloc = mock_context_alloc,
>
> @@ -161,6 +165,7 @@ static const struct intel_context_ops mock_context_ops = {
> .enter = intel_context_enter_engine,
> .exit = intel_context_exit_engine,
>
> + .reset = mock_context_reset,
> .destroy = mock_context_destroy,
> };
>
> --
> 2.25.0.rc0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915/gt: Discard stale context state from across idling
Quoting Chris Wilson (2019-12-30 16:01:11) > Before we idle, on parking, we switch to the kernel context such that we > have a scratch context loaded while the GPU idle, protecting any > precious user state. Be paranoid and assume that the idle state may have > been trashed, and reset the kernel_context image after idling. > > Signed-off-by: Chris Wilson > Cc: Imre Deak Reviewed-by: Matthew Auld -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915/gt: Discard stale context state from across idling
Before we idle, on parking, we switch to the kernel context such that we
have a scratch context loaded while the GPU idle, protecting any
precious user state. Be paranoid and assume that the idle state may have
been trashed, and reset the kernel_context image after idling.
Signed-off-by: Chris Wilson
Cc: Imre Deak
---
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 6 ++
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8
drivers/gpu/drm/i915/gt/mock_engine.c | 5 +
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index cd82f0baef49..1b9f73948f22 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -20,6 +20,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
{
struct intel_engine_cs *engine =
container_of(wf, typeof(*engine), wakeref);
+ struct intel_context *ce;
void *map;
ENGINE_TRACE(engine, "\n");
@@ -34,6 +35,11 @@ static int __engine_unpark(struct intel_wakeref *wf)
if (!IS_ERR_OR_NULL(map))
engine->pinned_default_state = map;
+ /* Discard stale context state from across idling */
+ ce = engine->kernel_context;
+ if (ce)
+ ce->ops->reset(ce);
+
if (engine->unpark)
engine->unpark(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 9b220c930ebc..d1c2f034296a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -213,16 +213,8 @@ int intel_gt_resume(struct intel_gt *gt)
intel_llc_enable(>llc);
for_each_engine(engine, gt, id) {
- struct intel_context *ce;
-
intel_engine_pm_get(engine);
- ce = engine->kernel_context;
- if (ce) {
- GEM_BUG_ON(!intel_context_is_pinned(ce));
- ce->ops->reset(ce);
- }
-
engine->serial++; /* kernel context lost */
err = engine->resume(engine);
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c
b/drivers/gpu/drm/i915/gt/mock_engine.c
index 4e1eafa94be9..d0e68ce9aa51 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -152,6 +152,10 @@ static int mock_context_pin(struct intel_context *ce)
return intel_context_active_acquire(ce);
}
+static void mock_context_reset(struct intel_context *ce)
+{
+}
+
static const struct intel_context_ops mock_context_ops = {
.alloc = mock_context_alloc,
@@ -161,6 +165,7 @@ static const struct intel_context_ops mock_context_ops = {
.enter = intel_context_enter_engine,
.exit = intel_context_exit_engine,
+ .reset = mock_context_reset,
.destroy = mock_context_destroy,
};
--
2.25.0.rc0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
