[Intel-gfx] [PATCH 2/4] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Sep 7 17:33:59 UTC 2017
On Fri, Sep 01, 2017 at 11:02:10AM +0530, Sagar Arun Kamble wrote:
> Tearing down of guc_ggtt_invalidate/guc_interrupts/guc_communication
> setup should happen towards end of reset/suspend as these are
> setup back again during recovery/resume.
>
> Prepared helpers intel_guc_pause and intel_guc_unpause that will do
> teardown/bringup of this setup along with suspension/resumption of GuC if
> loaded. Moved intel_guc_suspend, intel_guc_resume to intel_guc.c.
Please try to limit number of changes in single patch.
Also describe idea behind system/runtime functions.
>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 +-
> drivers/gpu/drm/i915/i915_gem.c | 6 +-
> drivers/gpu/drm/i915/i915_guc_submission.c | 52 ----------
> drivers/gpu/drm/i915/intel_guc.c | 152 +++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc.h | 9 +-
> drivers/gpu/drm/i915/intel_uc.c | 29 +-----
> 6 files changed, 169 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2ae730c..b2e8f95 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1690,8 +1690,6 @@ static int i915_drm_resume(struct drm_device *dev)
> }
> mutex_unlock(&dev->struct_mutex);
>
> - intel_guc_resume(dev_priv);
> -
> intel_modeset_init_hw(dev);
>
> spin_lock_irq(&dev_priv->irq_lock);
> @@ -2486,7 +2484,7 @@ static int intel_runtime_suspend(struct device *kdev)
> */
> i915_gem_runtime_suspend(dev_priv);
>
> - intel_guc_suspend(dev_priv);
> + intel_guc_runtime_suspend(&dev_priv->guc);
>
> intel_runtime_pm_disable_interrupts(dev_priv);
>
> @@ -2571,7 +2569,7 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - intel_guc_resume(dev_priv);
> + intel_guc_runtime_resume(&dev_priv->guc);
>
> if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e4cc08b..977500f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2846,6 +2846,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
>
> i915_gem_revoke_fences(dev_priv);
>
> + intel_guc_reset_prepare(&dev_priv->guc);
> +
> return err;
> }
>
> @@ -4574,8 +4576,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
> i915_gem_contexts_lost(dev_priv);
> mutex_unlock(&dev->struct_mutex);
>
> - intel_guc_suspend(dev_priv);
> -
> cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
> cancel_delayed_work_sync(&dev_priv->gt.retire_work);
>
> @@ -4592,6 +4592,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
> if (WARN_ON(!intel_engines_are_idle(dev_priv)))
> i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
>
> + intel_guc_system_suspend(&dev_priv->guc);
> +
> /*
> * Neither the BIOS, ourselves or any other kernel
> * expects the system to be in execlists mode on startup,
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 602ae8a..2f977ab 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1287,55 +1287,3 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
> guc_client_free(guc->execbuf_client);
> guc->execbuf_client = NULL;
> }
> -
> -/**
> - * intel_guc_suspend() - notify GuC entering suspend state
> - * @dev_priv: i915 device private
> - */
> -int intel_guc_suspend(struct drm_i915_private *dev_priv)
> -{
> - struct intel_guc *guc = &dev_priv->guc;
> - struct i915_gem_context *ctx;
> - u32 data[3];
> -
> - 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;
> - /* any value greater than GUC_POWER_D0 */
> - data[1] = GUC_POWER_D1;
> - /* first page is shared data with GuC */
> - data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
> -
> - return intel_guc_send(guc, data, ARRAY_SIZE(data));
> -}
> -
> -/**
> - * intel_guc_resume() - notify GuC resuming from suspend state
> - * @dev_priv: i915 device private
> - */
> -int intel_guc_resume(struct drm_i915_private *dev_priv)
> -{
> - struct intel_guc *guc = &dev_priv->guc;
> - struct i915_gem_context *ctx;
> - u32 data[3];
> -
> - 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;
> - data[1] = GUC_POWER_D0;
> - /* first page is shared data with GuC */
> - data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
> -
> - return intel_guc_send(guc, data, ARRAY_SIZE(data));
> -}
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 978a0e3..1fd8599 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -191,3 +191,155 @@ void intel_guc_auth_huc(struct intel_guc *guc, struct intel_uc_fw *huc_fw)
> out:
> i915_vma_unpin(vma);
> }
> +
> +int intel_guc_enable_communication(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> + if (HAS_GUC_CT(dev_priv))
> + return intel_guc_enable_ct(guc);
> +
> + guc->send = intel_guc_send_mmio;
> + return 0;
> +}
> +
> +void intel_guc_disable_communication(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> + if (HAS_GUC_CT(dev_priv))
> + intel_guc_disable_ct(guc);
> +
> + guc->send = intel_guc_send_nop;
> +}
> +
> +/**
> + * intel_guc_suspend() - notify GuC entering suspend state
> + */
> +static int intel_guc_suspend(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct i915_gem_context *ctx;
> + u32 data[3];
> +
> + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> + return 0;
> +
> + ctx = dev_priv->kernel_context;
> +
> + data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
> + /* any value greater than GUC_POWER_D0 */
> + data[1] = GUC_POWER_D1;
> + /* first page is shared data with GuC */
> + data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
> +
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> +}
> +
> +/**
> + * intel_guc_resume() - notify GuC resuming from suspend state
> + */
> +static int intel_guc_resume(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct i915_gem_context *ctx;
> + u32 data[3];
> +
> + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> + return 0;
> +
> + ctx = dev_priv->kernel_context;
> +
> + data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
> + data[1] = GUC_POWER_D0;
> + /* first page is shared data with GuC */
> + data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
> +
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> +}
> +
> +static void intel_guc_sanitize(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> + i915_ggtt_disable_guc(dev_priv);
> + intel_guc_disable_communication(guc);
> + gen9_disable_guc_interrupts(dev_priv);
> +}
> +
> +void intel_guc_reset_prepare(struct intel_guc *guc)
> +{
> + if (!i915.enable_guc_loading)
> + return;
> +
> + intel_guc_sanitize(guc);
> + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
> +}
> +
> +static int intel_guc_pause(struct intel_guc *guc)
> +{
> + int ret = 0;
> +
> + ret = intel_guc_suspend(guc);
> + intel_guc_sanitize(guc);
> +
> + return ret;
> +}
> +
> +static int intel_guc_unpause(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + int ret = 0;
> +
> + if (i915.guc_log_level >= 0)
> + gen9_enable_guc_interrupts(dev_priv);
> + intel_guc_enable_communication(guc);
> + i915_ggtt_enable_guc(dev_priv);
> + ret = intel_guc_resume(guc);
> +
> + return ret;
> +}
> +
> +int intel_guc_runtime_suspend(struct intel_guc *guc)
> +{
> + if (!i915.enable_guc_loading)
> + return 0;
> +
> + return intel_guc_pause(guc);
> +}
> +
> +int intel_guc_runtime_resume(struct intel_guc *guc)
> +{
> + if (!i915.enable_guc_loading)
> + return 0;
> +
> + return intel_guc_unpause(guc);
> +}
> +
> +int intel_guc_system_suspend(struct intel_guc *guc)
> +{
> + int ret = 0;
> +
> + if (!i915.enable_guc_loading)
> + return ret;
> +
> + ret = intel_guc_pause(guc);
> + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
> +
> + return ret;
> +}
> +
> +int intel_guc_system_resume(struct intel_guc *guc)
> +{
> + int ret = 0;
> +
> + if (!i915.enable_guc_loading)
> + return ret;
> +
> + /*
> + * Placeholder for GuC resume from system suspend/freeze states.
> + * Currently full reinitialization of GEM and GuC happens along
> + * these paths, Hence this function is doing nothing.
> + */
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index b329830..bf4dda0 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -162,6 +162,13 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> 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);
> void intel_guc_auth_huc(struct intel_guc *guc, struct intel_uc_fw *huc_fw);
> +int intel_guc_enable_communication(struct intel_guc *guc);
> +void intel_guc_disable_communication(struct intel_guc *guc);
> +void intel_guc_reset_prepare(struct intel_guc *guc);
> +int intel_guc_runtime_suspend(struct intel_guc *guc);
> +int intel_guc_runtime_resume(struct intel_guc *guc);
> +int intel_guc_system_suspend(struct intel_guc *guc);
> +int intel_guc_system_resume(struct intel_guc *guc);
>
> static inline int intel_guc_send(struct intel_guc *guc, const u32 *action,
> u32 len)
> @@ -177,8 +184,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
> /* intel_guc_loader.c */
> int intel_guc_select_fw(struct intel_guc *guc);
> int intel_guc_init_hw(struct intel_guc *guc);
> -int intel_guc_suspend(struct drm_i915_private *dev_priv);
> -int intel_guc_resume(struct drm_i915_private *dev_priv);
> u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>
> /* i915_guc_submission.c */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index a3fc4c8..30c004c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -265,27 +265,6 @@ static void guc_free_load_err_log(struct intel_guc *guc)
> i915_gem_object_put(guc->load_err_log);
> }
>
> -static int guc_enable_communication(struct intel_guc *guc)
> -{
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -
> - if (HAS_GUC_CT(dev_priv))
> - return intel_guc_enable_ct(guc);
> -
> - guc->send = intel_guc_send_mmio;
> - return 0;
> -}
> -
> -static void guc_disable_communication(struct intel_guc *guc)
> -{
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -
> - if (HAS_GUC_CT(dev_priv))
> - intel_guc_disable_ct(guc);
> -
> - guc->send = intel_guc_send_nop;
> -}
Why above functions are moved in this patch ?
> -
> int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> @@ -295,7 +274,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> if (!i915.enable_guc_loading)
> return 0;
>
> - guc_disable_communication(guc);
> + intel_guc_disable_communication(guc);
> gen9_reset_guc_interrupts(dev_priv);
>
> /* We need to notify the guc whenever we change the GGTT */
> @@ -347,7 +326,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>
> intel_guc_init_send_regs(guc);
>
> - ret = guc_enable_communication(guc);
> + ret = intel_guc_enable_communication(guc);
> if (ret)
> goto err_log_capture;
>
> @@ -373,7 +352,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> * marks the GPU as wedged until reset).
> */
> err_interrupts:
> - guc_disable_communication(guc);
> + intel_guc_disable_communication(guc);
> gen9_disable_guc_interrupts(dev_priv);
> err_log_capture:
> guc_capture_load_err_log(guc);
> @@ -410,7 +389,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> if (i915.enable_guc_submission)
> i915_guc_submission_disable(dev_priv);
>
> - guc_disable_communication(&dev_priv->guc);
> + intel_guc_disable_communication(&dev_priv->guc);
>
> if (i915.enable_guc_submission) {
> gen9_disable_guc_interrupts(dev_priv);
> --
> 1.9.1
>
More information about the Intel-gfx
mailing list