[Intel-gfx] [PATCH v10 9/9] drm/i915/guc: Fix GuC cleanup in unload path

[Sagar Arun Kamble]

On 9/28/2017 6:45 PM, Sagar Arun Kamble wrote:
>
>
> On 9/28/2017 5:25 PM, Chris Wilson wrote:
>> Quoting Sagar Arun Kamble (2017-09-27 10:30:39)
>>> -void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>>> +void intel_uc_cleanup(struct drm_i915_private *dev_priv)
>>>   {
>>>          guc_free_load_err_log(&dev_priv->guc);
>>>            if (!i915_modparams.enable_guc_loading)
>>>                  return;
>>>   -       guc_disable_communication(&dev_priv->guc);
>>> -
>>> -       if (i915_modparams.enable_guc_submission) {
>>> -               gen9_disable_guc_interrupts(dev_priv);
>>> -               i915_guc_submission_fini(dev_priv);
>>> -       }
>>> -
>>> -       i915_ggtt_disable_guc(dev_priv);
>>> +       if (i915_modparams.enable_guc_submission)
>>> +               i915_guc_submission_cleanup(dev_priv);
>> My preference would be for if (!guc->stage_desc_pool) return; inside
>> i915_guc_submission_cleanup().
>> -Chris
> Yes. I have taken similar input in the latest patch - 
> https://patchwork.freedesktop.org/patch/179405/
> In i915_guc_submission_cleanup we can cover destroy of stage_ids and 
> stage_desc_pool based on check you have suggested.
>  guc_ads_destroy is always required data so should we link with 
> stage_desc_pool check?
> intel_guc_log is optional so destroy need to be made dependent on 
> guc->log.vma
Realized that vma need not be checked so the check as you suggested 
looks more subtle here.