[Intel-gfx] [PATCH v2 6/6] drm/i915/uc: do not free err log on uc_fini

[Daniele Ceraolo Spurio]

On 3/25/20 10:58 AM, John Harrison wrote:
> On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
>> we do call uc_fini if there is an issue while loading the GuC, so we
>> can't delete in there the logs we need to debug the load failure.
>> Moving the log free to driver remove ensures the logs stick around ong
>> enough for us to dump them.
> I think this could be worded better and has a couple of typos.
> 

How about:

"We need to keep the GuC error logs around to debug the load failure, so 
we can't clean them in the error unwind, which includes uc_fini(). 
Moving the cleanup to driver remove ensures that the logs stick around 
long enough for us to dump them."

?

Daniele

> Otherwise it looks plausible.
> Reviewed-by: John Harrison <John.C.Harrison at Intel.com>
> 
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt.c    | 3 +--
>>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9 +++++++--
>>   drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 +
>>   3 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>> index 3dea8881e915..eda66b0d44bd 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -635,8 +635,7 @@ void intel_gt_driver_remove(struct intel_gt *gt)
>>   {
>>       __intel_gt_disable(gt);
>> -    intel_uc_fini_hw(&gt->uc);
>> -    intel_uc_fini(&gt->uc);
>> +    intel_uc_driver_remove(&gt->uc);
>>       intel_engines_release(gt);
>>   }
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index a4cbe06e06bd..b11e564ef22e 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -131,6 +131,13 @@ static void __uc_free_load_err_log(struct 
>> intel_uc *uc)
>>           i915_gem_object_put(log);
>>   }
>> +void intel_uc_driver_remove(struct intel_uc *uc)
>> +{
>> +    intel_uc_fini_hw(uc);
>> +    intel_uc_fini(uc);
>> +    __uc_free_load_err_log(uc);
>> +}
>> +
>>   static inline bool guc_communication_enabled(struct intel_guc *guc)
>>   {
>>       return intel_guc_ct_enabled(&guc->ct);
>> @@ -311,8 +318,6 @@ static void __uc_fini(struct intel_uc *uc)
>>   {
>>       intel_huc_fini(&uc->huc);
>>       intel_guc_fini(&uc->guc);
>> -
>> -    __uc_free_load_err_log(uc);
>>   }
>>   static int __uc_sanitize(struct intel_uc *uc)
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
>> index 5ae7b50b7dc1..9c954c589edf 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
>> @@ -34,6 +34,7 @@ struct intel_uc {
>>   void intel_uc_init_early(struct intel_uc *uc);
>>   void intel_uc_driver_late_release(struct intel_uc *uc);
>> +void intel_uc_driver_remove(struct intel_uc *uc);
>>   void intel_uc_init_mmio(struct intel_uc *uc);
>>   void intel_uc_reset_prepare(struct intel_uc *uc);
>>   void intel_uc_suspend(struct intel_uc *uc);
>