| Message ID | 1394441062-6601-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 10, 2014 at 09:44:22AM +0100, Daniel Vetter wrote: > It can happen ... > > Fix up the check to match pre-gen6 reality where we don't have hw > contexts and hence also don't need to set the reset status on them. > > This blows up when running any gpu reset testcase since for pre-gen6 > request->ctx is NULL. With this my ilk here is happy again. > > This regression has been introduced in > > commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e > Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Date: Thu Jan 30 16:01:15 2014 +0200 > > drm/i915: Use i915_hw_context to set reset stats > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Did you try playing around with setting last_context to private_default_context? That is more in line with the original outlined approach of "every platform has a context, be they fake, or real." > --- > drivers/gpu/drm/i915/i915_gem.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 92b0b4164b1d..25cc3f4f242e 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2291,8 +2291,10 @@ static void i915_set_reset_status(struct drm_i915_private *dev_priv, > { > struct i915_ctx_hang_stats *hs; > > - if (WARN_ON(!ctx)) > + if (!ctx) { > + WARN_ON(HAS_HW_CONTEXTS(dev_priv->dev)); > return; > + } > > hs = &ctx->hang_stats; > > -- > 1.8.1.4 >
On Mon, Mar 10, 2014 at 7:30 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > On Mon, Mar 10, 2014 at 09:44:22AM +0100, Daniel Vetter wrote: >> It can happen ... >> >> Fix up the check to match pre-gen6 reality where we don't have hw >> contexts and hence also don't need to set the reset status on them. >> >> This blows up when running any gpu reset testcase since for pre-gen6 >> request->ctx is NULL. With this my ilk here is happy again. >> >> This regression has been introduced in >> >> commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e >> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> Date: Thu Jan 30 16:01:15 2014 +0200 >> >> drm/i915: Use i915_hw_context to set reset stats >> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> Cc: Ben Widawsky <ben@bwidawsk.net> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Did you try playing around with setting last_context to > private_default_context? That is more in line with the original outlined > approach of "every platform has a context, be they fake, or real." Nope. Currently the tests for that are a bit busted, so I think we should do that when we fix things up generally. Mika seems to be working on this. For now this keeps my machines happy. There's also the problem that our QA seems to have missed this ... -Daniel
On Mon, Mar 10, 2014 at 09:30:22PM +0100, Daniel Vetter wrote: > On Mon, Mar 10, 2014 at 7:30 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Mon, Mar 10, 2014 at 09:44:22AM +0100, Daniel Vetter wrote: > >> It can happen ... > >> > >> Fix up the check to match pre-gen6 reality where we don't have hw > >> contexts and hence also don't need to set the reset status on them. > >> > >> This blows up when running any gpu reset testcase since for pre-gen6 > >> request->ctx is NULL. With this my ilk here is happy again. > >> > >> This regression has been introduced in > >> > >> commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e > >> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> Date: Thu Jan 30 16:01:15 2014 +0200 > >> > >> drm/i915: Use i915_hw_context to set reset stats > >> > >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> Cc: Ben Widawsky <ben@bwidawsk.net> > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Did you try playing around with setting last_context to > > private_default_context? That is more in line with the original outlined > > approach of "every platform has a context, be they fake, or real." > > Nope. Currently the tests for that are a bit busted, so I think we > should do that when we fix things up generally. Mika seems to be > working on this. For now this keeps my machines happy. There's also > the problem that our QA seems to have missed this ... > -Daniel As long as you've thought about it, lgtm, although I've not investigated if other areas of the code need it. Without last_context always != NULL, I wouldn't be surprised if you hit something similar elsewhere. Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 92b0b4164b1d..25cc3f4f242e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2291,8 +2291,10 @@ static void i915_set_reset_status(struct drm_i915_private *dev_priv, { struct i915_ctx_hang_stats *hs; - if (WARN_ON(!ctx)) + if (!ctx) { + WARN_ON(HAS_HW_CONTEXTS(dev_priv->dev)); return; + } hs = &ctx->hang_stats;
It can happen ... Fix up the check to match pre-gen6 reality where we don't have hw contexts and hence also don't need to set the reset status on them. This blows up when running any gpu reset testcase since for pre-gen6 request->ctx is NULL. With this my ilk here is happy again. This regression has been introduced in commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> Date: Thu Jan 30 16:01:15 2014 +0200 drm/i915: Use i915_hw_context to set reset stats Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)