* [PATCH v7 1/7] drm/i915/tgl: Add DC3CO required register and bits
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-11 9:08 ` Animesh Manna
2019-09-07 17:14 ` [PATCH v7 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
` (13 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Adding following definition to i915_reg.h
1. DC_STATE_EN register DC3CO bit fields and masks.
2. Transcoder EXITLINE register and its bit fields and mask.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 006cffd56be2..273a4ed8b3e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4135,6 +4135,7 @@ enum {
#define _VTOTAL_A 0x6000c
#define _VBLANK_A 0x60010
#define _VSYNC_A 0x60014
+#define _EXITLINE_A 0x60018
#define _PIPEASRC 0x6001c
#define _BCLRPAT_A 0x60020
#define _VSYNCSHIFT_A 0x60028
@@ -4181,11 +4182,16 @@ enum {
#define VTOTAL(trans) _MMIO_TRANS2(trans, _VTOTAL_A)
#define VBLANK(trans) _MMIO_TRANS2(trans, _VBLANK_A)
#define VSYNC(trans) _MMIO_TRANS2(trans, _VSYNC_A)
+#define EXITLINE(trans) _MMIO_TRANS2(trans, _EXITLINE_A)
#define BCLRPAT(trans) _MMIO_TRANS2(trans, _BCLRPAT_A)
#define VSYNCSHIFT(trans) _MMIO_TRANS2(trans, _VSYNCSHIFT_A)
#define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
#define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
+#define EXITLINE_ENABLE (1 << 31)
+#define EXITLINE_MASK (0x1fff)
+#define EXITLINE_SHIFT 0
+
/*
* HSW+ eDP PSR registers
*
@@ -10114,6 +10120,8 @@ enum skl_power_gate {
/* GEN9 DC */
#define DC_STATE_EN _MMIO(0x45504)
#define DC_STATE_DISABLE 0
+#define DC_STATE_EN_DC3CO (1 << 30)
+#define DC_STATE_DC3CO_STATUS (1 << 29)
#define DC_STATE_EN_UPTO_DC5 (1 << 0)
#define DC_STATE_EN_DC9 (1 << 3)
#define DC_STATE_EN_UPTO_DC6 (2 << 0)
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 1/7] drm/i915/tgl: Add DC3CO required register and bits
2019-09-07 17:14 ` [PATCH v7 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
@ 2019-09-11 9:08 ` Animesh Manna
0 siblings, 0 replies; 28+ messages in thread
From: Animesh Manna @ 2019-09-11 9:08 UTC (permalink / raw)
To: Anshuman Gupta, intel-gfx; +Cc: jani.nikula
On 9/7/2019 10:44 PM, Anshuman Gupta wrote:
> Adding following definition to i915_reg.h
> 1. DC_STATE_EN register DC3CO bit fields and masks.
> 2. Transcoder EXITLINE register and its bit fields and mask.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 006cffd56be2..273a4ed8b3e9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4135,6 +4135,7 @@ enum {
> #define _VTOTAL_A 0x6000c
> #define _VBLANK_A 0x60010
> #define _VSYNC_A 0x60014
> +#define _EXITLINE_A 0x60018
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> @@ -4181,11 +4182,16 @@ enum {
> #define VTOTAL(trans) _MMIO_TRANS2(trans, _VTOTAL_A)
> #define VBLANK(trans) _MMIO_TRANS2(trans, _VBLANK_A)
> #define VSYNC(trans) _MMIO_TRANS2(trans, _VSYNC_A)
> +#define EXITLINE(trans) _MMIO_TRANS2(trans, _EXITLINE_A)
> #define BCLRPAT(trans) _MMIO_TRANS2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _MMIO_TRANS2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
> #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A)
>
> +#define EXITLINE_ENABLE (1 << 31)
Need one extra space and same applicable below.
REG_BIT can be used. Please refer kernel doc added as header of this file.
Regards,
Animesh
> +#define EXITLINE_MASK (0x1fff)
> +#define EXITLINE_SHIFT 0
> +
> /*
> * HSW+ eDP PSR registers
> *
> @@ -10114,6 +10120,8 @@ enum skl_power_gate {
> /* GEN9 DC */
> #define DC_STATE_EN _MMIO(0x45504)
> #define DC_STATE_DISABLE 0
> +#define DC_STATE_EN_DC3CO (1 << 30)
> +#define DC_STATE_DC3CO_STATUS (1 << 29)
> #define DC_STATE_EN_UPTO_DC5 (1 << 0)
> #define DC_STATE_EN_DC9 (1 << 3)
> #define DC_STATE_EN_UPTO_DC6 (2 << 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
2019-09-07 17:14 ` [PATCH v7 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-11 9:12 ` Animesh Manna
2019-09-07 17:14 ` [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
` (12 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Enable dc3co state in enable_dc module param and add dc3co
enable mask to allowed_dc_mask and gen9_dc_mask.
v1: Adding enable_dc=3,4 options to enable DC3CO with DC5 and DC6
independently.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
.../drm/i915/display/intel_display_power.c | 29 ++++++++++++++-----
drivers/gpu/drm/i915/i915_params.c | 3 +-
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index ce88a27229ef..496fa1b53ffb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -698,7 +698,11 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
u32 mask;
mask = DC_STATE_EN_UPTO_DC5;
- if (INTEL_GEN(dev_priv) >= 11)
+
+ if (INTEL_GEN(dev_priv) >= 12)
+ mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6
+ | DC_STATE_EN_DC9;
+ else if (IS_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;
@@ -3927,14 +3931,17 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
int requested_dc;
int max_dc;
- if (INTEL_GEN(dev_priv) >= 11) {
- max_dc = 2;
+ if (INTEL_GEN(dev_priv) >= 12) {
+ max_dc = 4;
/*
* 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_GEN(dev_priv, 11)) {
+ max_dc = 2;
+ mask = DC_STATE_EN_DC9;
} else if (IS_GEN(dev_priv, 10) || IS_GEN9_BC(dev_priv)) {
max_dc = 2;
mask = 0;
@@ -3953,7 +3960,7 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
requested_dc = enable_dc;
} else if (enable_dc == -1) {
requested_dc = max_dc;
- } else if (enable_dc > max_dc && enable_dc <= 2) {
+ } else if (enable_dc > max_dc && enable_dc <= 4) {
DRM_DEBUG_KMS("Adjusting requested max DC state (%d->%d)\n",
enable_dc, max_dc);
requested_dc = max_dc;
@@ -3962,10 +3969,16 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
requested_dc = max_dc;
}
- if (requested_dc > 1)
- mask |= DC_STATE_EN_UPTO_DC6;
- if (requested_dc > 0)
- mask |= DC_STATE_EN_UPTO_DC5;
+ if (requested_dc == 4) {
+ mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6;
+ } else if (requested_dc == 3) {
+ mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC5;
+ } else {
+ if (requested_dc > 1)
+ mask |= DC_STATE_EN_UPTO_DC6;
+ if (requested_dc > 0)
+ mask |= DC_STATE_EN_UPTO_DC5;
+ }
DRM_DEBUG_KMS("Allowed DC state mask %02x\n", mask);
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 296452f9efe4..4f1806f65040 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -46,7 +46,8 @@ i915_param_named(modeset, int, 0400,
i915_param_named_unsafe(enable_dc, int, 0400,
"Enable power-saving display C-states. "
- "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
+ "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6; "
+ "3=up to DC5 with DC3CO; 4=up to DC6 with DC3CO)");
i915_param_named_unsafe(enable_fbc, int, 0600,
"Enable frame buffer compression for power savings "
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
2019-09-07 17:14 ` [PATCH v7 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
@ 2019-09-11 9:12 ` Animesh Manna
0 siblings, 0 replies; 28+ messages in thread
From: Animesh Manna @ 2019-09-11 9:12 UTC (permalink / raw)
To: Anshuman Gupta, intel-gfx; +Cc: jani.nikula
Hi,
On 9/7/2019 10:44 PM, Anshuman Gupta wrote:
> Enable dc3co state in enable_dc module param and add dc3co
> enable mask to allowed_dc_mask and gen9_dc_mask.
>
> v1: Adding enable_dc=3,4 options to enable DC3CO with DC5 and DC6
> independently.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> .../drm/i915/display/intel_display_power.c | 29 ++++++++++++++-----
> drivers/gpu/drm/i915/i915_params.c | 3 +-
> 2 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index ce88a27229ef..496fa1b53ffb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -698,7 +698,11 @@ static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
> u32 mask;
>
> mask = DC_STATE_EN_UPTO_DC5;
> - if (INTEL_GEN(dev_priv) >= 11)
> +
> + if (INTEL_GEN(dev_priv) >= 12)
> + mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6
> + | DC_STATE_EN_DC9;
> + else if (IS_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;
> @@ -3927,14 +3931,17 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> int requested_dc;
> int max_dc;
>
> - if (INTEL_GEN(dev_priv) >= 11) {
> - max_dc = 2;
> + if (INTEL_GEN(dev_priv) >= 12) {
> + max_dc = 4;
> /*
> * 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_GEN(dev_priv, 11)) {
> + max_dc = 2;
> + mask = DC_STATE_EN_DC9;
> } else if (IS_GEN(dev_priv, 10) || IS_GEN9_BC(dev_priv)) {
> max_dc = 2;
> mask = 0;
> @@ -3953,7 +3960,7 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> requested_dc = enable_dc;
> } else if (enable_dc == -1) {
> requested_dc = max_dc;
> - } else if (enable_dc > max_dc && enable_dc <= 2) {
> + } else if (enable_dc > max_dc && enable_dc <= 4) {
> DRM_DEBUG_KMS("Adjusting requested max DC state (%d->%d)\n",
> enable_dc, max_dc);
> requested_dc = max_dc;
> @@ -3962,10 +3969,16 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> requested_dc = max_dc;
> }
>
> - if (requested_dc > 1)
> - mask |= DC_STATE_EN_UPTO_DC6;
> - if (requested_dc > 0)
> - mask |= DC_STATE_EN_UPTO_DC5;
> + if (requested_dc == 4) {
> + mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6;
> + } else if (requested_dc == 3) {
> + mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC5;
> + } else {
> + if (requested_dc > 1)
> + mask |= DC_STATE_EN_UPTO_DC6;
> + if (requested_dc > 0)
> + mask |= DC_STATE_EN_UPTO_DC5;
> + }
Using switch statement will solve the purpose and code will look cleaner.
Regards,
Animesh
>
> DRM_DEBUG_KMS("Allowed DC state mask %02x\n", mask);
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 296452f9efe4..4f1806f65040 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -46,7 +46,8 @@ i915_param_named(modeset, int, 0400,
>
> i915_param_named_unsafe(enable_dc, int, 0400,
> "Enable power-saving display C-states. "
> - "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
> + "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6; "
> + "3=up to DC5 with DC3CO; 4=up to DC6 with DC3CO)");
>
> i915_param_named_unsafe(enable_fbc, int, 0600,
> "Enable frame buffer compression for power savings "
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
2019-09-07 17:14 ` [PATCH v7 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
2019-09-07 17:14 ` [PATCH v7 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-08 16:44 ` Imre Deak
2019-09-07 17:14 ` [PATCH v7 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
` (11 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Add max_dc_state and tgl_set_target_dc_state() API
in order to enable DC3CO state with existing DC states.
max_dc_state will enable/disable the desired DC state in
DC_STATE_EN reg when "DC Off" power well gets disable/enable.
v2: commit log improvement.
v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
to a appropriate place haswell_crtc_enable(). [Imre]
Changed the DC3CO power well enabled call back logic as
recommended in review comments. [Imre]
v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
v5: using udelay() instead of waiting for DC3CO exit status.
v6: Fixed minor unwanted change.
v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
.../drm/i915/display/intel_display_power.c | 111 ++++++++++++++----
.../drm/i915/display/intel_display_power.h | 3 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
3 files changed, 95 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 496fa1b53ffb..83b10f61ee42 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -772,6 +772,29 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
dev_priv->csr.dc_state = val & mask;
}
+static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
+{
+ if (!dev_priv->psr.sink_psr2_support)
+ return;
+
+ if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)
+ gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
+}
+
+static void tgl_disallow_dc3co(struct drm_i915_private *dev_priv)
+{
+ u32 val;
+
+ val = I915_READ(DC_STATE_EN);
+ val &= ~DC_STATE_DC3CO_STATUS;
+ I915_WRITE(DC_STATE_EN, val);
+ gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
+ /*
+ * Delay of 200us DC3CO Exit time B.Spec 49196
+ */
+ udelay(200);
+}
+
static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
{
assert_can_enable_dc9(dev_priv);
@@ -939,7 +962,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
- return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
+ return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
+ (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
}
static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
@@ -955,24 +979,32 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state cdclk_state = {};
- gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
+ if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
+ tgl_disallow_dc3co(dev_priv);
+ } else {
+ gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
- dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
- /* Can't read out voltage_level so can't use intel_cdclk_changed() */
- WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw, &cdclk_state));
+ dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
+ /*
+ * Can't read out voltage_level so can't use
+ * intel_cdclk_changed()
+ */
+ WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw,
+ &cdclk_state));
- gen9_assert_dbuf_enabled(dev_priv);
+ gen9_assert_dbuf_enabled(dev_priv);
- if (IS_GEN9_LP(dev_priv))
- bxt_verify_ddi_phy_power_wells(dev_priv);
+ if (IS_GEN9_LP(dev_priv))
+ bxt_verify_ddi_phy_power_wells(dev_priv);
- if (INTEL_GEN(dev_priv) >= 11)
- /*
- * DMC retains HW context only for port A, the other combo
- * PHY's HW context for port B is lost after DC transitions,
- * so we need to restore it manually.
- */
- intel_combo_phy_init(dev_priv);
+ if (INTEL_GEN(dev_priv) >= 11)
+ /*
+ * DMC retains HW context only for port A, the other
+ * combo PHY's HW context for port B is lost after
+ * DC transitions, so we need to restore it manually.
+ */
+ intel_combo_phy_init(dev_priv);
+ }
}
static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
@@ -987,10 +1019,48 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
if (!dev_priv->csr.dmc_payload)
return;
- 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);
+ if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
+ tgl_allow_dc3co(dev_priv);
+ } else if (dev_priv->csr.max_dc_state & DC_STATE_EN_UPTO_DC5_DC6_MASK) {
+ 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);
+ }
+}
+
+void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
+ bool psr2_deep_sleep)
+{
+ struct i915_power_well *power_well;
+ bool dc_off_enabled;
+ struct i915_power_domains *power_domains = &dev_priv->power_domains;
+
+ mutex_lock(&power_domains->lock);
+ power_well = lookup_power_well(dev_priv, TGL_DISP_DC_OFF);
+
+ if (!power_well)
+ goto unlock;
+
+ dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
+ power_well);
+ if (state == dev_priv->csr.max_dc_state)
+ goto unlock;
+
+ if (!dc_off_enabled) {
+ /*
+ * Need to disable the DC off power well to
+ * effect target DC state.
+ */
+ power_well->desc->ops->enable(dev_priv, power_well);
+ dev_priv->csr.max_dc_state = state;
+ power_well->desc->ops->disable(dev_priv, power_well);
+ goto unlock;
+ }
+ dev_priv->csr.max_dc_state = state;
+
+unlock:
+ mutex_unlock(&power_domains->lock);
}
static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
@@ -3610,7 +3680,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
.name = "DC off",
.domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
.ops = &gen9_dc_off_power_well_ops,
- .id = DISP_PW_ID_NONE,
+ .id = TGL_DISP_DC_OFF,
},
{
.name = "power well 2",
@@ -4039,6 +4109,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
dev_priv->csr.allowed_dc_mask =
get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
+ dev_priv->csr.max_dc_state = DC_STATE_EN_UPTO_DC5_DC6_MASK;
BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
mutex_init(&power_domains->lock);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 737b5def7fc6..69ebde992342 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -100,6 +100,7 @@ enum i915_power_well_id {
SKL_DISP_PW_MISC_IO,
SKL_DISP_PW_1,
SKL_DISP_PW_2,
+ TGL_DISP_DC_OFF,
};
#define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
@@ -256,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
void intel_display_power_resume_early(struct drm_i915_private *i915);
void intel_display_power_suspend(struct drm_i915_private *i915);
void intel_display_power_resume(struct drm_i915_private *i915);
+void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
+ bool psr2_deep_sleep);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index db7480831e52..999da5d2da0b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -336,6 +336,7 @@ struct intel_csr {
i915_reg_t mmioaddr[20];
u32 mmiodata[20];
u32 dc_state;
+ u32 max_dc_state;
u32 allowed_dc_mask;
intel_wakeref_t wakeref;
};
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
2019-09-07 17:14 ` [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
@ 2019-09-08 16:44 ` Imre Deak
2019-09-09 16:19 ` Anshuman Gupta
0 siblings, 1 reply; 28+ messages in thread
From: Imre Deak @ 2019-09-08 16:44 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Sat, Sep 07, 2019 at 10:44:39PM +0530, Anshuman Gupta wrote:
> Add max_dc_state and tgl_set_target_dc_state() API
> in order to enable DC3CO state with existing DC states.
> max_dc_state will enable/disable the desired DC state in
> DC_STATE_EN reg when "DC Off" power well gets disable/enable.
>
> v2: commit log improvement.
> v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
> Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
> Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
> to a appropriate place haswell_crtc_enable(). [Imre]
> Changed the DC3CO power well enabled call back logic as
> recommended in review comments. [Imre]
> v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
> v5: using udelay() instead of waiting for DC3CO exit status.
> v6: Fixed minor unwanted change.
> v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> .../drm/i915/display/intel_display_power.c | 111 ++++++++++++++----
> .../drm/i915/display/intel_display_power.h | 3 +
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 3 files changed, 95 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 496fa1b53ffb..83b10f61ee42 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -772,6 +772,29 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> dev_priv->csr.dc_state = val & mask;
> }
>
> +static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
Should be tgl_enable_dc3co(), to match the rest of DC state helpers.
> +{
> + if (!dev_priv->psr.sink_psr2_support)
> + return;
PSR knows when to enable DC3co, so no need to double-check that here.
> +
> + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)
This check is out-of-place wrt. the same checks for other DC states.
> + gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> +}
> +
> +static void tgl_disallow_dc3co(struct drm_i915_private *dev_priv)
> +{
> + u32 val;
> +
> + val = I915_READ(DC_STATE_EN);
> + val &= ~DC_STATE_DC3CO_STATUS;
> + I915_WRITE(DC_STATE_EN, val);
> + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> + /*
> + * Delay of 200us DC3CO Exit time B.Spec 49196
> + */
> + udelay(200);
> +}
> +
> static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> {
> assert_can_enable_dc9(dev_priv);
> @@ -939,7 +962,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
> static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> + return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
> + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
> }
>
> static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> @@ -955,24 +979,32 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state cdclk_state = {};
>
> - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> + tgl_disallow_dc3co(dev_priv);
> + } else {
With an early return you can avoid the extra diff and make reviewing
easier.
> + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>
> - dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> - /* Can't read out voltage_level so can't use intel_cdclk_changed() */
> - WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw, &cdclk_state));
> + dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> + /*
> + * Can't read out voltage_level so can't use
> + * intel_cdclk_changed()
> + */
> + WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw,
> + &cdclk_state));
>
> - gen9_assert_dbuf_enabled(dev_priv);
> + gen9_assert_dbuf_enabled(dev_priv);
>
> - if (IS_GEN9_LP(dev_priv))
> - bxt_verify_ddi_phy_power_wells(dev_priv);
> + if (IS_GEN9_LP(dev_priv))
> + bxt_verify_ddi_phy_power_wells(dev_priv);
>
> - if (INTEL_GEN(dev_priv) >= 11)
> - /*
> - * DMC retains HW context only for port A, the other combo
> - * PHY's HW context for port B is lost after DC transitions,
> - * so we need to restore it manually.
> - */
> - intel_combo_phy_init(dev_priv);
> + if (INTEL_GEN(dev_priv) >= 11)
> + /*
> + * DMC retains HW context only for port A, the other
> + * combo PHY's HW context for port B is lost after
> + * DC transitions, so we need to restore it manually.
> + */
> + intel_combo_phy_init(dev_priv);
> + }
> }
>
> static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> @@ -987,10 +1019,48 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> if (!dev_priv->csr.dmc_payload)
> return;
>
> - 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);
> + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
target_dc_state would be a better name and it shold be an exact state
instead of a mask.
> + tgl_allow_dc3co(dev_priv);
> + } else if (dev_priv->csr.max_dc_state & DC_STATE_EN_UPTO_DC5_DC6_MASK) {
> + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
We should make these checks more uniform, by checking here only
target_dc_state.
> + skl_enable_dc6(dev_priv);
> + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> + gen9_enable_dc5(dev_priv);
> + }
> +}
> +
> +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> + bool psr2_deep_sleep)
psr2_deep_sleep is a PSR internal stuff, while PSR is using the power
domains framework, so let's keep the deep sleep programming in the PSR
code.
> +{
> + struct i915_power_well *power_well;
> + bool dc_off_enabled;
> + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +
> + mutex_lock(&power_domains->lock);
> + power_well = lookup_power_well(dev_priv, TGL_DISP_DC_OFF);
> +
> + if (!power_well)
> + goto unlock;
> +
> + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> + power_well);
> + if (state == dev_priv->csr.max_dc_state)
> + goto unlock;
> +
> + if (!dc_off_enabled) {
> + /*
> + * Need to disable the DC off power well to
> + * effect target DC state.
> + */
> + power_well->desc->ops->enable(dev_priv, power_well);
> + dev_priv->csr.max_dc_state = state;
> + power_well->desc->ops->disable(dev_priv, power_well);
> + goto unlock;
> + }
> + dev_priv->csr.max_dc_state = state;
> +
> +unlock:
> + mutex_unlock(&power_domains->lock);
> }
>
> static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> @@ -3610,7 +3680,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> .name = "DC off",
> .domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
> .ops = &gen9_dc_off_power_well_ops,
> - .id = DISP_PW_ID_NONE,
> + .id = TGL_DISP_DC_OFF,
Let's assign this ID on all platforms for consistency.
> },
> {
> .name = "power well 2",
> @@ -4039,6 +4109,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> dev_priv->csr.allowed_dc_mask =
> get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
>
> + dev_priv->csr.max_dc_state = DC_STATE_EN_UPTO_DC5_DC6_MASK;
> BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
>
> mutex_init(&power_domains->lock);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 737b5def7fc6..69ebde992342 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -100,6 +100,7 @@ enum i915_power_well_id {
> SKL_DISP_PW_MISC_IO,
> SKL_DISP_PW_1,
> SKL_DISP_PW_2,
> + TGL_DISP_DC_OFF,
> };
>
> #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> @@ -256,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
> void intel_display_power_resume_early(struct drm_i915_private *i915);
> void intel_display_power_suspend(struct drm_i915_private *i915);
> void intel_display_power_resume(struct drm_i915_private *i915);
> +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> + bool psr2_deep_sleep);
>
> const char *
> intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index db7480831e52..999da5d2da0b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -336,6 +336,7 @@ struct intel_csr {
> i915_reg_t mmioaddr[20];
> u32 mmiodata[20];
> u32 dc_state;
> + u32 max_dc_state;
> u32 allowed_dc_mask;
> intel_wakeref_t wakeref;
> };
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
2019-09-08 16:44 ` Imre Deak
@ 2019-09-09 16:19 ` Anshuman Gupta
2019-09-11 8:21 ` Imre Deak
0 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-09 16:19 UTC (permalink / raw)
To: Imre Deak; +Cc: jani.nikula, intel-gfx
On 2019-09-08 at 19:44:35 +0300, Imre Deak wrote:
> On Sat, Sep 07, 2019 at 10:44:39PM +0530, Anshuman Gupta wrote:
Hi Imre,
Thanks for reviewing the pacthes i will rework the patches.
There are few comments from my side which will help to rework.
> > Add max_dc_state and tgl_set_target_dc_state() API
> > in order to enable DC3CO state with existing DC states.
> > max_dc_state will enable/disable the desired DC state in
> > DC_STATE_EN reg when "DC Off" power well gets disable/enable.
> >
> > v2: commit log improvement.
> > v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
> > Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
> > Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
> > to a appropriate place haswell_crtc_enable(). [Imre]
> > Changed the DC3CO power well enabled call back logic as
> > recommended in review comments. [Imre]
> > v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
> > v5: using udelay() instead of waiting for DC3CO exit status.
> > v6: Fixed minor unwanted change.
> > v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > .../drm/i915/display/intel_display_power.c | 111 ++++++++++++++----
> > .../drm/i915/display/intel_display_power.h | 3 +
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > 3 files changed, 95 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 496fa1b53ffb..83b10f61ee42 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -772,6 +772,29 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > dev_priv->csr.dc_state = val & mask;
> > }
> >
> > +static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
>
> Should be tgl_enable_dc3co(), to match the rest of DC state helpers.
>
> > +{
> > + if (!dev_priv->psr.sink_psr2_support)
> > + return;
>
> PSR knows when to enable DC3co, so no need to double-check that here.
>
> > +
> > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)
>
> This check is out-of-place wrt. the same checks for other DC states.
>
> > + gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> > +}
> > +
> > +static void tgl_disallow_dc3co(struct drm_i915_private *dev_priv)
> > +{
> > + u32 val;
> > +
> > + val = I915_READ(DC_STATE_EN);
> > + val &= ~DC_STATE_DC3CO_STATUS;
> > + I915_WRITE(DC_STATE_EN, val);
> > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > + /*
> > + * Delay of 200us DC3CO Exit time B.Spec 49196
> > + */
> > + udelay(200);
> > +}
> > +
> > static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > {
> > assert_can_enable_dc9(dev_priv);
> > @@ -939,7 +962,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
> > static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > - return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> > + return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
> > + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
> > }
> >
> > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> > @@ -955,24 +979,32 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> > {
> > struct intel_cdclk_state cdclk_state = {};
> >
> > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> > + tgl_disallow_dc3co(dev_priv);
> > + } else {
>
> With an early return you can avoid the extra diff and make reviewing
> easier.
>
> > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > - dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > - /* Can't read out voltage_level so can't use intel_cdclk_changed() */
> > - WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw, &cdclk_state));
> > + dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > + /*
> > + * Can't read out voltage_level so can't use
> > + * intel_cdclk_changed()
> > + */
> > + WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw,
> > + &cdclk_state));
> >
> > - gen9_assert_dbuf_enabled(dev_priv);
> > + gen9_assert_dbuf_enabled(dev_priv);
> >
> > - if (IS_GEN9_LP(dev_priv))
> > - bxt_verify_ddi_phy_power_wells(dev_priv);
> > + if (IS_GEN9_LP(dev_priv))
> > + bxt_verify_ddi_phy_power_wells(dev_priv);
> >
> > - if (INTEL_GEN(dev_priv) >= 11)
> > - /*
> > - * DMC retains HW context only for port A, the other combo
> > - * PHY's HW context for port B is lost after DC transitions,
> > - * so we need to restore it manually.
> > - */
> > - intel_combo_phy_init(dev_priv);
> > + if (INTEL_GEN(dev_priv) >= 11)
> > + /*
> > + * DMC retains HW context only for port A, the other
> > + * combo PHY's HW context for port B is lost after
> > + * DC transitions, so we need to restore it manually.
> > + */
> > + intel_combo_phy_init(dev_priv);
> > + }
> > }
> >
> > static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> > @@ -987,10 +1019,48 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> > if (!dev_priv->csr.dmc_payload)
> > return;
> >
> > - 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);
> > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
>
> target_dc_state would be a better name and it shold be an exact state
> instead of a mask.
>
> > + tgl_allow_dc3co(dev_priv);
> > + } else if (dev_priv->csr.max_dc_state & DC_STATE_EN_UPTO_DC5_DC6_MASK) {
> > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
>
> We should make these checks more uniform, by checking here only
> target_dc_state.
If i understand correctly this comment is about to have a uniform
condition target_dc_state in order to decide which DC state we
interested. Then we may require to check allowed_dc_mask accordingly in
tgl_set_target_dc_state(), but that would not be fitting for
older platforms.
What is your input on this? please correct me if i am wrong here.
>
> > + skl_enable_dc6(dev_priv);
> > + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > + gen9_enable_dc5(dev_priv);
> > + }
> > +}
> > +
> > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > + bool psr2_deep_sleep)
>
> psr2_deep_sleep is a PSR internal stuff, while PSR is using the power
> domains framework, so let's keep the deep sleep programming in the PSR
> code.
I wanted not to enable/disable psr2 deep sleep for each flip flush request.
Though PSR2 deep sleep is PSR internal stuff but it is H/W has tied with DC3CO.
That was intention to keep psr2_deep sleep enable/diable here (Doing in Patch 6).
In order to switch between DC3CO and DC5 we need to call
tgl_psr2_deep_sleep_disable/enable() from outside of PSR code.
please correct me if i am wrong here.
Thanks ,
Anshuman Gupta.
>
> > +{
> > + struct i915_power_well *power_well;
> > + bool dc_off_enabled;
> > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > +
> > + mutex_lock(&power_domains->lock);
> > + power_well = lookup_power_well(dev_priv, TGL_DISP_DC_OFF);
> > +
> > + if (!power_well)
> > + goto unlock;
> > +
> > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> > + power_well);
> > + if (state == dev_priv->csr.max_dc_state)
> > + goto unlock;
> > +
> > + if (!dc_off_enabled) {
> > + /*
> > + * Need to disable the DC off power well to
> > + * effect target DC state.
> > + */
> > + power_well->desc->ops->enable(dev_priv, power_well);
> > + dev_priv->csr.max_dc_state = state;
> > + power_well->desc->ops->disable(dev_priv, power_well);
> > + goto unlock;
> > + }
> > + dev_priv->csr.max_dc_state = state;
> > +
> > +unlock:
> > + mutex_unlock(&power_domains->lock);
> > }
> >
> > static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> > @@ -3610,7 +3680,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > .name = "DC off",
> > .domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > .ops = &gen9_dc_off_power_well_ops,
> > - .id = DISP_PW_ID_NONE,
> > + .id = TGL_DISP_DC_OFF,
>
> Let's assign this ID on all platforms for consistency.
IMO that should be done in different patch, as tgl_set_target_dc_state() is
only consumer of this id ?
What is your opinion on this ?
>
> > },
> > {
> > .name = "power well 2",
> > @@ -4039,6 +4109,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> > dev_priv->csr.allowed_dc_mask =
> > get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
> >
> > + dev_priv->csr.max_dc_state = DC_STATE_EN_UPTO_DC5_DC6_MASK;
> > BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
> >
> > mutex_init(&power_domains->lock);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 737b5def7fc6..69ebde992342 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -100,6 +100,7 @@ enum i915_power_well_id {
> > SKL_DISP_PW_MISC_IO,
> > SKL_DISP_PW_1,
> > SKL_DISP_PW_2,
> > + TGL_DISP_DC_OFF,
> > };
> >
> > #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> > @@ -256,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
> > void intel_display_power_resume_early(struct drm_i915_private *i915);
> > void intel_display_power_suspend(struct drm_i915_private *i915);
> > void intel_display_power_resume(struct drm_i915_private *i915);
> > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > + bool psr2_deep_sleep);
> >
> > const char *
> > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index db7480831e52..999da5d2da0b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -336,6 +336,7 @@ struct intel_csr {
> > i915_reg_t mmioaddr[20];
> > u32 mmiodata[20];
> > u32 dc_state;
> > + u32 max_dc_state;
> > u32 allowed_dc_mask;
> > intel_wakeref_t wakeref;
> > };
> > --
> > 2.21.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
2019-09-09 16:19 ` Anshuman Gupta
@ 2019-09-11 8:21 ` Imre Deak
2019-09-11 12:42 ` Anshuman Gupta
0 siblings, 1 reply; 28+ messages in thread
From: Imre Deak @ 2019-09-11 8:21 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Mon, Sep 09, 2019 at 09:49:17PM +0530, Anshuman Gupta wrote:
> On 2019-09-08 at 19:44:35 +0300, Imre Deak wrote:
> > On Sat, Sep 07, 2019 at 10:44:39PM +0530, Anshuman Gupta wrote:
> Hi Imre,
> Thanks for reviewing the pacthes i will rework the patches.
> There are few comments from my side which will help to rework.
> > > Add max_dc_state and tgl_set_target_dc_state() API
> > > in order to enable DC3CO state with existing DC states.
> > > max_dc_state will enable/disable the desired DC state in
> > > DC_STATE_EN reg when "DC Off" power well gets disable/enable.
> > >
> > > v2: commit log improvement.
> > > v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
> > > Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
> > > Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
> > > to a appropriate place haswell_crtc_enable(). [Imre]
> > > Changed the DC3CO power well enabled call back logic as
> > > recommended in review comments. [Imre]
> > > v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
> > > v5: using udelay() instead of waiting for DC3CO exit status.
> > > v6: Fixed minor unwanted change.
> > > v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
> > >
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > > .../drm/i915/display/intel_display_power.c | 111 ++++++++++++++----
> > > .../drm/i915/display/intel_display_power.h | 3 +
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > 3 files changed, 95 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 496fa1b53ffb..83b10f61ee42 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -772,6 +772,29 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > > dev_priv->csr.dc_state = val & mask;
> > > }
> > >
> > > +static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> >
> > Should be tgl_enable_dc3co(), to match the rest of DC state helpers.
> >
> > > +{
> > > + if (!dev_priv->psr.sink_psr2_support)
> > > + return;
> >
> > PSR knows when to enable DC3co, so no need to double-check that here.
> >
> > > +
> > > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)
> >
> > This check is out-of-place wrt. the same checks for other DC states.
> >
> > > + gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> > > +}
> > > +
> > > +static void tgl_disallow_dc3co(struct drm_i915_private *dev_priv)
> > > +{
> > > + u32 val;
> > > +
> > > + val = I915_READ(DC_STATE_EN);
> > > + val &= ~DC_STATE_DC3CO_STATUS;
> > > + I915_WRITE(DC_STATE_EN, val);
> > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > + /*
> > > + * Delay of 200us DC3CO Exit time B.Spec 49196
> > > + */
> > > + udelay(200);
> > > +}
> > > +
> > > static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > {
> > > assert_can_enable_dc9(dev_priv);
> > > @@ -939,7 +962,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
> > > static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> > > struct i915_power_well *power_well)
> > > {
> > > - return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> > > + return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
> > > + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
> > > }
> > >
> > > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> > > @@ -955,24 +979,32 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> > > {
> > > struct intel_cdclk_state cdclk_state = {};
> > >
> > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> > > + tgl_disallow_dc3co(dev_priv);
> > > + } else {
> >
> > With an early return you can avoid the extra diff and make reviewing
> > easier.
> >
> > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >
> > > - dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > > - /* Can't read out voltage_level so can't use intel_cdclk_changed() */
> > > - WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw, &cdclk_state));
> > > + dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > > + /*
> > > + * Can't read out voltage_level so can't use
> > > + * intel_cdclk_changed()
> > > + */
> > > + WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw,
> > > + &cdclk_state));
> > >
> > > - gen9_assert_dbuf_enabled(dev_priv);
> > > + gen9_assert_dbuf_enabled(dev_priv);
> > >
> > > - if (IS_GEN9_LP(dev_priv))
> > > - bxt_verify_ddi_phy_power_wells(dev_priv);
> > > + if (IS_GEN9_LP(dev_priv))
> > > + bxt_verify_ddi_phy_power_wells(dev_priv);
> > >
> > > - if (INTEL_GEN(dev_priv) >= 11)
> > > - /*
> > > - * DMC retains HW context only for port A, the other combo
> > > - * PHY's HW context for port B is lost after DC transitions,
> > > - * so we need to restore it manually.
> > > - */
> > > - intel_combo_phy_init(dev_priv);
> > > + if (INTEL_GEN(dev_priv) >= 11)
> > > + /*
> > > + * DMC retains HW context only for port A, the other
> > > + * combo PHY's HW context for port B is lost after
> > > + * DC transitions, so we need to restore it manually.
> > > + */
> > > + intel_combo_phy_init(dev_priv);
> > > + }
> > > }
> > >
> > > static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> > > @@ -987,10 +1019,48 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> > > if (!dev_priv->csr.dmc_payload)
> > > return;
> > >
> > > - 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);
> > > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> >
> > target_dc_state would be a better name and it shold be an exact state
> > instead of a mask.
> >
> > > + tgl_allow_dc3co(dev_priv);
> > > + } else if (dev_priv->csr.max_dc_state & DC_STATE_EN_UPTO_DC5_DC6_MASK) {
> > > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> >
> > We should make these checks more uniform, by checking here only
> > target_dc_state.
> If i understand correctly this comment is about to have a uniform
> condition target_dc_state in order to decide which DC state we
> interested. Then we may require to check allowed_dc_mask accordingly in
> tgl_set_target_dc_state(),
Yes, we could check here only target_dc_state, setting that one only to
values permitted by allowed_dc_mask.
> but that would not be fitting for
> older platforms.
> What is your input on this? please correct me if i am wrong here.
> >
> > > + skl_enable_dc6(dev_priv);
> > > + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > > + gen9_enable_dc5(dev_priv);
> > > + }
> > > +}
> > > +
> > > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > + bool psr2_deep_sleep)
> >
> > psr2_deep_sleep is a PSR internal stuff, while PSR is using the power
> > domains framework, so let's keep the deep sleep programming in the PSR
> > code.
> I wanted not to enable/disable psr2 deep sleep for each flip flush request.
> Though PSR2 deep sleep is PSR internal stuff but it is H/W has tied with DC3CO.
> That was intention to keep psr2_deep sleep enable/diable here (Doing in Patch 6).
> In order to switch between DC3CO and DC5 we need to call
> tgl_psr2_deep_sleep_disable/enable() from outside of PSR code.
> please correct me if i am wrong here.
I think it still belongs to the PSR code with other steps needed for
DC3co setup, like the idle frame programming. PSR deep sleep could also
be enabled/disabled independently of the DC state we enable (for
instance enable/disable deep sleep when both DC3co and DC5 is
disabled). It also needs the PSR lock, which again is internal to the
PSR code.
> Thanks ,
> Anshuman Gupta.
> >
> > > +{
> > > + struct i915_power_well *power_well;
> > > + bool dc_off_enabled;
> > > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > +
> > > + mutex_lock(&power_domains->lock);
> > > + power_well = lookup_power_well(dev_priv, TGL_DISP_DC_OFF);
> > > +
> > > + if (!power_well)
> > > + goto unlock;
> > > +
> > > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> > > + power_well);
> > > + if (state == dev_priv->csr.max_dc_state)
> > > + goto unlock;
> > > +
> > > + if (!dc_off_enabled) {
> > > + /*
> > > + * Need to disable the DC off power well to
> > > + * effect target DC state.
> > > + */
> > > + power_well->desc->ops->enable(dev_priv, power_well);
> > > + dev_priv->csr.max_dc_state = state;
> > > + power_well->desc->ops->disable(dev_priv, power_well);
> > > + goto unlock;
> > > + }
> > > + dev_priv->csr.max_dc_state = state;
> > > +
> > > +unlock:
> > > + mutex_unlock(&power_domains->lock);
> > > }
> > >
> > > static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> > > @@ -3610,7 +3680,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > > .name = "DC off",
> > > .domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > > .ops = &gen9_dc_off_power_well_ops,
> > > - .id = DISP_PW_ID_NONE,
> > > + .id = TGL_DISP_DC_OFF,
> >
> > Let's assign this ID on all platforms for consistency.
> IMO that should be done in different patch, as tgl_set_target_dc_state() is
> only consumer of this id ?
> What is your opinion on this ?
> >
> > > },
> > > {
> > > .name = "power well 2",
> > > @@ -4039,6 +4109,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> > > dev_priv->csr.allowed_dc_mask =
> > > get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
> > >
> > > + dev_priv->csr.max_dc_state = DC_STATE_EN_UPTO_DC5_DC6_MASK;
> > > BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
> > >
> > > mutex_init(&power_domains->lock);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index 737b5def7fc6..69ebde992342 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > @@ -100,6 +100,7 @@ enum i915_power_well_id {
> > > SKL_DISP_PW_MISC_IO,
> > > SKL_DISP_PW_1,
> > > SKL_DISP_PW_2,
> > > + TGL_DISP_DC_OFF,
> > > };
> > >
> > > #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> > > @@ -256,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
> > > void intel_display_power_resume_early(struct drm_i915_private *i915);
> > > void intel_display_power_suspend(struct drm_i915_private *i915);
> > > void intel_display_power_resume(struct drm_i915_private *i915);
> > > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > + bool psr2_deep_sleep);
> > >
> > > const char *
> > > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index db7480831e52..999da5d2da0b 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -336,6 +336,7 @@ struct intel_csr {
> > > i915_reg_t mmioaddr[20];
> > > u32 mmiodata[20];
> > > u32 dc_state;
> > > + u32 max_dc_state;
> > > u32 allowed_dc_mask;
> > > intel_wakeref_t wakeref;
> > > };
> > > --
> > > 2.21.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
2019-09-11 8:21 ` Imre Deak
@ 2019-09-11 12:42 ` Anshuman Gupta
0 siblings, 0 replies; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-11 12:42 UTC (permalink / raw)
To: Imre Deak; +Cc: jani.nikula, intel-gfx
On 2019-09-11 at 11:21:42 +0300, Imre Deak wrote:
> On Mon, Sep 09, 2019 at 09:49:17PM +0530, Anshuman Gupta wrote:
> > On 2019-09-08 at 19:44:35 +0300, Imre Deak wrote:
> > > On Sat, Sep 07, 2019 at 10:44:39PM +0530, Anshuman Gupta wrote:
> > Hi Imre,
> > Thanks for reviewing the pacthes i will rework the patches.
> > There are few comments from my side which will help to rework.
> > > > Add max_dc_state and tgl_set_target_dc_state() API
> > > > in order to enable DC3CO state with existing DC states.
> > > > max_dc_state will enable/disable the desired DC state in
> > > > DC_STATE_EN reg when "DC Off" power well gets disable/enable.
> > > >
> > > > v2: commit log improvement.
> > > > v3: Used intel_wait_for_register to wait for DC3CO exit. [Imre]
> > > > Used gen9_set_dc_state() to allow/disallow DC3CO. [Imre]
> > > > Moved transcoder psr2 exit line enablement from tgl_allow_dc3co()
> > > > to a appropriate place haswell_crtc_enable(). [Imre]
> > > > Changed the DC3CO power well enabled call back logic as
> > > > recommended in review comments. [Imre]
> > > > v4: Used wait_for_us() instead of intel_wait_for_reg(). [Imre (IRC)]
> > > > v5: using udelay() instead of waiting for DC3CO exit status.
> > > > v6: Fixed minor unwanted change.
> > > > v7: Removed DC3CO powerwell and POWER_DOMAIN_VIDEO.
> > > >
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > > .../drm/i915/display/intel_display_power.c | 111 ++++++++++++++----
> > > > .../drm/i915/display/intel_display_power.h | 3 +
> > > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > > 3 files changed, 95 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > index 496fa1b53ffb..83b10f61ee42 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > @@ -772,6 +772,29 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > > > dev_priv->csr.dc_state = val & mask;
> > > > }
> > > >
> > > > +static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> > >
> > > Should be tgl_enable_dc3co(), to match the rest of DC state helpers.
> > >
> > > > +{
> > > > + if (!dev_priv->psr.sink_psr2_support)
> > > > + return;
> > >
> > > PSR knows when to enable DC3co, so no need to double-check that here.
> > >
> > > > +
> > > > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)
> > >
> > > This check is out-of-place wrt. the same checks for other DC states.
> > >
> > > > + gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> > > > +}
> > > > +
> > > > +static void tgl_disallow_dc3co(struct drm_i915_private *dev_priv)
> > > > +{
> > > > + u32 val;
> > > > +
> > > > + val = I915_READ(DC_STATE_EN);
> > > > + val &= ~DC_STATE_DC3CO_STATUS;
> > > > + I915_WRITE(DC_STATE_EN, val);
> > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > > + /*
> > > > + * Delay of 200us DC3CO Exit time B.Spec 49196
> > > > + */
> > > > + udelay(200);
> > > > +}
> > > > +
> > > > static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > > > {
> > > > assert_can_enable_dc9(dev_priv);
> > > > @@ -939,7 +962,8 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
> > > > static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> > > > struct i915_power_well *power_well)
> > > > {
> > > > - return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> > > > + return ((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC3CO) == 0 &&
> > > > + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0);
> > > > }
> > > >
> > > > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> > > > @@ -955,24 +979,32 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> > > > {
> > > > struct intel_cdclk_state cdclk_state = {};
> > > >
> > > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> > > > + tgl_disallow_dc3co(dev_priv);
> > > > + } else {
> > >
> > > With an early return you can avoid the extra diff and make reviewing
> > > easier.
> > >
> > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > >
> > > > - dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > > > - /* Can't read out voltage_level so can't use intel_cdclk_changed() */
> > > > - WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw, &cdclk_state));
> > > > + dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > > > + /*
> > > > + * Can't read out voltage_level so can't use
> > > > + * intel_cdclk_changed()
> > > > + */
> > > > + WARN_ON(intel_cdclk_needs_modeset(&dev_priv->cdclk.hw,
> > > > + &cdclk_state));
> > > >
> > > > - gen9_assert_dbuf_enabled(dev_priv);
> > > > + gen9_assert_dbuf_enabled(dev_priv);
> > > >
> > > > - if (IS_GEN9_LP(dev_priv))
> > > > - bxt_verify_ddi_phy_power_wells(dev_priv);
> > > > + if (IS_GEN9_LP(dev_priv))
> > > > + bxt_verify_ddi_phy_power_wells(dev_priv);
> > > >
> > > > - if (INTEL_GEN(dev_priv) >= 11)
> > > > - /*
> > > > - * DMC retains HW context only for port A, the other combo
> > > > - * PHY's HW context for port B is lost after DC transitions,
> > > > - * so we need to restore it manually.
> > > > - */
> > > > - intel_combo_phy_init(dev_priv);
> > > > + if (INTEL_GEN(dev_priv) >= 11)
> > > > + /*
> > > > + * DMC retains HW context only for port A, the other
> > > > + * combo PHY's HW context for port B is lost after
> > > > + * DC transitions, so we need to restore it manually.
> > > > + */
> > > > + intel_combo_phy_init(dev_priv);
> > > > + }
> > > > }
> > > >
> > > > static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> > > > @@ -987,10 +1019,48 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> > > > if (!dev_priv->csr.dmc_payload)
> > > > return;
> > > >
> > > > - 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);
> > > > + if (dev_priv->csr.max_dc_state & DC_STATE_EN_DC3CO) {
> > >
> > > target_dc_state would be a better name and it shold be an exact state
> > > instead of a mask.
> > >
> > > > + tgl_allow_dc3co(dev_priv);
> > > > + } else if (dev_priv->csr.max_dc_state & DC_STATE_EN_UPTO_DC5_DC6_MASK) {
> > > > + if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> > >
> > > We should make these checks more uniform, by checking here only
> > > target_dc_state.
> > If i understand correctly this comment is about to have a uniform
> > condition target_dc_state in order to decide which DC state we
> > interested. Then we may require to check allowed_dc_mask accordingly in
> > tgl_set_target_dc_state(),
>
> Yes, we could check here only target_dc_state, setting that one only to
> values permitted by allowed_dc_mask.
For older platform setting target_dc_state value permitted by allowed_dc_mask
i am planning to have a allowed_dc_mask_to_target_dc_state() func, which
will be called from intel_power_domains_init, and initilizes target_dc_state.
for tgl onwards tgl_set_target_dc_state() will do it.
Hope it will be ok ?
>
> > but that would not be fitting for
> > older platforms.
> > What is your input on this? please correct me if i am wrong here.
> > >
> > > > + skl_enable_dc6(dev_priv);
> > > > + else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> > > > + gen9_enable_dc5(dev_priv);
> > > > + }
> > > > +}
> > > > +
> > > > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > > + bool psr2_deep_sleep)
> > >
> > > psr2_deep_sleep is a PSR internal stuff, while PSR is using the power
> > > domains framework, so let's keep the deep sleep programming in the PSR
> > > code.
> > I wanted not to enable/disable psr2 deep sleep for each flip flush request.
> > Though PSR2 deep sleep is PSR internal stuff but it is H/W has tied with DC3CO.
> > That was intention to keep psr2_deep sleep enable/diable here (Doing in Patch 6).
> > In order to switch between DC3CO and DC5 we need to call
> > tgl_psr2_deep_sleep_disable/enable() from outside of PSR code.
> > please correct me if i am wrong here.
>
> I think it still belongs to the PSR code with other steps needed for
> DC3co setup, like the idle frame programming. PSR deep sleep could also
> be enabled/disabled independently of the DC state we enable (for
> instance enable/disable deep sleep when both DC3co and DC5 is
> disabled). It also needs the PSR lock, which again is internal to the
> PSR code.
>
> > Thanks ,
> > Anshuman Gupta.
> > >
> > > > +{
> > > > + struct i915_power_well *power_well;
> > > > + bool dc_off_enabled;
> > > > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > +
> > > > + mutex_lock(&power_domains->lock);
> > > > + power_well = lookup_power_well(dev_priv, TGL_DISP_DC_OFF);
> > > > +
> > > > + if (!power_well)
> > > > + goto unlock;
> > > > +
> > > > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> > > > + power_well);
> > > > + if (state == dev_priv->csr.max_dc_state)
> > > > + goto unlock;
> > > > +
> > > > + if (!dc_off_enabled) {
> > > > + /*
> > > > + * Need to disable the DC off power well to
> > > > + * effect target DC state.
> > > > + */
> > > > + power_well->desc->ops->enable(dev_priv, power_well);
> > > > + dev_priv->csr.max_dc_state = state;
> > > > + power_well->desc->ops->disable(dev_priv, power_well);
> > > > + goto unlock;
> > > > + }
> > > > + dev_priv->csr.max_dc_state = state;
> > > > +
> > > > +unlock:
> > > > + mutex_unlock(&power_domains->lock);
> > > > }
> > > >
> > > > static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> > > > @@ -3610,7 +3680,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > > > .name = "DC off",
> > > > .domains = TGL_DISPLAY_DC_OFF_POWER_DOMAINS,
> > > > .ops = &gen9_dc_off_power_well_ops,
> > > > - .id = DISP_PW_ID_NONE,
> > > > + .id = TGL_DISP_DC_OFF,
> > >
> > > Let's assign this ID on all platforms for consistency.
> > IMO that should be done in different patch, as tgl_set_target_dc_state() is
> > only consumer of this id ?
> > What is your opinion on this ?
> > >
> > > > },
> > > > {
> > > > .name = "power well 2",
> > > > @@ -4039,6 +4109,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> > > > dev_priv->csr.allowed_dc_mask =
> > > > get_allowed_dc_mask(dev_priv, i915_modparams.enable_dc);
> > > >
> > > > + dev_priv->csr.max_dc_state = DC_STATE_EN_UPTO_DC5_DC6_MASK;
> > > > BUILD_BUG_ON(POWER_DOMAIN_NUM > 64);
> > > >
> > > > mutex_init(&power_domains->lock);
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > index 737b5def7fc6..69ebde992342 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > @@ -100,6 +100,7 @@ enum i915_power_well_id {
> > > > SKL_DISP_PW_MISC_IO,
> > > > SKL_DISP_PW_1,
> > > > SKL_DISP_PW_2,
> > > > + TGL_DISP_DC_OFF,
> > > > };
> > > >
> > > > #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> > > > @@ -256,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915);
> > > > void intel_display_power_resume_early(struct drm_i915_private *i915);
> > > > void intel_display_power_suspend(struct drm_i915_private *i915);
> > > > void intel_display_power_resume(struct drm_i915_private *i915);
> > > > +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > > + bool psr2_deep_sleep);
> > > >
> > > > const char *
> > > > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index db7480831e52..999da5d2da0b 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -336,6 +336,7 @@ struct intel_csr {
> > > > i915_reg_t mmioaddr[20];
> > > > u32 mmiodata[20];
> > > > u32 dc_state;
> > > > + u32 max_dc_state;
> > > > u32 allowed_dc_mask;
> > > > intel_wakeref_t wakeref;
> > > > };
> > > > --
> > > > 2.21.0
> > > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (2 preceding siblings ...)
2019-09-07 17:14 ` [PATCH v7 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-08 17:10 ` Imre Deak
2019-09-07 17:14 ` [PATCH v7 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
` (10 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
DC3CO enabling B.Specs sequence requires to enable end configure
exit scanlines to TRANS_EXITLINE register, programming this register
has to be part of modeset sequence as this can't be change when
transcoder or port is enabled.
When system boots with only eDP panel there may not be real
modeset as BIOS has already programmed the necessary registers,
therefore it needs to force a modeset at bootup to enable and configure
DC3CO exitline.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 5 +
.../drm/i915/display/intel_display_power.c | 108 ++++++++++++++++++
.../drm/i915/display/intel_display_power.h | 7 ++
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 2 +-
drivers/gpu/drm/i915/intel_pm.h | 2 +
7 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3b5275ab66cf..84488f87d058 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
int clock_limit = dev_priv->max_dotclk_freq;
+ tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
+
if (INTEL_GEN(dev_priv) < 4) {
clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
@@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->pixel_multiplier = 1;
}
+ tgl_dc3co_crtc_get_config(pipe_config);
+
out:
for_each_power_domain(power_domain, power_domain_mask)
intel_display_power_put(dev_priv,
@@ -12739,6 +12743,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(pixel_multiplier);
PIPE_CONF_CHECK_I(output_format);
+ PIPE_CONF_CHECK_BOOL(has_dc3co_exitline);
PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 83b10f61ee42..ecce118b5b0e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -19,6 +19,7 @@
#include "intel_hotplug.h"
#include "intel_sideband.h"
#include "intel_tc.h"
+#include "intel_pm.h"
bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id);
@@ -772,6 +773,113 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
dev_priv->csr.dc_state = val & mask;
}
+void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
+{
+ struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+ u32 val;
+
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ val = I915_READ(EXITLINE(cstate->cpu_transcoder));
+ val &= ~EXITLINE_ENABLE;
+ I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
+}
+
+void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
+{
+ u32 linetime_us, val, exit_scanlines;
+ u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
+ struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
+ if (WARN_ON(!linetime_us))
+ return;
+ /*
+ * DC3CO Exit time 200us B.Spec 49196
+ * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
+ * Exit line event need to program above calculated scan lines before
+ * next VBLANK.
+ */
+ exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
+ if (WARN_ON(exit_scanlines > crtc_vdisplay))
+ return;
+
+ exit_scanlines = crtc_vdisplay - exit_scanlines;
+ exit_scanlines <<= EXITLINE_SHIFT;
+ val = I915_READ(EXITLINE(cstate->cpu_transcoder));
+ val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
+ val |= exit_scanlines;
+ val |= EXITLINE_ENABLE;
+ I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
+}
+
+static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
+{
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct drm_connector *connector;
+ struct drm_connector_state *connector_state;
+ int i;
+
+ for_each_new_connector_in_state(state, connector, connector_state, i) {
+ if (connector->status == connector_status_connected &&
+ connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/*
+ * DC3CO requires to enable exitline and program DC3CO requires
+ * exit scanlines to TRANS_EXITLINE register, which should only
+ * change before transcoder or port are enabled.
+ * This requires to disable the fastset at boot for eDP output.
+ */
+void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
+ struct intel_crtc_state *crtc_state)
+{
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
+ return;
+
+ dev_priv->csr.dc3co_crtc = NULL;
+
+ if (crtc_state->has_psr2 && crtc_state->base.active) {
+ if (tgl_dc3co_is_edp_connected(crtc_state)) {
+ crtc_state->has_dc3co_exitline = true;
+ dev_priv->csr.dc3co_crtc =
+ to_intel_crtc(crtc_state->base.crtc);
+ }
+ }
+}
+
+void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
+{
+ u32 val;
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
+ return;
+
+ if (crtc_state->cpu_transcoder == TRANSCODER_A) {
+ val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
+ if (val & EXITLINE_ENABLE)
+ crtc_state->has_dc3co_exitline = true;
+ else
+ crtc_state->has_dc3co_exitline = false;
+ }
+}
+
static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
{
if (!dev_priv->psr.sink_psr2_support)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 69ebde992342..d77a5a53f543 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -12,6 +12,8 @@
struct drm_i915_private;
struct intel_encoder;
+struct intel_crtc_state;
+struct intel_atomic_state;
enum intel_display_power_domain {
POWER_DOMAIN_DISPLAY_CORE,
@@ -259,6 +261,11 @@ void intel_display_power_suspend(struct drm_i915_private *i915);
void intel_display_power_resume(struct drm_i915_private *i915);
void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
bool psr2_deep_sleep);
+void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
+ struct intel_crtc_state *crtc_state);
+void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
+void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
+void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index d5cc4b810d9e..22f8fe0a34d0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -871,6 +871,7 @@ struct intel_crtc_state {
bool has_psr;
bool has_psr2;
+ bool has_dc3co_exitline;
/*
* Frequence the dpll for the port should run at. Differs from the
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 999da5d2da0b..3218b0319852 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -339,6 +339,8 @@ struct intel_csr {
u32 max_dc_state;
u32 allowed_dc_mask;
intel_wakeref_t wakeref;
+ /* cache the crtc on which DC3CO will be allowed */
+ struct intel_crtc *dc3co_crtc;
};
enum i915_cache_level {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 528e90ed5de4..7352c0cb88d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
return ret;
}
-static uint_fixed_16_16_t
+uint_fixed_16_16_t
intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
{
u32 pixel_rate;
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index e3573e1e16e3..454e92c06dff 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -8,6 +8,7 @@
#include <linux/types.h>
+#include "i915_drv.h"
#include "i915_reg.h"
struct drm_device;
@@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
+uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
2019-09-07 17:14 ` [PATCH v7 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
@ 2019-09-08 17:10 ` Imre Deak
0 siblings, 0 replies; 28+ messages in thread
From: Imre Deak @ 2019-09-08 17:10 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Sat, Sep 07, 2019 at 10:44:40PM +0530, Anshuman Gupta wrote:
> DC3CO enabling B.Specs sequence requires to enable end configure
> exit scanlines to TRANS_EXITLINE register, programming this register
> has to be part of modeset sequence as this can't be change when
> transcoder or port is enabled.
> When system boots with only eDP panel there may not be real
> modeset as BIOS has already programmed the necessary registers,
> therefore it needs to force a modeset at bootup to enable and configure
> DC3CO exitline.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 5 +
> .../drm/i915/display/intel_display_power.c | 108 ++++++++++++++++++
> .../drm/i915/display/intel_display_power.h | 7 ++
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.h | 2 +
> 7 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3b5275ab66cf..84488f87d058 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> int clock_limit = dev_priv->max_dotclk_freq;
>
> + tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
> +
This is an encoder specific state so should be done from the encoder
compute config hook.
> if (INTEL_GEN(dev_priv) < 4) {
> clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
>
> @@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->pixel_multiplier = 1;
> }
>
> + tgl_dc3co_crtc_get_config(pipe_config);
> +
> out:
> for_each_power_domain(power_domain, power_domain_mask)
> intel_display_power_put(dev_priv,
> @@ -12739,6 +12743,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> PIPE_CONF_CHECK_I(pixel_multiplier);
> PIPE_CONF_CHECK_I(output_format);
> + PIPE_CONF_CHECK_BOOL(has_dc3co_exitline);
> PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
> if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 83b10f61ee42..ecce118b5b0e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -19,6 +19,7 @@
> #include "intel_hotplug.h"
> #include "intel_sideband.h"
> #include "intel_tc.h"
> +#include "intel_pm.h"
>
> bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> enum i915_power_well_id power_well_id);
> @@ -772,6 +773,113 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> dev_priv->csr.dc_state = val & mask;
> }
>
> +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
Please add these in the patch that uses them, to make review easier.
> +{
> + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> + u32 val;
> +
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> + val &= ~EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> +}
> +
> +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> +{
> + u32 linetime_us, val, exit_scanlines;
> + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
> + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> +
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
> + if (WARN_ON(!linetime_us))
> + return;
> + /*
> + * DC3CO Exit time 200us B.Spec 49196
> + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
> + * Exit line event need to program above calculated scan lines before
> + * next VBLANK.
> + */
> + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
exit_scanlines could be the state that you calculate during the PSR compute
config phase, only programming that calculated state here.
> + if (WARN_ON(exit_scanlines > crtc_vdisplay))
> + return;
> +
> + exit_scanlines = crtc_vdisplay - exit_scanlines;
> + exit_scanlines <<= EXITLINE_SHIFT;
> + val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
> + val |= exit_scanlines;
> + val |= EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> +}
> +
> +static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_atomic_state *state = crtc_state->base.state;
> + struct drm_connector *connector;
> + struct drm_connector_state *connector_state;
> + int i;
> +
> + for_each_new_connector_in_state(state, connector, connector_state, i) {
> + if (connector->status == connector_status_connected &&
> + connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +/*
> + * DC3CO requires to enable exitline and program DC3CO requires
> + * exit scanlines to TRANS_EXITLINE register, which should only
> + * change before transcoder or port are enabled.
> + * This requires to disable the fastset at boot for eDP output.
> + */
> +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> + struct intel_crtc_state *crtc_state)
> +{
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> + return;
> +
> + dev_priv->csr.dc3co_crtc = NULL;
This will correspond to psr.pipe, so I can't see why we need to
calcualte it separately.
> +
> + if (crtc_state->has_psr2 && crtc_state->base.active) {
> + if (tgl_dc3co_is_edp_connected(crtc_state)) {
> + crtc_state->has_dc3co_exitline = true;
I think we should rather calculate the exit_scanline value itself (as
you calculate that now in tgl_enable_psr2_transcoder_exitline()). By
doing that in the encoder compute config hook, we'll also know that we
have eDP connected, so wouldn't have to check for that either.
> + dev_priv->csr.dc3co_crtc =
> + to_intel_crtc(crtc_state->base.crtc);
> + }
> + }
> +}
> +
> +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> +{
> + u32 val;
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> +
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> + return;
> +
> + if (crtc_state->cpu_transcoder == TRANSCODER_A) {
> + val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> + if (val & EXITLINE_ENABLE)
> + crtc_state->has_dc3co_exitline = true;
> + else
> + crtc_state->has_dc3co_exitline = false;
Based on the above we should read out the exit_scanline value itself (in
the encoder's get_config hook).
> + }
> +}
> +
> static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> {
> if (!dev_priv->psr.sink_psr2_support)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 69ebde992342..d77a5a53f543 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -12,6 +12,8 @@
>
> struct drm_i915_private;
> struct intel_encoder;
> +struct intel_crtc_state;
> +struct intel_atomic_state;
>
> enum intel_display_power_domain {
> POWER_DOMAIN_DISPLAY_CORE,
> @@ -259,6 +261,11 @@ void intel_display_power_suspend(struct drm_i915_private *i915);
> void intel_display_power_resume(struct drm_i915_private *i915);
> void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> bool psr2_deep_sleep);
> +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> + struct intel_crtc_state *crtc_state);
> +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
>
> const char *
> intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d5cc4b810d9e..22f8fe0a34d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -871,6 +871,7 @@ struct intel_crtc_state {
>
> bool has_psr;
> bool has_psr2;
> + bool has_dc3co_exitline;
>
> /*
> * Frequence the dpll for the port should run at. Differs from the
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 999da5d2da0b..3218b0319852 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -339,6 +339,8 @@ struct intel_csr {
> u32 max_dc_state;
> u32 allowed_dc_mask;
> intel_wakeref_t wakeref;
> + /* cache the crtc on which DC3CO will be allowed */
> + struct intel_crtc *dc3co_crtc;
> };
>
> enum i915_cache_level {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 528e90ed5de4..7352c0cb88d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
> return ret;
> }
>
> -static uint_fixed_16_16_t
> +uint_fixed_16_16_t
> intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
> {
> u32 pixel_rate;
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index e3573e1e16e3..454e92c06dff 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -8,6 +8,7 @@
>
> #include <linux/types.h>
>
> +#include "i915_drv.h"
> #include "i915_reg.h"
>
> struct drm_device;
> @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
> u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
>
> u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
> +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
>
> unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 5/7] drm/i915/tgl: DC3CO PSR2 helper
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (3 preceding siblings ...)
2019-09-07 17:14 ` [PATCH v7 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-08 17:20 ` Imre Deak
2019-09-07 17:14 ` [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
` (9 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Add dc3co helper functions to enable/disable psr2 deep sleep.
Adhere B.Specs by disallow DC3CO state before PSR2 exit.
Enable PSR2 exitline event and program the desired scanlines
to exit DC3CO in intel_psr_enable function at modeset path.
Disable the DC3CO exitline in order to maintian consistent
pipe config state in encoder disable path.
v1: moved calling of tgl_enable_psr2_transcoder_exitline() to
intel_psr_enable(). [Imre]
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 51 ++++++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_psr.h | 2 +
2 files changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index b3c7eef53bf3..95b12946c72a 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -534,6 +534,48 @@ transcoder_has_psr2(struct drm_i915_private *dev_priv, enum transcoder trans)
return trans == TRANSCODER_EDP;
}
+static void psr2_program_idle_frames(struct drm_i915_private *dev_priv,
+ u32 idle_frames)
+{
+ u32 val;
+
+ idle_frames <<= EDP_PSR2_IDLE_FRAME_SHIFT;
+ val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
+ val &= ~EDP_PSR2_IDLE_FRAME_MASK;
+ val |= idle_frames;
+ I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
+}
+
+void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv)
+{
+ int idle_frames = 0;
+
+ psr2_program_idle_frames(dev_priv, idle_frames);
+}
+
+void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv)
+{
+ int idle_frames;
+
+ /*
+ * Let's use 6 as the minimum to cover all known cases including the
+ * off-by-one issue that HW has in some cases.
+ */
+ idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
+ idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1);
+ psr2_program_idle_frames(dev_priv, idle_frames);
+}
+
+static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv)
+{
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ cancel_delayed_work(&dev_priv->csr.idle_work);
+ /* Before PSR2 exit disallow dc3co*/
+ tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
+}
+
static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
struct intel_crtc_state *crtc_state)
{
@@ -799,6 +841,10 @@ void intel_psr_enable(struct intel_dp *intel_dp,
WARN_ON(dev_priv->drrs.dp);
+ /* Enable PSR2 transcoder exit line */
+ if (crtc_state->has_psr2)
+ tgl_enable_psr2_transcoder_exitline(crtc_state);
+
mutex_lock(&dev_priv->psr.lock);
if (!psr_global_enabled(dev_priv->psr.debug)) {
@@ -829,6 +875,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
}
if (dev_priv->psr.psr2_enabled) {
+ tgl_disallow_dc3co_on_psr2_exit(dev_priv);
val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
WARN_ON(!(val & EDP_PSR2_ENABLE));
val &= ~EDP_PSR2_ENABLE;
@@ -895,6 +942,10 @@ void intel_psr_disable(struct intel_dp *intel_dp,
if (WARN_ON(!CAN_PSR(dev_priv)))
return;
+ /* Disable PSR2 transcoder exit line */
+ if (old_crtc_state->has_psr2)
+ tgl_disable_psr2_transcoder_exitline(old_crtc_state);
+
mutex_lock(&dev_priv->psr.lock);
intel_psr_disable_locked(intel_dp);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 46e4de8b8cd5..75a9862f36fd 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -35,5 +35,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp);
int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
u32 *out_value);
bool intel_psr_enabled(struct intel_dp *intel_dp);
+void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv);
+void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv);
#endif /* __INTEL_PSR_H__ */
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 5/7] drm/i915/tgl: DC3CO PSR2 helper
2019-09-07 17:14 ` [PATCH v7 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
@ 2019-09-08 17:20 ` Imre Deak
0 siblings, 0 replies; 28+ messages in thread
From: Imre Deak @ 2019-09-08 17:20 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Sat, Sep 07, 2019 at 10:44:41PM +0530, Anshuman Gupta wrote:
> Add dc3co helper functions to enable/disable psr2 deep sleep.
> Adhere B.Specs by disallow DC3CO state before PSR2 exit.
> Enable PSR2 exitline event and program the desired scanlines
> to exit DC3CO in intel_psr_enable function at modeset path.
> Disable the DC3CO exitline in order to maintian consistent
> pipe config state in encoder disable path.
>
> v1: moved calling of tgl_enable_psr2_transcoder_exitline() to
> intel_psr_enable(). [Imre]
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 51 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_psr.h | 2 +
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3c7eef53bf3..95b12946c72a 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -534,6 +534,48 @@ transcoder_has_psr2(struct drm_i915_private *dev_priv, enum transcoder trans)
> return trans == TRANSCODER_EDP;
> }
>
> +static void psr2_program_idle_frames(struct drm_i915_private *dev_priv,
> + u32 idle_frames)
> +{
> + u32 val;
> +
> + idle_frames <<= EDP_PSR2_IDLE_FRAME_SHIFT;
> + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> + val &= ~EDP_PSR2_IDLE_FRAME_MASK;
> + val |= idle_frames;
> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
> +}
> +
> +void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv)
> +{
> + int idle_frames = 0;
No need for the above.
> +
> + psr2_program_idle_frames(dev_priv, idle_frames);
> +}
> +
> +void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv)
> +{
> + int idle_frames;
> +
> + /*
> + * Let's use 6 as the minimum to cover all known cases including the
> + * off-by-one issue that HW has in some cases.
> + */
> + idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> + idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1);
> + psr2_program_idle_frames(dev_priv, idle_frames);
> +}
> +
> +static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv)
> +{
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + cancel_delayed_work(&dev_priv->csr.idle_work);
This is racy, as the work could be still running at this point.
> + /* Before PSR2 exit disallow dc3co*/
> + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
The third deep_sleep param is not needed, since we want deep_sleep only
with DC6 (and not with DC3co), but programming deep_sleep is a PSR
internal stuff, so should be done here rather than the power domain
code.
> +}
> +
> static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state)
> {
> @@ -799,6 +841,10 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>
> WARN_ON(dev_priv->drrs.dp);
>
> + /* Enable PSR2 transcoder exit line */
> + if (crtc_state->has_psr2)
> + tgl_enable_psr2_transcoder_exitline(crtc_state);
The transcoder is enabled already at this point, so we shouldn't
change the exit line reg.
> +
> mutex_lock(&dev_priv->psr.lock);
>
> if (!psr_global_enabled(dev_priv->psr.debug)) {
> @@ -829,6 +875,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> }
>
> if (dev_priv->psr.psr2_enabled) {
> + tgl_disallow_dc3co_on_psr2_exit(dev_priv);
> val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> WARN_ON(!(val & EDP_PSR2_ENABLE));
> val &= ~EDP_PSR2_ENABLE;
> @@ -895,6 +942,10 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> if (WARN_ON(!CAN_PSR(dev_priv)))
> return;
>
> + /* Disable PSR2 transcoder exit line */
> + if (old_crtc_state->has_psr2)
> + tgl_disable_psr2_transcoder_exitline(old_crtc_state);
Similarly, this isn't the right spot for changing the exit line reg.
> +
> mutex_lock(&dev_priv->psr.lock);
>
> intel_psr_disable_locked(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 46e4de8b8cd5..75a9862f36fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -35,5 +35,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp);
> int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> u32 *out_value);
> bool intel_psr_enabled(struct intel_dp *intel_dp);
> +void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv);
> +void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv);
>
> #endif /* __INTEL_PSR_H__ */
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (4 preceding siblings ...)
2019-09-07 17:14 ` [PATCH v7 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-08 17:55 ` Imre Deak
2019-09-07 17:14 ` [PATCH v7 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
` (8 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
DC3CO is useful power state, when DMC detects PSR2 idle frame
while an active video playback, playing 30fps video on 60hz panel
is the classic example of this use case.
DC5 and DC6 saves more power, but can't be entered during video
playback because there are not enough idle frames in a row to meet
most PSR2 panel deep sleep entry requirement typically 4 frames.
It will be worthy to enable DC3CO after completion of each flip
and switch back to DC5 when display is idle, as driver doesn't
differentiate between video playback and a normal flip.
It is safer to allow DC5 after 6 idle frame, as PSR2 requires
minimum 6 idle frame.
v2: calculated s/w state to switch over dc3co when there is an
update. [Imre]
used cancel_delayed_work_sync() in order to avoid any race
with already scheduled delayed work. [Imre]
v3: cancel_delayed_work_sync() may blocked the commit work.
Hence dropping it, dc5_idle_thread() checks the valid wakeref before
putting the reference count, which avoids any chances of dropping
a zero wakeref. [Imre (IRC)]
v4: use frontbuffer flush mechanism. [Imre]
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 2 +
.../drm/i915/display/intel_display_power.c | 79 +++++++++++++++++++
.../drm/i915/display/intel_display_power.h | 6 ++
.../gpu/drm/i915/display/intel_frontbuffer.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
5 files changed, 89 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 84488f87d058..2754e8ee693f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16206,6 +16206,7 @@ int intel_modeset_init(struct drm_device *dev)
init_llist_head(&dev_priv->atomic_helper.free_list);
INIT_WORK(&dev_priv->atomic_helper.free_work,
intel_atomic_helper_free_state_worker);
+ INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
intel_init_quirks(dev_priv);
@@ -17147,6 +17148,7 @@ void intel_modeset_driver_remove(struct drm_device *dev)
flush_workqueue(dev_priv->modeset_wq);
flush_work(&dev_priv->atomic_helper.free_work);
+ flush_delayed_work(&dev_priv->csr.idle_work);
WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
/*
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index ecce118b5b0e..c110f04c9733 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -20,6 +20,7 @@
#include "intel_sideband.h"
#include "intel_tc.h"
#include "intel_pm.h"
+#include "intel_psr.h"
bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id);
@@ -773,6 +774,27 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
dev_priv->csr.dc_state = val & mask;
}
+static u32 intel_get_frame_time_us(const struct intel_crtc_state *cstate)
+{
+ u32 pixel_rate, crtc_htotal, crtc_vtotal;
+ u32 frametime_us;
+
+ if (!cstate || !cstate->base.active)
+ return 0;
+
+ pixel_rate = cstate->pixel_rate;
+
+ if (WARN_ON(pixel_rate == 0))
+ return 0;
+
+ crtc_htotal = cstate->base.adjusted_mode.crtc_htotal;
+ crtc_vtotal = cstate->base.adjusted_mode.crtc_vtotal;
+ frametime_us = DIV_ROUND_UP(crtc_htotal * crtc_vtotal * 1000ULL,
+ pixel_rate);
+
+ return frametime_us;
+}
+
void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
{
struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
@@ -817,6 +839,49 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
}
+void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits, enum fb_op_origin origin)
+{
+ struct intel_crtc_state *cstate;
+ u32 delay;
+ unsigned int busy_frontbuffer_bits;
+
+ if (!IS_TIGERLAKE(dev_priv))
+ return;
+
+ if (origin != ORIGIN_FLIP)
+ return;
+
+ if (!dev_priv->csr.dc3co_crtc)
+ return;
+
+ cstate = to_intel_crtc_state(dev_priv->csr.dc3co_crtc->base.state);
+ frontbuffer_bits &=
+ INTEL_FRONTBUFFER_ALL_MASK(dev_priv->csr.dc3co_crtc->pipe);
+ busy_frontbuffer_bits &= ~frontbuffer_bits;
+
+ mutex_lock(&dev_priv->psr.lock);
+
+ if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
+ goto unlock;
+
+ /*
+ * At every flip frontbuffer flush first cancel the delayed work,
+ * when delayed schedules that means display has been idle
+ * for the 6 idle frame.
+ */
+ if (!busy_frontbuffer_bits) {
+ cancel_delayed_work(&dev_priv->csr.idle_work);
+ tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO, false);
+ delay = DC5_REQ_IDLE_FRAMES * intel_get_frame_time_us(cstate);
+ schedule_delayed_work(&dev_priv->csr.idle_work,
+ usecs_to_jiffies(delay));
+ }
+
+unlock:
+ mutex_unlock(&dev_priv->psr.lock);
+}
+
static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
{
struct drm_atomic_state *state = crtc_state->base.state;
@@ -880,6 +945,14 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
}
}
+void tgl_dc5_idle_thread(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, typeof(*dev_priv), csr.idle_work.work);
+
+ tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
+}
+
static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
{
if (!dev_priv->psr.sink_psr2_support)
@@ -1155,6 +1228,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
if (state == dev_priv->csr.max_dc_state)
goto unlock;
+ if (!psr2_deep_sleep)
+ tgl_psr2_deep_sleep_disable(dev_priv);
+
if (!dc_off_enabled) {
/*
* Need to disable the DC off power well to
@@ -1167,6 +1243,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
}
dev_priv->csr.max_dc_state = state;
+ if (psr2_deep_sleep)
+ tgl_psr2_deep_sleep_enable(dev_priv);
+
unlock:
mutex_unlock(&power_domains->lock);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index d77a5a53f543..df096d64c744 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -9,6 +9,9 @@
#include "intel_display.h"
#include "intel_runtime_pm.h"
#include "i915_reg.h"
+#include "intel_frontbuffer.h"
+
+#define DC5_REQ_IDLE_FRAMES 6
struct drm_i915_private;
struct intel_encoder;
@@ -266,6 +269,9 @@ void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
+void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits, enum fb_op_origin origin);
+void tgl_dc5_idle_thread(struct work_struct *work);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index fc40dc1fdbcc..c3b10f6e4382 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -90,6 +90,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
might_sleep();
intel_edp_drrs_flush(i915, frontbuffer_bits);
intel_psr_flush(i915, frontbuffer_bits, origin);
+ tgl_dc3co_flush(i915, frontbuffer_bits, origin);
intel_fbc_flush(i915, frontbuffer_bits, origin);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3218b0319852..fe71119a458e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -338,6 +338,7 @@ struct intel_csr {
u32 dc_state;
u32 max_dc_state;
u32 allowed_dc_mask;
+ struct delayed_work idle_work;
intel_wakeref_t wakeref;
/* cache the crtc on which DC3CO will be allowed */
struct intel_crtc *dc3co_crtc;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
2019-09-07 17:14 ` [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
@ 2019-09-08 17:55 ` Imre Deak
2019-09-10 9:56 ` Anshuman Gupta
0 siblings, 1 reply; 28+ messages in thread
From: Imre Deak @ 2019-09-08 17:55 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Sat, Sep 07, 2019 at 10:44:42PM +0530, Anshuman Gupta wrote:
> DC3CO is useful power state, when DMC detects PSR2 idle frame
> while an active video playback, playing 30fps video on 60hz panel
> is the classic example of this use case.
> DC5 and DC6 saves more power, but can't be entered during video
> playback because there are not enough idle frames in a row to meet
> most PSR2 panel deep sleep entry requirement typically 4 frames.
Please also explain why DC3co will be enabled only for flips but not for
other frontbuffer flush events (ORIGIN_CS/DIRTYFB etc.) and that we
could enable it for those too by switching to manual PSR tracking and
programming only 1 idle frame for deep sleep (see below).
Also explaining that the frontbuffer invalidate event doesn't need to be
acted on (b/c of PSR exit) would be helpful.
>
> It will be worthy to enable DC3CO after completion of each flip
> and switch back to DC5 when display is idle, as driver doesn't
> differentiate between video playback and a normal flip.
> It is safer to allow DC5 after 6 idle frame, as PSR2 requires
> minimum 6 idle frame.
It would be clearer to say here that after a flip we enable DC3co, after
which we wait manually 6 frames (by scheduling the idle frame work) at
which point we enable PSR deep sleep with 6 idle frames. After this 6
idle frames the HW will enter deep sleep and DC5 will be entered
after this by DMC at some point.
The claim that we _have_ to make the HW wait for 6 idle frames before it
enters deep sleep doesn't make much sense to me, would be good to see a
reference to that if it really exists. That setting seems to only serve
the purpose to avoid update lags, but in the future (as discussed with
Ville) we should switch to manual PSR tracking and for that we would
program the HW to wait only 1 idle frame before entering deep sleep and
rely only on the manual 6 idle frame wait (via the idle frame work) to
avoid update lags.
>
> v2: calculated s/w state to switch over dc3co when there is an
> update. [Imre]
> used cancel_delayed_work_sync() in order to avoid any race
> with already scheduled delayed work. [Imre]
> v3: cancel_delayed_work_sync() may blocked the commit work.
> Hence dropping it, dc5_idle_thread() checks the valid wakeref before
> putting the reference count, which avoids any chances of dropping
> a zero wakeref. [Imre (IRC)]
> v4: use frontbuffer flush mechanism. [Imre]
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +
> .../drm/i915/display/intel_display_power.c | 79 +++++++++++++++++++
> .../drm/i915/display/intel_display_power.h | 6 ++
> .../gpu/drm/i915/display/intel_frontbuffer.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 5 files changed, 89 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 84488f87d058..2754e8ee693f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16206,6 +16206,7 @@ int intel_modeset_init(struct drm_device *dev)
> init_llist_head(&dev_priv->atomic_helper.free_list);
> INIT_WORK(&dev_priv->atomic_helper.free_work,
> intel_atomic_helper_free_state_worker);
> + INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
>
> intel_init_quirks(dev_priv);
>
> @@ -17147,6 +17148,7 @@ void intel_modeset_driver_remove(struct drm_device *dev)
> flush_workqueue(dev_priv->modeset_wq);
>
> flush_work(&dev_priv->atomic_helper.free_work);
> + flush_delayed_work(&dev_priv->csr.idle_work);
This is racy as the work could be still running, but also would leave a
few other places with the work running like suspend, so let's just make
sure that it's not running any more after encoder disabling.
> WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index ecce118b5b0e..c110f04c9733 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -20,6 +20,7 @@
> #include "intel_sideband.h"
> #include "intel_tc.h"
> #include "intel_pm.h"
> +#include "intel_psr.h"
>
> bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> enum i915_power_well_id power_well_id);
> @@ -773,6 +774,27 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> dev_priv->csr.dc_state = val & mask;
> }
>
> +static u32 intel_get_frame_time_us(const struct intel_crtc_state *cstate)
> +{
> + u32 pixel_rate, crtc_htotal, crtc_vtotal;
> + u32 frametime_us;
> +
> + if (!cstate || !cstate->base.active)
> + return 0;
> +
> + pixel_rate = cstate->pixel_rate;
> +
> + if (WARN_ON(pixel_rate == 0))
> + return 0;
> +
> + crtc_htotal = cstate->base.adjusted_mode.crtc_htotal;
> + crtc_vtotal = cstate->base.adjusted_mode.crtc_vtotal;
> + frametime_us = DIV_ROUND_UP(crtc_htotal * crtc_vtotal * 1000ULL,
> + pixel_rate);
> +
> + return frametime_us;
> +}
> +
> void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> {
> struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> @@ -817,6 +839,49 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> }
>
> +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits, enum fb_op_origin origin)
> +{
> + struct intel_crtc_state *cstate;
> + u32 delay;
> + unsigned int busy_frontbuffer_bits;
> +
> + if (!IS_TIGERLAKE(dev_priv))
> + return;
> +
> + if (origin != ORIGIN_FLIP)
> + return;
> +
> + if (!dev_priv->csr.dc3co_crtc)
> + return;
> +
> + cstate = to_intel_crtc_state(dev_priv->csr.dc3co_crtc->base.state);
> + frontbuffer_bits &=
> + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->csr.dc3co_crtc->pipe);
> + busy_frontbuffer_bits &= ~frontbuffer_bits;
Hrm, this looks wrong. Also it depends on the PSR mechanism, so this
whole DC3co flush logic should rather be done from the PSR flush func,
using psr.pipe.
> +
> + mutex_lock(&dev_priv->psr.lock);
> +
> + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
> + goto unlock;
> +
> + /*
> + * At every flip frontbuffer flush first cancel the delayed work,
> + * when delayed schedules that means display has been idle
> + * for the 6 idle frame.
> + */
> + if (!busy_frontbuffer_bits) {
> + cancel_delayed_work(&dev_priv->csr.idle_work);
The above is racy.
> + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO, false);
> + delay = DC5_REQ_IDLE_FRAMES * intel_get_frame_time_us(cstate);
> + schedule_delayed_work(&dev_priv->csr.idle_work,
> + usecs_to_jiffies(delay));
> + }
> +
> +unlock:
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> +
> static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
> {
> struct drm_atomic_state *state = crtc_state->base.state;
> @@ -880,6 +945,14 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> }
> }
>
> +void tgl_dc5_idle_thread(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, typeof(*dev_priv), csr.idle_work.work);
> +
> + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
So it would result in enabling deep sleep, but without the PSR lock.
That's one reason we should really keep the PSR specific parts here.
> +}
> +
> static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> {
> if (!dev_priv->psr.sink_psr2_support)
> @@ -1155,6 +1228,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> if (state == dev_priv->csr.max_dc_state)
> goto unlock;
>
> + if (!psr2_deep_sleep)
> + tgl_psr2_deep_sleep_disable(dev_priv);
> +
> if (!dc_off_enabled) {
> /*
> * Need to disable the DC off power well to
> @@ -1167,6 +1243,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> }
> dev_priv->csr.max_dc_state = state;
>
> + if (psr2_deep_sleep)
> + tgl_psr2_deep_sleep_enable(dev_priv);
> +
> unlock:
> mutex_unlock(&power_domains->lock);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index d77a5a53f543..df096d64c744 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -9,6 +9,9 @@
> #include "intel_display.h"
> #include "intel_runtime_pm.h"
> #include "i915_reg.h"
> +#include "intel_frontbuffer.h"
> +
> +#define DC5_REQ_IDLE_FRAMES 6
No need for a define for this.
>
> struct drm_i915_private;
> struct intel_encoder;
> @@ -266,6 +269,9 @@ void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits, enum fb_op_origin origin);
> +void tgl_dc5_idle_thread(struct work_struct *work);
>
> const char *
> intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index fc40dc1fdbcc..c3b10f6e4382 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -90,6 +90,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> might_sleep();
> intel_edp_drrs_flush(i915, frontbuffer_bits);
> intel_psr_flush(i915, frontbuffer_bits, origin);
> + tgl_dc3co_flush(i915, frontbuffer_bits, origin);
> intel_fbc_flush(i915, frontbuffer_bits, origin);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3218b0319852..fe71119a458e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -338,6 +338,7 @@ struct intel_csr {
> u32 dc_state;
> u32 max_dc_state;
> u32 allowed_dc_mask;
> + struct delayed_work idle_work;
> intel_wakeref_t wakeref;
> /* cache the crtc on which DC3CO will be allowed */
> struct intel_crtc *dc3co_crtc;
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
2019-09-08 17:55 ` Imre Deak
@ 2019-09-10 9:56 ` Anshuman Gupta
2019-09-11 8:50 ` Imre Deak
0 siblings, 1 reply; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-10 9:56 UTC (permalink / raw)
To: Imre Deak; +Cc: jani.nikula, intel-gfx
On 2019-09-08 at 20:55:17 +0300, Imre Deak wrote:
Hi Imre ,
Thanks for review, could you please provide your response on below
comments.
> On Sat, Sep 07, 2019 at 10:44:42PM +0530, Anshuman Gupta wrote:
> > DC3CO is useful power state, when DMC detects PSR2 idle frame
> > while an active video playback, playing 30fps video on 60hz panel
> > is the classic example of this use case.
> > DC5 and DC6 saves more power, but can't be entered during video
> > playback because there are not enough idle frames in a row to meet
> > most PSR2 panel deep sleep entry requirement typically 4 frames.
>
> Please also explain why DC3co will be enabled only for flips but not for
> other frontbuffer flush events (ORIGIN_CS/DIRTYFB etc.) and that we
> could enable it for those too by switching to manual PSR tracking and
> programming only 1 idle frame for deep sleep (see below).
>
> Also explaining that the frontbuffer invalidate event doesn't need to be
> acted on (b/c of PSR exit) would be helpful.
>
> >
> > It will be worthy to enable DC3CO after completion of each flip
> > and switch back to DC5 when display is idle, as driver doesn't
> > differentiate between video playback and a normal flip.
> > It is safer to allow DC5 after 6 idle frame, as PSR2 requires
> > minimum 6 idle frame.
>
> It would be clearer to say here that after a flip we enable DC3co, after
> which we wait manually 6 frames (by scheduling the idle frame work) at
> which point we enable PSR deep sleep with 6 idle frames. After this 6
> idle frames the HW will enter deep sleep and DC5 will be entered
> after this by DMC at some point.
>
> The claim that we _have_ to make the HW wait for 6 idle frames before it
> enters deep sleep doesn't make much sense to me, would be good to see a
> reference to that if it really exists. That setting seems to only serve
> the purpose to avoid update lags, but in the future (as discussed with
> Ville) we should switch to manual PSR tracking and for that we would
> program the HW to wait only 1 idle frame before entering deep sleep and
> rely only on the manual 6 idle frame wait (via the idle frame work) to
> avoid update lags.
>
> >
> > v2: calculated s/w state to switch over dc3co when there is an
> > update. [Imre]
> > used cancel_delayed_work_sync() in order to avoid any race
> > with already scheduled delayed work. [Imre]
> > v3: cancel_delayed_work_sync() may blocked the commit work.
> > Hence dropping it, dc5_idle_thread() checks the valid wakeref before
> > putting the reference count, which avoids any chances of dropping
> > a zero wakeref. [Imre (IRC)]
> > v4: use frontbuffer flush mechanism. [Imre]
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 2 +
> > .../drm/i915/display/intel_display_power.c | 79 +++++++++++++++++++
> > .../drm/i915/display/intel_display_power.h | 6 ++
> > .../gpu/drm/i915/display/intel_frontbuffer.c | 1 +
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > 5 files changed, 89 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 84488f87d058..2754e8ee693f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -16206,6 +16206,7 @@ int intel_modeset_init(struct drm_device *dev)
> > init_llist_head(&dev_priv->atomic_helper.free_list);
> > INIT_WORK(&dev_priv->atomic_helper.free_work,
> > intel_atomic_helper_free_state_worker);
> > + INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
> >
> > intel_init_quirks(dev_priv);
> >
> > @@ -17147,6 +17148,7 @@ void intel_modeset_driver_remove(struct drm_device *dev)
> > flush_workqueue(dev_priv->modeset_wq);
> >
> > flush_work(&dev_priv->atomic_helper.free_work);
> > + flush_delayed_work(&dev_priv->csr.idle_work);
>
> This is racy as the work could be still running, but also would leave a
> few other places with the work running like suspend, so let's just make
> sure that it's not running any more after encoder disabling.
>
> > WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
> >
> > /*
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index ecce118b5b0e..c110f04c9733 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -20,6 +20,7 @@
> > #include "intel_sideband.h"
> > #include "intel_tc.h"
> > #include "intel_pm.h"
> > +#include "intel_psr.h"
> >
> > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> > enum i915_power_well_id power_well_id);
> > @@ -773,6 +774,27 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > dev_priv->csr.dc_state = val & mask;
> > }
> >
> > +static u32 intel_get_frame_time_us(const struct intel_crtc_state *cstate)
> > +{
> > + u32 pixel_rate, crtc_htotal, crtc_vtotal;
> > + u32 frametime_us;
> > +
> > + if (!cstate || !cstate->base.active)
> > + return 0;
> > +
> > + pixel_rate = cstate->pixel_rate;
> > +
> > + if (WARN_ON(pixel_rate == 0))
> > + return 0;
> > +
> > + crtc_htotal = cstate->base.adjusted_mode.crtc_htotal;
> > + crtc_vtotal = cstate->base.adjusted_mode.crtc_vtotal;
> > + frametime_us = DIV_ROUND_UP(crtc_htotal * crtc_vtotal * 1000ULL,
> > + pixel_rate);
> > +
> > + return frametime_us;
> > +}
> > +
> > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > {
> > struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > @@ -817,6 +839,49 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > }
> >
> > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > + unsigned int frontbuffer_bits, enum fb_op_origin origin)
> > +{
> > + struct intel_crtc_state *cstate;
> > + u32 delay;
> > + unsigned int busy_frontbuffer_bits;
> > +
> > + if (!IS_TIGERLAKE(dev_priv))
> > + return;
> > +
> > + if (origin != ORIGIN_FLIP)
> > + return;
> > +
> > + if (!dev_priv->csr.dc3co_crtc)
> > + return;
> > +
> > + cstate = to_intel_crtc_state(dev_priv->csr.dc3co_crtc->base.state);
> > + frontbuffer_bits &=
> > + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->csr.dc3co_crtc->pipe);
> > + busy_frontbuffer_bits &= ~frontbuffer_bits;
>
> Hrm, this looks wrong. Also it depends on the PSR mechanism, so this
> whole DC3co flush logic should rather be done from the PSR flush func,
> using psr.pipe.
>
Hmm initially i have planned to have entire logic under psr flush func
but PSR invalidate/flush logic for ORIGIN_FLIP relies on H/W tracking,
PSR invalidate and flush call has early return for ORIGIN_FLIP
unlike DC3CO case where we are only interested in ORIGIN_FLIP requests.
> > +
> > + mutex_lock(&dev_priv->psr.lock);
> > +
> > + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
> > + goto unlock;
> > +
> > + /*
> > + * At every flip frontbuffer flush first cancel the delayed work,
> > + * when delayed schedules that means display has been idle
> > + * for the 6 idle frame.
> > + */
> > + if (!busy_frontbuffer_bits) {
> > + cancel_delayed_work(&dev_priv->csr.idle_work);
>
> The above is racy.
Yes tgl_dc5_idle_thread() can even run after this but in that case also
tgl_set_target_state() is protected by the power domain locks.
Do u see any other harm of it apart from setting target_dc_state
immediately to DC5/DC6 after it sets to DC3CO (IMO this would be a little
penalty for a VBLANK in next page flip it will again set target_dc_state to DC3CO).
I can think of two possible solutions to avoid this race.
1. cancel_delayed_work_sync().
2. Having a flag to indicate that delayed work has been canceled
and same can be used by tgl_dc5_idle_thread() to have an early return.
Please suggest your opinion on it.
>
> > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO, false);
> > + delay = DC5_REQ_IDLE_FRAMES * intel_get_frame_time_us(cstate);
> > + schedule_delayed_work(&dev_priv->csr.idle_work,
> > + usecs_to_jiffies(delay));
> > + }
> > +
> > +unlock:
> > + mutex_unlock(&dev_priv->psr.lock);
> > +}
> > +
> > static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
> > {
> > struct drm_atomic_state *state = crtc_state->base.state;
> > @@ -880,6 +945,14 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > }
> > }
> >
> > +void tgl_dc5_idle_thread(struct work_struct *work)
> > +{
> > + struct drm_i915_private *dev_priv =
> > + container_of(work, typeof(*dev_priv), csr.idle_work.work);
> > +
> > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
>
> So it would result in enabling deep sleep, but without the PSR lock.
> That's one reason we should really keep the PSR specific parts here.
>
> > +}
> > +
> > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> > {
> > if (!dev_priv->psr.sink_psr2_support)
> > @@ -1155,6 +1228,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > if (state == dev_priv->csr.max_dc_state)
> > goto unlock;
> >
> > + if (!psr2_deep_sleep)
> > + tgl_psr2_deep_sleep_disable(dev_priv);
> > +
> > if (!dc_off_enabled) {
> > /*
> > * Need to disable the DC off power well to
> > @@ -1167,6 +1243,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > }
> > dev_priv->csr.max_dc_state = state;
> >
> > + if (psr2_deep_sleep)
> > + tgl_psr2_deep_sleep_enable(dev_priv);
> > +
> > unlock:
> > mutex_unlock(&power_domains->lock);
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index d77a5a53f543..df096d64c744 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -9,6 +9,9 @@
> > #include "intel_display.h"
> > #include "intel_runtime_pm.h"
> > #include "i915_reg.h"
> > +#include "intel_frontbuffer.h"
> > +
> > +#define DC5_REQ_IDLE_FRAMES 6
>
> No need for a define for this.
>
> >
> > struct drm_i915_private;
> > struct intel_encoder;
> > @@ -266,6 +269,9 @@ void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > + unsigned int frontbuffer_bits, enum fb_op_origin origin);
> > +void tgl_dc5_idle_thread(struct work_struct *work);
> >
> > const char *
> > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > index fc40dc1fdbcc..c3b10f6e4382 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > @@ -90,6 +90,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> > might_sleep();
> > intel_edp_drrs_flush(i915, frontbuffer_bits);
> > intel_psr_flush(i915, frontbuffer_bits, origin);
> > + tgl_dc3co_flush(i915, frontbuffer_bits, origin);
> > intel_fbc_flush(i915, frontbuffer_bits, origin);
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3218b0319852..fe71119a458e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -338,6 +338,7 @@ struct intel_csr {
> > u32 dc_state;
> > u32 max_dc_state;
> > u32 allowed_dc_mask;
> > + struct delayed_work idle_work;
> > intel_wakeref_t wakeref;
> > /* cache the crtc on which DC3CO will be allowed */
> > struct intel_crtc *dc3co_crtc;
> > --
> > 2.21.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
2019-09-10 9:56 ` Anshuman Gupta
@ 2019-09-11 8:50 ` Imre Deak
2019-09-11 9:57 ` Anshuman Gupta
0 siblings, 1 reply; 28+ messages in thread
From: Imre Deak @ 2019-09-11 8:50 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx
On Tue, Sep 10, 2019 at 03:26:20PM +0530, Anshuman Gupta wrote:
> On 2019-09-08 at 20:55:17 +0300, Imre Deak wrote:
> Hi Imre ,
> Thanks for review, could you please provide your response on below
> comments.
> > On Sat, Sep 07, 2019 at 10:44:42PM +0530, Anshuman Gupta wrote:
> > > DC3CO is useful power state, when DMC detects PSR2 idle frame
> > > while an active video playback, playing 30fps video on 60hz panel
> > > is the classic example of this use case.
> > > DC5 and DC6 saves more power, but can't be entered during video
> > > playback because there are not enough idle frames in a row to meet
> > > most PSR2 panel deep sleep entry requirement typically 4 frames.
> >
> > Please also explain why DC3co will be enabled only for flips but not for
> > other frontbuffer flush events (ORIGIN_CS/DIRTYFB etc.) and that we
> > could enable it for those too by switching to manual PSR tracking and
> > programming only 1 idle frame for deep sleep (see below).
> >
> > Also explaining that the frontbuffer invalidate event doesn't need to be
> > acted on (b/c of PSR exit) would be helpful.
> >
> > >
> > > It will be worthy to enable DC3CO after completion of each flip
> > > and switch back to DC5 when display is idle, as driver doesn't
> > > differentiate between video playback and a normal flip.
> > > It is safer to allow DC5 after 6 idle frame, as PSR2 requires
> > > minimum 6 idle frame.
> >
> > It would be clearer to say here that after a flip we enable DC3co, after
> > which we wait manually 6 frames (by scheduling the idle frame work) at
> > which point we enable PSR deep sleep with 6 idle frames. After this 6
> > idle frames the HW will enter deep sleep and DC5 will be entered
> > after this by DMC at some point.
> >
> > The claim that we _have_ to make the HW wait for 6 idle frames before it
> > enters deep sleep doesn't make much sense to me, would be good to see a
> > reference to that if it really exists. That setting seems to only serve
> > the purpose to avoid update lags, but in the future (as discussed with
> > Ville) we should switch to manual PSR tracking and for that we would
> > program the HW to wait only 1 idle frame before entering deep sleep and
> > rely only on the manual 6 idle frame wait (via the idle frame work) to
> > avoid update lags.
> >
> > >
> > > v2: calculated s/w state to switch over dc3co when there is an
> > > update. [Imre]
> > > used cancel_delayed_work_sync() in order to avoid any race
> > > with already scheduled delayed work. [Imre]
> > > v3: cancel_delayed_work_sync() may blocked the commit work.
> > > Hence dropping it, dc5_idle_thread() checks the valid wakeref before
> > > putting the reference count, which avoids any chances of dropping
> > > a zero wakeref. [Imre (IRC)]
> > > v4: use frontbuffer flush mechanism. [Imre]
> > >
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 2 +
> > > .../drm/i915/display/intel_display_power.c | 79 +++++++++++++++++++
> > > .../drm/i915/display/intel_display_power.h | 6 ++
> > > .../gpu/drm/i915/display/intel_frontbuffer.c | 1 +
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > 5 files changed, 89 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 84488f87d058..2754e8ee693f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -16206,6 +16206,7 @@ int intel_modeset_init(struct drm_device *dev)
> > > init_llist_head(&dev_priv->atomic_helper.free_list);
> > > INIT_WORK(&dev_priv->atomic_helper.free_work,
> > > intel_atomic_helper_free_state_worker);
> > > + INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
> > >
> > > intel_init_quirks(dev_priv);
> > >
> > > @@ -17147,6 +17148,7 @@ void intel_modeset_driver_remove(struct drm_device *dev)
> > > flush_workqueue(dev_priv->modeset_wq);
> > >
> > > flush_work(&dev_priv->atomic_helper.free_work);
> > > + flush_delayed_work(&dev_priv->csr.idle_work);
> >
> > This is racy as the work could be still running, but also would leave a
> > few other places with the work running like suspend, so let's just make
> > sure that it's not running any more after encoder disabling.
> >
> > > WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
> > >
> > > /*
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index ecce118b5b0e..c110f04c9733 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -20,6 +20,7 @@
> > > #include "intel_sideband.h"
> > > #include "intel_tc.h"
> > > #include "intel_pm.h"
> > > +#include "intel_psr.h"
> > >
> > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> > > enum i915_power_well_id power_well_id);
> > > @@ -773,6 +774,27 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > > dev_priv->csr.dc_state = val & mask;
> > > }
> > >
> > > +static u32 intel_get_frame_time_us(const struct intel_crtc_state *cstate)
> > > +{
> > > + u32 pixel_rate, crtc_htotal, crtc_vtotal;
> > > + u32 frametime_us;
> > > +
> > > + if (!cstate || !cstate->base.active)
> > > + return 0;
> > > +
> > > + pixel_rate = cstate->pixel_rate;
> > > +
> > > + if (WARN_ON(pixel_rate == 0))
> > > + return 0;
> > > +
> > > + crtc_htotal = cstate->base.adjusted_mode.crtc_htotal;
> > > + crtc_vtotal = cstate->base.adjusted_mode.crtc_vtotal;
> > > + frametime_us = DIV_ROUND_UP(crtc_htotal * crtc_vtotal * 1000ULL,
> > > + pixel_rate);
> > > +
> > > + return frametime_us;
> > > +}
> > > +
> > > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > > @@ -817,6 +839,49 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > > I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > > }
> > >
> > > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > > + unsigned int frontbuffer_bits, enum fb_op_origin origin)
> > > +{
> > > + struct intel_crtc_state *cstate;
> > > + u32 delay;
> > > + unsigned int busy_frontbuffer_bits;
> > > +
> > > + if (!IS_TIGERLAKE(dev_priv))
> > > + return;
> > > +
> > > + if (origin != ORIGIN_FLIP)
> > > + return;
> > > +
> > > + if (!dev_priv->csr.dc3co_crtc)
> > > + return;
> > > +
> > > + cstate = to_intel_crtc_state(dev_priv->csr.dc3co_crtc->base.state);
> > > + frontbuffer_bits &=
> > > + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->csr.dc3co_crtc->pipe);
> > > + busy_frontbuffer_bits &= ~frontbuffer_bits;
> >
> > Hrm, this looks wrong. Also it depends on the PSR mechanism, so this
> > whole DC3co flush logic should rather be done from the PSR flush func,
> > using psr.pipe.
> >
> Hmm initially i have planned to have entire logic under psr flush func
> but PSR invalidate/flush logic for ORIGIN_FLIP relies on H/W tracking,
> PSR invalidate and flush call has early return for ORIGIN_FLIP
> unlike DC3CO case where we are only interested in ORIGIN_FLIP requests.
intel_psr_flush() would be very similar to what we need to do for
DC3co/ORIGIN_FLIP, except adjusting psr.busy_frontbuffer_bits. I think
eventually we could switch to full manual tracking as mentioned earlier,
so we'd also enable/disable PSR manually in case of ORIGIN_FLIP.
> > > +
> > > + mutex_lock(&dev_priv->psr.lock);
> > > +
> > > + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
> > > + goto unlock;
> > > +
> > > + /*
> > > + * At every flip frontbuffer flush first cancel the delayed work,
> > > + * when delayed schedules that means display has been idle
> > > + * for the 6 idle frame.
> > > + */
> > > + if (!busy_frontbuffer_bits) {
> > > + cancel_delayed_work(&dev_priv->csr.idle_work);
> >
> > The above is racy.
> Yes tgl_dc5_idle_thread() can even run after this but in that case also
> tgl_set_target_state() is protected by the power domain locks.
> Do u see any other harm of it apart from setting target_dc_state
> immediately to DC5/DC6 after it sets to DC3CO (IMO this would be a little
> penalty for a VBLANK in next page flip it will again set target_dc_state to DC3CO).
>
> I can think of two possible solutions to avoid this race.
> 1. cancel_delayed_work_sync().
> 2. Having a flag to indicate that delayed work has been canceled
> and same can be used by tgl_dc5_idle_thread() to have an early return.
> Please suggest your opinion on it.
One way would be, instead of canceling the work and scheduling a new one,
extend the timer of it and do an early return in the work if another one
is already pending (due to the extension).
During PSR disabling (as part of encoder disabling)
cancel_delayed_work_sync() could be used then.
> >
> > > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO, false);
> > > + delay = DC5_REQ_IDLE_FRAMES * intel_get_frame_time_us(cstate);
> > > + schedule_delayed_work(&dev_priv->csr.idle_work,
> > > + usecs_to_jiffies(delay));
> > > + }
> > > +
> > > +unlock:
> > > + mutex_unlock(&dev_priv->psr.lock);
> > > +}
> > > +
> > > static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
> > > {
> > > struct drm_atomic_state *state = crtc_state->base.state;
> > > @@ -880,6 +945,14 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > > }
> > > }
> > >
> > > +void tgl_dc5_idle_thread(struct work_struct *work)
> > > +{
> > > + struct drm_i915_private *dev_priv =
> > > + container_of(work, typeof(*dev_priv), csr.idle_work.work);
> > > +
> > > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
> >
> > So it would result in enabling deep sleep, but without the PSR lock.
> > That's one reason we should really keep the PSR specific parts here.
> >
> > > +}
> > > +
> > > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> > > {
> > > if (!dev_priv->psr.sink_psr2_support)
> > > @@ -1155,6 +1228,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > if (state == dev_priv->csr.max_dc_state)
> > > goto unlock;
> > >
> > > + if (!psr2_deep_sleep)
> > > + tgl_psr2_deep_sleep_disable(dev_priv);
> > > +
> > > if (!dc_off_enabled) {
> > > /*
> > > * Need to disable the DC off power well to
> > > @@ -1167,6 +1243,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > }
> > > dev_priv->csr.max_dc_state = state;
> > >
> > > + if (psr2_deep_sleep)
> > > + tgl_psr2_deep_sleep_enable(dev_priv);
> > > +
> > > unlock:
> > > mutex_unlock(&power_domains->lock);
> > > }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index d77a5a53f543..df096d64c744 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > @@ -9,6 +9,9 @@
> > > #include "intel_display.h"
> > > #include "intel_runtime_pm.h"
> > > #include "i915_reg.h"
> > > +#include "intel_frontbuffer.h"
> > > +
> > > +#define DC5_REQ_IDLE_FRAMES 6
> >
> > No need for a define for this.
> >
> > >
> > > struct drm_i915_private;
> > > struct intel_encoder;
> > > @@ -266,6 +269,9 @@ void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > > void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > > void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > > + unsigned int frontbuffer_bits, enum fb_op_origin origin);
> > > +void tgl_dc5_idle_thread(struct work_struct *work);
> > >
> > > const char *
> > > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > index fc40dc1fdbcc..c3b10f6e4382 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > @@ -90,6 +90,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> > > might_sleep();
> > > intel_edp_drrs_flush(i915, frontbuffer_bits);
> > > intel_psr_flush(i915, frontbuffer_bits, origin);
> > > + tgl_dc3co_flush(i915, frontbuffer_bits, origin);
> > > intel_fbc_flush(i915, frontbuffer_bits, origin);
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 3218b0319852..fe71119a458e 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -338,6 +338,7 @@ struct intel_csr {
> > > u32 dc_state;
> > > u32 max_dc_state;
> > > u32 allowed_dc_mask;
> > > + struct delayed_work idle_work;
> > > intel_wakeref_t wakeref;
> > > /* cache the crtc on which DC3CO will be allowed */
> > > struct intel_crtc *dc3co_crtc;
> > > --
> > > 2.21.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
2019-09-11 8:50 ` Imre Deak
@ 2019-09-11 9:57 ` Anshuman Gupta
0 siblings, 0 replies; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-11 9:57 UTC (permalink / raw)
To: Imre Deak; +Cc: jani.nikula, intel-gfx
On 2019-09-11 at 11:50:26 +0300, Imre Deak wrote:
> On Tue, Sep 10, 2019 at 03:26:20PM +0530, Anshuman Gupta wrote:
> > On 2019-09-08 at 20:55:17 +0300, Imre Deak wrote:
> > Hi Imre ,
> > Thanks for review, could you please provide your response on below
> > comments.
> > > On Sat, Sep 07, 2019 at 10:44:42PM +0530, Anshuman Gupta wrote:
> > > > DC3CO is useful power state, when DMC detects PSR2 idle frame
> > > > while an active video playback, playing 30fps video on 60hz panel
> > > > is the classic example of this use case.
> > > > DC5 and DC6 saves more power, but can't be entered during video
> > > > playback because there are not enough idle frames in a row to meet
> > > > most PSR2 panel deep sleep entry requirement typically 4 frames.
> > >
> > > Please also explain why DC3co will be enabled only for flips but not for
> > > other frontbuffer flush events (ORIGIN_CS/DIRTYFB etc.) and that we
> > > could enable it for those too by switching to manual PSR tracking and
> > > programming only 1 idle frame for deep sleep (see below).
> > >
> > > Also explaining that the frontbuffer invalidate event doesn't need to be
> > > acted on (b/c of PSR exit) would be helpful.
> > >
> > > >
> > > > It will be worthy to enable DC3CO after completion of each flip
> > > > and switch back to DC5 when display is idle, as driver doesn't
> > > > differentiate between video playback and a normal flip.
> > > > It is safer to allow DC5 after 6 idle frame, as PSR2 requires
> > > > minimum 6 idle frame.
> > >
> > > It would be clearer to say here that after a flip we enable DC3co, after
> > > which we wait manually 6 frames (by scheduling the idle frame work) at
> > > which point we enable PSR deep sleep with 6 idle frames. After this 6
> > > idle frames the HW will enter deep sleep and DC5 will be entered
> > > after this by DMC at some point.
> > >
> > > The claim that we _have_ to make the HW wait for 6 idle frames before it
> > > enters deep sleep doesn't make much sense to me, would be good to see a
> > > reference to that if it really exists. That setting seems to only serve
> > > the purpose to avoid update lags, but in the future (as discussed with
> > > Ville) we should switch to manual PSR tracking and for that we would
> > > program the HW to wait only 1 idle frame before entering deep sleep and
> > > rely only on the manual 6 idle frame wait (via the idle frame work) to
> > > avoid update lags.
> > >
> > > >
> > > > v2: calculated s/w state to switch over dc3co when there is an
> > > > update. [Imre]
> > > > used cancel_delayed_work_sync() in order to avoid any race
> > > > with already scheduled delayed work. [Imre]
> > > > v3: cancel_delayed_work_sync() may blocked the commit work.
> > > > Hence dropping it, dc5_idle_thread() checks the valid wakeref before
> > > > putting the reference count, which avoids any chances of dropping
> > > > a zero wakeref. [Imre (IRC)]
> > > > v4: use frontbuffer flush mechanism. [Imre]
> > > >
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +
> > > > .../drm/i915/display/intel_display_power.c | 79 +++++++++++++++++++
> > > > .../drm/i915/display/intel_display_power.h | 6 ++
> > > > .../gpu/drm/i915/display/intel_frontbuffer.c | 1 +
> > > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > > 5 files changed, 89 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 84488f87d058..2754e8ee693f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -16206,6 +16206,7 @@ int intel_modeset_init(struct drm_device *dev)
> > > > init_llist_head(&dev_priv->atomic_helper.free_list);
> > > > INIT_WORK(&dev_priv->atomic_helper.free_work,
> > > > intel_atomic_helper_free_state_worker);
> > > > + INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
> > > >
> > > > intel_init_quirks(dev_priv);
> > > >
> > > > @@ -17147,6 +17148,7 @@ void intel_modeset_driver_remove(struct drm_device *dev)
> > > > flush_workqueue(dev_priv->modeset_wq);
> > > >
> > > > flush_work(&dev_priv->atomic_helper.free_work);
> > > > + flush_delayed_work(&dev_priv->csr.idle_work);
> > >
> > > This is racy as the work could be still running, but also would leave a
> > > few other places with the work running like suspend, so let's just make
> > > sure that it's not running any more after encoder disabling.
> > >
> > > > WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
> > > >
> > > > /*
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > index ecce118b5b0e..c110f04c9733 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > @@ -20,6 +20,7 @@
> > > > #include "intel_sideband.h"
> > > > #include "intel_tc.h"
> > > > #include "intel_pm.h"
> > > > +#include "intel_psr.h"
> > > >
> > > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> > > > enum i915_power_well_id power_well_id);
> > > > @@ -773,6 +774,27 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > > > dev_priv->csr.dc_state = val & mask;
> > > > }
> > > >
> > > > +static u32 intel_get_frame_time_us(const struct intel_crtc_state *cstate)
> > > > +{
> > > > + u32 pixel_rate, crtc_htotal, crtc_vtotal;
> > > > + u32 frametime_us;
> > > > +
> > > > + if (!cstate || !cstate->base.active)
> > > > + return 0;
> > > > +
> > > > + pixel_rate = cstate->pixel_rate;
> > > > +
> > > > + if (WARN_ON(pixel_rate == 0))
> > > > + return 0;
> > > > +
> > > > + crtc_htotal = cstate->base.adjusted_mode.crtc_htotal;
> > > > + crtc_vtotal = cstate->base.adjusted_mode.crtc_vtotal;
> > > > + frametime_us = DIV_ROUND_UP(crtc_htotal * crtc_vtotal * 1000ULL,
> > > > + pixel_rate);
> > > > +
> > > > + return frametime_us;
> > > > +}
> > > > +
> > > > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > > > @@ -817,6 +839,49 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > > > I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > > > }
> > > >
> > > > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > > > + unsigned int frontbuffer_bits, enum fb_op_origin origin)
> > > > +{
> > > > + struct intel_crtc_state *cstate;
> > > > + u32 delay;
> > > > + unsigned int busy_frontbuffer_bits;
> > > > +
> > > > + if (!IS_TIGERLAKE(dev_priv))
> > > > + return;
> > > > +
> > > > + if (origin != ORIGIN_FLIP)
> > > > + return;
> > > > +
> > > > + if (!dev_priv->csr.dc3co_crtc)
> > > > + return;
> > > > +
> > > > + cstate = to_intel_crtc_state(dev_priv->csr.dc3co_crtc->base.state);
> > > > + frontbuffer_bits &=
> > > > + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->csr.dc3co_crtc->pipe);
> > > > + busy_frontbuffer_bits &= ~frontbuffer_bits;
> > >
> > > Hrm, this looks wrong. Also it depends on the PSR mechanism, so this
> > > whole DC3co flush logic should rather be done from the PSR flush func,
> > > using psr.pipe.
> > >
> > Hmm initially i have planned to have entire logic under psr flush func
> > but PSR invalidate/flush logic for ORIGIN_FLIP relies on H/W tracking,
> > PSR invalidate and flush call has early return for ORIGIN_FLIP
> > unlike DC3CO case where we are only interested in ORIGIN_FLIP requests.
>
> intel_psr_flush() would be very similar to what we need to do for
> DC3co/ORIGIN_FLIP, except adjusting psr.busy_frontbuffer_bits. I think
> eventually we could switch to full manual tracking as mentioned earlier,
> so we'd also enable/disable PSR manually in case of ORIGIN_FLIP.
So i will use the different function tgl_dc3co_flush() with a comment "in
future we will use intel_psr_flush when we will switch to full manual tracking
for PSR" and will use the psr.pipe.
>
> > > > +
> > > > + mutex_lock(&dev_priv->psr.lock);
> > > > +
> > > > + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
> > > > + goto unlock;
> > > > +
> > > > + /*
> > > > + * At every flip frontbuffer flush first cancel the delayed work,
> > > > + * when delayed schedules that means display has been idle
> > > > + * for the 6 idle frame.
> > > > + */
> > > > + if (!busy_frontbuffer_bits) {
> > > > + cancel_delayed_work(&dev_priv->csr.idle_work);
> > >
> > > The above is racy.
> > Yes tgl_dc5_idle_thread() can even run after this but in that case also
> > tgl_set_target_state() is protected by the power domain locks.
> > Do u see any other harm of it apart from setting target_dc_state
> > immediately to DC5/DC6 after it sets to DC3CO (IMO this would be a little
> > penalty for a VBLANK in next page flip it will again set target_dc_state to DC3CO).
> >
> > I can think of two possible solutions to avoid this race.
> > 1. cancel_delayed_work_sync().
> > 2. Having a flag to indicate that delayed work has been canceled
> > and same can be used by tgl_dc5_idle_thread() to have an early return.
> > Please suggest your opinion on it.
>
> One way would be, instead of canceling the work and scheduling a new one,
> extend the timer of it and do an early return in the work if another one
> is already pending (due to the extension).
>
> During PSR disabling (as part of encoder disabling)
> cancel_delayed_work_sync() could be used then.
I am using cancel_delayed_work_sync() for next version
in tgl_disable_psr2_transcoder_exitline() func which calls from
intel_psr_disable() as part of encoder disable.
Thanks,
Anshuman Gupta.
>
> > >
> > > > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO, false);
> > > > + delay = DC5_REQ_IDLE_FRAMES * intel_get_frame_time_us(cstate);
> > > > + schedule_delayed_work(&dev_priv->csr.idle_work,
> > > > + usecs_to_jiffies(delay));
> > > > + }
> > > > +
> > > > +unlock:
> > > > + mutex_unlock(&dev_priv->psr.lock);
> > > > +}
> > > > +
> > > > static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state *crtc_state)
> > > > {
> > > > struct drm_atomic_state *state = crtc_state->base.state;
> > > > @@ -880,6 +945,14 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > > > }
> > > > }
> > > >
> > > > +void tgl_dc5_idle_thread(struct work_struct *work)
> > > > +{
> > > > + struct drm_i915_private *dev_priv =
> > > > + container_of(work, typeof(*dev_priv), csr.idle_work.work);
> > > > +
> > > > + tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6, true);
> > >
> > > So it would result in enabling deep sleep, but without the PSR lock.
> > > That's one reason we should really keep the PSR specific parts here.
> > >
> > > > +}
> > > > +
> > > > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv)
> > > > {
> > > > if (!dev_priv->psr.sink_psr2_support)
> > > > @@ -1155,6 +1228,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > > if (state == dev_priv->csr.max_dc_state)
> > > > goto unlock;
> > > >
> > > > + if (!psr2_deep_sleep)
> > > > + tgl_psr2_deep_sleep_disable(dev_priv);
> > > > +
> > > > if (!dc_off_enabled) {
> > > > /*
> > > > * Need to disable the DC off power well to
> > > > @@ -1167,6 +1243,9 @@ void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state,
> > > > }
> > > > dev_priv->csr.max_dc_state = state;
> > > >
> > > > + if (psr2_deep_sleep)
> > > > + tgl_psr2_deep_sleep_enable(dev_priv);
> > > > +
> > > > unlock:
> > > > mutex_unlock(&power_domains->lock);
> > > > }
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > index d77a5a53f543..df096d64c744 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > @@ -9,6 +9,9 @@
> > > > #include "intel_display.h"
> > > > #include "intel_runtime_pm.h"
> > > > #include "i915_reg.h"
> > > > +#include "intel_frontbuffer.h"
> > > > +
> > > > +#define DC5_REQ_IDLE_FRAMES 6
> > >
> > > No need for a define for this.
> > >
> > > >
> > > > struct drm_i915_private;
> > > > struct intel_encoder;
> > > > @@ -266,6 +269,9 @@ void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > > > void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > > > void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > > > void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > > > +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> > > > + unsigned int frontbuffer_bits, enum fb_op_origin origin);
> > > > +void tgl_dc5_idle_thread(struct work_struct *work);
> > > >
> > > > const char *
> > > > intel_display_power_domain_str(enum intel_display_power_domain domain);
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > > index fc40dc1fdbcc..c3b10f6e4382 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > > > @@ -90,6 +90,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> > > > might_sleep();
> > > > intel_edp_drrs_flush(i915, frontbuffer_bits);
> > > > intel_psr_flush(i915, frontbuffer_bits, origin);
> > > > + tgl_dc3co_flush(i915, frontbuffer_bits, origin);
> > > > intel_fbc_flush(i915, frontbuffer_bits, origin);
> > > > }
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 3218b0319852..fe71119a458e 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -338,6 +338,7 @@ struct intel_csr {
> > > > u32 dc_state;
> > > > u32 max_dc_state;
> > > > u32 allowed_dc_mask;
> > > > + struct delayed_work idle_work;
> > > > intel_wakeref_t wakeref;
> > > > /* cache the crtc on which DC3CO will be allowed */
> > > > struct intel_crtc *dc3co_crtc;
> > > > --
> > > > 2.21.0
> > > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v7 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (5 preceding siblings ...)
2019-09-07 17:14 ` [PATCH v7 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
@ 2019-09-07 17:14 ` Anshuman Gupta
2019-09-07 17:34 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev7) Patchwork
` (7 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Anshuman Gupta @ 2019-09-07 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Adding DC3CO counter in i915_dmc_info debugfs will be
useful for DC3CO validation.
DMC firmware uses DMC_DEBUG3 register as DC3CO counter
register on TGL, as per B.Specs DMC_DEBUG3 is general
purpose register.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
drivers/gpu/drm/i915/i915_reg.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 708855e051b5..161c4bcd3756 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2397,6 +2397,12 @@ static int i915_dmc_info(struct seq_file *m, void *unused)
seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
CSR_VERSION_MINOR(csr->version));
+ /*
+ * TGL DMC f/w uses DMC_DEBUG3 register for DC3CO counter.
+ */
+ if (IS_TIGERLAKE(dev_priv))
+ seq_printf(m, "DC3CO count: %d\n", I915_READ(DMC_DEBUG3));
+
if (INTEL_GEN(dev_priv) >= 12) {
dc5_reg = TGL_DMC_DEBUG_DC5_COUNT;
dc6_reg = TGL_DMC_DEBUG_DC6_COUNT;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 273a4ed8b3e9..0fb86a78c4a4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7251,6 +7251,8 @@ enum {
#define TGL_DMC_DEBUG_DC5_COUNT _MMIO(0x101084)
#define TGL_DMC_DEBUG_DC6_COUNT _MMIO(0x101088)
+#define DMC_DEBUG3 _MMIO(0x101090)
+
/* interrupts */
#define DE_MASTER_IRQ_CONTROL (1 << 31)
#define DE_SPRITEB_FLIP_DONE (1 << 29)
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev7)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (6 preceding siblings ...)
2019-09-07 17:14 ` [PATCH v7 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
@ 2019-09-07 17:34 ` Patchwork
2019-09-07 17:37 ` ✗ Fi.CI.SPARSE: " Patchwork
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-07 17:34 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev7)
URL : https://patchwork.freedesktop.org/series/64923/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b9b649158ff7 drm/i915/tgl: Add DC3CO required register and bits
d747f1009d07 drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
d700315d1374 drm/i915/tgl: Enable DC3CO state in "DC Off" power well
-:56: CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
#56: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:795:
+ udelay(200);
total: 0 errors, 0 warnings, 1 checks, 173 lines checked
20f1f169303f drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
f0066404eeed drm/i915/tgl: DC3CO PSR2 helper
6bc7d57d681e drm/i915/tgl: switch between dc3co and dc5 based on display idleness
fb593dad0f1a drm/i915/tgl: Add DC3CO counter in i915_dmc_info
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✗ Fi.CI.SPARSE: warning for DC3CO Support for TGL (rev7)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (7 preceding siblings ...)
2019-09-07 17:34 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev7) Patchwork
@ 2019-09-07 17:37 ` Patchwork
2019-09-07 17:58 ` ✓ Fi.CI.BAT: success " Patchwork
` (5 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-07 17:37 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev7)
URL : https://patchwork.freedesktop.org/series/64923/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/tgl: Add DC3CO required register and bits
Okay!
Commit: drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
Okay!
Commit: drm/i915/tgl: Enable DC3CO state in "DC Off" power well
Okay!
Commit: drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✓ Fi.CI.BAT: success for DC3CO Support for TGL (rev7)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (8 preceding siblings ...)
2019-09-07 17:37 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-09-07 17:58 ` Patchwork
2019-09-07 19:03 ` ✗ Fi.CI.IGT: failure " Patchwork
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-07 17:58 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev7)
URL : https://patchwork.freedesktop.org/series/64923/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6849 -> Patchwork_14315
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/
Known issues
------------
Here are the changes found in Patchwork_14315 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_flink_basic@double-flink:
- fi-icl-u3: [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u3/igt@gem_flink_basic@double-flink.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/fi-icl-u3/igt@gem_flink_basic@double-flink.html
* igt@kms_chamelium@dp-edid-read:
- fi-icl-u2: [PASS][3] -> [FAIL][4] ([fdo#109483] / [fdo#109635 ])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][5] ([fdo#107718]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@i915_module_load@reload-no-display:
- fi-icl-u3: [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u3/igt@i915_module_load@reload-no-display.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/fi-icl-u3/igt@i915_module_load@reload-no-display.html
#### Warnings ####
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][9] ([fdo#111096]) -> [FAIL][10] ([fdo#111407])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
Participating hosts (52 -> 45)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6849 -> Patchwork_14315
CI-20190529: 20190529
CI_DRM_6849: d4111d3dd34455068de02ce18b796a631c7fe3a9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14315: fb593dad0f1abb97f45a36ee6d2a346abbcebd8a @ git://anongit.freedesktop.org/gfx-ci/linux
== Kernel 32bit build ==
Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/build_32bit.log
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHK include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready (#1)
Building modules, stage 2.
MODPOST 117 modules
ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
scripts/Makefile.modpost:103: recipe for target 'modules-modpost' failed
make[1]: *** [modules-modpost] Error 1
Makefile:1301: recipe for target 'modules' failed
make: *** [modules] Error 2
== Linux commits ==
fb593dad0f1a drm/i915/tgl: Add DC3CO counter in i915_dmc_info
6bc7d57d681e drm/i915/tgl: switch between dc3co and dc5 based on display idleness
f0066404eeed drm/i915/tgl: DC3CO PSR2 helper
20f1f169303f drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
d700315d1374 drm/i915/tgl: Enable DC3CO state in "DC Off" power well
d747f1009d07 drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
b9b649158ff7 drm/i915/tgl: Add DC3CO required register and bits
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✗ Fi.CI.IGT: failure for DC3CO Support for TGL (rev7)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (9 preceding siblings ...)
2019-09-07 17:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-07 19:03 ` Patchwork
2019-09-08 5:46 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev8) Patchwork
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-07 19:03 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev7)
URL : https://patchwork.freedesktop.org/series/64923/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6849_full -> Patchwork_14315_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_14315_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_14315_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_14315_full:
### IGT changes ###
#### Possible regressions ####
* igt@kms_flip@dpms-vs-vblank-race-interruptible:
- shard-glk: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-glk7/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-glk8/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
Known issues
------------
Here are the changes found in Patchwork_14315_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-kbl: [PASS][3] -> [INCOMPLETE][4] ([fdo#103665])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-kbl3/igt@gem_ctx_isolation@rcs0-s3.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-kbl1/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_ctx_shared@exec-single-timeline-bsd:
- shard-iclb: [PASS][5] -> [SKIP][6] ([fdo#110841])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
* igt@gem_ctx_shared@q-smoketest-bsd2:
- shard-iclb: [PASS][7] -> [SKIP][8] ([fdo#109276]) +8 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb1/igt@gem_ctx_shared@q-smoketest-bsd2.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb6/igt@gem_ctx_shared@q-smoketest-bsd2.html
* igt@gem_eio@reset-stress:
- shard-apl: [PASS][9] -> [FAIL][10] ([fdo#109661])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-apl8/igt@gem_eio@reset-stress.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-apl4/igt@gem_eio@reset-stress.html
* igt@gem_exec_schedule@in-order-bsd:
- shard-iclb: [PASS][11] -> [SKIP][12] ([fdo#111325]) +2 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb7/igt@gem_exec_schedule@in-order-bsd.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb1/igt@gem_exec_schedule@in-order-bsd.html
* igt@gem_tiled_swapping@non-threaded:
- shard-glk: [PASS][13] -> [DMESG-WARN][14] ([fdo#108686])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-glk5/igt@gem_tiled_swapping@non-threaded.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-glk4/igt@gem_tiled_swapping@non-threaded.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167]) +4 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@fbcpsr-suspend:
- shard-skl: [PASS][17] -> [INCOMPLETE][18] ([fdo#104108] / [fdo#106978])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl10/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl4/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
* igt@kms_psr@psr2_cursor_plane_move:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441]) +1 similar issue
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb4/igt@kms_psr@psr2_cursor_plane_move.html
* igt@kms_setmode@basic:
- shard-hsw: [PASS][21] -> [FAIL][22] ([fdo#99912])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-hsw4/igt@kms_setmode@basic.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-hsw4/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-apl: [PASS][23] -> [DMESG-WARN][24] ([fdo#108566]) +3 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-apl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-apl8/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
* igt@perf@blocking:
- shard-skl: [PASS][25] -> [FAIL][26] ([fdo#110728])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl7/igt@perf@blocking.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl6/igt@perf@blocking.html
#### Possible fixes ####
* igt@gem_ctx_isolation@bcs0-s3:
- shard-kbl: [DMESG-WARN][27] ([fdo#103313]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-kbl6/igt@gem_ctx_isolation@bcs0-s3.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-kbl3/igt@gem_ctx_isolation@bcs0-s3.html
* igt@gem_exec_async@concurrent-writes-bsd:
- shard-iclb: [SKIP][29] ([fdo#111325]) -> [PASS][30] +4 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb8/igt@gem_exec_async@concurrent-writes-bsd.html
* igt@gem_exec_suspend@basic-s3:
- shard-kbl: [INCOMPLETE][31] ([fdo#103665]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-kbl3/igt@gem_exec_suspend@basic-s3.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-kbl6/igt@gem_exec_suspend@basic-s3.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [INCOMPLETE][33] ([fdo#104108]) -> [PASS][34] +1 similar issue
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl4/igt@gem_softpin@noreloc-s3.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl4/igt@gem_softpin@noreloc-s3.html
* igt@gem_tiled_swapping@non-threaded:
- shard-apl: [DMESG-WARN][35] ([fdo#108686]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-apl4/igt@gem_tiled_swapping@non-threaded.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-apl6/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_wait@basic-wait-write-all:
- shard-skl: [DMESG-WARN][37] ([fdo#106107]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl5/igt@gem_wait@basic-wait-write-all.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl7/igt@gem_wait@basic-wait-write-all.html
* igt@i915_suspend@fence-restore-tiled2untiled:
- shard-apl: [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +7 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
* igt@kms_cursor_crc@pipe-b-cursor-suspend:
- shard-skl: [INCOMPLETE][41] ([fdo#110741]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl10/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw: [FAIL][43] ([fdo#105767]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
* igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
- shard-glk: [FAIL][45] ([fdo#105363]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
* igt@kms_flip@flip-vs-suspend-interruptible:
- shard-skl: [INCOMPLETE][47] ([fdo#109507]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl8/igt@kms_flip@flip-vs-suspend-interruptible.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl10/igt@kms_flip@flip-vs-suspend-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: [FAIL][49] ([fdo#103167]) -> [PASS][50] +6 similar issues
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [FAIL][51] ([fdo#108145]) -> [PASS][52]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [FAIL][53] ([fdo#108145] / [fdo#110403]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_psr@psr2_sprite_blt:
- shard-iclb: [SKIP][55] ([fdo#109441]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb3/igt@kms_psr@psr2_sprite_blt.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
* igt@prime_vgem@fence-wait-bsd2:
- shard-iclb: [SKIP][57] ([fdo#109276]) -> [PASS][58] +11 similar issues
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb7/igt@prime_vgem@fence-wait-bsd2.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
#### Warnings ####
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: [SKIP][59] ([fdo#109349]) -> [DMESG-WARN][60] ([fdo#107724])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-iclb3/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- shard-skl: [FAIL][61] ([fdo#108040]) -> [FAIL][62] ([fdo#103167])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-skl4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-skl4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
- shard-apl: [INCOMPLETE][63] ([fdo#103927]) -> [SKIP][64] ([fdo#109271])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/shard-apl2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14315/shard-apl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt.html
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
[fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
[fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
[fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6849 -> Patchwork_14315
CI-20190529: 20190529
CI_DRM_6849: d4111d3dd34455068de02ce18b796a631c7fe3a9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14315: fb593dad0f1abb97f45a36ee6d2a346abbcebd8a @ 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_14315/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev8)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (10 preceding siblings ...)
2019-09-07 19:03 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-09-08 5:46 ` Patchwork
2019-09-08 5:49 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-08 5:46 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev8)
URL : https://patchwork.freedesktop.org/series/64923/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
5f6bfdba251e drm/i915/tgl: Add DC3CO required register and bits
0d688fd512cf drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
5f17b9ab767f drm/i915/tgl: Enable DC3CO state in "DC Off" power well
-:56: CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
#56: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:795:
+ udelay(200);
total: 0 errors, 0 warnings, 1 checks, 173 lines checked
cc4a3c84e086 drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
3559a4f92a27 drm/i915/tgl: DC3CO PSR2 helper
799a2cf9ae05 drm/i915/tgl: switch between dc3co and dc5 based on display idleness
bbb46e48c0a2 drm/i915/tgl: Add DC3CO counter in i915_dmc_info
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✗ Fi.CI.SPARSE: warning for DC3CO Support for TGL (rev8)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (11 preceding siblings ...)
2019-09-08 5:46 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev8) Patchwork
@ 2019-09-08 5:49 ` Patchwork
2019-09-08 6:10 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-08 7:19 ` ✓ Fi.CI.IGT: " Patchwork
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-08 5:49 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev8)
URL : https://patchwork.freedesktop.org/series/64923/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/tgl: Add DC3CO required register and bits
Okay!
Commit: drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
Okay!
Commit: drm/i915/tgl: Enable DC3CO state in "DC Off" power well
Okay!
Commit: drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✓ Fi.CI.BAT: success for DC3CO Support for TGL (rev8)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (12 preceding siblings ...)
2019-09-08 5:49 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-09-08 6:10 ` Patchwork
2019-09-08 7:19 ` ✓ Fi.CI.IGT: " Patchwork
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-08 6:10 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev8)
URL : https://patchwork.freedesktop.org/series/64923/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6850 -> Patchwork_14319
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/
Known issues
------------
Here are the changes found in Patchwork_14319 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_hangcheck:
- fi-icl-u3: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#108569])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][3] -> [FAIL][4] ([fdo#111407])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- {fi-icl-u4}: [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u4/igt@gem_ctx_create@basic-files.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-icl-u4/igt@gem_ctx_create@basic-files.html
* igt@kms_busy@basic-flip-c:
- fi-kbl-7500u: [SKIP][7] ([fdo#109271] / [fdo#109278]) -> [PASS][8] +2 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
* igt@kms_chamelium@dp-edid-read:
- fi-cml-u2: [FAIL][9] ([fdo#109483]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-icl-u2: [DMESG-WARN][11] ([fdo#110595]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
Participating hosts (52 -> 45)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6850 -> Patchwork_14319
CI-20190529: 20190529
CI_DRM_6850: edc92c96939102c2a59e389195e8d04049e9a022 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14319: bbb46e48c0a231ce57350b17d8fee75dd8a4c2bd @ git://anongit.freedesktop.org/gfx-ci/linux
== Kernel 32bit build ==
Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/build_32bit.log
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHK include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready (#1)
Building modules, stage 2.
MODPOST 117 modules
ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
scripts/Makefile.modpost:103: recipe for target 'modules-modpost' failed
make[1]: *** [modules-modpost] Error 1
Makefile:1301: recipe for target 'modules' failed
make: *** [modules] Error 2
== Linux commits ==
bbb46e48c0a2 drm/i915/tgl: Add DC3CO counter in i915_dmc_info
799a2cf9ae05 drm/i915/tgl: switch between dc3co and dc5 based on display idleness
3559a4f92a27 drm/i915/tgl: DC3CO PSR2 helper
cc4a3c84e086 drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
5f17b9ab767f drm/i915/tgl: Enable DC3CO state in "DC Off" power well
0d688fd512cf drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
5f6bfdba251e drm/i915/tgl: Add DC3CO required register and bits
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* ✓ Fi.CI.IGT: success for DC3CO Support for TGL (rev8)
2019-09-07 17:14 [PATCH v7 0/7] DC3CO Support for TGL Anshuman Gupta
` (13 preceding siblings ...)
2019-09-08 6:10 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-08 7:19 ` Patchwork
14 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-09-08 7:19 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: intel-gfx
== Series Details ==
Series: DC3CO Support for TGL (rev8)
URL : https://patchwork.freedesktop.org/series/64923/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6850_full -> Patchwork_14319_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_14319_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +4 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-apl3/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_exec_async@concurrent-writes-bsd:
- shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#111325]) +7 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb5/igt@gem_exec_async@concurrent-writes-bsd.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb1/igt@gem_exec_async@concurrent-writes-bsd.html
* igt@gem_exec_schedule@promotion-bsd1:
- shard-iclb: [PASS][5] -> [SKIP][6] ([fdo#109276]) +21 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb1/igt@gem_exec_schedule@promotion-bsd1.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb5/igt@gem_exec_schedule@promotion-bsd1.html
* igt@kms_atomic@plane_cursor_legacy:
- shard-snb: [PASS][7] -> [SKIP][8] ([fdo#109271])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-snb5/igt@kms_atomic@plane_cursor_legacy.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-snb5/igt@kms_atomic@plane_cursor_legacy.html
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
- shard-glk: [PASS][9] -> [FAIL][10] ([fdo#104873])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
- shard-iclb: [PASS][11] -> [FAIL][12] ([fdo#103167]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [PASS][13] -> [FAIL][14] ([fdo#108145] / [fdo#110403])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [PASS][15] -> [SKIP][16] ([fdo#109642] / [fdo#111068])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb5/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@psr2_sprite_mmap_gtt:
- shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +2 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][19] -> [FAIL][20] ([fdo#99912])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl1/igt@kms_setmode@basic.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-apl1/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- shard-skl: [PASS][21] -> [INCOMPLETE][22] ([fdo#104108])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl6/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-skl2/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
#### Possible fixes ####
* igt@gem_exec_balancer@smoke:
- shard-iclb: [SKIP][23] ([fdo#110854]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb8/igt@gem_exec_balancer@smoke.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb4/igt@gem_exec_balancer@smoke.html
* igt@gem_exec_schedule@preemptive-hang-bsd:
- shard-iclb: [SKIP][25] ([fdo#111325]) -> [PASS][26] +3 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: [FAIL][27] ([fdo#105363]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@basic:
- shard-iclb: [FAIL][29] ([fdo#103167]) -> [PASS][30] +4 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_frontbuffer_tracking@basic.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb1/igt@kms_frontbuffer_tracking@basic.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [FAIL][31] ([fdo#108145]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [FAIL][33] ([fdo#103166]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36] +1 similar issue
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-apl: [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +5 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-apl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
* igt@prime_busy@after-bsd2:
- shard-iclb: [SKIP][39] ([fdo#109276]) -> [PASS][40] +14 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb3/igt@prime_busy@after-bsd2.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb2/igt@prime_busy@after-bsd2.html
#### Warnings ####
* igt@gem_mocs_settings@mocs-reset-bsd2:
- shard-iclb: [SKIP][41] ([fdo#109276]) -> [FAIL][42] ([fdo#111330]) +1 similar issue
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: [DMESG-WARN][43] ([fdo#107724]) -> [SKIP][44] ([fdo#109349])
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-iclb4/igt@kms_dp_dsc@basic-dsc-enable-edp.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- shard-skl: [FAIL][45] ([fdo#103167]) -> [FAIL][46] ([fdo#108040])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14319/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6850 -> Patchwork_14319
CI-20190529: 20190529
CI_DRM_6850: edc92c96939102c2a59e389195e8d04049e9a022 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14319: bbb46e48c0a231ce57350b17d8fee75dd8a4c2bd @ 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_14319/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread