Re: [RFC 05/11] drm/i915: Extending i915_gem_check_wedge to check engine reset in progress

Tomas Elf
2015-06-09T11:08:40Z

On 08/06/2015 18:24, Chris Wilson wrote:
> On Mon, Jun 08, 2015 at 06:03:23PM +0100, Tomas Elf wrote:
>> @@ -4089,11 +4130,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
>> unsigned reset_counter;
>> int ret;
>>
>> - ret = i915_gem_wait_for_error(&dev_priv->gpu_error);
>> - if (ret)
>> - return ret;
>> -
>> - ret = i915_gem_check_wedge(&dev_priv->gpu_error, false);
>> + ret = i915_gem_wait_for_error(dev_priv);
>> if (ret)
>> return ret;
>>
>> @@ -4112,9 +4149,17 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
>> if (target == NULL)
>> return 0;
>>
>> - ret = __i915_wait_request(target, reset_counter, true, NULL, NULL);
>> - if (ret == 0)
>> - queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
>> + if (target->ring) {
>> + if (i915_gem_check_wedge(dev_priv, NULL, false))
>> + return -EIO;
>> +
>> + ret = __i915_wait_request(target, reset_counter, true, NULL,
>> + NULL);
>> +
>> + if (ret == 0)
>> + queue_delayed_work(dev_priv->wq,
>> + &dev_priv->mm.retire_work, 0);
>> + }
>
> This breaks an important bit of ABI. throttle() is used to detect when
> the GPU is hung, even when the client is idle (i.e. when the client
> starts up or is switched to).
> -Chris
>

Yeah, this is pretty silly, we've actually phased out this change from
GMIN but somehow it survived in my local tree. There should be no
changes to i915_gem_ring_throttle().

Good catch!

Thanks,
Tomas