[Intel-gfx] [PATCH v6 14/20] drm/i915/guc: Add support for reset engine using GuC commands

[Michel Thierry]

On 19/04/17 03:27, Chris Wilson wrote:
> On Tue, Apr 18, 2017 at 01:23:29PM -0700, Michel Thierry wrote:
>> This patch adds per engine reset and recovery (TDR) support when GuC is
>> used to submit workloads to GPU.
>>
>> In the case of i915 directly submission to ELSP, driver manages hang
>> detection, recovery and resubmission. With GuC submission these tasks
>> are shared between driver and GuC. i915 is still responsible for detecting
>> a hang, and when it does it only requests GuC to reset that Engine. GuC
>> internally manages acquiring forcewake and idling the engine before actually
>> resetting it.
>>
>> Once the reset is successful, i915 takes over again and handles resubmission.
>> The scheduler in i915 knows which requests are pending so after resetting
>> a engine, pending workloads/requests are resubmitted again.
>>
>> v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
>> non-guc funtion names.
>>
>> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
>> Signed-off-by: Jeff McGee <jeff.mcgee at intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>> ---
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 7df278fe492e..6295760098a1 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1176,14 +1176,15 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
>>
>>  	/* After a GPU reset, we may have requests to replay */
>>  	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>> -	if (!i915.enable_guc_submission && !execlists_elsp_idle(engine)) {
>> +	if (!execlists_elsp_idle(engine)) {
>>  		DRM_DEBUG_DRIVER("Restarting %s from requests [0x%x, 0x%x]\n",
>>  				 engine->name,
>>  				 port_seqno(&engine->execlist_port[0]),
>>  				 port_seqno(&engine->execlist_port[1]));
>>  		engine->execlist_port[0].count = 0;
>>  		engine->execlist_port[1].count = 0;
>> -		execlists_submit_ports(engine);
>> +		if (!dev_priv->guc.execbuf_client)
>> +			execlists_submit_ports(engine);
>
> Not sure what you were intending to do here as this only resets the
> submission count -- which is not used by guc dequeue. Some merit in the
> making the code look similar, certainly adds the dbg message but I think
> it is unrelated to the rest of the patch.

Yes, it only keeps the same debug message (originally added to check it 
was taking the right path). I can remove if you think it doesn't provide 
anything useful.