| Message ID | 1428678710-11462-1-git-send-email-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2015-04-10 at 20:41 +0530, Animesh Manna wrote: > From: Suketu Shah <suketu.j.shah@intel.com> > > Add triggers for DC6 as per details provided in skl_enable_dc6 > and skl_disable_dc6 implementations. > > Also Call POSTING_READ for every write to a register to ensure > it is written to immediately > > v1: Remove POSTING_READ and intel_prepare_ddi calls as they've been added in previous patches. > > v2: > 1] Remove check for backlight disabled as it should be the case by that time. > 2] Mark DC5 as disabled when enabling DC6. > 3] Return from DC5-disabling function early if DC5 is already be disabled which can happen > due to DC6-enabling earlier. > 3] Ensure CSR firmware is loaded after resume from DC6 as corresponding memory contents won't > be retained after runtime-suspend. > 4] Ensure that CSR isn't identified as loaded before CSR-loading program is called during > runtime-resume. > > v3: Rebase to latest > Modified as per review comments from Imre and after discussion with Art: > 1] DC6 should be preferably enabled when PG2 is disabled by SW as the check for PG1 being > disabled is taken of by HW to enter DC6, and disabled when PG2 is enabled respectively. > This helps save more power, especially in the case when display is disabled but GT is > enabled. Accordingly, replacing DC5 trigger sequence with DC6 for SKL. > 2] DC6 could be enabled from intel_runtime_suspend() function, if DC5 is already enabled. > 3] Move CSR-load-status setting code from intel_runtime_suspend function to a new function. > > v4: > 1] Enable/disable DC6 only when toggling the power-well using a newly defined macro ENABLE_DC6. > > v5: > 1] Load CSR on system resume too as firmware may be lost on system suspend preventing > enabling DC5, DC6. > 2] DDI buffers shouldn't be programmed during driver-load/resume as it's already done > during modeset initialization then and also that the encoder list is still uninitialized by > then. Therefore, call intel_prepare_ddi function right after disabling DC6 but outside > skl_disable_dc6 function and not during driver-load/resume. > > v6: > 1] Rebase to latest. > 2] Move SKL_ENABLE_DC6 macro definition from intel_display.c to intel_runtime_pm.c. > > v7: > 1) Refactored the code for removing the warning got from checkpatch. > 2) After adding dmc ver 1.0 support rebased on top of nightly. (Animesh) > > v8: > - Reverted the changes done in v7. > - Removed the condition check in skl_prepare_resune(). (Animesh) > > Issue: VIZ-2819 > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com> > Signed-off-by: Suketu Shah <suketu.j.shah@intel.com> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 30 +++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 40 ++++++++++++++++++++++----------- > 2 files changed, 57 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index acd0e2b..7a1123c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -594,6 +594,8 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv) > static int intel_suspend_complete(struct drm_i915_private *dev_priv); > static int vlv_resume_prepare(struct drm_i915_private *dev_priv, > bool rpm_resume); > +static int skl_resume_prepare(struct drm_i915_private *dev_priv); > + > > static int i915_drm_suspend(struct drm_device *dev) > { > @@ -808,6 +810,8 @@ static int i915_drm_resume_early(struct drm_device *dev) > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > hsw_disable_pc8(dev_priv); > + else if (IS_SKYLAKE(dev_priv)) > + ret = skl_resume_prepare(dev_priv); > > intel_uncore_sanitize(dev); > intel_power_domains_init_hw(dev_priv); > @@ -1022,6 +1026,19 @@ static int i915_pm_resume(struct device *dev) > return i915_drm_resume(drm_dev); > } > > +static int skl_suspend_complete(struct drm_i915_private *dev_priv) > +{ > + /* Enabling DC6 is not a hard requirement to enter runtime D3 */ > + > + /* > + * This is to ensure that CSR isn't identified as loaded before > + * CSR-loading program is called during runtime-resume. > + */ > + intel_csr_load_status_set(dev_priv, false); This sets the wrong state now, should be something like intel_csr_state_set(dev_priv, FW_UNINITIALIZED); > + > + return 0; > +} > + > static int hsw_suspend_complete(struct drm_i915_private *dev_priv) > { > hsw_enable_pc8(dev_priv); > @@ -1029,6 +1046,15 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv) > return 0; > } > > +static int skl_resume_prepare(struct drm_i915_private *dev_priv) > +{ > + struct drm_device *dev = dev_priv->dev; > + > + intel_csr_load_program(dev); > + > + return 0; > +} > + > /* > * Save all Gunit registers that may be lost after a D3 and a subsequent > * S0i[R123] transition. The list of registers needing a save/restore is > @@ -1487,6 +1513,8 @@ static int intel_runtime_resume(struct device *device) > > if (IS_GEN6(dev_priv)) > intel_init_pch_refclk(dev); > + else if (IS_SKYLAKE(dev)) > + ret = skl_resume_prepare(dev_priv); > else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > hsw_disable_pc8(dev_priv); > else if (IS_VALLEYVIEW(dev_priv)) > @@ -1519,6 +1547,8 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv) > struct drm_device *dev = dev_priv->dev; > int ret; > > + if (IS_SKYLAKE(dev)) > + ret = skl_suspend_complete(dev_priv); > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > ret = hsw_suspend_complete(dev_priv); > else if (IS_VALLEYVIEW(dev)) > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index ea95df7..6cf54c1 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -50,7 +50,8 @@ > * present for a given platform. > */ > > -#define GEN9_ENABLE_DC5(dev) (IS_SKYLAKE(dev)) > +#define GEN9_ENABLE_DC5(dev) 0 > +#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > > #define for_each_power_well(i, power_well, domain_mask, power_domains) \ > for (i = 0; \ > @@ -438,8 +439,6 @@ static void skl_disable_dc6(struct drm_i915_private *dev_priv) > val &= ~DC_STATE_EN_UPTO_DC6; > I915_WRITE(DC_STATE_EN, val); > POSTING_READ(DC_STATE_EN); > - > - intel_prepare_ddi(dev); > } > > static void skl_set_power_well(struct drm_i915_private *dev_priv, > @@ -489,9 +488,21 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > !I915_READ(HSW_PWR_WELL_BIOS), > "Invalid for power well status to be enabled, unless done by the BIOS, \ > when request is to disable!\n"); > - if (GEN9_ENABLE_DC5(dev) && > - power_well->data == SKL_DISP_PW_2) > - gen9_disable_dc5(dev_priv); > + if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && > + power_well->data == SKL_DISP_PW_2) { > + if (SKL_ENABLE_DC6(dev)) { > + skl_disable_dc6(dev_priv); > + /* > + * DDI buffer programming unnecessary during driver-load/resume > + * as it's already done during modeset initialization then. > + * It's also invalid here as encoder list is still uninitialized. > + */ > + if (!dev_priv->power_domains.initializing) > + intel_prepare_ddi(dev); > + } else { > + gen9_disable_dc5(dev_priv); > + } > + } > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask); > } > > @@ -509,22 +520,25 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > POSTING_READ(HSW_PWR_WELL_DRIVER); > DRM_DEBUG_KMS("Disabling %s\n", power_well->name); > > - if (GEN9_ENABLE_DC5(dev) && > + if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && > power_well->data == SKL_DISP_PW_2) { > if (dev_priv->csr.states <= FW_LOADING) { > /* > - * TODO: wait for a completion event or > - * similar here instead of busy > - * waiting using wait_for function. > - */ > + * TODO: wait for a completion event or similar here instead of busy > + * waiting using wait_for function. > + */ > if (wait_for( > intel_csr_load_status_get( > dev_priv), 1000)) > DRM_ERROR("Timed out waiting for CSR to be loaded!"); > else > - gen9_enable_dc5(dev_priv); > + if (SKL_ENABLE_DC6(dev)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); > } else { > - DRM_ERROR("Cannot enable DC5 as CSR failed to load!"); > + DRM_ERROR("Cannot enable %s as CSR failed to load!", > + SKL_ENABLE_DC6(dev) ? "DC6" : "DC5"); > } > } > }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index acd0e2b..7a1123c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -594,6 +594,8 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv) static int intel_suspend_complete(struct drm_i915_private *dev_priv); static int vlv_resume_prepare(struct drm_i915_private *dev_priv, bool rpm_resume); +static int skl_resume_prepare(struct drm_i915_private *dev_priv); + static int i915_drm_suspend(struct drm_device *dev) { @@ -808,6 +810,8 @@ static int i915_drm_resume_early(struct drm_device *dev) if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_disable_pc8(dev_priv); + else if (IS_SKYLAKE(dev_priv)) + ret = skl_resume_prepare(dev_priv); intel_uncore_sanitize(dev); intel_power_domains_init_hw(dev_priv); @@ -1022,6 +1026,19 @@ static int i915_pm_resume(struct device *dev) return i915_drm_resume(drm_dev); } +static int skl_suspend_complete(struct drm_i915_private *dev_priv) +{ + /* Enabling DC6 is not a hard requirement to enter runtime D3 */ + + /* + * This is to ensure that CSR isn't identified as loaded before + * CSR-loading program is called during runtime-resume. + */ + intel_csr_load_status_set(dev_priv, false); + + return 0; +} + static int hsw_suspend_complete(struct drm_i915_private *dev_priv) { hsw_enable_pc8(dev_priv); @@ -1029,6 +1046,15 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv) return 0; } +static int skl_resume_prepare(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv->dev; + + intel_csr_load_program(dev); + + return 0; +} + /* * Save all Gunit registers that may be lost after a D3 and a subsequent * S0i[R123] transition. The list of registers needing a save/restore is @@ -1487,6 +1513,8 @@ static int intel_runtime_resume(struct device *device) if (IS_GEN6(dev_priv)) intel_init_pch_refclk(dev); + else if (IS_SKYLAKE(dev)) + ret = skl_resume_prepare(dev_priv); else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_disable_pc8(dev_priv); else if (IS_VALLEYVIEW(dev_priv)) @@ -1519,6 +1547,8 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv) struct drm_device *dev = dev_priv->dev; int ret; + if (IS_SKYLAKE(dev)) + ret = skl_suspend_complete(dev_priv); if (IS_HASWELL(dev) || IS_BROADWELL(dev)) ret = hsw_suspend_complete(dev_priv); else if (IS_VALLEYVIEW(dev)) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index ea95df7..6cf54c1 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -50,7 +50,8 @@ * present for a given platform. */ -#define GEN9_ENABLE_DC5(dev) (IS_SKYLAKE(dev)) +#define GEN9_ENABLE_DC5(dev) 0 +#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) #define for_each_power_well(i, power_well, domain_mask, power_domains) \ for (i = 0; \ @@ -438,8 +439,6 @@ static void skl_disable_dc6(struct drm_i915_private *dev_priv) val &= ~DC_STATE_EN_UPTO_DC6; I915_WRITE(DC_STATE_EN, val); POSTING_READ(DC_STATE_EN); - - intel_prepare_ddi(dev); } static void skl_set_power_well(struct drm_i915_private *dev_priv, @@ -489,9 +488,21 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, !I915_READ(HSW_PWR_WELL_BIOS), "Invalid for power well status to be enabled, unless done by the BIOS, \ when request is to disable!\n"); - if (GEN9_ENABLE_DC5(dev) && - power_well->data == SKL_DISP_PW_2) - gen9_disable_dc5(dev_priv); + if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && + power_well->data == SKL_DISP_PW_2) { + if (SKL_ENABLE_DC6(dev)) { + skl_disable_dc6(dev_priv); + /* + * DDI buffer programming unnecessary during driver-load/resume + * as it's already done during modeset initialization then. + * It's also invalid here as encoder list is still uninitialized. + */ + if (!dev_priv->power_domains.initializing) + intel_prepare_ddi(dev); + } else { + gen9_disable_dc5(dev_priv); + } + } I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask); } @@ -509,22 +520,25 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, POSTING_READ(HSW_PWR_WELL_DRIVER); DRM_DEBUG_KMS("Disabling %s\n", power_well->name); - if (GEN9_ENABLE_DC5(dev) && + if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && power_well->data == SKL_DISP_PW_2) { if (dev_priv->csr.states <= FW_LOADING) { /* - * TODO: wait for a completion event or - * similar here instead of busy - * waiting using wait_for function. - */ + * TODO: wait for a completion event or similar here instead of busy + * waiting using wait_for function. + */ if (wait_for( intel_csr_load_status_get( dev_priv), 1000)) DRM_ERROR("Timed out waiting for CSR to be loaded!"); else - gen9_enable_dc5(dev_priv); + if (SKL_ENABLE_DC6(dev)) + skl_enable_dc6(dev_priv); + else + gen9_enable_dc5(dev_priv); } else { - DRM_ERROR("Cannot enable DC5 as CSR failed to load!"); + DRM_ERROR("Cannot enable %s as CSR failed to load!", + SKL_ENABLE_DC6(dev) ? "DC6" : "DC5"); } } }