Message ID | 1429174334-12089-6-git-send-email-animesh.manna@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On to, 2015-04-16 at 14:22 +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 | 43 +++++++++++++++++++++++++++------ > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index acd0e2b..0f590e3 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, 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); This needs to be rebased on the recent BXT changes. While at it please fix the IS_SKYLAKE check to be an 'else if' condition as above for consistency (botched in the BXT series and missed in my review). > 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)) This needs to be rebased as above and the 'else if' ladder kept for consistency. With the above fixed this is: Reviewed-by: Imre Deak <imre.deak@intel.com> > 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 da8c18d..7e6908e 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -49,7 +49,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; \ > @@ -397,6 +398,16 @@ static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > POSTING_READ(DC_STATE_EN); > } > > +static void skl_enable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > +static void skl_disable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > static void skl_set_power_well(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well, bool enable) > { > @@ -444,9 +455,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); > } > > @@ -464,17 +487,23 @@ 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) { > enum csr_state state; > - > + /* TODO: wait for a completion event or > + * similar here instead of busy > + * waiting using wait_for function. > + */ > wait_for((state = intel_csr_load_status_get(dev_priv)) != > FW_UNINITIALIZED, 1000); > if (state != FW_LOADED) > DRM_ERROR("CSR firmware not ready (%d)\n", > state); > else > - gen9_enable_dc5(dev_priv); > + if (SKL_ENABLE_DC6(dev)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); > } > } > }
On Thu, Apr 16, 2015 at 02:22:11PM +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 | 43 +++++++++++++++++++++++++++------ > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index acd0e2b..0f590e3 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, 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); I resolved the conflicts with bxt support here assuming that there's no functional conflicts since dc6 is skl-only. But please double-check just to make sure. > 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 da8c18d..7e6908e 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -49,7 +49,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; \ > @@ -397,6 +398,16 @@ static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > POSTING_READ(DC_STATE_EN); > } > > +static void skl_enable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > +static void skl_disable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > static void skl_set_power_well(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well, bool enable) > { > @@ -444,9 +455,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); > } > > @@ -464,17 +487,23 @@ 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) { > enum csr_state state; > - > + /* TODO: wait for a completion event or > + * similar here instead of busy > + * waiting using wait_for function. > + */ > wait_for((state = intel_csr_load_status_get(dev_priv)) != > FW_UNINITIALIZED, 1000); > if (state != FW_LOADED) > DRM_ERROR("CSR firmware not ready (%d)\n", > state); > else > - gen9_enable_dc5(dev_priv); > + if (SKL_ENABLE_DC6(dev)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); Yep, skl_set_power_well is definitely going over the top. Rule of thumb is that you shouldn't use more than 3 tabs in a function, maybe 4 for an if (error_condition) baile_out; check. -Daniel > } > } > } > -- > 2.0.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Apr 16, 2015 at 02:22:11PM +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 | 43 +++++++++++++++++++++++++++------ > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index acd0e2b..0f590e3 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); Please no forward declarations of static functions. I know that vlv_resume_prepare is kinda misplaced already, but that's not a good excuse. Can you please do another fixup patch to juggle these around? We might even want to extract i915_suspend.c or similar (now that the ums gunk is gone) to keep these all together. Not sure about that ... -Daniel > + > > 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, 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 da8c18d..7e6908e 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -49,7 +49,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; \ > @@ -397,6 +398,16 @@ static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > POSTING_READ(DC_STATE_EN); > } > > +static void skl_enable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > +static void skl_disable_dc6(struct drm_i915_private *dev_priv) > +{ > + /* TODO: Implementation to be done. */ > +} > + > static void skl_set_power_well(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well, bool enable) > { > @@ -444,9 +455,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); > } > > @@ -464,17 +487,23 @@ 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) { > enum csr_state state; > - > + /* TODO: wait for a completion event or > + * similar here instead of busy > + * waiting using wait_for function. > + */ > wait_for((state = intel_csr_load_status_get(dev_priv)) != > FW_UNINITIALIZED, 1000); > if (state != FW_LOADED) > DRM_ERROR("CSR firmware not ready (%d)\n", > state); > else > - gen9_enable_dc5(dev_priv); > + if (SKL_ENABLE_DC6(dev)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); > } > } > } > -- > 2.0.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index acd0e2b..0f590e3 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, 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 da8c18d..7e6908e 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -49,7 +49,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; \ @@ -397,6 +398,16 @@ static void gen9_disable_dc5(struct drm_i915_private *dev_priv) POSTING_READ(DC_STATE_EN); } +static void skl_enable_dc6(struct drm_i915_private *dev_priv) +{ + /* TODO: Implementation to be done. */ +} + +static void skl_disable_dc6(struct drm_i915_private *dev_priv) +{ + /* TODO: Implementation to be done. */ +} + static void skl_set_power_well(struct drm_i915_private *dev_priv, struct i915_power_well *power_well, bool enable) { @@ -444,9 +455,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); } @@ -464,17 +487,23 @@ 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) { enum csr_state state; - + /* TODO: wait for a completion event or + * similar here instead of busy + * waiting using wait_for function. + */ wait_for((state = intel_csr_load_status_get(dev_priv)) != FW_UNINITIALIZED, 1000); if (state != FW_LOADED) DRM_ERROR("CSR firmware not ready (%d)\n", state); else - gen9_enable_dc5(dev_priv); + if (SKL_ENABLE_DC6(dev)) + skl_enable_dc6(dev_priv); + else + gen9_enable_dc5(dev_priv); } } }