| Message ID | 1506432285-14979-4-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Sagar Arun Kamble (2017-09-26 14:24:41) > Currently GPU is reset at the end of suspend via i915_gem_sanitize. > On resume, GuC will not be loaded until intel_uc_init_hw happens > during GEM resume flow but action to exit sleep wll be send to GuC > considering the FW load status. To make sure we don't invoke that > action update GuC FW load status through new helper > intel_guc_sanitize. Conditional check based on FW fetch status should > take care of driver init flow. > > v2: Rebase. > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > drivers/gpu/drm/i915/intel_uc.c | 8 ++++++++ > drivers/gpu/drm/i915/intel_uc.h | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8e6e2bd..0d9a107 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4528,6 +4528,8 @@ void i915_gem_sanitize(struct drm_i915_private *i915) > mutex_unlock(&i915->drm.struct_mutex); > } > > + intel_guc_sanitize(&i915->guc); > + > /* > * If we inherit context state from the BIOS or earlier occupants > * of the GPU, the GPU may be in an inconsistent state when we > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index b9376e4..29d43f8 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -581,3 +581,11 @@ int intel_guc_sample_forcewake(struct intel_guc *guc) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +void intel_guc_sanitize(struct intel_guc *guc) > +{ > + if (guc->fw.fetch_status == INTEL_UC_FIRMWARE_SUCCESS) > + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING; > + else > + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; No, sanitize should not be carrying old state over. We are supposed to be scrubbing hw state. (Elsewhere we detaint user state.) -Chris
On 9/26/2017 7:16 PM, Chris Wilson wrote: > Quoting Sagar Arun Kamble (2017-09-26 14:24:41) >> Currently GPU is reset at the end of suspend via i915_gem_sanitize. >> On resume, GuC will not be loaded until intel_uc_init_hw happens >> during GEM resume flow but action to exit sleep wll be send to GuC >> considering the FW load status. To make sure we don't invoke that >> action update GuC FW load status through new helper >> intel_guc_sanitize. Conditional check based on FW fetch status should >> take care of driver init flow. >> >> v2: Rebase. >> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem.c | 2 ++ >> drivers/gpu/drm/i915/intel_uc.c | 8 ++++++++ >> drivers/gpu/drm/i915/intel_uc.h | 1 + >> 3 files changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 8e6e2bd..0d9a107 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -4528,6 +4528,8 @@ void i915_gem_sanitize(struct drm_i915_private *i915) >> mutex_unlock(&i915->drm.struct_mutex); >> } >> >> + intel_guc_sanitize(&i915->guc); >> + >> /* >> * If we inherit context state from the BIOS or earlier occupants >> * of the GPU, the GPU may be in an inconsistent state when we >> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c >> index b9376e4..29d43f8 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -581,3 +581,11 @@ int intel_guc_sample_forcewake(struct intel_guc *guc) >> >> return intel_guc_send(guc, action, ARRAY_SIZE(action)); >> } >> + >> +void intel_guc_sanitize(struct intel_guc *guc) >> +{ >> + if (guc->fw.fetch_status == INTEL_UC_FIRMWARE_SUCCESS) >> + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING; >> + else >> + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; > No, sanitize should not be carrying old state over. We are supposed to > be scrubbing hw state. (Elsewhere we detaint user state.) > -Chris Will make this " guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; " unconditionally as GuC FW will not be loaded from hereon. Will not depend on fetch status.
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8e6e2bd..0d9a107 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4528,6 +4528,8 @@ void i915_gem_sanitize(struct drm_i915_private *i915) mutex_unlock(&i915->drm.struct_mutex); } + intel_guc_sanitize(&i915->guc); + /* * If we inherit context state from the BIOS or earlier occupants * of the GPU, the GPU may be in an inconsistent state when we diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index b9376e4..29d43f8 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -581,3 +581,11 @@ int intel_guc_sample_forcewake(struct intel_guc *guc) return intel_guc_send(guc, action, ARRAY_SIZE(action)); } + +void intel_guc_sanitize(struct intel_guc *guc) +{ + if (guc->fw.fetch_status == INTEL_UC_FIRMWARE_SUCCESS) + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING; + else + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; +} diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 0a79e17..6163ff9 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -213,6 +213,7 @@ struct intel_huc { int intel_uc_suspend(struct drm_i915_private *dev_priv); int intel_uc_resume(struct drm_i915_private *dev_priv); int intel_guc_sample_forcewake(struct intel_guc *guc); +void intel_guc_sanitize(struct intel_guc *guc); int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
Currently GPU is reset at the end of suspend via i915_gem_sanitize. On resume, GuC will not be loaded until intel_uc_init_hw happens during GEM resume flow but action to exit sleep wll be send to GuC considering the FW load status. To make sure we don't invoke that action update GuC FW load status through new helper intel_guc_sanitize. Conditional check based on FW fetch status should take care of driver init flow. v2: Rebase. Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/intel_uc.c | 8 ++++++++ drivers/gpu/drm/i915/intel_uc.h | 1 + 3 files changed, 11 insertions(+)