| Message ID | 1507712056-25030-14-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 11 Oct 2017 10:54:08 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > GuC/HuC resume operation depends on whether firmwares are available > in the WOPCM region. This is known through register WOPCM_SIZE BIT(0). > > If it indicates WOPCM is locked (bit is set) we just need to send action > to GuC to resume and enable other related GuC functionality such as > communication, interrupts and submission. > > If it indicates WOPCM is not locked then we need to first reload the > GuC/HuC and then do all resume tasks. Currently on resume from sleep, > GuC/HuC are not loaded as GPU is reset at the end of suspend/early > resume. > So we will have to reload the firmware and send action to resume. > Resume will be done through uc_init_hw from gem_init_hw based on newly > introduced state "guc->suspended". During gem_init_hw firmware load will > be skipped based on resume status during intel_uc_resume. > > Also updated the accesses to dev_priv->guc and dev_priv->huc structure by > reusing initial declared pointer. > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_guc_reg.h | 1 + > drivers/gpu/drm/i915/intel_guc.c | 2 + > drivers/gpu/drm/i915/intel_guc.h | 3 ++ > drivers/gpu/drm/i915/intel_uc.c | 99 > +++++++++++++++++++++++++++++-------- > 4 files changed, 85 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h > b/drivers/gpu/drm/i915/i915_guc_reg.h > index 35cf991..532296b 100644 > --- a/drivers/gpu/drm/i915/i915_guc_reg.h > +++ b/drivers/gpu/drm/i915/i915_guc_reg.h > @@ -75,6 +75,7 @@ > /* Defines WOPCM space available to GuC firmware */ > #define GUC_WOPCM_SIZE _MMIO(0xc050) > +#define GUC_WOPCM_LOCKED BIT(0) > /* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */ > #define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */ > #define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */ > diff --git a/drivers/gpu/drm/i915/intel_guc.c > b/drivers/gpu/drm/i915/intel_guc.c > index 55a0158..73be382 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -65,6 +65,8 @@ void intel_guc_init_early(struct intel_guc *guc) > mutex_init(&guc->send_mutex); > guc->send = intel_guc_send_nop; > guc->notify = gen8_guc_raise_irq; > + guc->suspended = false; > + guc->skip_load_on_resume = false; > } > int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len) > diff --git a/drivers/gpu/drm/i915/intel_guc.h > b/drivers/gpu/drm/i915/intel_guc.h > index a587210..9f84033 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -38,6 +38,9 @@ struct intel_guc { > struct intel_guc_log log; > struct intel_guc_ct ct; > + bool suspended; > + bool skip_load_on_resume; maybe bool xxx:1 to save space. > + > /* Log snapshot if GuC errors during load */ > struct drm_i915_gem_object *load_err_log; > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 1365724..f641872 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -152,14 +152,35 @@ static void guc_disable_communication(struct > intel_guc *guc) > guc->send = intel_guc_send_nop; > } > +static inline bool guc_wopcm_locked(struct intel_guc *guc) > +{ > + struct drm_i915_private *dev_priv = guc_to_i915(guc); > + > + return I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_LOCKED; what about adding !! trick? > +} > + > int intel_uc_init_hw(struct drm_i915_private *dev_priv) > { > struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > int ret, attempts; > if (!i915_modparams.enable_guc_loading) > return 0; > + /* > + * If on resume from sleep GuC was available we resumed GuC during > + * i915_gem_resume. We need to skip load here. Reset > skip_load_on_resume > + * to allow load during module reload/reset/next resume behavior. > + */ > + if (guc->skip_load_on_resume) { > + guc->skip_load_on_resume = false; > + return 0; > + } > + > + WARN_ON_ONCE(guc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); > + WARN_ON_ONCE(huc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); > + > guc_disable_communication(guc); > gen9_reset_guc_interrupts(dev_priv); > @@ -197,8 +218,8 @@ int intel_uc_init_hw(struct drm_i915_private > *dev_priv) > if (ret) > goto err_submission; > - intel_huc_init_hw(&dev_priv->huc); > - ret = intel_guc_init_hw(&dev_priv->guc); > + intel_huc_init_hw(huc); > + ret = intel_guc_init_hw(guc); > if (ret == 0 || ret != -EAGAIN) > break; > @@ -214,7 +235,21 @@ int intel_uc_init_hw(struct drm_i915_private > *dev_priv) > if (ret) > goto err_log_capture; > - intel_huc_auth(&dev_priv->huc); > + /* > + * If WOPCM was not locked during resume from sleep, GuC/HuC need to > + * be reloaded. For this we are using intel_uc_init_hw path as we want > + * to handle HuC/GuC reload, GuC resume, HuC authentication and > + * submission enabling etc. all here. > + */ > + if (guc->suspended) { > + ret = intel_guc_resume(guc); > + if (ret) > + DRM_ERROR("GuC resume failed (%d)." > + " GuC functions may not work\n", ret); > + guc->suspended = false; > + } > + > + intel_huc_auth(huc); > if (i915_guc_submission_initialized(guc)) { > if (i915_modparams.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > @@ -305,6 +340,8 @@ int intel_uc_suspend(struct drm_i915_private > *dev_priv) > gen9_disable_guc_interrupts(dev_priv); > guc_disable_communication(guc); > + guc->suspended = true; > + > goto out; > out_suspend: > @@ -326,28 +363,50 @@ int intel_uc_suspend(struct drm_i915_private > *dev_priv) > void intel_uc_resume(struct drm_i915_private *dev_priv) > { > struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > int ret; > - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) > - return; > - > - ret = guc_enable_communication(guc); > - if (ret) { > - DRM_DEBUG_DRIVER("GuC communication enable failed (%d)\n", ret); > + if (!guc->suspended) > return; > - } > - if (i915_modparams.guc_log_level >= 0) > - gen9_enable_guc_interrupts(dev_priv); > + /* > + * If WOPCM is locked then GuC and HuC are still loaded. We just > + * need to enable communication with GuC, enable interrupts, > + * invoke GuC action to resume from sleep and enable submission. > + * If WOPCM is not locked it is similar to fresh boot and we need > + * reload the GuC/HuC firmwares and enable other GuC related > + * mechanisms. Post reloading GuC we need to send action to resume > + * from sleep for GuC to restore its state prior to suspend. > + */ > + if (guc_wopcm_locked(guc)) { > + huc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; > + guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; Hmm, as we didn't clear load_status (or at least I can't find it) then load_status should still be INTEL_UC_FIRMWARE_SUCCESS here. Maybe instead of introducing new skip_load_on_resume flag we can just rely on load_status that is correctly maintained across suspend resume. > - ret = intel_guc_resume(guc); > - if (ret) > - DRM_ERROR("GuC resume failed (%d)." > - "GuC functions may not work\n", ret); > + ret = guc_enable_communication(guc); > + if (ret) { > + DRM_DEBUG_DRIVER("GuC communication enable failed" > + " (%d)\n", ret); Btw, the only path in guc_enable_communication that can fail reports error so maybe this extra debug is not necessary ? Maybe better option will be to reorg this code to include note about forced GuC reload during init_hw ? > + return; > + } > - i915_guc_submission_enable(dev_priv); > + if (i915_modparams.guc_log_level >= 0) > + gen9_enable_guc_interrupts(dev_priv); > - DRM_DEBUG_DRIVER("GuC submission %s\n", > - i915_guc_submission_enabled(guc) ? > - "enabled" : "disabled"); > + ret = intel_guc_resume(guc); > + if (ret) > + DRM_ERROR("GuC resume failed (%d)." > + " GuC functions may not work\n", ret); > + Hmm, is it safe to continue with submission after failing resume ? > + i915_guc_submission_enable(dev_priv); > + > + DRM_DEBUG_DRIVER("GuC submission %s\n", > + i915_guc_submission_enabled(guc) ? > + "enabled" : "disabled"); > + guc->suspended = false; > + guc->skip_load_on_resume = true; > + } else { > + DRM_DEBUG_DRIVER("GuC not available. Resume will be done" > + " during i915_gem_init_hw\n"); > + guc->skip_load_on_resume = false; > + } > }
On 10/11/2017 10:36 PM, Michal Wajdeczko wrote: > On Wed, 11 Oct 2017 10:54:08 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> GuC/HuC resume operation depends on whether firmwares are available >> in the WOPCM region. This is known through register WOPCM_SIZE BIT(0). >> >> If it indicates WOPCM is locked (bit is set) we just need to send action >> to GuC to resume and enable other related GuC functionality such as >> communication, interrupts and submission. >> >> If it indicates WOPCM is not locked then we need to first reload the >> GuC/HuC and then do all resume tasks. Currently on resume from sleep, >> GuC/HuC are not loaded as GPU is reset at the end of suspend/early >> resume. >> So we will have to reload the firmware and send action to resume. >> Resume will be done through uc_init_hw from gem_init_hw based on newly >> introduced state "guc->suspended". During gem_init_hw firmware load will >> be skipped based on resume status during intel_uc_resume. >> >> Also updated the accesses to dev_priv->guc and dev_priv->huc >> structure by >> reusing initial declared pointer. >> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_guc_reg.h | 1 + >> drivers/gpu/drm/i915/intel_guc.c | 2 + >> drivers/gpu/drm/i915/intel_guc.h | 3 ++ >> drivers/gpu/drm/i915/intel_uc.c | 99 >> +++++++++++++++++++++++++++++-------- >> 4 files changed, 85 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h >> b/drivers/gpu/drm/i915/i915_guc_reg.h >> index 35cf991..532296b 100644 >> --- a/drivers/gpu/drm/i915/i915_guc_reg.h >> +++ b/drivers/gpu/drm/i915/i915_guc_reg.h >> @@ -75,6 +75,7 @@ >> /* Defines WOPCM space available to GuC firmware */ >> #define GUC_WOPCM_SIZE _MMIO(0xc050) >> +#define GUC_WOPCM_LOCKED BIT(0) >> /* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */ >> #define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */ >> #define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */ >> diff --git a/drivers/gpu/drm/i915/intel_guc.c >> b/drivers/gpu/drm/i915/intel_guc.c >> index 55a0158..73be382 100644 >> --- a/drivers/gpu/drm/i915/intel_guc.c >> +++ b/drivers/gpu/drm/i915/intel_guc.c >> @@ -65,6 +65,8 @@ void intel_guc_init_early(struct intel_guc *guc) >> mutex_init(&guc->send_mutex); >> guc->send = intel_guc_send_nop; >> guc->notify = gen8_guc_raise_irq; >> + guc->suspended = false; >> + guc->skip_load_on_resume = false; >> } >> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 >> len) >> diff --git a/drivers/gpu/drm/i915/intel_guc.h >> b/drivers/gpu/drm/i915/intel_guc.h >> index a587210..9f84033 100644 >> --- a/drivers/gpu/drm/i915/intel_guc.h >> +++ b/drivers/gpu/drm/i915/intel_guc.h >> @@ -38,6 +38,9 @@ struct intel_guc { >> struct intel_guc_log log; >> struct intel_guc_ct ct; >> + bool suspended; >> + bool skip_load_on_resume; > > maybe bool xxx:1 to save space. Ok. Will update. > >> + >> /* Log snapshot if GuC errors during load */ >> struct drm_i915_gem_object *load_err_log; >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 1365724..f641872 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -152,14 +152,35 @@ static void guc_disable_communication(struct >> intel_guc *guc) >> guc->send = intel_guc_send_nop; >> } >> +static inline bool guc_wopcm_locked(struct intel_guc *guc) >> +{ >> + struct drm_i915_private *dev_priv = guc_to_i915(guc); >> + >> + return I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_LOCKED; > > what about adding !! trick? Sure. Will add. > >> +} >> + >> int intel_uc_init_hw(struct drm_i915_private *dev_priv) >> { >> struct intel_guc *guc = &dev_priv->guc; >> + struct intel_huc *huc = &dev_priv->huc; >> int ret, attempts; >> if (!i915_modparams.enable_guc_loading) >> return 0; >> + /* >> + * If on resume from sleep GuC was available we resumed GuC during >> + * i915_gem_resume. We need to skip load here. Reset >> skip_load_on_resume >> + * to allow load during module reload/reset/next resume behavior. >> + */ >> + if (guc->skip_load_on_resume) { >> + guc->skip_load_on_resume = false; >> + return 0; >> + } >> + >> + WARN_ON_ONCE(guc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); >> + WARN_ON_ONCE(huc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); >> + >> guc_disable_communication(guc); >> gen9_reset_guc_interrupts(dev_priv); >> @@ -197,8 +218,8 @@ int intel_uc_init_hw(struct drm_i915_private >> *dev_priv) >> if (ret) >> goto err_submission; >> - intel_huc_init_hw(&dev_priv->huc); >> - ret = intel_guc_init_hw(&dev_priv->guc); >> + intel_huc_init_hw(huc); >> + ret = intel_guc_init_hw(guc); >> if (ret == 0 || ret != -EAGAIN) >> break; >> @@ -214,7 +235,21 @@ int intel_uc_init_hw(struct drm_i915_private >> *dev_priv) >> if (ret) >> goto err_log_capture; >> - intel_huc_auth(&dev_priv->huc); >> + /* >> + * If WOPCM was not locked during resume from sleep, GuC/HuC >> need to >> + * be reloaded. For this we are using intel_uc_init_hw path as >> we want >> + * to handle HuC/GuC reload, GuC resume, HuC authentication and >> + * submission enabling etc. all here. >> + */ >> + if (guc->suspended) { >> + ret = intel_guc_resume(guc); >> + if (ret) >> + DRM_ERROR("GuC resume failed (%d)." >> + " GuC functions may not work\n", ret); >> + guc->suspended = false; >> + } >> + >> + intel_huc_auth(huc); >> if (i915_guc_submission_initialized(guc)) { >> if (i915_modparams.guc_log_level >= 0) >> gen9_enable_guc_interrupts(dev_priv); >> @@ -305,6 +340,8 @@ int intel_uc_suspend(struct drm_i915_private >> *dev_priv) >> gen9_disable_guc_interrupts(dev_priv); >> guc_disable_communication(guc); >> + guc->suspended = true; >> + >> goto out; >> out_suspend: >> @@ -326,28 +363,50 @@ int intel_uc_suspend(struct drm_i915_private >> *dev_priv) >> void intel_uc_resume(struct drm_i915_private *dev_priv) >> { >> struct intel_guc *guc = &dev_priv->guc; >> + struct intel_huc *huc = &dev_priv->huc; >> int ret; >> - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) >> - return; >> - >> - ret = guc_enable_communication(guc); >> - if (ret) { >> - DRM_DEBUG_DRIVER("GuC communication enable failed (%d)\n", >> ret); >> + if (!guc->suspended) >> return; >> - } >> - if (i915_modparams.guc_log_level >= 0) >> - gen9_enable_guc_interrupts(dev_priv); >> + /* >> + * If WOPCM is locked then GuC and HuC are still loaded. We just >> + * need to enable communication with GuC, enable interrupts, >> + * invoke GuC action to resume from sleep and enable submission. >> + * If WOPCM is not locked it is similar to fresh boot and we need >> + * reload the GuC/HuC firmwares and enable other GuC related >> + * mechanisms. Post reloading GuC we need to send action to resume >> + * from sleep for GuC to restore its state prior to suspend. >> + */ >> + if (guc_wopcm_locked(guc)) { >> + huc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; >> + guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; > > Hmm, as we didn't clear load_status (or at least I can't find it) then > load_status should still be INTEL_UC_FIRMWARE_SUCCESS here. > > Maybe instead of introducing new skip_load_on_resume flag we can > just rely on load_status that is correctly maintained across suspend > resume. load status will be cleared in the uc_sanitize and that patch was later in the sequence. Have pulled earlier in the v14. We can drop the skip_load_on_resume as you suggested. will update. > >> - ret = intel_guc_resume(guc); >> - if (ret) >> - DRM_ERROR("GuC resume failed (%d)." >> - "GuC functions may not work\n", ret); >> + ret = guc_enable_communication(guc); >> + if (ret) { >> + DRM_DEBUG_DRIVER("GuC communication enable failed" >> + " (%d)\n", ret); > > Btw, the only path in guc_enable_communication that can fail reports > error so maybe this extra debug is not necessary ? ok will remove. > > Maybe better option will be to reorg this code to include note about > forced GuC reload during init_hw ? ok. > >> + return; >> + } >> - i915_guc_submission_enable(dev_priv); >> + if (i915_modparams.guc_log_level >= 0) >> + gen9_enable_guc_interrupts(dev_priv); >> - DRM_DEBUG_DRIVER("GuC submission %s\n", >> - i915_guc_submission_enabled(guc) ? >> - "enabled" : "disabled"); >> + ret = intel_guc_resume(guc); >> + if (ret) >> + DRM_ERROR("GuC resume failed (%d)." >> + " GuC functions may not work\n", ret); >> + > > Hmm, is it safe to continue with submission after failing resume ? It may not so will revert all earlier steps and fall back to execlists. > >> + i915_guc_submission_enable(dev_priv); >> + >> + DRM_DEBUG_DRIVER("GuC submission %s\n", >> + i915_guc_submission_enabled(guc) ? >> + "enabled" : "disabled"); >> + guc->suspended = false; >> + guc->skip_load_on_resume = true; >> + } else { >> + DRM_DEBUG_DRIVER("GuC not available. Resume will be done" >> + " during i915_gem_init_hw\n"); >> + guc->skip_load_on_resume = false; >> + } >> }
diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h index 35cf991..532296b 100644 --- a/drivers/gpu/drm/i915/i915_guc_reg.h +++ b/drivers/gpu/drm/i915/i915_guc_reg.h @@ -75,6 +75,7 @@ /* Defines WOPCM space available to GuC firmware */ #define GUC_WOPCM_SIZE _MMIO(0xc050) +#define GUC_WOPCM_LOCKED BIT(0) /* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */ #define GUC_WOPCM_TOP (0x80 << 12) /* 512KB */ #define BXT_GUC_WOPCM_RC6_RESERVED (0x10 << 12) /* 64KB */ diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index 55a0158..73be382 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -65,6 +65,8 @@ void intel_guc_init_early(struct intel_guc *guc) mutex_init(&guc->send_mutex); guc->send = intel_guc_send_nop; guc->notify = gen8_guc_raise_irq; + guc->suspended = false; + guc->skip_load_on_resume = false; } int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len) diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index a587210..9f84033 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -38,6 +38,9 @@ struct intel_guc { struct intel_guc_log log; struct intel_guc_ct ct; + bool suspended; + bool skip_load_on_resume; + /* Log snapshot if GuC errors during load */ struct drm_i915_gem_object *load_err_log; diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 1365724..f641872 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -152,14 +152,35 @@ static void guc_disable_communication(struct intel_guc *guc) guc->send = intel_guc_send_nop; } +static inline bool guc_wopcm_locked(struct intel_guc *guc) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + + return I915_READ(GUC_WOPCM_SIZE) & GUC_WOPCM_LOCKED; +} + int intel_uc_init_hw(struct drm_i915_private *dev_priv) { struct intel_guc *guc = &dev_priv->guc; + struct intel_huc *huc = &dev_priv->huc; int ret, attempts; if (!i915_modparams.enable_guc_loading) return 0; + /* + * If on resume from sleep GuC was available we resumed GuC during + * i915_gem_resume. We need to skip load here. Reset skip_load_on_resume + * to allow load during module reload/reset/next resume behavior. + */ + if (guc->skip_load_on_resume) { + guc->skip_load_on_resume = false; + return 0; + } + + WARN_ON_ONCE(guc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); + WARN_ON_ONCE(huc->fw.load_status == INTEL_UC_FIRMWARE_SUCCESS); + guc_disable_communication(guc); gen9_reset_guc_interrupts(dev_priv); @@ -197,8 +218,8 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) if (ret) goto err_submission; - intel_huc_init_hw(&dev_priv->huc); - ret = intel_guc_init_hw(&dev_priv->guc); + intel_huc_init_hw(huc); + ret = intel_guc_init_hw(guc); if (ret == 0 || ret != -EAGAIN) break; @@ -214,7 +235,21 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) if (ret) goto err_log_capture; - intel_huc_auth(&dev_priv->huc); + /* + * If WOPCM was not locked during resume from sleep, GuC/HuC need to + * be reloaded. For this we are using intel_uc_init_hw path as we want + * to handle HuC/GuC reload, GuC resume, HuC authentication and + * submission enabling etc. all here. + */ + if (guc->suspended) { + ret = intel_guc_resume(guc); + if (ret) + DRM_ERROR("GuC resume failed (%d)." + " GuC functions may not work\n", ret); + guc->suspended = false; + } + + intel_huc_auth(huc); if (i915_guc_submission_initialized(guc)) { if (i915_modparams.guc_log_level >= 0) gen9_enable_guc_interrupts(dev_priv); @@ -305,6 +340,8 @@ int intel_uc_suspend(struct drm_i915_private *dev_priv) gen9_disable_guc_interrupts(dev_priv); guc_disable_communication(guc); + guc->suspended = true; + goto out; out_suspend: @@ -326,28 +363,50 @@ int intel_uc_suspend(struct drm_i915_private *dev_priv) void intel_uc_resume(struct drm_i915_private *dev_priv) { struct intel_guc *guc = &dev_priv->guc; + struct intel_huc *huc = &dev_priv->huc; int ret; - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) - return; - - ret = guc_enable_communication(guc); - if (ret) { - DRM_DEBUG_DRIVER("GuC communication enable failed (%d)\n", ret); + if (!guc->suspended) return; - } - if (i915_modparams.guc_log_level >= 0) - gen9_enable_guc_interrupts(dev_priv); + /* + * If WOPCM is locked then GuC and HuC are still loaded. We just + * need to enable communication with GuC, enable interrupts, + * invoke GuC action to resume from sleep and enable submission. + * If WOPCM is not locked it is similar to fresh boot and we need + * reload the GuC/HuC firmwares and enable other GuC related + * mechanisms. Post reloading GuC we need to send action to resume + * from sleep for GuC to restore its state prior to suspend. + */ + if (guc_wopcm_locked(guc)) { + huc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; + guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS; - ret = intel_guc_resume(guc); - if (ret) - DRM_ERROR("GuC resume failed (%d)." - "GuC functions may not work\n", ret); + ret = guc_enable_communication(guc); + if (ret) { + DRM_DEBUG_DRIVER("GuC communication enable failed" + " (%d)\n", ret); + return; + } - i915_guc_submission_enable(dev_priv); + if (i915_modparams.guc_log_level >= 0) + gen9_enable_guc_interrupts(dev_priv); - DRM_DEBUG_DRIVER("GuC submission %s\n", - i915_guc_submission_enabled(guc) ? - "enabled" : "disabled"); + ret = intel_guc_resume(guc); + if (ret) + DRM_ERROR("GuC resume failed (%d)." + " GuC functions may not work\n", ret); + + i915_guc_submission_enable(dev_priv); + + DRM_DEBUG_DRIVER("GuC submission %s\n", + i915_guc_submission_enabled(guc) ? + "enabled" : "disabled"); + guc->suspended = false; + guc->skip_load_on_resume = true; + } else { + DRM_DEBUG_DRIVER("GuC not available. Resume will be done" + " during i915_gem_init_hw\n"); + guc->skip_load_on_resume = false; + } }
GuC/HuC resume operation depends on whether firmwares are available in the WOPCM region. This is known through register WOPCM_SIZE BIT(0). If it indicates WOPCM is locked (bit is set) we just need to send action to GuC to resume and enable other related GuC functionality such as communication, interrupts and submission. If it indicates WOPCM is not locked then we need to first reload the GuC/HuC and then do all resume tasks. Currently on resume from sleep, GuC/HuC are not loaded as GPU is reset at the end of suspend/early resume. So we will have to reload the firmware and send action to resume. Resume will be done through uc_init_hw from gem_init_hw based on newly introduced state "guc->suspended". During gem_init_hw firmware load will be skipped based on resume status during intel_uc_resume. Also updated the accesses to dev_priv->guc and dev_priv->huc structure by reusing initial declared pointer. Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_guc_reg.h | 1 + drivers/gpu/drm/i915/intel_guc.c | 2 + drivers/gpu/drm/i915/intel_guc.h | 3 ++ drivers/gpu/drm/i915/intel_uc.c | 99 +++++++++++++++++++++++++++++-------- 4 files changed, 85 insertions(+), 20 deletions(-)