| Message ID | 1506055758-25442-3-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 22 Sep 2017 06:49:13 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > Apart from configuring interrupts, we need to update the ggtt invalidate > interface and GuC communication on suspend. This functionality can be > reused for other suspend and reset paths. > > v2: Rebase w.r.t removal of GuC code restructuring. > > v3: Removed GuC specific helpers as tasks other than send H2G for > sleep/resume are to be done from uc generic functions. (Michal) > > 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_guc_submission.c | 5 ---- > drivers/gpu/drm/i915/intel_uc.c | 39 > ++++++++++++++++++++++++++++-- > 2 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > b/drivers/gpu/drm/i915/i915_guc_submission.c > index e191d56..cd5ef8b 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private > *dev_priv) > if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) > return 0; > - gen9_disable_guc_interrupts(dev_priv); > - > ctx = dev_priv->kernel_context; > data[0] = INTEL_GUC_ACTION_ENTER_S_STATE; > @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private > *dev_priv) > if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) > return 0; > - if (i915.guc_log_level >= 0) > - gen9_enable_guc_interrupts(dev_priv); > - > ctx = dev_priv->kernel_context; > data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 8e4d8b0..66d2cff 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc > *guc) > int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) > { > - return intel_guc_suspend(dev_priv); > + int ret; > + > + if (!i915.enable_guc_loading) > + return 0; > + > + ret = intel_guc_suspend(dev_priv); > + if (ret) { > + DRM_ERROR("GuC runtime suspend failed (%d)\n", ret); > + return ret; > + } > + > + i915_ggtt_disable_guc(dev_priv); > + gen9_disable_guc_interrupts(dev_priv); > + guc_disable_communication(&dev_priv->guc); > + > + return 0; > } > int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) > { > - return intel_guc_resume(dev_priv); > + int ret; > + > + if (!i915.enable_guc_loading) > + return 0; > + > + ret = guc_enable_communication(&dev_priv->guc); > + if (ret) { > + DRM_ERROR("GuC enable communication failed (%d)\n", ret); Not sure if we need extra message here, as Guc CT has its own error messages > + return ret; > + } > + if (i915.guc_log_level >= 0) > + gen9_enable_guc_interrupts(dev_priv); > + i915_ggtt_enable_guc(dev_priv); > + > + ret = intel_guc_resume(dev_priv); > + if (ret) { > + DRM_ERROR("GuC runtime resume failed (%d)\n", ret); > + return ret; > + } > + > + return 0; Maybe better would be to have single error message: out: if (ret) DRM_ERROR("Failed to resume uC runtime (%d)\n", ret); return ret; > } > int intel_uc_suspend(struct drm_i915_private *dev_priv)
On 9/22/2017 1:08 PM, Michal Wajdeczko wrote: > On Fri, 22 Sep 2017 06:49:13 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> Apart from configuring interrupts, we need to update the ggtt invalidate >> interface and GuC communication on suspend. This functionality can be >> reused for other suspend and reset paths. >> >> v2: Rebase w.r.t removal of GuC code restructuring. >> >> v3: Removed GuC specific helpers as tasks other than send H2G for >> sleep/resume are to be done from uc generic functions. (Michal) >> >> 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_guc_submission.c | 5 ---- >> drivers/gpu/drm/i915/intel_uc.c | 39 >> ++++++++++++++++++++++++++++-- >> 2 files changed, 37 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c >> b/drivers/gpu/drm/i915/i915_guc_submission.c >> index e191d56..cd5ef8b 100644 >> --- a/drivers/gpu/drm/i915/i915_guc_submission.c >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c >> @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private >> *dev_priv) >> if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) >> return 0; >> - gen9_disable_guc_interrupts(dev_priv); >> - >> ctx = dev_priv->kernel_context; >> data[0] = INTEL_GUC_ACTION_ENTER_S_STATE; >> @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private >> *dev_priv) >> if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) >> return 0; >> - if (i915.guc_log_level >= 0) >> - gen9_enable_guc_interrupts(dev_priv); >> - >> ctx = dev_priv->kernel_context; >> data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 8e4d8b0..66d2cff 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc >> *guc) >> int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) >> { >> - return intel_guc_suspend(dev_priv); >> + int ret; >> + >> + if (!i915.enable_guc_loading) >> + return 0; >> + >> + ret = intel_guc_suspend(dev_priv); >> + if (ret) { >> + DRM_ERROR("GuC runtime suspend failed (%d)\n", ret); >> + return ret; >> + } >> + >> + i915_ggtt_disable_guc(dev_priv); >> + gen9_disable_guc_interrupts(dev_priv); >> + guc_disable_communication(&dev_priv->guc); >> + >> + return 0; >> } >> int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) >> { >> - return intel_guc_resume(dev_priv); >> + int ret; >> + >> + if (!i915.enable_guc_loading) >> + return 0; >> + >> + ret = guc_enable_communication(&dev_priv->guc); >> + if (ret) { >> + DRM_ERROR("GuC enable communication failed (%d)\n", ret); > > Not sure if we need extra message here, as Guc CT has its own error > messages Ok. Will remove these. > >> + return ret; >> + } >> + if (i915.guc_log_level >= 0) >> + gen9_enable_guc_interrupts(dev_priv); >> + i915_ggtt_enable_guc(dev_priv); >> + >> + ret = intel_guc_resume(dev_priv); >> + if (ret) { >> + DRM_ERROR("GuC runtime resume failed (%d)\n", ret); >> + return ret; >> + } >> + >> + return 0; > > Maybe better would be to have single error message: > > out: > if (ret) > DRM_ERROR("Failed to resume uC runtime (%d)\n", ret); > return ret; Will update like this. Thanks. > >> } >> int intel_uc_suspend(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index e191d56..cd5ef8b 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) return 0; - gen9_disable_guc_interrupts(dev_priv); - ctx = dev_priv->kernel_context; data[0] = INTEL_GUC_ACTION_ENTER_S_STATE; @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) return 0; - if (i915.guc_log_level >= 0) - gen9_enable_guc_interrupts(dev_priv); - ctx = dev_priv->kernel_context; data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 8e4d8b0..66d2cff 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc *guc) int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) { - return intel_guc_suspend(dev_priv); + int ret; + + if (!i915.enable_guc_loading) + return 0; + + ret = intel_guc_suspend(dev_priv); + if (ret) { + DRM_ERROR("GuC runtime suspend failed (%d)\n", ret); + return ret; + } + + i915_ggtt_disable_guc(dev_priv); + gen9_disable_guc_interrupts(dev_priv); + guc_disable_communication(&dev_priv->guc); + + return 0; } int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) { - return intel_guc_resume(dev_priv); + int ret; + + if (!i915.enable_guc_loading) + return 0; + + ret = guc_enable_communication(&dev_priv->guc); + if (ret) { + DRM_ERROR("GuC enable communication failed (%d)\n", ret); + return ret; + } + if (i915.guc_log_level >= 0) + gen9_enable_guc_interrupts(dev_priv); + i915_ggtt_enable_guc(dev_priv); + + ret = intel_guc_resume(dev_priv); + if (ret) { + DRM_ERROR("GuC runtime resume failed (%d)\n", ret); + return ret; + } + + return 0; } int intel_uc_suspend(struct drm_i915_private *dev_priv)
Apart from configuring interrupts, we need to update the ggtt invalidate interface and GuC communication on suspend. This functionality can be reused for other suspend and reset paths. v2: Rebase w.r.t removal of GuC code restructuring. v3: Removed GuC specific helpers as tasks other than send H2G for sleep/resume are to be done from uc generic functions. (Michal) 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_guc_submission.c | 5 ---- drivers/gpu/drm/i915/intel_uc.c | 39 ++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 7 deletions(-)