[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Srivatsa, Anusha]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Rodrigo Vivi]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[James Ausmus]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Rodrigo Vivi]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[James Ausmus]

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

✓ Fi.CI.BAT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)

[Patchwork]

== 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

✓ Fi.CI.IGT: success for drm/i915/icl: Enable DC9 as lowest possible state during screen-off (rev4)

[Patchwork]

== 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

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Rodrigo Vivi]

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

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Rodrigo Vivi]

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

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Yadav, Jyoti R]

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

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Rodrigo Vivi]

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

Re: [PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Srivatsa, Anusha]

>-----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

[PATCH] drm/i915/icl: Enable DC9 as lowest possible state during screen-off

[Anusha Srivatsa]

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