Re: [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers

["Michal Wajdeczko" <michal.wajdeczko@xxxxxxxxx>]

On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble  
<sagar.a.kamble@xxxxxxxxx> wrote:

> Prepared generic helpers intel_uc_suspend, intel_uc_resume,
> intel_uc_runtime_suspend, intel_uc_runtime_resume. Added
> error handling to all the calls for suspend/resume.
>
> v2: Rebase w.r.t removal of GuC code restructuring.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_gem.c |  7 ++++++-
>  drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h |  4 ++++
>  4 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 5c111ea..8635f40 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  	}
>  	mutex_unlock(&dev->struct_mutex);
> -	intel_guc_resume(dev_priv);
> +	/*
> +	 * NB: Full gem reinitialization is being done above, hence
> +	 * intel_uc_resume will be of no use. Currently intel_uc_resume
> +	 * is nop. If full reinitialization is removed, will  need to put
> +	 * functionality to resume from sleep in intel_uc_resume.
> +	 */
> +	ret = intel_uc_resume(dev_priv);
> +	if (ret)
> +		DRM_ERROR("failed to resume uc\n");
> 	intel_modeset_init_hw(dev);
> @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device  
> *kdev)
>  	 */
>  	i915_gem_runtime_suspend(dev_priv);
> -	intel_guc_suspend(dev_priv);
> +	ret = intel_uc_runtime_suspend(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", ret);
> +		enable_rpm_wakeref_asserts(dev_priv);
> +		return ret;
> +	}
> 	intel_runtime_pm_disable_interrupts(dev_priv);
> @@ -2578,7 +2591,11 @@ 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);
> +	ret = intel_uc_runtime_resume(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("uc runtime resume failed (%d)\n", ret);
> +		return ret;
> +	}
> 	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 c4bf348..dd56d45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4539,7 +4539,11 @@ 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);
> +	ret = intel_uc_suspend(dev_priv);

Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ?

Michal

> +	if (ret) {
> +		DRM_ERROR("uc suspend failed (%d)\n", ret);
> +		goto out;
> +	}
> 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
>  	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
> @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private  
> *dev_priv)
> err_unlock:
>  	mutex_unlock(&dev->struct_mutex);
> +out:
>  	intel_runtime_pm_put(dev_priv);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 0178ba4..8e4d8b0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc  
> *guc)
> 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
> +
> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_suspend(dev_priv);
> +}
> +
> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_resume(dev_priv);
> +}
> +
> +int intel_uc_suspend(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_suspend(dev_priv);
> +}
> +
> +int intel_uc_resume(struct drm_i915_private *dev_priv)
> +{
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 7703c9a..3d33a51 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -208,6 +208,10 @@ struct intel_huc {
>  void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
> +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);
>  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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx