[Intel-gfx] [PATCH 28/28] drm/i915: Replace hangcheck by heartbeats

[Bloomfield, Jon]

> -----Original Message-----
> From: Chris Wilson <chris at chris-wilson.co.uk>
> Sent: Monday, August 26, 2019 9:57 AM
> To: Bloomfield, Jon <jon.bloomfield at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>; Ursulin, Tvrtko
> <tvrtko.ursulin at intel.com>
> Subject: RE: [PATCH 28/28] drm/i915: Replace hangcheck by heartbeats
> 
> > Isn't engine->heartbeat now NULL in some cases?
> 
> engine->heartbeat, the worker
> vs
> engine->last_heartbeat
> 
Doh!

> Maybe,
> 
> struct intel_engine_heartbeat {
> 	work;
> 	systole;
> };
> 
> 
> > > diff --git a/drivers/gpu/drm/i915/i915_getparam.c
> > > b/drivers/gpu/drm/i915/i915_getparam.c
> > > index 5d9101376a3d..e6c351080593 100644
> > > --- a/drivers/gpu/drm/i915/i915_getparam.c
> > > +++ b/drivers/gpu/drm/i915/i915_getparam.c
> > > @@ -78,8 +78,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void
> > > *data,
> > >                       return -ENODEV;
> > >               break;
> > >       case I915_PARAM_HAS_GPU_RESET:
> > > -             value = i915_modparams.enable_hangcheck &&
> > > -                     intel_has_gpu_reset(i915);
> > > +             value = intel_has_gpu_reset(i915);
> >
> > Don't understand this tweak. We haven't really changed the essence of
> hangcheck, just improved it, so why do we change this get_param?
> 
> I deleted the modparam in earlier patches. But anticipated you would
> object...

Ok, I see. But then shouldn't we just be checking the new param for a non-zero timeout? That would then be equivalent.
Or, it seems fair to conclude that this never made sense, but then it really ought to be a separate patch to remove the association between HAS_GPU_RESET and hangcheck.

> -Chris