Re: [PATCH v8 6/8] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend

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

On 9/26/2017 7:27 PM, Michał Winiarski wrote:
> On Tue, Sep 26, 2017 at 02:49:48PM +0100, Chris Wilson wrote:
>> Quoting Sagar Arun Kamble (2017-09-26 14:24:43)
>>> With this patch we disable GuC submission in i915_drm_suspend path.
>>> This will destroy the client which will be setup back again. We also
>>> reuse the complete sanitization done via intel_uc_runtime_suspend in
>>> this path. Post i915_drm_resume, this state is recreated by
>>> intel_uc_init_hw hence we need not have similar reuse for intel_uc_resume.
>>> This also fixes issue where intel_uc_fini_hw was being called after GPU
>>> reset happening in i915_gem_suspend during i915_driver_unload.
>>>
>>> v2: Rebase w.r.t removal of GuC code restructuring. Added struct_mutex
>>> protection for i915_guc_submission_disable.
>>>
>>> v3: Rebase w.r.t updated GuC suspend function name.
>>>
>>> v4: Added lockdep assert in i915_guc_submission_enable/disable.
>>> Refined intel_uc_suspend to remove unnecessary locals and simplify
>>> return. (Michal Winiarski)
>>> Removed comment in guc_client_free about ignoring failure for
>>> destroy_doorbell. (Oscar)
>>> Rebase w.r.t i915_modparams change.
>>>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>>> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++++---
>>>   drivers/gpu/drm/i915/intel_uc.c            | 11 +++++++----
>>>   2 files changed, 19 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
>>> index d1d6c0d..0c56765 100644
>>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>>> @@ -885,9 +885,6 @@ static void guc_client_free(struct i915_guc_client *client)
>>>           * Be sure to drop any locks
>>>           */
>>>   
>>> -       /* 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 */
>>>          destroy_doorbell(client);
>>>          guc_stage_desc_fini(client->guc, client);
>>>          i915_gem_object_unpin_map(client->vma->obj);
>>> @@ -1154,6 +1151,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>>>                       sizeof(struct guc_wq_item) *
>>>                       I915_NUM_ENGINES > GUC_WQ_SIZE);
>>>   
>>> +       /*
>>> +        * Assert that drm.struct_mutex is held.
>> Ahem.
>>
>>> +        * Needed for GuC client vma binding.
>>> +        */
>>> +       lockdep_assert_held(&dev_priv->drm.struct_mutex);
>> If you don't directly depend on struct_mutex, don't assert it. Otherwise
>> the person who removes that requirement will get very confused and upset.
>> -Chris
> My bad - I suggested that.
> Failed to notice *why* we're taking the mutex, and that we're already have
> lockdep assert in the right place (__i915_gem_object_release_unless_active).
>
> Sorry :)
>
> -Michał
Will remove this. Thanks.
>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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