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.

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

Reply via email to