Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

On 9/22/2017 12:03 AM, Oscar Mateo wrote:
> <SNIP>
>
>
> On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
>> Teardown of GuC HW/SW state was not properly done in unload path.
>> guc_submission_disable was called as part of intel_uc_fini_hw which
>> happens post gem_unload in the i915_driver_unload path.
>> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
>> function does cleanup.
>> To differentiate the tasks during suspend and unload w.r.t GuC this
>> patch introduces new function i915_gem_fini which in addition to
>> disabling GuC interfaces also disables GuC submission during which
>> communication with GuC is needed for destroying doorbell.
>> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
>> GuC operations. To achieve this, new helpers i915_gem_context_suspend
>> and i915_gem_suspend_complete are prepared.
>>
>
> Sagar, I just realized this patch would make this comment in 
> guc_client_free superfluous, right?:
>
>     /* FIXME: in many cases, by the time we get here the GuC has been
>      * reset, so we cannot destroy the doorbell properly. Ignore the
>      * error message for now */
>
> Can you make sure you remove it in this patch?
>
> Thanks!
Yes. Will need to remove this comment :)
Will make this change as part of 
https://patchwork.freedesktop.org/patch/178189/
How about another comment before it? i915_gem_suspend is ensuring no 
outstanding submissions before coming to
uc_suspend. So shall we remove that as well?

         /*
          * XXX: wait for any outstanding submissions before freeing memory.
          * Be sure to drop any locks
          */



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