Re: [Intel-gfx] [PATCH 2/7] drm/i915/selftests: Use a sacrificial context for hang testing
Quoting Mika Kuoppala (2018-02-05 14:02:55) > Chris Wilsonwrites: > > diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c > > b/drivers/gpu/drm/i915/selftests/mock_context.c > > index bbf80d42e793..501becc47c0c 100644 > > --- a/drivers/gpu/drm/i915/selftests/mock_context.c > > +++ b/drivers/gpu/drm/i915/selftests/mock_context.c > > @@ -92,3 +92,14 @@ live_context(struct drm_i915_private *i915, struct > > drm_file *file) > > > > return i915_gem_create_context(i915, file->driver_priv); > > } > > + > > +struct i915_gem_context * > > +kernel_context(struct drm_i915_private *i915) > > kernel_context_create would be more symmetric. Already established mock_context(), live_context() hence kernel_context(). Which will comes first C++ in the kernel or the will to rename? ;) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915/selftests: Use a sacrificial context for hang testing
Chris Wilsonwrites: > Avoid injecting hangs in to the i915->kernel_context in case the GPU > reset leaves corruption in the context image in its wake (leading to > continual failures and system hangs after the selftests are ostensibly > complete). Use a sacrificial kernel_context instead. > > v2: Closing a context is tricky; export a function (for selftests) from > i915_gem_context.c to get it right. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Michel Thierry --- > drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 39 > +--- > drivers/gpu/drm/i915/selftests/mock_context.c| 11 +++ > drivers/gpu/drm/i915/selftests/mock_context.h| 3 ++ > 3 files changed, 36 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > index a4f4ff22389b..e0b662a2b8ae 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > @@ -33,6 +33,7 @@ struct hang { > struct drm_i915_private *i915; > struct drm_i915_gem_object *hws; > struct drm_i915_gem_object *obj; > + struct i915_gem_context *ctx; > u32 *seqno; > u32 *batch; > }; > @@ -45,9 +46,15 @@ static int hang_init(struct hang *h, struct > drm_i915_private *i915) > memset(h, 0, sizeof(*h)); > h->i915 = i915; > > + h->ctx = kernel_context(i915); > + if (IS_ERR(h->ctx)) > + return PTR_ERR(h->ctx); > + > h->hws = i915_gem_object_create_internal(i915, PAGE_SIZE); > - if (IS_ERR(h->hws)) > - return PTR_ERR(h->hws); > + if (IS_ERR(h->hws)) { > + err = PTR_ERR(h->hws); > + goto err_ctx; > + } > > h->obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > if (IS_ERR(h->obj)) { > @@ -79,6 +86,8 @@ static int hang_init(struct hang *h, struct > drm_i915_private *i915) > i915_gem_object_put(h->obj); > err_hws: > i915_gem_object_put(h->hws); > +err_ctx: > + kernel_context_close(h->ctx); > return err; > } > > @@ -196,9 +205,7 @@ static int emit_recurse_batch(struct hang *h, > } > > static struct drm_i915_gem_request * > -hang_create_request(struct hang *h, > - struct intel_engine_cs *engine, > - struct i915_gem_context *ctx) > +hang_create_request(struct hang *h, struct intel_engine_cs *engine) > { > struct drm_i915_gem_request *rq; > int err; > @@ -225,7 +232,7 @@ hang_create_request(struct hang *h, > h->batch = vaddr; > } > > - rq = i915_gem_request_alloc(engine, ctx); > + rq = i915_gem_request_alloc(engine, h->ctx); > if (IS_ERR(rq)) > return rq; > > @@ -306,6 +313,8 @@ static void hang_fini(struct hang *h) > i915_gem_object_unpin_map(h->hws); > i915_gem_object_put(h->hws); > > + kernel_context_close(h->ctx); > + > flush_test(h->i915, I915_WAIT_LOCKED); > } > > @@ -341,7 +350,7 @@ static int igt_hang_sanitycheck(void *arg) > if (!intel_engine_can_store_dword(engine)) > continue; > > - rq = hang_create_request(, engine, i915->kernel_context); > + rq = hang_create_request(, engine); > if (IS_ERR(rq)) { > err = PTR_ERR(rq); > pr_err("Failed to create request for %s, err=%d\n", > @@ -478,8 +487,7 @@ static int __igt_reset_engine(struct drm_i915_private > *i915, bool active) > struct drm_i915_gem_request *rq; > > mutex_lock(>drm.struct_mutex); > - rq = hang_create_request(, engine, > - i915->kernel_context); > + rq = hang_create_request(, engine); > if (IS_ERR(rq)) { > err = PTR_ERR(rq); > mutex_unlock(>drm.struct_mutex); > @@ -686,8 +694,7 @@ static int __igt_reset_engine_others(struct > drm_i915_private *i915, > struct drm_i915_gem_request *rq; > > mutex_lock(>drm.struct_mutex); > - rq = hang_create_request(, engine, > - i915->kernel_context); > + rq = hang_create_request(, engine); > if (IS_ERR(rq)) { > err = PTR_ERR(rq); > mutex_unlock(>drm.struct_mutex); > @@ -842,7 +849,7 @@ static int igt_wait_reset(void *arg) > if (err) > goto unlock; > > - rq = hang_create_request(, i915->engine[RCS],
[Intel-gfx] [PATCH 2/7] drm/i915/selftests: Use a sacrificial context for hang testing
Avoid injecting hangs in to the i915->kernel_context in case the GPU reset leaves corruption in the context image in its wake (leading to continual failures and system hangs after the selftests are ostensibly complete). Use a sacrificial kernel_context instead. v2: Closing a context is tricky; export a function (for selftests) from i915_gem_context.c to get it right. Signed-off-by: Chris WilsonCc: Mika Kuoppala Cc: Michel Thierry i915 = i915; + h->ctx = kernel_context(i915); + if (IS_ERR(h->ctx)) + return PTR_ERR(h->ctx); + h->hws = i915_gem_object_create_internal(i915, PAGE_SIZE); - if (IS_ERR(h->hws)) - return PTR_ERR(h->hws); + if (IS_ERR(h->hws)) { + err = PTR_ERR(h->hws); + goto err_ctx; + } h->obj = i915_gem_object_create_internal(i915, PAGE_SIZE); if (IS_ERR(h->obj)) { @@ -79,6 +86,8 @@ static int hang_init(struct hang *h, struct drm_i915_private *i915) i915_gem_object_put(h->obj); err_hws: i915_gem_object_put(h->hws); +err_ctx: + kernel_context_close(h->ctx); return err; } @@ -196,9 +205,7 @@ static int emit_recurse_batch(struct hang *h, } static struct drm_i915_gem_request * -hang_create_request(struct hang *h, - struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +hang_create_request(struct hang *h, struct intel_engine_cs *engine) { struct drm_i915_gem_request *rq; int err; @@ -225,7 +232,7 @@ hang_create_request(struct hang *h, h->batch = vaddr; } - rq = i915_gem_request_alloc(engine, ctx); + rq = i915_gem_request_alloc(engine, h->ctx); if (IS_ERR(rq)) return rq; @@ -306,6 +313,8 @@ static void hang_fini(struct hang *h) i915_gem_object_unpin_map(h->hws); i915_gem_object_put(h->hws); + kernel_context_close(h->ctx); + flush_test(h->i915, I915_WAIT_LOCKED); } @@ -341,7 +350,7 @@ static int igt_hang_sanitycheck(void *arg) if (!intel_engine_can_store_dword(engine)) continue; - rq = hang_create_request(, engine, i915->kernel_context); + rq = hang_create_request(, engine); if (IS_ERR(rq)) { err = PTR_ERR(rq); pr_err("Failed to create request for %s, err=%d\n", @@ -478,8 +487,7 @@ static int __igt_reset_engine(struct drm_i915_private *i915, bool active) struct drm_i915_gem_request *rq; mutex_lock(>drm.struct_mutex); - rq = hang_create_request(, engine, -i915->kernel_context); + rq = hang_create_request(, engine); if (IS_ERR(rq)) { err = PTR_ERR(rq); mutex_unlock(>drm.struct_mutex); @@ -686,8 +694,7 @@ static int __igt_reset_engine_others(struct drm_i915_private *i915, struct drm_i915_gem_request *rq; mutex_lock(>drm.struct_mutex); - rq = hang_create_request(, engine, -i915->kernel_context); + rq = hang_create_request(, engine); if (IS_ERR(rq)) { err = PTR_ERR(rq); mutex_unlock(>drm.struct_mutex); @@ -842,7 +849,7 @@ static int igt_wait_reset(void *arg) if (err) goto unlock; - rq = hang_create_request(, i915->engine[RCS], i915->kernel_context); + rq = hang_create_request(, i915->engine[RCS]); if (IS_ERR(rq)) { err = PTR_ERR(rq); goto fini; @@