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

[John Harrison <John.C.Harrison@xxxxxxxxx>]

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.

Otherwise it looks plausible.
Reviewed-by: John Harrison <John.C.Harrison@xxxxxxxxx>

> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
>   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);

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