Re: [PATCH v8 03/20] drm/i915: Modify error handler for per engine hang recovery

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Michel Thierry <michel.thierry@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v8 03/20] drm/i915: Modify error handler for per engine hang recovery
Date: Tue, 06 Jun 2017 11:16:24 +0100	[thread overview]
Message-ID: <149674418469.14666.13666326723893798280@mail.alporthouse.com> (raw)
In-Reply-To: <01ed00eb-0662-1419-e1b4-46e232aecd6e@intel.com>

Quoting Michel Thierry (2017-06-06 01:40:31)
> On 6/4/2017 5:06 AM, Chris Wilson wrote:
> > And whilst I'm here, we need to extend I915_PARAM_HAS_GPU_RESET to
> > indicate having per-engine resets for the complimentary set of igt.
> > -Chris
> > 
> 
> Something like this?
> 
>          case I915_PARAM_HAS_GPU_RESET:
> -               value = i915.enable_hangcheck && 
> intel_has_gpu_reset(dev_priv);
> +               value = i915.enable_hangcheck;
> +               if (value)
> +                       value = intel_has_reset_engine(dev_priv) ? 2 :
> +                               intel_has_gpu_reset(dev_priv) ? 1 : 0;
>                  break;
> 
> (you'll probably think of a nicer way to do it)

I didn't think it was sensible to advertise reset-engine support without
global reset (or the hangcheck to detect the error), and for the time
being we can keep thinking of this as an integer rather than a set of
flags.

So I was just thinking of

value = i915.enable_hangcheck && intel_has_gpu_reset(dev_priv);
if (value && intel_has_reset_engine(dev_priv))
	value = 2;

If you want to propose breaking it into flags

value = !!i915.enable_hangcheck; /* can't remember if this is bool */
if (intel_has_gpu_reset(dev_priv))
	value |= BIT(1);
if (intel_has_reset_engine(dev_priv))
	value |= BIT(2);

Then we need to teach igt to look at the flags.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx