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

[Chris Wilson]

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 at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at 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