Re: [PATCH] drm/i915/uc: Fini hw even if GuC is not running

[Fernando Pacheco <fernando.pacheco@intel.com>]

On 8/13/19 1:18 PM, Michal Wajdeczko wrote:
> On Tue, 13 Aug 2019 18:26:28 +0200, Fernando Pacheco <fernando.pacheco@intel.com> wrote:
>
>> We should not be skipping uc_fini_hw on finding GuC
>> is no longer running. There is plenty of hw and internal
>> state that can be cleaned up without having to communicate
>> with GuC.
>>
>> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index 0dc2b0cf4604..c698cddc14dc 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -521,7 +521,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
>>  {
>>      struct intel_guc *guc = &uc->guc;
>> -    if (!intel_guc_is_running(guc))
>> +    if (!intel_uc_supports_guc(uc))
>
> there is a huge difference between is_running vs supports_guc
> and choosing supports_guc is optimist approach as we can fail
> to fetch guc fw and abort early, so maybe
>
>     if (!intel_uc_fw_is_available(&guc->fw))

This is the better check. Thanks!

>
> would be closer to reality (assuming we don't fail on wopcm
> (hmm, maybe we should force fw state to FAIL in such case?)

That would make sense to me. The fail on wopcm directly
affects the state of the fw because we abort the upload
as a result.

Thanks,
Fernando

>
>
>>          return;
>>     if (intel_uc_supports_guc_submission(uc))

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx