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