[Intel-gfx] [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands

Michel Thierry michel.thierry at intel.com
Mon Oct 30 21:08:39 UTC 2017


On 30/10/17 13:58, Chris Wilson wrote:
> Quoting Michel Thierry (2017-10-30 18:56:15)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index af745749509c..02fb35744f66 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1984,10 +1984,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
>>                  goto out;
>>          }
>>   
>> -       ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
>> +       if (!engine->i915->guc.execbuf_client)
>> +               ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
>> +       else
>> +               ret = intel_guc_reset_engine(engine);
> 
> While redundant, the interface cries out for
> 	intel_guc_reset_engine(guc, engine);
> even though, in this case we would be using
> 	intel_guc_reset_engine(&engine->i915->guc, engine);
> 

I'll change the name and pass the guc as parameter.

> I would also be tempted to change intel_gpu_reset to
> 	intel_gpu_reset_engine(engine->i915, engine);
> 
> intel_gt_reset_engine?
> 
And then it becomes a wrapper of intel_gpu_reset? (intel_gpu_reset is 
used for full reset path and in older platforms too).

> Just trying to make our language consistent along each path.
> 

It's true that it would look better as you suggest:

if (!guc_client)
	intel_gt_reset_engine(i915, engine);
else
	intel_guc_reset_engine(guc, engine);

Thanks,

-Michel


More information about the Intel-gfx mailing list