* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-23 18:32 Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-23 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index baac35f698f9..6691b9ee95db 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e9a926fca04..529ff19a5e48 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..ef08313cf359 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:32 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
@ 2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Srivatsa, Anusha @ 2018-10-23 18:44 UTC (permalink / raw)
To: intel-gfx; +Cc: Vivi, Rodrigo
Rodrigo, this patch is tested by Jyoti. Can you review it?
Anusha
>-----Original Message-----
>From: Srivatsa, Anusha
>Sent: Tuesday, October 23, 2018 11:32 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Manna, Animesh <animesh.manna@intel.com>; Deak, Imre
><imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Ausmus, James
><james.ausmus@intel.com>; Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Subject: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-
>off
>
>From: Animesh Manna <animesh.manna@intel.com>
>
>ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
>DC5/6 when appropriate.
>
>v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
>v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
>v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
>v5: (Anusha Srivatsa)
>- rebased against the latest upstream changes.
>
>v6: (Anusha Srivatsa)
>- rebased.Use INTEL_GEN consistently.
>- Simplify the code (Rodrigo)
>
>v7: rebased. Change order according to platforms(Jyoti)
>
>Cc: Imre Deak <imre.deak@intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>Signed-off-by: James Ausmus <james.ausmus@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>index baac35f698f9..6691b9ee95db 100644
>--- a/drivers/gpu/drm/i915/i915_drv.c
>+++ b/drivers/gpu/drm/i915/i915_drv.c
>@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device
>*dev)
>
> intel_uncore_resume_early(dev_priv);
>
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@ -
>2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
>+ icl_display_core_uninit(dev_priv);
>+ bxt_enable_dc9(dev_priv);
>+ } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@ -
>3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend,
>bios?\n");
>
>- if (IS_GEN9_LP(dev_priv)) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
>+ bxt_disable_dc9(dev_priv);
>+ icl_display_core_init(dev_priv, true);
>+ if (dev_priv->csr.dmc_payload) {
>+ if (dev_priv->csr.allowed_dc_mask &
>+ DC_STATE_EN_UPTO_DC6)
>+ skl_enable_dc6(dev_priv);
>+ else if (dev_priv->csr.allowed_dc_mask &
>+ DC_STATE_EN_UPTO_DC5)
>+ gen9_enable_dc5(dev_priv);
>+ }
>+ } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index 0e9a926fca04..529ff19a5e48 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private
>*dev_priv); void bxt_disable_dc9(struct drm_i915_private *dev_priv); void
>gen9_enable_dc5(struct drm_i915_private *dev_priv); unsigned int
>skl_cdclk_get_vco(unsigned int freq);
>+void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config); void
>intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, @@ -1989,6 +1990,8
>@@ int intel_power_domains_init(struct drm_i915_private *); void
>intel_power_domains_cleanup(struct drm_i915_private *dev_priv); void
>intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
>+void icl_display_core_init(struct drm_i915_private *dev_priv, bool
>+resume); void icl_display_core_uninit(struct drm_i915_private
>+*dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv); void
>intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
>diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>b/drivers/gpu/drm/i915/intel_runtime_pm.c
>index 5f5416eb9644..ef08313cf359 100644
>--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private
>*dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
>- if (IS_GEN9_LP(dev_priv))
>+ if (INTEL_GEN(dev_priv) >= 11)
>+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
>+ else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
>@@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
>-
>- intel_power_sequencer_reset(dev_priv);
>+ if (INTEL_GEN(dev_priv) < 11)
>+ intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9); }
>
>@@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct
>drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
>-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
>+void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
>@@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct
>drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
>- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
>+ if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
>- mask = 0;
>- } else if (IS_GEN9_LP(dev_priv)) {
>- max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
>+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
>+ max_dc = 2;
>+ mask = 0;
>+ } else if (IS_GEN9_LP(dev_priv)) {
>+ max_dc = 1;
>+ mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
>@@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct
>drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
>-static void icl_display_core_init(struct drm_i915_private *dev_priv,
>- bool resume)
>+void icl_display_core_init(struct drm_i915_private *dev_priv,
>+ bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
> struct i915_power_well *well;
>@@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct
>drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
>-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
> struct i915_power_well *well;
>--
>2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:32 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
@ 2018-10-23 18:52 ` Rodrigo Vivi
2018-10-23 20:01 ` James Ausmus
2018-10-23 19:24 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4) Patchwork
2018-10-23 22:17 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 18:52 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f698f9..6691b9ee95db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0e9a926fca04..529ff19a5e48 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..ef08313cf359 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
I'm sorry if this was discussed already, but why is this only
needed on BXT?
Could we have a comment here or mention on commit message?
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)
2018-10-23 18:32 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
@ 2018-10-23 19:24 ` Patchwork
2018-10-23 22:17 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-23 19:24 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)
URL : https://patchwork.freedesktop.org/series/49447/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5023 -> Patchwork_10550 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/49447/revisions/4/mbox/
== Known issues ==
Here are the changes found in Patchwork_10550 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@amdgpu/amd_basic@cs-compute:
fi-kbl-8809g: NOTRUN -> FAIL (fdo#108094)
igt@amdgpu/amd_prime@amd-to-i915:
fi-kbl-8809g: NOTRUN -> FAIL (fdo#107341)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
fi-byt-clapper: PASS -> FAIL (fdo#107362)
==== Possible fixes ====
igt@drv_selftest@live_execlists:
fi-apl-guc: INCOMPLETE (fdo#106693) -> PASS
igt@gem_ctx_switch@basic-default:
fi-icl-u: DMESG-FAIL -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-byt-clapper: FAIL (fdo#107362) -> PASS
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#108094 https://bugs.freedesktop.org/show_bug.cgi?id=108094
== Participating hosts (49 -> 45) ==
Additional (1): fi-kbl-7560u
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5023 -> Patchwork_10550
CI_DRM_5023: 166bc98d7b77005943ab670506f164783cdc3f56 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4688: fa6dbf8c048961356fd642df047cb58ab49309b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10550: c0cabc73fc4a863abfca4922650e5eeba9b12722 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
c0cabc73fc4a drm/i915/icl: Enable DC9 as lowest possible state during screen-off
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10550/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 18:52 ` Rodrigo Vivi
@ 2018-10-23 20:01 ` James Ausmus
2018-10-23 20:34 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: James Ausmus @ 2018-10-23 20:01 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > From: Animesh Manna <animesh.manna@intel.com>
> >
> > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > DC5/6 when appropriate.
> >
> > v2: (James Ausmus)
> > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > i915_drm_suspend_early
> > - Add DC9 to gen9_dc_mask for ICL
> > - Re-order GEN checks for newest platform first
> > - Use INTEL_GEN instead of INTEL_INFO->gen
> > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > - Consolidate GEN checks
> >
> > v3: (James Ausmus)
> > - Also allow DC6 for ICL (Imre, Art)
> > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> >
> > v4: (James Ausmus)
> > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > PPS regs are Always On
> > - Rebase against upstream changes
> >
> > v5: (Anusha Srivatsa)
> > - rebased against the latest upstream changes.
> >
> > v6: (Anusha Srivatsa)
> > - rebased.Use INTEL_GEN consistently.
> > - Simplify the code (Rodrigo)
> >
> > v7: rebased. Change order according to platforms(Jyoti)
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > 3 files changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index baac35f698f9..6691b9ee95db 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> >
> > intel_uncore_resume_early(dev_priv);
> >
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > gen9_sanitize_dc_state(dev_priv);
> > bxt_disable_dc9(dev_priv);
> > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > intel_uncore_suspend(dev_priv);
> >
> > ret = 0;
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + icl_display_core_uninit(dev_priv);
> > + bxt_enable_dc9(dev_priv);
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > bxt_display_core_uninit(dev_priv);
> > bxt_enable_dc9(dev_priv);
> > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > if (intel_uncore_unclaimed_mmio(dev_priv))
> > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> >
> > - if (IS_GEN9_LP(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + bxt_disable_dc9(dev_priv);
> > + icl_display_core_init(dev_priv, true);
> > + if (dev_priv->csr.dmc_payload) {
> > + if (dev_priv->csr.allowed_dc_mask &
> > + DC_STATE_EN_UPTO_DC6)
> > + skl_enable_dc6(dev_priv);
> > + else if (dev_priv->csr.allowed_dc_mask &
> > + DC_STATE_EN_UPTO_DC5)
> > + gen9_enable_dc5(dev_priv);
> > + }
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > bxt_disable_dc9(dev_priv);
> > bxt_display_core_init(dev_priv, true);
> > if (dev_priv->csr.dmc_payload &&
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 0e9a926fca04..529ff19a5e48 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > struct intel_crtc_state *pipe_config);
> > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 5f5416eb9644..ef08313cf359 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > u32 mask;
> >
> > mask = DC_STATE_EN_UPTO_DC5;
> > - if (IS_GEN9_LP(dev_priv))
> > + if (INTEL_GEN(dev_priv) >= 11)
> > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > + else if (IS_GEN9_LP(dev_priv))
> > mask |= DC_STATE_EN_DC9;
> > else
> > mask |= DC_STATE_EN_UPTO_DC6;
> > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > assert_can_enable_dc9(dev_priv);
> >
> > DRM_DEBUG_KMS("Enabling DC9\n");
> > -
> > - intel_power_sequencer_reset(dev_priv);
> > + if (INTEL_GEN(dev_priv) < 11)
> > + intel_power_sequencer_reset(dev_priv);
>
> I'm sorry if this was discussed already, but why is this only
> needed on BXT?
>
> Could we have a comment here or mention on commit message?
It is in the commit message! ;)
<snip>
> > v4: (James Ausmus)
> > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > PPS regs are Always On
</snip>
Enough, or need more?
-James
>
> > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > }
> >
> > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > assert_csr_loaded(dev_priv);
> > }
> >
> > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > {
> > assert_can_enable_dc6(dev_priv);
> >
> > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > int requested_dc;
> > int max_dc;
> >
> > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > max_dc = 2;
> > - mask = 0;
> > - } else if (IS_GEN9_LP(dev_priv)) {
> > - max_dc = 1;
> > /*
> > * DC9 has a separate HW flow from the rest of the DC states,
> > * not depending on the DMC firmware. It's needed by system
> > * suspend/resume, so allow it unconditionally.
> > */
> > mask = DC_STATE_EN_DC9;
> > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > + max_dc = 2;
> > + mask = 0;
> > + } else if (IS_GEN9_LP(dev_priv)) {
> > + max_dc = 1;
> > + mask = DC_STATE_EN_DC9;
> > } else {
> > max_dc = 0;
> > mask = 0;
> > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > I915_WRITE(CHICKEN_MISC_2, val);
> > }
> >
> > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > - bool resume)
> > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > + bool resume)
> > {
> > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > struct i915_power_well *well;
> > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > intel_csr_load_program(dev_priv);
> > }
> >
> > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > {
> > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > struct i915_power_well *well;
> > --
> > 2.17.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 20:01 ` James Ausmus
@ 2018-10-23 20:34 ` Rodrigo Vivi
2018-10-23 20:38 ` James Ausmus
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 20:34 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 01:01:11PM -0700, James Ausmus wrote:
> On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> > On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > > From: Animesh Manna <animesh.manna@intel.com>
> > >
> > > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > > DC5/6 when appropriate.
> > >
> > > v2: (James Ausmus)
> > > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > > i915_drm_suspend_early
> > > - Add DC9 to gen9_dc_mask for ICL
> > > - Re-order GEN checks for newest platform first
> > > - Use INTEL_GEN instead of INTEL_INFO->gen
> > > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > > - Consolidate GEN checks
> > >
> > > v3: (James Ausmus)
> > > - Also allow DC6 for ICL (Imre, Art)
> > > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> > >
> > > v4: (James Ausmus)
> > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > PPS regs are Always On
> > > - Rebase against upstream changes
> > >
> > > v5: (Anusha Srivatsa)
> > > - rebased against the latest upstream changes.
> > >
> > > v6: (Anusha Srivatsa)
> > > - rebased.Use INTEL_GEN consistently.
> > > - Simplify the code (Rodrigo)
> > >
> > > v7: rebased. Change order according to platforms(Jyoti)
> > >
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > > 3 files changed, 36 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index baac35f698f9..6691b9ee95db 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > >
> > > intel_uncore_resume_early(dev_priv);
> > >
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > > gen9_sanitize_dc_state(dev_priv);
> > > bxt_disable_dc9(dev_priv);
> > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > > intel_uncore_suspend(dev_priv);
> > >
> > > ret = 0;
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > + icl_display_core_uninit(dev_priv);
> > > + bxt_enable_dc9(dev_priv);
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > bxt_display_core_uninit(dev_priv);
> > > bxt_enable_dc9(dev_priv);
> > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > > if (intel_uncore_unclaimed_mmio(dev_priv))
> > > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> > >
> > > - if (IS_GEN9_LP(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > + bxt_disable_dc9(dev_priv);
> > > + icl_display_core_init(dev_priv, true);
> > > + if (dev_priv->csr.dmc_payload) {
> > > + if (dev_priv->csr.allowed_dc_mask &
> > > + DC_STATE_EN_UPTO_DC6)
> > > + skl_enable_dc6(dev_priv);
> > > + else if (dev_priv->csr.allowed_dc_mask &
> > > + DC_STATE_EN_UPTO_DC5)
> > > + gen9_enable_dc5(dev_priv);
> > > + }
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > bxt_disable_dc9(dev_priv);
> > > bxt_display_core_init(dev_priv, true);
> > > if (dev_priv->csr.dmc_payload &&
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 0e9a926fca04..529ff19a5e48 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > > struct intel_crtc_state *pipe_config);
> > > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 5f5416eb9644..ef08313cf359 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > > u32 mask;
> > >
> > > mask = DC_STATE_EN_UPTO_DC5;
> > > - if (IS_GEN9_LP(dev_priv))
> > > + if (INTEL_GEN(dev_priv) >= 11)
> > > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > > + else if (IS_GEN9_LP(dev_priv))
> > > mask |= DC_STATE_EN_DC9;
> > > else
> > > mask |= DC_STATE_EN_UPTO_DC6;
> > > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > assert_can_enable_dc9(dev_priv);
> > >
> > > DRM_DEBUG_KMS("Enabling DC9\n");
> > > -
> > > - intel_power_sequencer_reset(dev_priv);
> > > + if (INTEL_GEN(dev_priv) < 11)
> > > + intel_power_sequencer_reset(dev_priv);
> >
> > I'm sorry if this was discussed already, but why is this only
> > needed on BXT?
> >
> > Could we have a comment here or mention on commit message?
>
> It is in the commit message! ;)
ops... my bad. sorry.
I didn't read the revision history. :/
>
> <snip>
> > > v4: (James Ausmus)
> > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > PPS regs are Always On
Is this because it is gen11+ or because PPS regs on icl is on PCH and BXT
doesn't have it?
in other words: Is the gen check better here or should we use HAS_PCH_SPLIT ?
and is this a possible thing to happen on the future?
> </snip>
>
> Enough, or need more?
I believe a comment close to the if is more appropriated to remind
us what to do in the future. ;)
Thanks,
Rodrigo.
>
> -James
>
> >
> > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > > }
> > >
> > > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > > assert_csr_loaded(dev_priv);
> > > }
> > >
> > > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > {
> > > assert_can_enable_dc6(dev_priv);
> > >
> > > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > > int requested_dc;
> > > int max_dc;
> > >
> > > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > max_dc = 2;
> > > - mask = 0;
> > > - } else if (IS_GEN9_LP(dev_priv)) {
> > > - max_dc = 1;
> > > /*
> > > * DC9 has a separate HW flow from the rest of the DC states,
> > > * not depending on the DMC firmware. It's needed by system
> > > * suspend/resume, so allow it unconditionally.
> > > */
> > > mask = DC_STATE_EN_DC9;
> > > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > > + max_dc = 2;
> > > + mask = 0;
> > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > + max_dc = 1;
> > > + mask = DC_STATE_EN_DC9;
> > > } else {
> > > max_dc = 0;
> > > mask = 0;
> > > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > I915_WRITE(CHICKEN_MISC_2, val);
> > > }
> > >
> > > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > - bool resume)
> > > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > + bool resume)
> > > {
> > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > struct i915_power_well *well;
> > > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > intel_csr_load_program(dev_priv);
> > > }
> > >
> > > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > {
> > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > struct i915_power_well *well;
> > > --
> > > 2.17.1
> > >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-23 20:34 ` Rodrigo Vivi
@ 2018-10-23 20:38 ` James Ausmus
0 siblings, 0 replies; 18+ messages in thread
From: James Ausmus @ 2018-10-23 20:38 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Oct 23, 2018 at 01:34:54PM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 23, 2018 at 01:01:11PM -0700, James Ausmus wrote:
> > On Tue, Oct 23, 2018 at 11:52:31AM -0700, Rodrigo Vivi wrote:
> > > On Tue, Oct 23, 2018 at 11:32:21AM -0700, Anusha Srivatsa wrote:
> > > > From: Animesh Manna <animesh.manna@intel.com>
> > > >
> > > > ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> > > > DC5/6 when appropriate.
> > > >
> > > > v2: (James Ausmus)
> > > > - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> > > > i915_drm_suspend_early
> > > > - Add DC9 to gen9_dc_mask for ICL
> > > > - Re-order GEN checks for newest platform first
> > > > - Use INTEL_GEN instead of INTEL_INFO->gen
> > > > - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> > > > - Consolidate GEN checks
> > > >
> > > > v3: (James Ausmus)
> > > > - Also allow DC6 for ICL (Imre, Art)
> > > > - Simplify !(GEN >= 11) to GEN < 11 (Imre)
> > > >
> > > > v4: (James Ausmus)
> > > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > > PPS regs are Always On
> > > > - Rebase against upstream changes
> > > >
> > > > v5: (Anusha Srivatsa)
> > > > - rebased against the latest upstream changes.
> > > >
> > > > v6: (Anusha Srivatsa)
> > > > - rebased.Use INTEL_GEN consistently.
> > > > - Simplify the code (Rodrigo)
> > > >
> > > > v7: rebased. Change order according to platforms(Jyoti)
> > > >
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> > > > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> > > > 3 files changed, 36 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index baac35f698f9..6691b9ee95db 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> > > >
> > > > intel_uncore_resume_early(dev_priv);
> > > >
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> > > > gen9_sanitize_dc_state(dev_priv);
> > > > bxt_disable_dc9(dev_priv);
> > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> > > > intel_uncore_suspend(dev_priv);
> > > >
> > > > ret = 0;
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > + icl_display_core_uninit(dev_priv);
> > > > + bxt_enable_dc9(dev_priv);
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > bxt_display_core_uninit(dev_priv);
> > > > bxt_enable_dc9(dev_priv);
> > > > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> > > > if (intel_uncore_unclaimed_mmio(dev_priv))
> > > > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> > > >
> > > > - if (IS_GEN9_LP(dev_priv)) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > + bxt_disable_dc9(dev_priv);
> > > > + icl_display_core_init(dev_priv, true);
> > > > + if (dev_priv->csr.dmc_payload) {
> > > > + if (dev_priv->csr.allowed_dc_mask &
> > > > + DC_STATE_EN_UPTO_DC6)
> > > > + skl_enable_dc6(dev_priv);
> > > > + else if (dev_priv->csr.allowed_dc_mask &
> > > > + DC_STATE_EN_UPTO_DC5)
> > > > + gen9_enable_dc5(dev_priv);
> > > > + }
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > bxt_disable_dc9(dev_priv);
> > > > bxt_display_core_init(dev_priv, true);
> > > > if (dev_priv->csr.dmc_payload &&
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 0e9a926fca04..529ff19a5e48 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> > > > void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> > > > void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> > > > unsigned int skl_cdclk_get_vco(unsigned int freq);
> > > > +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> > > > void intel_dp_get_m_n(struct intel_crtc *crtc,
> > > > struct intel_crtc_state *pipe_config);
> > > > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> > > > @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> > > > void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> > > > void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> > > > +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> > > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> > > > void intel_power_domains_disable(struct drm_i915_private *dev_priv);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index 5f5416eb9644..ef08313cf359 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> > > > u32 mask;
> > > >
> > > > mask = DC_STATE_EN_UPTO_DC5;
> > > > - if (IS_GEN9_LP(dev_priv))
> > > > + if (INTEL_GEN(dev_priv) >= 11)
> > > > + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> > > > + else if (IS_GEN9_LP(dev_priv))
> > > > mask |= DC_STATE_EN_DC9;
> > > > else
> > > > mask |= DC_STATE_EN_UPTO_DC6;
> > > > @@ -633,8 +635,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > > assert_can_enable_dc9(dev_priv);
> > > >
> > > > DRM_DEBUG_KMS("Enabling DC9\n");
> > > > -
> > > > - intel_power_sequencer_reset(dev_priv);
> > > > + if (INTEL_GEN(dev_priv) < 11)
> > > > + intel_power_sequencer_reset(dev_priv);
> > >
> > > I'm sorry if this was discussed already, but why is this only
> > > needed on BXT?
> > >
> > > Could we have a comment here or mention on commit message?
> >
> > It is in the commit message! ;)
>
> ops... my bad. sorry.
>
> I didn't read the revision history. :/
>
> >
> > <snip>
> > > > v4: (James Ausmus)
> > > > - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> > > > PPS regs are Always On
>
> Is this because it is gen11+ or because PPS regs on icl is on PCH and BXT
> doesn't have it?
>
> in other words: Is the gen check better here or should we use HAS_PCH_SPLIT ?
>
> and is this a possible thing to happen on the future?
It is due to the PCH split - good call. I saw no sign in BSpec of this
changing in the future, though, so not sure whether GEN >= 11, or
HAS_PCH_SPLIT is the better check.
>
> > </snip>
> >
> > Enough, or need more?
>
> I believe a comment close to the if is more appropriated to remind
> us what to do in the future. ;)
Agreed. :)
Thanks!
-James
>
> Thanks,
> Rodrigo.
>
> >
> > -James
> >
> > >
> > > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > > > }
> > > >
> > > > @@ -716,7 +718,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> > > > assert_csr_loaded(dev_priv);
> > > > }
> > > >
> > > > -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > > +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> > > > {
> > > > assert_can_enable_dc6(dev_priv);
> > > >
> > > > @@ -2978,17 +2980,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> > > > int requested_dc;
> > > > int max_dc;
> > > >
> > > > - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> > > > + if (INTEL_GEN(dev_priv) >= 11) {
> > > > max_dc = 2;
> > > > - mask = 0;
> > > > - } else if (IS_GEN9_LP(dev_priv)) {
> > > > - max_dc = 1;
> > > > /*
> > > > * DC9 has a separate HW flow from the rest of the DC states,
> > > > * not depending on the DMC firmware. It's needed by system
> > > > * suspend/resume, so allow it unconditionally.
> > > > */
> > > > mask = DC_STATE_EN_DC9;
> > > > + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> > > > + max_dc = 2;
> > > > + mask = 0;
> > > > + } else if (IS_GEN9_LP(dev_priv)) {
> > > > + max_dc = 1;
> > > > + mask = DC_STATE_EN_DC9;
> > > > } else {
> > > > max_dc = 0;
> > > > mask = 0;
> > > > @@ -3539,8 +3544,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > I915_WRITE(CHICKEN_MISC_2, val);
> > > > }
> > > >
> > > > -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > - bool resume)
> > > > +void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > + bool resume)
> > > > {
> > > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > struct i915_power_well *well;
> > > > @@ -3592,7 +3597,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > > > intel_csr_load_program(dev_priv);
> > > > }
> > > >
> > > > -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> > > > {
> > > > struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > struct i915_power_well *well;
> > > > --
> > > > 2.17.1
> > > >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)
2018-10-23 18:32 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
` (2 preceding siblings ...)
2018-10-23 19:24 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4) Patchwork
@ 2018-10-23 22:17 ` Patchwork
3 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-10-23 22:17 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)
URL : https://patchwork.freedesktop.org/series/49447/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5023_full -> Patchwork_10550_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10550_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_await@wide-contexts:
shard-glk: PASS -> FAIL (fdo#106680)
shard-apl: PASS -> FAIL (fdo#106680)
igt@gem_linear_blits@interruptible:
shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_busy@extended-pageflip-hang-newfb-render-b:
shard-apl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
shard-snb: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_color@pipe-a-legacy-gamma:
shard-apl: PASS -> FAIL (fdo#104782, fdo#108145)
igt@kms_cursor_crc@cursor-128x128-dpms:
shard-apl: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-apl: PASS -> FAIL (fdo#103232, fdo#103191)
igt@kms_flip@2x-plain-flip-fb-recreate:
shard-glk: PASS -> FAIL (fdo#100368)
igt@kms_flip@2x-wf_vblank-ts-check-interruptible:
shard-hsw: PASS -> INCOMPLETE (fdo#103540)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-rte:
shard-apl: PASS -> FAIL (fdo#105682, fdo#103167)
igt@kms_frontbuffer_tracking@fbc-tilingchange:
shard-glk: PASS -> FAIL (fdo#103167)
igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108146)
igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
shard-apl: PASS -> FAIL (fdo#108145)
igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
shard-apl: PASS -> FAIL (fdo#103166) +2
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
shard-kbl: NOTRUN -> FAIL (fdo#99912)
igt@pm_rpm@universal-planes:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
==== Possible fixes ====
igt@kms_busy@extended-modeset-hang-newfb-render-c:
shard-skl: DMESG-WARN (fdo#107956) -> PASS +1
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: FAIL (fdo#108145) -> PASS +1
igt@kms_cursor_crc@cursor-256x256-onscreen:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-64x21-offscreen:
shard-skl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-size-change:
shard-apl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
shard-glk: DMESG-WARN (fdo#105763, fdo#106538) -> PASS +1
igt@kms_draw_crc@draw-method-rgb565-mmap-cpu-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@busy-flip-interruptible:
shard-skl: FAIL (fdo#103257) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-skl: FAIL (fdo#105363) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
shard-apl: FAIL (fdo#103167) -> PASS +3
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
shard-glk: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu:
shard-skl: FAIL (fdo#103167) -> PASS +1
igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
shard-apl: FAIL (fdo#103166) -> PASS +1
igt@kms_rotation_crc@sprite-rotation-180:
shard-apl: INCOMPLETE (fdo#103927) -> PASS
igt@kms_vblank@pipe-a-ts-continuation-suspend:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@pm_rpm@legacy-planes-dpms:
shard-skl: INCOMPLETE (fdo#107807, fdo#105959) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103257 https://bugs.freedesktop.org/show_bug.cgi?id=103257
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5023 -> Patchwork_10550
CI_DRM_5023: 166bc98d7b77005943ab670506f164783cdc3f56 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4688: fa6dbf8c048961356fd642df047cb58ab49309b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10550: c0cabc73fc4a863abfca4922650e5eeba9b12722 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10550/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
@ 2018-10-30 17:56 ` Rodrigo Vivi
0 siblings, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-30 17:56 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Mon, Oct 29, 2018 at 03:14:10PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> v8: rebased. Change the check from platform specific to
> HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Tested-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
pushed to dinq, thanks for the patch, answers and tests
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++---------
> 3 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ad13da61d7a..6bdcd5a3d7b7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2157,7 +2157,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2924,7 +2924,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3009,7 +3012,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 268afb6d2746..e4eaa40bd5f1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1695,6 +1695,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -2045,6 +2046,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..b1901a6c17be 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,13 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + /*
> + * Power sequencer reset is not needed on
> + * platforms with South Display Engine on PCH,
> + * because PPS registers are always on.
> + */
> + if (!HAS_PCH_SPLIT(dev_priv))
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +723,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2985,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3549,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3602,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-29 22:14 Anusha Srivatsa
2018-10-30 17:56 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-29 22:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
v8: rebased. Change the check from platform specific to
HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Tested-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++---------
3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ad13da61d7a..6bdcd5a3d7b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2157,7 +2157,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2924,7 +2924,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3009,7 +3012,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 268afb6d2746..e4eaa40bd5f1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1695,6 +1695,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -2045,6 +2046,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..b1901a6c17be 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,13 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ /*
+ * Power sequencer reset is not needed on
+ * platforms with South Display Engine on PCH,
+ * because PPS registers are always on.
+ */
+ if (!HAS_PCH_SPLIT(dev_priv))
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +723,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2985,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3549,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3602,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-10-24 23:01 Anusha Srivatsa
@ 2018-10-24 23:22 ` Rodrigo Vivi
0 siblings, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-10-24 23:22 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Wed, Oct 24, 2018 at 04:01:09PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> v7: rebased. Change order according to platforms(Jyoti)
>
> v8: rebased. Change the check from platform specific to
> HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 31 ++++++++++++++++---------
> 3 files changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f698f9..6691b9ee95db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0e9a926fca04..529ff19a5e48 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> @@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 5f5416eb9644..31d08f452dcb 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -633,8 +635,12 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + /* In case of Gen11+, PPS registers are always on, due
/*
* In case...
(but can be fixed while merging.
> + * to PCH split. Reset power sequencer on platforms
> + * without the PCH split.
> + */
What about:
/*
* Power sequencer reset is not needed on platforms with
* South Display Engine on PCH, because PPS registers are always on.
*/
with or without accepting this bikeshed
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + if (!HAS_PCH_SPLIT(dev_priv))
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -716,7 +722,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2978,17 +2984,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3539,8 +3548,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3592,7 +3601,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-10-24 23:01 Anusha Srivatsa
2018-10-24 23:22 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-10-24 23:01 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
v7: rebased. Change order according to platforms(Jyoti)
v8: rebased. Change the check from platform specific to
HAS_PCH_SPLIT(). Add comment in code to be more clear.(Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 31 ++++++++++++++++---------
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index baac35f698f9..6691b9ee95db 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2156,7 +2156,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2923,7 +2923,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3008,7 +3011,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e9a926fca04..529ff19a5e48 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1640,6 +1640,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
@@ -1989,6 +1990,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5f5416eb9644..31d08f452dcb 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -560,7 +560,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -633,8 +635,12 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ /* In case of Gen11+, PPS registers are always on, due
+ * to PCH split. Reset power sequencer on platforms
+ * without the PCH split.
+ */
+ if (!HAS_PCH_SPLIT(dev_priv))
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -716,7 +722,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2978,17 +2984,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
} else {
max_dc = 0;
mask = 0;
@@ -3539,8 +3548,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3592,7 +3601,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-19 20:06 Anusha Srivatsa
@ 2018-10-23 10:01 ` Yadav, Jyoti R
0 siblings, 0 replies; 18+ messages in thread
From: Yadav, Jyoti R @ 2018-10-23 10:01 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
On 9/20/2018 1:36 AM, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
>
> v6: (Anusha Srivatsa)
> - rebased.Use INTEL_GEN consistently.
> - Simplify the code (Rodrigo)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 44e2c0f5ec50..036f33fa5626 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2155,7 +2155,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2922,7 +2922,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -3007,7 +3010,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..f0385fe5bb15 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> @@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 0fdabce647ab..5271ca9418de 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2969,17 +2971,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> + if (INTEL_GEN(dev_priv) >= 11) {
> max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN9_LP(dev_priv)) {
> + max_dc = 1;
> + mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> +
Please double confirm that Gen checks are in order. I could see see Gen
9 coming before Gen10. Usually we prefer to put new Gens first.
Also i tested this patch on ICL B1 HW and could see that display is
entering into Dc9 when DMC FW is loaded.
With this we can add "Tested-by" tag with my name for this patch.
> mask = 0;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3514,8 +3519,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3569,7 +3574,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> intel_csr_load_program(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-19 20:06 Anusha Srivatsa
2018-10-23 10:01 ` Yadav, Jyoti R
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-19 20:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
v6: (Anusha Srivatsa)
- rebased.Use INTEL_GEN consistently.
- Simplify the code (Rodrigo)
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 44e2c0f5ec50..036f33fa5626 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2155,7 +2155,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2922,7 +2922,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -3007,7 +3010,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..f0385fe5bb15 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 0fdabce647ab..5271ca9418de 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2969,17 +2971,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
+ if (INTEL_GEN(dev_priv) >= 11) {
max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN9_LP(dev_priv)) {
+ max_dc = 1;
+ mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3514,8 +3519,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3569,7 +3574,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
intel_csr_load_program(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-13 20:13 ` Rodrigo Vivi
@ 2018-09-17 18:45 ` Srivatsa, Anusha
0 siblings, 0 replies; 18+ messages in thread
From: Srivatsa, Anusha @ 2018-09-17 18:45 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Thursday, September 13, 2018 1:14 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Manna, Animesh
><animesh.manna@intel.com>; Deak, Imre <imre.deak@intel.com>; Ausmus,
>James <james.ausmus@intel.com>
>Subject: Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during
>screen-off
>
>On Thu, Sep 13, 2018 at 12:31:09PM -0700, Anusha Srivatsa wrote:
>> From: Animesh Manna <animesh.manna@intel.com>
>>
>> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and
>> enable
>> DC5/6 when appropriate.
>>
>> v2: (James Ausmus)
>> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
>> i915_drm_suspend_early
>> - Add DC9 to gen9_dc_mask for ICL
>> - Re-order GEN checks for newest platform first
>> - Use INTEL_GEN instead of INTEL_INFO->gen
>> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
>> - Consolidate GEN checks
>>
>> v3: (James Ausmus)
>> - Also allow DC6 for ICL (Imre, Art)
>> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>>
>> v4: (James Ausmus)
>> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
>> PPS regs are Always On
>> - Rebase against upstream changes
>>
>> v5: (Anusha Srivatsa)
>> - rebased against the latest upstream changes.
>
>First concern with this patch is regarding the tests...
>How is this getting tested? Are you able to see DC6 and DC9?
>
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> Signed-off-by: James Ausmus <james.ausmus@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
>> drivers/gpu/drm/i915/intel_drv.h | 3 +++
>> drivers/gpu/drm/i915/intel_runtime_pm.c | 29
>> +++++++++++++++----------
>> 3 files changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c index 2ddf8538cb47..86a83e0a7ef2
>> 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct
>> drm_device *dev)
>>
>> intel_uncore_resume_early(dev_priv);
>>
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
>> gen9_sanitize_dc_state(dev_priv);
>> bxt_disable_dc9(dev_priv);
>> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@
>> -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
>> intel_uncore_suspend(dev_priv);
>>
>> ret = 0;
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (IS_ICELAKE(dev_priv)) {
>> + icl_display_core_uninit(dev_priv);
>> + bxt_enable_dc9(dev_priv);
>> + } else if (IS_GEN9_LP(dev_priv)) {
>> bxt_display_core_uninit(dev_priv);
>> bxt_enable_dc9(dev_priv);
>> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@
>> -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
>> if (intel_uncore_unclaimed_mmio(dev_priv))
>> DRM_DEBUG_DRIVER("Unclaimed access during suspend,
>bios?\n");
>>
>> - if (IS_GEN9_LP(dev_priv)) {
>> + if (IS_ICELAKE(dev_priv)) {
>
>commit message mention the use of INTEL_GEN instead of ICELAKE, but it seems
>we are missing some replacements here....
Actually double checked with the internal version of this patch, seems like IS_ICELAKE was what was used in the code...
Maybe change the commit log?
>
>> + bxt_disable_dc9(dev_priv);
>> + icl_display_core_init(dev_priv, true);
>> + if (dev_priv->csr.dmc_payload) {
>> + if (dev_priv->csr.allowed_dc_mask &
>> + DC_STATE_EN_UPTO_DC6)
>> + skl_enable_dc6(dev_priv);
>> + else if (dev_priv->csr.allowed_dc_mask &
>> + DC_STATE_EN_UPTO_DC5)
>> + gen9_enable_dc5(dev_priv);
>> + }
>> + } else if (IS_GEN9_LP(dev_priv)) {
>> bxt_disable_dc9(dev_priv);
>> bxt_display_core_init(dev_priv, true);
>> if (dev_priv->csr.dmc_payload &&
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index bf1c38728a59..f0385fe5bb15 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private
>> *dev_priv); void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>> void gen9_enable_dc5(struct drm_i915_private *dev_priv); unsigned int
>> skl_cdclk_get_vco(unsigned int freq);
>> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
>> void intel_dp_get_m_n(struct intel_crtc *crtc,
>> struct intel_crtc_state *pipe_config); void
>> intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n); @@
>> -1966,6 +1967,8 @@ int intel_power_domains_init(struct
>> drm_i915_private *); void intel_power_domains_cleanup(struct
>> drm_i915_private *dev_priv); void intel_power_domains_init_hw(struct
>> drm_i915_private *dev_priv, bool resume); void
>> intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
>> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool
>> +resume); void icl_display_core_uninit(struct drm_i915_private
>> +*dev_priv);
>> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
>> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index 480dadb1047b..3e2c936217f8 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private
>*dev_priv)
>> u32 mask;
>>
>> mask = DC_STATE_EN_UPTO_DC5;
>> - if (IS_GEN9_LP(dev_priv))
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
>> + else if (IS_GEN9_LP(dev_priv))
>> mask |= DC_STATE_EN_DC9;
>> else
>> mask |= DC_STATE_EN_UPTO_DC6;
>> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private
>*dev_priv)
>> assert_can_enable_dc9(dev_priv);
>>
>> DRM_DEBUG_KMS("Enabling DC9\n");
>> -
>> - intel_power_sequencer_reset(dev_priv);
>> + if (INTEL_GEN(dev_priv) < 11)
>> + intel_power_sequencer_reset(dev_priv);
>> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9); }
>>
>> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct
>drm_i915_private *dev_priv)
>> assert_csr_loaded(dev_priv);
>> }
>>
>> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
>> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
>> {
>> assert_can_enable_dc6(dev_priv);
>>
>> @@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct
>drm_i915_private *dev_priv,
>> int requested_dc;
>> int max_dc;
>>
>> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
>> - max_dc = 2;
>> - mask = 0;
>> - } else if (IS_GEN9_LP(dev_priv)) {
>> - max_dc = 1;
>> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
>> + if (INTEL_GEN(dev_priv) >= 11)
>> + max_dc = 2;
>
>This is supper confusing... Could we create a new full block for INTEL_GEN >= 11?
Yes...that sounds good.
Anusha
>
>> + else
>> + max_dc = 1;
>> /*
>> * DC9 has a separate HW flow from the rest of the DC states,
>> * not depending on the DMC firmware. It's needed by system
>> * suspend/resume, so allow it unconditionally.
>> */
>> mask = DC_STATE_EN_DC9;
>> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
>> + max_dc = 2;
>> + mask = 0;
>> } else {
>> max_dc = 0;
>> mask = 0;
>> @@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct
>drm_i915_private *dev_priv)
>> I915_WRITE(CHICKEN_MISC_2, val);
>> }
>>
>> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
>> - bool resume)
>> +void icl_display_core_init(struct drm_i915_private *dev_priv,
>> + bool resume)
>> {
>> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
>> struct i915_power_well *well;
>> @@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct
>drm_i915_private *dev_priv,
>> icl_mbus_init(dev_priv);
>> }
>>
>> -static void icl_display_core_uninit(struct drm_i915_private
>> *dev_priv)
>> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>> {
>> struct i915_power_domains *power_domains = &dev_priv-
>>power_domains;
>> struct i915_power_well *well;
>> --
>> 2.17.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
2018-09-13 19:31 Anusha Srivatsa
@ 2018-09-13 20:13 ` Rodrigo Vivi
2018-09-17 18:45 ` Srivatsa, Anusha
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-09-13 20:13 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Thu, Sep 13, 2018 at 12:31:09PM -0700, Anusha Srivatsa wrote:
> From: Animesh Manna <animesh.manna@intel.com>
>
> ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
> DC5/6 when appropriate.
>
> v2: (James Ausmus)
> - Also handle ICL as GEN9_LP in i915_drm_suspend_late and
> i915_drm_suspend_early
> - Add DC9 to gen9_dc_mask for ICL
> - Re-order GEN checks for newest platform first
> - Use INTEL_GEN instead of INTEL_INFO->gen
> - Use INTEL_GEN >= 11 instead of IS_ICELAKE
> - Consolidate GEN checks
>
> v3: (James Ausmus)
> - Also allow DC6 for ICL (Imre, Art)
> - Simplify !(GEN >= 11) to GEN < 11 (Imre)
>
> v4: (James Ausmus)
> - Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
> PPS regs are Always On
> - Rebase against upstream changes
>
> v5: (Anusha Srivatsa)
> - rebased against the latest upstream changes.
First concern with this patch is regarding the tests...
How is this getting tested? Are you able to see DC6 and DC9?
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++----------
> 3 files changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2ddf8538cb47..86a83e0a7ef2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>
> intel_uncore_resume_early(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> gen9_sanitize_dc_state(dev_priv);
> bxt_disable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
> intel_uncore_suspend(dev_priv);
>
> ret = 0;
> - if (IS_GEN9_LP(dev_priv)) {
> + if (IS_ICELAKE(dev_priv)) {
> + icl_display_core_uninit(dev_priv);
> + bxt_enable_dc9(dev_priv);
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_display_core_uninit(dev_priv);
> bxt_enable_dc9(dev_priv);
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> @@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
> if (intel_uncore_unclaimed_mmio(dev_priv))
> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>
> - if (IS_GEN9_LP(dev_priv)) {
> + if (IS_ICELAKE(dev_priv)) {
commit message mention the use of INTEL_GEN instead of ICELAKE,
but it seems we are missing some replacements here....
> + bxt_disable_dc9(dev_priv);
> + icl_display_core_init(dev_priv, true);
> + if (dev_priv->csr.dmc_payload) {
> + if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC6)
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask &
> + DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> + } else if (IS_GEN9_LP(dev_priv)) {
> bxt_disable_dc9(dev_priv);
> bxt_display_core_init(dev_priv, true);
> if (dev_priv->csr.dmc_payload &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..f0385fe5bb15 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void gen9_enable_dc5(struct drm_i915_private *dev_priv);
> unsigned int skl_cdclk_get_vco(unsigned int freq);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> @@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
> void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
> +void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_power_domains_enable(struct drm_i915_private *dev_priv);
> void intel_power_domains_disable(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 480dadb1047b..3e2c936217f8 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (IS_GEN9_LP(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 11)
> + mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
> + else if (IS_GEN9_LP(dev_priv))
> mask |= DC_STATE_EN_DC9;
> else
> mask |= DC_STATE_EN_UPTO_DC6;
> @@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> assert_can_enable_dc9(dev_priv);
>
> DRM_DEBUG_KMS("Enabling DC9\n");
> -
> - intel_power_sequencer_reset(dev_priv);
> + if (INTEL_GEN(dev_priv) < 11)
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
>
> @@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> assert_csr_loaded(dev_priv);
> }
>
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc6(dev_priv);
>
> @@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
> - max_dc = 2;
> - mask = 0;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - max_dc = 1;
> + if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 11)
> + max_dc = 2;
This is supper confusing... Could we create a new full block for INTEL_GEN >= 11?
> + else
> + max_dc = 1;
> /*
> * DC9 has a separate HW flow from the rest of the DC states,
> * not depending on the DMC firmware. It's needed by system
> * suspend/resume, so allow it unconditionally.
> */
> mask = DC_STATE_EN_DC9;
> + } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
> + max_dc = 2;
> + mask = 0;
> } else {
> max_dc = 0;
> mask = 0;
> @@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> I915_WRITE(CHICKEN_MISC_2, val);
> }
>
> -static void icl_display_core_init(struct drm_i915_private *dev_priv,
> - bool resume)
> +void icl_display_core_init(struct drm_i915_private *dev_priv,
> + bool resume)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> @@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> icl_mbus_init(dev_priv);
> }
>
> -static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> +void icl_display_core_uninit(struct drm_i915_private *dev_priv)
> {
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *well;
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-13 19:31 Anusha Srivatsa
2018-09-13 20:13 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-13 19:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 ++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++----------
3 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2ddf8538cb47..86a83e0a7ef2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..f0385fe5bb15 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1627,6 +1627,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1966,6 +1967,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 480dadb1047b..3e2c936217f8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
- max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11)
+ max_dc = 2;
+ else
+ max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3513,8 +3518,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
- bool resume)
+void icl_display_core_init(struct drm_i915_private *dev_priv,
+ bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
@@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
icl_mbus_init(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off
@ 2018-09-10 19:10 Anusha Srivatsa
0 siblings, 0 replies; 18+ messages in thread
From: Anusha Srivatsa @ 2018-09-10 19:10 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Animesh Manna <animesh.manna@intel.com>
ICL supports DC5, DC6, and DC9. Enable DC9 during screen-off, and enable
DC5/6 when appropriate.
v2: (James Ausmus)
- Also handle ICL as GEN9_LP in i915_drm_suspend_late and
i915_drm_suspend_early
- Add DC9 to gen9_dc_mask for ICL
- Re-order GEN checks for newest platform first
- Use INTEL_GEN instead of INTEL_INFO->gen
- Use INTEL_GEN >= 11 instead of IS_ICELAKE
- Consolidate GEN checks
v3: (James Ausmus)
- Also allow DC6 for ICL (Imre, Art)
- Simplify !(GEN >= 11) to GEN < 11 (Imre)
v4: (James Ausmus)
- Don't call intel_power_sequencer_reset after DC9 for Gen11+, as the
PPS regs are Always On
- Rebase against upstream changes
v5: (Anusha Srivatsa)
- rebased against the latest upstream changes.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 27 +++++++++++++++----------
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5dd7fc582e6f..2b1c1b9e0077 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1855,7 +1855,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv);
- if (IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2622,7 +2622,10 @@ static int intel_runtime_suspend(struct device *kdev)
intel_uncore_suspend(dev_priv);
ret = 0;
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ icl_display_core_uninit(dev_priv);
+ bxt_enable_dc9(dev_priv);
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_display_core_uninit(dev_priv);
bxt_enable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
@@ -2707,7 +2710,18 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- if (IS_GEN9_LP(dev_priv)) {
+ if (IS_ICELAKE(dev_priv)) {
+ bxt_disable_dc9(dev_priv);
+ icl_display_core_init(dev_priv, true);
+ if (dev_priv->csr.dmc_payload) {
+ if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC6)
+ skl_enable_dc6(dev_priv);
+ else if (dev_priv->csr.allowed_dc_mask &
+ DC_STATE_EN_UPTO_DC5)
+ gen9_enable_dc5(dev_priv);
+ }
+ } else if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
if (dev_priv->csr.dmc_payload &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5731215210a..3a06f58a3459 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1625,6 +1625,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
void bxt_disable_dc9(struct drm_i915_private *dev_priv);
void gen9_enable_dc5(struct drm_i915_private *dev_priv);
unsigned int skl_cdclk_get_vco(unsigned int freq);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
@@ -1962,6 +1963,8 @@ int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);
+void icl_display_core_init(struct drm_i915_private *dev_priv, bool resume);
+void icl_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_power_domains_enable(struct drm_i915_private *dev_priv);
void intel_power_domains_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 480dadb1047b..33ad0b751576 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -551,7 +551,9 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (IS_GEN9_LP(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 11)
+ mask |= DC_STATE_EN_UPTO_DC6 | DC_STATE_EN_DC9;
+ else if (IS_GEN9_LP(dev_priv))
mask |= DC_STATE_EN_DC9;
else
mask |= DC_STATE_EN_UPTO_DC6;
@@ -624,8 +626,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
assert_can_enable_dc9(dev_priv);
DRM_DEBUG_KMS("Enabling DC9\n");
-
- intel_power_sequencer_reset(dev_priv);
+ if (INTEL_GEN(dev_priv) < 11)
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -707,7 +709,7 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
assert_csr_loaded(dev_priv);
}
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc6(dev_priv);
@@ -2968,17 +2970,20 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (IS_GEN9_BC(dev_priv) || INTEL_INFO(dev_priv)->gen >= 10) {
- max_dc = 2;
- mask = 0;
- } else if (IS_GEN9_LP(dev_priv)) {
- max_dc = 1;
+ if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 11)
+ max_dc = 2;
+ else
+ max_dc = 1;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
* suspend/resume, so allow it unconditionally.
*/
mask = DC_STATE_EN_DC9;
+ } else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv)) {
+ max_dc = 2;
+ mask = 0;
} else {
max_dc = 0;
mask = 0;
@@ -3513,7 +3518,7 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
I915_WRITE(CHICKEN_MISC_2, val);
}
-static void icl_display_core_init(struct drm_i915_private *dev_priv,
+void icl_display_core_init(struct drm_i915_private *dev_priv,
bool resume)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -3565,7 +3570,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
icl_mbus_init(dev_priv);
}
-static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
+void icl_display_core_uninit(struct drm_i915_private *dev_priv)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *well;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-30 17:56 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-23 18:32 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-23 18:44 ` Srivatsa, Anusha
2018-10-23 18:52 ` Rodrigo Vivi
2018-10-23 20:01 ` James Ausmus
2018-10-23 20:34 ` Rodrigo Vivi
2018-10-23 20:38 ` James Ausmus
2018-10-23 19:24 ` ✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4) Patchwork
2018-10-23 22:17 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2018-10-29 22:14 [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off Anusha Srivatsa
2018-10-30 17:56 ` Rodrigo Vivi
2018-10-24 23:01 Anusha Srivatsa
2018-10-24 23:22 ` Rodrigo Vivi
2018-09-19 20:06 Anusha Srivatsa
2018-10-23 10:01 ` Yadav, Jyoti R
2018-09-13 19:31 Anusha Srivatsa
2018-09-13 20:13 ` Rodrigo Vivi
2018-09-17 18:45 ` Srivatsa, Anusha
2018-09-10 19:10 Anusha Srivatsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.