[Intel-gfx] [PATCH v3 1/1] drm/i915: Sharing Gfx Clock, Wake and Gunit save/restore logic using common handler for runtime/system s/r paths
Daniel Vetter
daniel at ffwll.ch
Tue Aug 12 14:00:41 CEST 2014
On Tue, Aug 12, 2014 at 04:21:55PM +0530, sagar.a.kamble at intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble at intel.com>
>
> v1:
> Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of gunit registers need to be followed in
> PM suspend and resume path similar to runtime suspend and resume.
>
> v2:
> 1. Keeping GT access, wake, gunit save/restore related helpers static.
> 2. Moved GT access check, Wake Control, Gunit state save to end of i915_drm_freeze.
> 3. Reusing the sequence in runtime_suspend/resume path at macro level.
>
> v3:
> 1. Prepared common handlers for platform specific tasks to be done before HW suspend and after HW resume from
> D0i3.
> 2. Changed commit header.
>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Goel, Akash <akash.goel at intel.com>
> Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2
> Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>
Yeah, this looks roughly like what I'd expected, thanks.
I think there's some room now to consolidate hw deinit/init code further
between system s/r and rpm paths, but at least we've started. Please find
one of the rpm domain experts to review this so I can merge it.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 130 ++++++++++++++++++++++++++--------------
> 1 file changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ec96f9a..4440722 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -494,6 +494,11 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
> return true;
> }
>
> +
> +static int pre_hw_suspend_deinit(struct drm_i915_private *dev_priv);
> +static int post_hw_resume_init(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend);
> +
> static int i915_drm_freeze(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -614,15 +619,16 @@ void intel_console_resume(struct work_struct *work)
> static int i915_drm_thaw_early(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret = 0;
>
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - hsw_disable_pc8(dev_priv);
> + /* Restore any platform specific registers/clk state */
> + ret = post_hw_resume_init(dev_priv, false);
>
> intel_uncore_early_sanitize(dev, true);
> intel_uncore_sanitize(dev);
> intel_power_domains_init_hw(dev_priv);
>
> - return 0;
> + return ret;
> }
>
> static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> @@ -908,6 +914,7 @@ static int i915_pm_suspend_late(struct device *dev)
> struct pci_dev *pdev = to_pci_dev(dev);
> struct drm_device *drm_dev = pci_get_drvdata(pdev);
> struct drm_i915_private *dev_priv = drm_dev->dev_private;
> + int ret = 0;
>
> /*
> * We have a suspedn ordering issue with the snd-hda driver also
> @@ -921,13 +928,13 @@ static int i915_pm_suspend_late(struct device *dev)
> if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> return 0;
>
> - if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
> - hsw_enable_pc8(dev_priv);
> + /* Save any platform specific registers/clk state needed post resume */
> + ret = pre_hw_suspend_deinit(dev_priv);
>
> pci_disable_device(pdev);
> pci_set_power_state(pdev, PCI_D3hot);
>
> - return 0;
> + return ret;
> }
>
> static int i915_pm_resume_early(struct device *dev)
> @@ -983,23 +990,26 @@ static int i915_pm_poweroff(struct device *dev)
> return i915_drm_freeze(drm_dev);
> }
>
> -static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int hsw_suspend(struct drm_i915_private *dev_priv)
> {
> hsw_enable_pc8(dev_priv);
>
> return 0;
> }
>
> -static int snb_runtime_resume(struct drm_i915_private *dev_priv)
> +static int snb_resume(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> {
> struct drm_device *dev = dev_priv->dev;
>
> - intel_init_pch_refclk(dev);
> + if (resume_from_rpm_suspend)
> + intel_init_pch_refclk(dev);
>
> return 0;
> }
>
> -static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
> +static int hsw_resume(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> {
> hsw_disable_pc8(dev_priv);
>
> @@ -1295,10 +1305,10 @@ static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
> I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
> }
>
> -static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int vlv_suspend(struct drm_i915_private *dev_priv)
> {
> u32 mask;
> - int err;
> + int ret = 0;
>
> /*
> * Bspec defines the following GT well on flags as debug only, so
> @@ -1311,20 +1321,19 @@ static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
>
> vlv_check_no_gt_access(dev_priv);
>
> - err = vlv_force_gfx_clock(dev_priv, true);
> - if (err)
> + ret = vlv_force_gfx_clock(dev_priv, true);
> + if (ret)
> goto err1;
>
> - err = vlv_allow_gt_wake(dev_priv, false);
> - if (err)
> + ret = vlv_allow_gt_wake(dev_priv, false);
> + if (ret)
> goto err2;
> vlv_save_gunit_s0ix_state(dev_priv);
>
> - err = vlv_force_gfx_clock(dev_priv, false);
> - if (err)
> + ret = vlv_force_gfx_clock(dev_priv, false);
> + if (ret)
> goto err2;
> -
> - return 0;
> + return ret;
>
> err2:
> /* For safety always re-enable waking and disable gfx clock forcing */
> @@ -1332,14 +1341,15 @@ err2:
> err1:
> vlv_force_gfx_clock(dev_priv, false);
>
> - return err;
> + return ret;
> }
>
> -static int vlv_runtime_resume(struct drm_i915_private *dev_priv)
> +static int vlv_resume(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> {
> struct drm_device *dev = dev_priv->dev;
> int err;
> - int ret;
> + int ret = 0;
>
> /*
> * If any of the steps fail just try to continue, that's the best we
> @@ -1360,8 +1370,10 @@ static int vlv_runtime_resume(struct drm_i915_private *dev_priv)
>
> vlv_check_no_gt_access(dev_priv);
>
> - intel_init_clock_gating(dev);
> - i915_gem_restore_fences(dev);
> + if (resume_from_rpm_suspend) {
> + intel_init_clock_gating(dev);
> + i915_gem_restore_fences(dev);
> + }
>
> return ret;
> }
> @@ -1413,16 +1425,8 @@ static int intel_runtime_suspend(struct device *device)
> cancel_work_sync(&dev_priv->rps.work);
> intel_runtime_pm_disable_interrupts(dev);
>
> - if (IS_GEN6(dev)) {
> - ret = 0;
> - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> - ret = hsw_runtime_suspend(dev_priv);
> - } else if (IS_VALLEYVIEW(dev)) {
> - ret = vlv_runtime_suspend(dev_priv);
> - } else {
> - ret = -ENODEV;
> - WARN_ON(1);
> - }
> + /* Save any platform specific registers/clk state needed post resume */
> + ret = pre_hw_suspend_deinit(dev_priv);
>
> if (ret) {
> DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
> @@ -1461,16 +1465,8 @@ static int intel_runtime_resume(struct device *device)
> intel_opregion_notify_adapter(dev, PCI_D0);
> dev_priv->pm.suspended = false;
>
> - if (IS_GEN6(dev)) {
> - ret = snb_runtime_resume(dev_priv);
> - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> - ret = hsw_runtime_resume(dev_priv);
> - } else if (IS_VALLEYVIEW(dev)) {
> - ret = vlv_runtime_resume(dev_priv);
> - } else {
> - WARN_ON(1);
> - ret = -ENODEV;
> - }
> + /* Restore any platform specific registers/clk state */
> + ret = post_hw_resume_init(dev_priv, true);
>
> /*
> * No point of rolling back things in case of an error, as the best
> @@ -1490,6 +1486,50 @@ static int intel_runtime_resume(struct device *device)
> return ret;
> }
>
> +/* This handler is used in system/runtime suspend path to reuse
> + * Gfx clock, Wake control, Gunit state save related functionaility for VLV.
> + */
> +static int pre_hw_suspend_deinit(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + int ret = 0;
> +
> + if (IS_GEN6(dev)) {
> + ret = 0;
> + } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> + ret = hsw_suspend(dev_priv);
> + } else if (IS_VALLEYVIEW(dev)) {
> + ret = vlv_suspend(dev_priv);
> + } else {
> + ret = -ENODEV;
> + WARN_ON(1);
> + }
> +
> + return ret;
> +}
> +
> +/* This handler is used in system/runtime resume path to reuse
> + * Gfx clock, Wake control, Gunit state restore related functionaility for VLV.
> + */
> +static int post_hw_resume_init(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + int ret = 0;
> +
> + if (IS_GEN6(dev)) {
> + ret = snb_resume(dev_priv, resume_from_rpm_suspend);
> + } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> + ret = hsw_resume(dev_priv, resume_from_rpm_suspend);
> + } else if (IS_VALLEYVIEW(dev)) {
> + ret = vlv_resume(dev_priv, resume_from_rpm_suspend);
> + } else {
> + WARN_ON(1);
> + ret = -ENODEV;
> + }
> + return ret;
> +}
> +
> static const struct dev_pm_ops i915_pm_ops = {
> .suspend = i915_pm_suspend,
> .suspend_late = i915_pm_suspend_late,
> --
> 1.8.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list