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

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

 



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

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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