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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 09/21/2017 12:09 PM, Sagar Arun Kamble wrote:


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
         */

I have the impression that this comment was here as a future design guide for Direct Submission. It seems to be the case for almost all the "XXX" comments in that file, like:

    /* XXX: wait for any interrupts */
    /* XXX: wait for workqueue to drain */

Since Direct Submission is not being considered at the moment, maybe we can just drop of all them (or transform them into something more innocuous than a "XXX").
What do people think?

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux