All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/7] DC3CO Support for TGL
@ 2019-09-13  8:23 Anshuman Gupta
  2019-09-13  8:23 ` [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

v8 revision is a rework of series, which has fixed the review comments
provided by Imre and Animesh.

Anshuman Gupta (7):
  drm/i915/tgl: Add DC3CO required register and bits
  drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
  drm/i915/tgl: Enable DC3CO state in "DC Off" power well
  drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
  drm/i915/tgl: DC3CO PSR2 helper
  drm/i915/tgl: switch between dc3co and dc5 based on display idleness
  drm/i915/tgl: Add DC3CO counter in i915_dmc_info

 drivers/gpu/drm/i915/display/intel_display.c  |   5 +
 .../drm/i915/display/intel_display_power.c    | 332 +++++++++++++++++-
 .../drm/i915/display/intel_display_power.h    |  13 +
 .../drm/i915/display/intel_display_types.h    |   1 +
 .../gpu/drm/i915/display/intel_frontbuffer.c  |   1 +
 drivers/gpu/drm/i915/display/intel_psr.c      |  52 +++
 drivers/gpu/drm/i915/display/intel_psr.h      |   2 +
 drivers/gpu/drm/i915/i915_debugfs.c           |   6 +
 drivers/gpu/drm/i915/i915_drv.h               |   3 +
 drivers/gpu/drm/i915/i915_params.c            |   3 +-
 drivers/gpu/drm/i915/i915_reg.h               |  10 +
 drivers/gpu/drm/i915/intel_pm.c               |   2 +-
 drivers/gpu/drm/i915/intel_pm.h               |   2 +
 13 files changed, 415 insertions(+), 17 deletions(-)

-- 
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] 22+ messages in thread

* [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-16 13:24   ` Animesh Manna
  2019-09-13  8:23 ` [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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.

v1: Use of REG_BIT and using extra space for EXITLINE_ macro
    definition. [Animesh]

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 bf37ecebc82f..6bfebab9a441 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4138,6 +4138,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
@@ -4184,11 +4185,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	REG_BIT(31)
+#define   EXITLINE_MASK		REG_GENMASK(12, 0)
+#define   EXITLINE_SHIFT	0
+
 /*
  * HSW+ eDP PSR registers
  *
@@ -10118,6 +10124,8 @@ enum skl_power_gate {
 /* GEN9 DC */
 #define DC_STATE_EN			_MMIO(0x45504)
 #define  DC_STATE_DISABLE		0
+#define  DC_STATE_EN_DC3CO		REG_BIT(30)
+#define  DC_STATE_DC3CO_STATUS		REG_BIT(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] 22+ messages in thread

* [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
  2019-09-13  8:23 ` [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-16 13:25   ` Animesh Manna
  2019-09-13  8:23 ` [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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. [Animesh]
v2: Using a switch statement for cleaner code. [Animesh]

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, 25 insertions(+), 7 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..24cd9320ad4c 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,20 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
 		requested_dc = max_dc;
 	}
 
-	if (requested_dc > 1)
+	switch (requested_dc) {
+	case 4:
+		mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6;
+		break;
+	case 3:
+		mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC5;
+		break;
+	case 2:
 		mask |= DC_STATE_EN_UPTO_DC6;
-	if (requested_dc > 0)
+		break;
+	case 1:
 		mask |= DC_STATE_EN_UPTO_DC5;
+		break;
+	}
 
 	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] 22+ messages in thread

* [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
  2019-09-13  8:23 ` [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
  2019-09-13  8:23 ` [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-23 16:21   ` Imre Deak
  2019-09-13  8:23 ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Add target_dc_state and tgl_set_target_dc_state() API
in order to enable DC3CO state with existing DC states.
target_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.
v8: Uniform checks by using only target_dc_state instead of allowed_dc_mask
    in "DC off" power well callback. [Imre]
    Adding "DC off" power well id to older platforms. [Imre]
    Removed psr2_deep_sleep flag from tgl_set_target_dc_state. [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>
---
 .../drm/i915/display/intel_display_power.c    | 102 ++++++++++++++++--
 .../drm/i915/display/intel_display_power.h    |   2 +
 drivers/gpu/drm/i915/i915_drv.h               |   1 +
 3 files changed, 96 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 24cd9320ad4c..7965e07257a0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -772,6 +772,36 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
 	dev_priv->csr.dc_state = val & mask;
 }
 
+static void
+allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
+{
+	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
+		dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC6;
+	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
+		dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC5;
+}
+
+static void tgl_enable_dc3co(struct drm_i915_private *dev_priv)
+{
+	DRM_DEBUG_KMS("Enabling DC3CO\n");
+	gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
+}
+
+static void tgl_disable_dc3co(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	DRM_DEBUG_KMS("Disabling DC3CO\n");
+	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 +969,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,6 +986,11 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
 {
 	struct intel_cdclk_state cdclk_state = {};
 
+	if (dev_priv->csr.target_dc_state == DC_STATE_EN_DC3CO) {
+		tgl_disable_dc3co(dev_priv);
+		return;
+	}
+
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
 	dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
@@ -987,12 +1023,59 @@ 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)
+	if (dev_priv->csr.target_dc_state == DC_STATE_EN_DC3CO)
+		tgl_enable_dc3co(dev_priv);
+	else if (dev_priv->csr.target_dc_state == DC_STATE_EN_UPTO_DC6)
 		skl_enable_dc6(dev_priv);
-	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
+	else if (dev_priv->csr.target_dc_state == DC_STATE_EN_UPTO_DC5)
 		gen9_enable_dc5(dev_priv);
 }
 
+void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state)
+{
+	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, SKL_DISP_DC_OFF);
+
+	if (!power_well)
+		goto unlock;
+
+	if (state == dev_priv->csr.target_dc_state)
+		goto unlock;
+
+	/*
+	 *  Compute the DC5/6 state wrt to the permisisble allowed dc mask.
+	 *  DC3CO allowed mask has already been checked in tgl_dc3co_flush
+	 *  It doesn't need to check here again.
+	 */
+	if (state != DC_STATE_EN_DC3CO) {
+		if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
+			state = DC_STATE_EN_UPTO_DC6;
+		else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
+			state = DC_STATE_EN_UPTO_DC5;
+	}
+
+	dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
+							   power_well);
+	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.target_dc_state = state;
+		power_well->desc->ops->disable(dev_priv, power_well);
+		goto unlock;
+	}
+		dev_priv->csr.target_dc_state = state;
+
+unlock:
+	mutex_unlock(&power_domains->lock);
+}
+
 static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
 					 struct i915_power_well *power_well)
 {
@@ -2938,7 +3021,7 @@ static const struct i915_power_well_desc skl_power_wells[] = {
 		.name = "DC off",
 		.domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
 		.ops = &gen9_dc_off_power_well_ops,
-		.id = DISP_PW_ID_NONE,
+		.id = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -3020,7 +3103,7 @@ static const struct i915_power_well_desc bxt_power_wells[] = {
 		.name = "DC off",
 		.domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS,
 		.ops = &gen9_dc_off_power_well_ops,
-		.id = DISP_PW_ID_NONE,
+		.id = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -3080,7 +3163,7 @@ static const struct i915_power_well_desc glk_power_wells[] = {
 		.name = "DC off",
 		.domains = GLK_DISPLAY_DC_OFF_POWER_DOMAINS,
 		.ops = &gen9_dc_off_power_well_ops,
-		.id = DISP_PW_ID_NONE,
+		.id = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -3249,7 +3332,7 @@ static const struct i915_power_well_desc cnl_power_wells[] = {
 		.name = "DC off",
 		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,
 		.ops = &gen9_dc_off_power_well_ops,
-		.id = DISP_PW_ID_NONE,
+		.id = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -3377,7 +3460,7 @@ static const struct i915_power_well_desc icl_power_wells[] = {
 		.name = "DC off",
 		.domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS,
 		.ops = &gen9_dc_off_power_well_ops,
-		.id = DISP_PW_ID_NONE,
+		.id = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -3610,7 +3693,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 = SKL_DISP_DC_OFF,
 	},
 	{
 		.name = "power well 2",
@@ -4043,6 +4126,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);
 
+	allowed_dc_mask_to_target_dc_state(dev_priv);
 	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..13fc705799fd 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,
+	SKL_DISP_DC_OFF,
 };
 
 #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
@@ -256,6 +257,7 @@ 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);
 
 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 84fb0245cf62..68fb732c24c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -337,6 +337,7 @@ struct intel_csr {
 	i915_reg_t mmioaddr[20];
 	u32 mmiodata[20];
 	u32 dc_state;
+	u32 target_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] 22+ messages in thread

* [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (2 preceding siblings ...)
  2019-09-13  8:23 ` [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-23 16:26   ` Imre Deak
  2019-09-13  8:23 ` [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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 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    | 104 ++++++++++++++++++
 .../drm/i915/display/intel_display_power.h    |   7 ++
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/intel_pm.c               |   2 +-
 drivers/gpu/drm/i915/intel_pm.h               |   2 +
 6 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a19f8c73f2e0..6b7b8d2112a5 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 7965e07257a0..42eabcdecf00 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,109 @@ 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 (!cstate->has_dc3co_exitline)
+		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 (!cstate->has_dc3co_exitline)
+		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;
+
+	if (crtc_state->has_psr2 && crtc_state->base.active) {
+		if (tgl_dc3co_is_edp_connected(crtc_state))
+			crtc_state->has_dc3co_exitline = true;
+	}
+}
+
+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;
+
+	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
+	if (crtc_state->cpu_transcoder != TRANSCODER_A)
+		return;
+
+	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
+
+	if (val & EXITLINE_ENABLE)
+		crtc_state->has_dc3co_exitline = true;
+}
+
 static void
 allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 13fc705799fd..bf68f26a5a37 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,
@@ -258,6 +260,11 @@ 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);
+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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d0ceb272551f..5679d1c1d560 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] 22+ messages in thread

* [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (3 preceding siblings ...)
  2019-09-13  8:23 ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-23 16:42   ` Imre Deak
  2019-09-13  8:23 ` [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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 +
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 3 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index b3c7eef53bf3..11d37f96ce71 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);
+}
+
 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__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 68fb732c24c8..4521b9381db3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -494,6 +494,7 @@ struct i915_psr {
 	bool link_standby;
 	bool colorimetry_support;
 	bool psr2_enabled;
+	bool psr2_deep_slp_disabled;
 	u8 sink_sync_latency;
 	ktime_t last_entry_attempt;
 	ktime_t last_exit;
-- 
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] 22+ messages in thread

* [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (4 preceding siblings ...)
  2019-09-13  8:23 ` [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-23 16:46   ` Imre Deak
  2019-09-13  8:23 ` [PATCH v8 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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.

B.Specs:49196 has a restriction to enable DC3CO only for Video Playback.
It will be worthy to enable DC3CO after completion of each pageflip
and switch back to DC5 when display is idle because driver doesn't
differentiate between video playback and a normal pageflip.
We will use Frontbuffer flush call tgl_dc3co_flush() to enable DC3CO
state only for ORIGIN_FLIP flush call, because DC3CO state has primarily
targeted for VPB use case. We are not interested here for frontbuffer
invalidates calls because that triggers PSR2 exit, which will
explicitly disable DC3CO.

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.
As PSR2 existing implementation is using minimum 6 idle frames for
deep sleep, it is safer to enable DC5/6 after 6 idle frames
(By scheduling a delayed work of 6 idle frames, once DC3CO has been
enabled after a pageflip).

After manually waiting for 6 idle frames DC5/6 will be enabled and
PSR2 deep sleep idle frames will be restored to 6 idle frames, at this
point DMC will triggers DC5/6 once PSR2 enters to deep sleep after
6 idle frames.
In future when we will enable S/W PSR2 tracking, we can change the
PSR2 required deep sleep idle frames to 1 so DMC can trigger the
DC5/6 immediately after S/W manual waiting of 6 idle frames get
complete.

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: Used frontbuffer flush mechanism. [Imre]
v5: Used psr.pipe to extract frontbuffer busy bits. [Imre]
    Used cancel_delayed_work_sync() in encoder disable path. [Imre]
    Used mod_delayed_work() instead of cancelling and scheduling a
    delayed work. [Imre]
    Used psr.lock in tgl_dc5_idle_thread() to enable psr2 deep
    sleep. [Imre]
    Removed DC5_REQ_IDLE_FRAMES macro. [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>
---
 .../drm/i915/display/intel_display_power.c    | 97 +++++++++++++++++++
 .../drm/i915/display/intel_display_power.h    |  4 +
 .../gpu/drm/i915/display/intel_frontbuffer.c  |  1 +
 drivers/gpu/drm/i915/display/intel_psr.c      |  1 +
 drivers/gpu/drm/i915/i915_drv.h               |  1 +
 5 files changed, 104 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 42eabcdecf00..a605047cb28a 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);
@@ -784,6 +806,9 @@ void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
 	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
 	val &= ~EXITLINE_ENABLE;
 	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
+
+	/* As psr2 encoder has disabled, cancel the dc5 idle delayed work */
+	cancel_delayed_work_sync(&dev_priv->csr.idle_work);
 }
 
 void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
@@ -817,6 +842,60 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
 	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
 }
 
+/*
+ * When we will enable manual PSR2 S/W tracking in future
+ * we will implement this entire DC3CO flush logic in
+ * intel_psr_flush().
+ */
+void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
+		     unsigned int frontbuffer_bits, enum fb_op_origin origin)
+{
+	struct intel_crtc_state *cstate;
+	struct intel_crtc *crtc;
+	u32 delay;
+	unsigned int busy_frontbuffer_bits;
+
+	if (!IS_TIGERLAKE(dev_priv))
+		return;
+
+	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
+		return;
+
+	if (origin != ORIGIN_FLIP)
+		return;
+
+	mutex_lock(&dev_priv->psr.lock);
+	crtc = intel_get_crtc_for_pipe(dev_priv, dev_priv->psr.pipe);
+	cstate = to_intel_crtc_state(crtc->base.state);
+
+	frontbuffer_bits &=
+		INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe);
+
+	busy_frontbuffer_bits &= ~frontbuffer_bits;
+
+	if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
+		goto unlock;
+
+	/*
+	 * At every flip frontbuffer flush modified delay of delayed work,
+	 * when delayed schedules that means display has been idle.
+	 */
+	if (!busy_frontbuffer_bits) {
+		if (!dev_priv->psr.psr2_deep_slp_disabled) {
+			tgl_psr2_deep_sleep_disable(dev_priv);
+			dev_priv->psr.psr2_deep_slp_disabled = true;
+		}
+		tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO);
+		/* DC5/DC6 required idle frames = 6 */
+		delay = 6 * intel_get_frame_time_us(cstate);
+		mod_delayed_work(system_wq, &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;
@@ -876,6 +955,23 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
 		crtc_state->has_dc3co_exitline = true;
 }
 
+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);
+
+	/* If delayed work is pending, it is not idle */
+	if (delayed_work_pending(&dev_priv->csr.idle_work))
+		return;
+
+	DRM_DEBUG_KMS("DC5/6 idle thread\n");
+	mutex_lock(&dev_priv->psr.lock);
+	tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
+	tgl_psr2_deep_sleep_enable(dev_priv);
+	dev_priv->psr.psr2_deep_slp_disabled = false;
+	mutex_unlock(&dev_priv->psr.lock);
+}
+
 static void
 allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
 {
@@ -4237,6 +4333,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 
 	INIT_DELAYED_WORK(&power_domains->async_put_work,
 			  intel_display_power_put_async_work);
+	INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
 
 	/*
 	 * The enabling order will be from lower to higher indexed wells,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index bf68f26a5a37..87725a23fead 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -9,6 +9,7 @@
 #include "intel_display.h"
 #include "intel_runtime_pm.h"
 #include "i915_reg.h"
+#include "intel_frontbuffer.h"
 
 struct drm_i915_private;
 struct intel_encoder;
@@ -265,6 +266,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/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 11d37f96ce71..552eaf2bd16f 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -574,6 +574,7 @@ static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv)
 	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);
+	dev_priv->psr.psr2_deep_slp_disabled = false;
 }
 
 static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4521b9381db3..25591c128c59 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -339,6 +339,7 @@ struct intel_csr {
 	u32 dc_state;
 	u32 target_dc_state;
 	u32 allowed_dc_mask;
+	struct delayed_work idle_work;
 	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] 22+ messages in thread

* [PATCH v8 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (5 preceding siblings ...)
  2019-09-13  8:23 ` [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
@ 2019-09-13  8:23 ` Anshuman Gupta
  2019-09-13 12:17 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev9) Patchwork
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-13  8:23 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 e5835337f022..5bb0a299b520 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2404,6 +2404,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 6bfebab9a441..3ad75d0fb71a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7255,6 +7255,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] 22+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev9)
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (6 preceding siblings ...)
  2019-09-13  8:23 ` [PATCH v8 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
@ 2019-09-13 12:17 ` Patchwork
  2019-09-13 12:20 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-09-13 12:17 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: DC3CO Support for TGL (rev9)
URL   : https://patchwork.freedesktop.org/series/64923/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c3f36fdec82d drm/i915/tgl: Add DC3CO required register and bits
0a2bd8b728de drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
90dfa5c2071e drm/i915/tgl: Enable DC3CO state in "DC Off" power well
-:67: CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
#67: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:802:
+	udelay(200);

total: 0 errors, 0 warnings, 1 checks, 193 lines checked
de039280c902 drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
97cfc089ad24 drm/i915/tgl: DC3CO PSR2 helper
66cd4a46a92d drm/i915/tgl: switch between dc3co and dc5 based on display idleness
a6a25c98f5fa 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] 22+ messages in thread

* ✗ Fi.CI.SPARSE: warning for DC3CO Support for TGL (rev9)
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (7 preceding siblings ...)
  2019-09-13 12:17 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev9) Patchwork
@ 2019-09-13 12:20 ` Patchwork
  2019-09-13 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-09-14 10:21 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-09-13 12:20 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: DC3CO Support for TGL (rev9)
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] 22+ messages in thread

* ✓ Fi.CI.BAT: success for DC3CO Support for TGL (rev9)
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (8 preceding siblings ...)
  2019-09-13 12:20 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-09-13 12:37 ` Patchwork
  2019-09-14 10:21 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-09-13 12:37 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: DC3CO Support for TGL (rev9)
URL   : https://patchwork.freedesktop.org/series/64923/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6888 -> Patchwork_14397
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14397:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic:
    - {fi-icl-u4}:        [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-icl-u4/igt@gem_exec_suspend@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-icl-u4/igt@gem_exec_suspend@basic.html

  
Known issues
------------

  Here are the changes found in Patchwork_14397 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_linear_blits@basic:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-icl-u3/igt@gem_linear_blits@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-icl-u3/igt@gem_linear_blits@basic.html

  * igt@i915_module_load@reload:
    - fi-icl-u3:          [PASS][5] -> [DMESG-WARN][6] ([fdo#107724] / [fdo#111214])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-icl-u3/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-icl-u3/igt@i915_module_load@reload.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic:
    - fi-bxt-dsi:         [INCOMPLETE][7] ([fdo#103927]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-bxt-dsi/igt@gem_ctx_create@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-bxt-dsi/igt@gem_ctx_create@basic.html

  * igt@gem_ctx_create@basic-files:
    - {fi-tgl-u2}:        [INCOMPLETE][9] -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-tgl-u2/igt@gem_ctx_create@basic-files.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-tgl-u2/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_gttfill@basic:
    - {fi-tgl-u}:         [INCOMPLETE][11] ([fdo#111593]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-tgl-u/igt@gem_exec_gttfill@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-tgl-u/igt@gem_exec_gttfill@basic.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-icl-u3:          [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14] +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][15] ([fdo#111096]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][17] ([fdo#102614]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593


Participating hosts (54 -> 47)
------------------------------

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

  CI-20190529: 20190529
  CI_DRM_6888: 52e9cd0877ee673ba1bb80c7c7be2e53c0821084 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5179: 3374cd0b048f9c277b2815bf80502f9f89680176 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14397: a6a25c98f5fae01d2175428deb153e07959dce58 @ 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_14397/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 ==

a6a25c98f5fa drm/i915/tgl: Add DC3CO counter in i915_dmc_info
66cd4a46a92d drm/i915/tgl: switch between dc3co and dc5 based on display idleness
97cfc089ad24 drm/i915/tgl: DC3CO PSR2 helper
de039280c902 drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
90dfa5c2071e drm/i915/tgl: Enable DC3CO state in "DC Off" power well
0a2bd8b728de drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
c3f36fdec82d drm/i915/tgl: Add DC3CO required register and bits

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.IGT: success for DC3CO Support for TGL (rev9)
  2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
                   ` (9 preceding siblings ...)
  2019-09-13 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-14 10:21 ` Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-09-14 10:21 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: DC3CO Support for TGL (rev9)
URL   : https://patchwork.freedesktop.org/series/64923/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6888_full -> Patchwork_14397_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_14397_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_ctx_shared@q-smoketest-bsd2:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +11 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb1/igt@gem_ctx_shared@q-smoketest-bsd2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb3/igt@gem_ctx_shared@q-smoketest-bsd2.html

  * igt@gem_eio@in-flight-internal-1us:
    - shard-skl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#106107])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl9/igt@gem_eio@in-flight-internal-1us.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl10/igt@gem_eio@in-flight-internal-1us.html

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [PASS][7] -> [FAIL][8] ([fdo#109661])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-snb1/igt@gem_eio@unwedge-stress.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-snb7/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#111325]) +4 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb5/igt@gem_exec_schedule@wide-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-apl1/igt@i915_suspend@debugfs-reader.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-apl5/igt@i915_suspend@debugfs-reader.html

  * igt@kms_flip@2x-plain-flip-ts-check-interruptible:
    - shard-glk:          [PASS][13] -> [FAIL][14] ([fdo#100368])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-glk5/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-glk9/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-hsw:          [PASS][15] -> [DMESG-FAIL][16] ([fdo#102614])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-hsw8/igt@kms_flip@dpms-vs-vblank-race.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-hsw5/igt@kms_flip@dpms-vs-vblank-race.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#105363])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108134])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl9/igt@kms_flip_tiling@flip-to-x-tiled.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl9/igt@kms_flip_tiling@flip-to-x-tiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103167]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145] / [fdo#110403])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103166])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109642] / [fdo#111068])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb7/igt@kms_psr2_su@page_flip.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][31] -> [FAIL][32] ([fdo#99912])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-apl1/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-apl6/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-b-query-forked-hang:
    - shard-apl:          [PASS][33] -> [INCOMPLETE][34] ([fdo#103927]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-apl8/igt@kms_vblank@pipe-b-query-forked-hang.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-apl5/igt@kms_vblank@pipe-b-query-forked-hang.html

  * igt@perf@polling:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([fdo#110728])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl10/igt@perf@polling.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl4/igt@perf@polling.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-iclb:         [FAIL][37] ([fdo#109661]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb8/igt@gem_eio@reset-stress.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb5/igt@gem_eio@reset-stress.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][39] ([fdo#110854]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb6/igt@gem_exec_balancer@smoke.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb2/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@out-order-bsd2:
    - shard-iclb:         [SKIP][41] ([fdo#109276]) -> [PASS][42] +16 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb3/igt@gem_exec_schedule@out-order-bsd2.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb1/igt@gem_exec_schedule@out-order-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][43] ([fdo#111325]) -> [PASS][44] +5 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb6/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_softpin@softpin:
    - shard-apl:          [INCOMPLETE][45] ([fdo#103927]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-apl5/igt@gem_softpin@softpin.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-apl5/igt@gem_softpin@softpin.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-kbl:          [DMESG-WARN][47] ([fdo#108686]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-kbl2/igt@gem_tiled_swapping@non-threaded.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-kbl4/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][49] ([fdo#108566]) -> [PASS][50] +4 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-apl6/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-hsw:          [FAIL][51] ([fdo#103355]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions.html

  * igt@kms_flip@2x-flip-vs-fences-interruptible:
    - shard-hsw:          [INCOMPLETE][53] ([fdo#103540]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-hsw4/igt@kms_flip@2x-flip-vs-fences-interruptible.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-hsw1/igt@kms_flip@2x-flip-vs-fences-interruptible.html

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-skl:          [FAIL][55] ([fdo#100368]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl2/igt@kms_flip@plain-flip-ts-check-interruptible.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl2/igt@kms_flip@plain-flip-ts-check-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][57] ([fdo#103167]) -> [PASS][58] +5 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][59] ([fdo#108145]) -> [PASS][60] +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [SKIP][61] ([fdo#109441]) -> [PASS][62] +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb6/igt@kms_psr@psr2_primary_blt.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb2/igt@kms_psr@psr2_primary_blt.html

  * igt@kms_vblank@pipe-b-query-forked-hang:
    - shard-iclb:         [INCOMPLETE][63] ([fdo#107713]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb7/igt@kms_vblank@pipe-b-query-forked-hang.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb1/igt@kms_vblank@pipe-b-query-forked-hang.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][65] ([fdo#111330]) -> [SKIP][66] ([fdo#109276])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6888/shard-iclb1/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14397/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [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#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [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#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [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_6888 -> Patchwork_14397

  CI-20190529: 20190529
  CI_DRM_6888: 52e9cd0877ee673ba1bb80c7c7be2e53c0821084 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5179: 3374cd0b048f9c277b2815bf80502f9f89680176 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14397: a6a25c98f5fae01d2175428deb153e07959dce58 @ 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_14397/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits
  2019-09-13  8:23 ` [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
@ 2019-09-16 13:24   ` Animesh Manna
  0 siblings, 0 replies; 22+ messages in thread
From: Animesh Manna @ 2019-09-16 13:24 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx; +Cc: jani.nikula



On 9/13/2019 1:53 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.
>
> v1: Use of REG_BIT and using extra space for EXITLINE_ macro
>      definition. [Animesh]
>
> 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 bf37ecebc82f..6bfebab9a441 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4138,6 +4138,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
> @@ -4184,11 +4185,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	REG_BIT(31)
> +#define   EXITLINE_MASK		REG_GENMASK(12, 0)
> +#define   EXITLINE_SHIFT	0
> +
>   /*
>    * HSW+ eDP PSR registers
>    *
> @@ -10118,6 +10124,8 @@ enum skl_power_gate {
>   /* GEN9 DC */
>   #define DC_STATE_EN			_MMIO(0x45504)
>   #define  DC_STATE_DISABLE		0
> +#define  DC_STATE_EN_DC3CO		REG_BIT(30)
> +#define  DC_STATE_DC3CO_STATUS		REG_BIT(29)
>   #define  DC_STATE_EN_UPTO_DC5		(1 << 0)
>   #define  DC_STATE_EN_DC9		(1 << 3)
>   #define  DC_STATE_EN_UPTO_DC6		(2 << 0)
The purpose of adding the register is not clear by looking at this patch.
Commit description can be improved if register definition want to keep 
as separate patch.
With improved commit description,
Can add Reviewed-by: Animesh Manna <animesh.manna@intel.com>


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask
  2019-09-13  8:23 ` [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
@ 2019-09-16 13:25   ` Animesh Manna
  0 siblings, 0 replies; 22+ messages in thread
From: Animesh Manna @ 2019-09-16 13:25 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx; +Cc: jani.nikula



On 9/13/2019 1:53 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. [Animesh]
> v2: Using a switch statement for cleaner code. [Animesh]
>
> 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, 25 insertions(+), 7 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..24cd9320ad4c 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,20 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
>   		requested_dc = max_dc;
>   	}
>   
> -	if (requested_dc > 1)
> +	switch (requested_dc) {
> +	case 4:
> +		mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC6;
> +		break;
> +	case 3:
> +		mask |= DC_STATE_EN_DC3CO | DC_STATE_EN_UPTO_DC5;
> +		break;
> +	case 2:
>   		mask |= DC_STATE_EN_UPTO_DC6;
> -	if (requested_dc > 0)
> +		break;
> +	case 1:
>   		mask |= DC_STATE_EN_UPTO_DC5;
> +		break;
> +	}
>   
>   	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 "

Changes looks ok to me.
Can add Reviewed-by: Animesh Manna <animesh.manna@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well
  2019-09-13  8:23 ` [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
@ 2019-09-23 16:21   ` Imre Deak
  0 siblings, 0 replies; 22+ messages in thread
From: Imre Deak @ 2019-09-23 16:21 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Fri, Sep 13, 2019 at 01:53:35PM +0530, Anshuman Gupta wrote:
> Add target_dc_state and tgl_set_target_dc_state() API
> in order to enable DC3CO state with existing DC states.
> target_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.
> v8: Uniform checks by using only target_dc_state instead of allowed_dc_mask
>     in "DC off" power well callback. [Imre]
>     Adding "DC off" power well id to older platforms. [Imre]
>     Removed psr2_deep_sleep flag from tgl_set_target_dc_state. [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>
> ---
>  .../drm/i915/display/intel_display_power.c    | 102 ++++++++++++++++--
>  .../drm/i915/display/intel_display_power.h    |   2 +
>  drivers/gpu/drm/i915/i915_drv.h               |   1 +
>  3 files changed, 96 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 24cd9320ad4c..7965e07257a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -772,6 +772,36 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
>  	dev_priv->csr.dc_state = val & mask;
>  }
>  
> +static void
> +allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> +{
> +	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> +		dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC6;
> +	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> +		dev_priv->csr.target_dc_state = DC_STATE_EN_UPTO_DC5;
> +}
> +
> +static void tgl_enable_dc3co(struct drm_i915_private *dev_priv)
> +{
> +	DRM_DEBUG_KMS("Enabling DC3CO\n");
> +	gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> +}
> +
> +static void tgl_disable_dc3co(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	DRM_DEBUG_KMS("Disabling DC3CO\n");
> +	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 +969,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,6 +986,11 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_cdclk_state cdclk_state = {};
>  
> +	if (dev_priv->csr.target_dc_state == DC_STATE_EN_DC3CO) {
> +		tgl_disable_dc3co(dev_priv);
> +		return;
> +	}
> +
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> @@ -987,12 +1023,59 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>  	if (!dev_priv->csr.dmc_payload)
>  		return;
>  

Using a switch here would be clearer.

> -	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> +	if (dev_priv->csr.target_dc_state == DC_STATE_EN_DC3CO)
> +		tgl_enable_dc3co(dev_priv);
> +	else if (dev_priv->csr.target_dc_state == DC_STATE_EN_UPTO_DC6)
>  		skl_enable_dc6(dev_priv);
> -	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> +	else if (dev_priv->csr.target_dc_state == DC_STATE_EN_UPTO_DC5)
>  		gen9_enable_dc5(dev_priv);
>  }
>  
> +void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state)
> +{
> +	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, SKL_DISP_DC_OFF);
> +
> +	if (!power_well)

WARN_ON() on the above condition, since it shouldn't happen?

> +		goto unlock;
> +
> +	if (state == dev_priv->csr.target_dc_state)
> +		goto unlock;
> +
> +	/*
> +	 *  Compute the DC5/6 state wrt to the permisisble allowed dc mask.
> +	 *  DC3CO allowed mask has already been checked in tgl_dc3co_flush
> +	 *  It doesn't need to check here again.

It would be better not to special case DC3co here, check it too against
allowed_dc_mask and bail out afterwards if the adjusted state equals
target_dc_state.

> +	 */
> +	if (state != DC_STATE_EN_DC3CO) {
> +		if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
> +			state = DC_STATE_EN_UPTO_DC6;
> +		else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
> +			state = DC_STATE_EN_UPTO_DC5;
> +	}
> +
> +	dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv,
> +							   power_well);
> +	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.target_dc_state = state;
> +		power_well->desc->ops->disable(dev_priv, power_well);
> +		goto unlock;
> +	}
> +		dev_priv->csr.target_dc_state = state;

Would be clearer by:

	if (dc_off_was_disabled)
		power_well->enable();

	target_dc_state = state;

	if (dc_off_was_disabled)
		power_Well->disable();

> +
> +unlock:
> +	mutex_unlock(&power_domains->lock);
> +}
> +
>  static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
>  					 struct i915_power_well *power_well)
>  {
> @@ -2938,7 +3021,7 @@ static const struct i915_power_well_desc skl_power_wells[] = {
>  		.name = "DC off",
>  		.domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
>  		.ops = &gen9_dc_off_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> +		.id = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -3020,7 +3103,7 @@ static const struct i915_power_well_desc bxt_power_wells[] = {
>  		.name = "DC off",
>  		.domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS,
>  		.ops = &gen9_dc_off_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> +		.id = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -3080,7 +3163,7 @@ static const struct i915_power_well_desc glk_power_wells[] = {
>  		.name = "DC off",
>  		.domains = GLK_DISPLAY_DC_OFF_POWER_DOMAINS,
>  		.ops = &gen9_dc_off_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> +		.id = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -3249,7 +3332,7 @@ static const struct i915_power_well_desc cnl_power_wells[] = {
>  		.name = "DC off",
>  		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,
>  		.ops = &gen9_dc_off_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> +		.id = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -3377,7 +3460,7 @@ static const struct i915_power_well_desc icl_power_wells[] = {
>  		.name = "DC off",
>  		.domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS,
>  		.ops = &gen9_dc_off_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> +		.id = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -3610,7 +3693,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 = SKL_DISP_DC_OFF,
>  	},
>  	{
>  		.name = "power well 2",
> @@ -4043,6 +4126,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);
>  
> +	allowed_dc_mask_to_target_dc_state(dev_priv);
>  	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..13fc705799fd 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,
> +	SKL_DISP_DC_OFF,
>  };
>  
>  #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
> @@ -256,6 +257,7 @@ 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);
>  
>  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 84fb0245cf62..68fb732c24c8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -337,6 +337,7 @@ struct intel_csr {
>  	i915_reg_t mmioaddr[20];
>  	u32 mmiodata[20];
>  	u32 dc_state;
> +	u32 target_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] 22+ messages in thread

* Re: [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
  2019-09-13  8:23 ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
@ 2019-09-23 16:26   ` Imre Deak
  2019-09-24  6:13     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.y Anshuman Gupta
  2019-09-25  9:10     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
  0 siblings, 2 replies; 22+ messages in thread
From: Imre Deak @ 2019-09-23 16:26 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Fri, Sep 13, 2019 at 01:53:36PM +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 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>

You missed my review comments on v7:
- Computing the state should be done from a DP encoder compute config
  hook, which will then make tgl_dc3co_is_edp_connected() redundant.
- Instead of has_dc3co_exitline dc3co_exitline should be computed (and
  read out), which could be used then when we need to program it.

> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
>  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
>  .../drm/i915/display/intel_display_power.h    |   7 ++
>  .../drm/i915/display/intel_display_types.h    |   1 +
>  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
>  drivers/gpu/drm/i915/intel_pm.h               |   2 +
>  6 files changed, 120 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a19f8c73f2e0..6b7b8d2112a5 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 7965e07257a0..42eabcdecf00 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,109 @@ 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 (!cstate->has_dc3co_exitline)
> +		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 (!cstate->has_dc3co_exitline)
> +		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;
> +
> +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> +		if (tgl_dc3co_is_edp_connected(crtc_state))
> +			crtc_state->has_dc3co_exitline = true;
> +	}
> +}
> +
> +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;
> +
> +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> +		return;
> +
> +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> +
> +	if (val & EXITLINE_ENABLE)
> +		crtc_state->has_dc3co_exitline = true;
> +}
> +
>  static void
>  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 13fc705799fd..bf68f26a5a37 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,
> @@ -258,6 +260,11 @@ 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);
> +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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d0ceb272551f..5679d1c1d560 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] 22+ messages in thread

* Re: [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper
  2019-09-13  8:23 ` [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
@ 2019-09-23 16:42   ` Imre Deak
  2019-09-23 17:56     ` Anshuman Gupta
  0 siblings, 1 reply; 22+ messages in thread
From: Imre Deak @ 2019-09-23 16:42 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Fri, Sep 13, 2019 at 01:53:37PM +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>

You missed some of the review comments for v7, see below.

> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 51 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_psr.h |  2 +
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  3 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3c7eef53bf3..11d37f96ce71 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 a variable for that.

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

Could we use crtc_state->dc3co_exitline instead (which we would set only
whenever we want to enable DC3co)?

> +
> +	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);
> +}
> +
>  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);

This is too late to program the EXITLINE reg, since the transcoder is
already enabled.

> +
>  	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);

So DC5/6 will be reenabled, but I can't see how PSR deep sleep gets
reenabled. I think we should have a function that we call both from here
and the idle thread that reenables DC5/6 and also PSR deep sleep.

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

The transcoder is still active here so we can't program the EXITLINE reg
at this point.

Please add tgl_enable/disable_psr2_transcoder_exitline() in this patch
where the functions are actually used, the review is difficult when I
have to jump between the two patches.

OTOH tgl_psr2_deep_sleep_enable/disable() is used in a follow-up patch,
but you define them already here; those should be also moved where they
are used for easier review.

> +
>  	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__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 68fb732c24c8..4521b9381db3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -494,6 +494,7 @@ struct i915_psr {
>  	bool link_standby;
>  	bool colorimetry_support;
>  	bool psr2_enabled;
> +	bool psr2_deep_slp_disabled;
>  	u8 sink_sync_latency;
>  	ktime_t last_entry_attempt;
>  	ktime_t last_exit;
> -- 
> 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] 22+ messages in thread

* Re: [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness
  2019-09-13  8:23 ` [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
@ 2019-09-23 16:46   ` Imre Deak
  0 siblings, 0 replies; 22+ messages in thread
From: Imre Deak @ 2019-09-23 16:46 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Fri, Sep 13, 2019 at 01:53:38PM +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.
> 
> B.Specs:49196 has a restriction to enable DC3CO only for Video Playback.
> It will be worthy to enable DC3CO after completion of each pageflip
> and switch back to DC5 when display is idle because driver doesn't
> differentiate between video playback and a normal pageflip.
> We will use Frontbuffer flush call tgl_dc3co_flush() to enable DC3CO
> state only for ORIGIN_FLIP flush call, because DC3CO state has primarily
> targeted for VPB use case. We are not interested here for frontbuffer
> invalidates calls because that triggers PSR2 exit, which will
> explicitly disable DC3CO.
> 
> 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.
> As PSR2 existing implementation is using minimum 6 idle frames for
> deep sleep, it is safer to enable DC5/6 after 6 idle frames
> (By scheduling a delayed work of 6 idle frames, once DC3CO has been
> enabled after a pageflip).
> 
> After manually waiting for 6 idle frames DC5/6 will be enabled and
> PSR2 deep sleep idle frames will be restored to 6 idle frames, at this
> point DMC will triggers DC5/6 once PSR2 enters to deep sleep after
> 6 idle frames.
> In future when we will enable S/W PSR2 tracking, we can change the
> PSR2 required deep sleep idle frames to 1 so DMC can trigger the
> DC5/6 immediately after S/W manual waiting of 6 idle frames get
> complete.
> 
> 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: Used frontbuffer flush mechanism. [Imre]
> v5: Used psr.pipe to extract frontbuffer busy bits. [Imre]
>     Used cancel_delayed_work_sync() in encoder disable path. [Imre]
>     Used mod_delayed_work() instead of cancelling and scheduling a
>     delayed work. [Imre]
>     Used psr.lock in tgl_dc5_idle_thread() to enable psr2 deep
>     sleep. [Imre]
>     Removed DC5_REQ_IDLE_FRAMES macro. [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>
> ---
>  .../drm/i915/display/intel_display_power.c    | 97 +++++++++++++++++++
>  .../drm/i915/display/intel_display_power.h    |  4 +
>  .../gpu/drm/i915/display/intel_frontbuffer.c  |  1 +
>  drivers/gpu/drm/i915/display/intel_psr.c      |  1 +
>  drivers/gpu/drm/i915/i915_drv.h               |  1 +
>  5 files changed, 104 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 42eabcdecf00..a605047cb28a 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);
> @@ -784,6 +806,9 @@ void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
>  	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
>  	val &= ~EXITLINE_ENABLE;
>  	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> +
> +	/* As psr2 encoder has disabled, cancel the dc5 idle delayed work */
> +	cancel_delayed_work_sync(&dev_priv->csr.idle_work);
>  }
>  
>  void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> @@ -817,6 +842,60 @@ void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
>  	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
>  }
>  
> +/*
> + * When we will enable manual PSR2 S/W tracking in future
> + * we will implement this entire DC3CO flush logic in
> + * intel_psr_flush().
> + */
> +void tgl_dc3co_flush(struct drm_i915_private *dev_priv,
> +		     unsigned int frontbuffer_bits, enum fb_op_origin origin)
> +{
> +	struct intel_crtc_state *cstate;
> +	struct intel_crtc *crtc;
> +	u32 delay;
> +	unsigned int busy_frontbuffer_bits;
> +
> +	if (!IS_TIGERLAKE(dev_priv))
> +		return;
> +
> +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> +		return;

Could we use crtc_state->dc3co_exitline instead of the two checks above?

> +
> +	if (origin != ORIGIN_FLIP)
> +		return;
> +
> +	mutex_lock(&dev_priv->psr.lock);
> +	crtc = intel_get_crtc_for_pipe(dev_priv, dev_priv->psr.pipe);
> +	cstate = to_intel_crtc_state(crtc->base.state);
> +
> +	frontbuffer_bits &=
> +		INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe);
> +
> +	busy_frontbuffer_bits &= ~frontbuffer_bits;

This is still wrong, busy_frontbuffer_bits is not inited.

> +
> +	if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active)
> +		goto unlock;
> +
> +	/*
> +	 * At every flip frontbuffer flush modified delay of delayed work,
> +	 * when delayed schedules that means display has been idle.
> +	 */
> +	if (!busy_frontbuffer_bits) {
> +		if (!dev_priv->psr.psr2_deep_slp_disabled) {

I think reenabling PSR deep sleep if it was already enabled is not a
problem, we don't need a flag just to avoid that.

> +			tgl_psr2_deep_sleep_disable(dev_priv);
> +			dev_priv->psr.psr2_deep_slp_disabled = true;
> +		}
> +		tgl_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO);
> +		/* DC5/DC6 required idle frames = 6 */
> +		delay = 6 * intel_get_frame_time_us(cstate);
> +		mod_delayed_work(system_wq, &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;
> @@ -876,6 +955,23 @@ void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
>  		crtc_state->has_dc3co_exitline = true;
>  }
>  
> +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);
> +
> +	/* If delayed work is pending, it is not idle */
> +	if (delayed_work_pending(&dev_priv->csr.idle_work))
> +		return;

The above should be checked with the psr.lock held to avoid a race.

> +
> +	DRM_DEBUG_KMS("DC5/6 idle thread\n");
> +	mutex_lock(&dev_priv->psr.lock);
> +	tgl_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
> +	tgl_psr2_deep_sleep_enable(dev_priv);
> +	dev_priv->psr.psr2_deep_slp_disabled = false;
> +	mutex_unlock(&dev_priv->psr.lock);
> +}
> +
>  static void
>  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
>  {
> @@ -4237,6 +4333,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  
>  	INIT_DELAYED_WORK(&power_domains->async_put_work,
>  			  intel_display_power_put_async_work);
> +	INIT_DELAYED_WORK(&dev_priv->csr.idle_work, tgl_dc5_idle_thread);
>  
>  	/*
>  	 * The enabling order will be from lower to higher indexed wells,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index bf68f26a5a37..87725a23fead 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -9,6 +9,7 @@
>  #include "intel_display.h"
>  #include "intel_runtime_pm.h"
>  #include "i915_reg.h"
> +#include "intel_frontbuffer.h"
>  
>  struct drm_i915_private;
>  struct intel_encoder;
> @@ -265,6 +266,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/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 11d37f96ce71..552eaf2bd16f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -574,6 +574,7 @@ static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv)
>  	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);
> +	dev_priv->psr.psr2_deep_slp_disabled = false;
>  }
>  
>  static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4521b9381db3..25591c128c59 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -339,6 +339,7 @@ struct intel_csr {
>  	u32 dc_state;
>  	u32 target_dc_state;
>  	u32 allowed_dc_mask;
> +	struct delayed_work idle_work;
>  	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] 22+ messages in thread

* Re: [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper
  2019-09-23 16:42   ` Imre Deak
@ 2019-09-23 17:56     ` Anshuman Gupta
  0 siblings, 0 replies; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-23 17:56 UTC (permalink / raw)
  To: Imre Deak; +Cc: jani.nikula, intel-gfx

On 2019-09-23 at 19:42:02 +0300, Imre Deak wrote:
> On Fri, Sep 13, 2019 at 01:53:37PM +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>
> 
> You missed some of the review comments for v7, see below.
Ah, somehow i missed this mail itself. https://patchwork.freedesktop.org/patch/329538/?series=64923&rev=8
I will address these comments.
below there are some follow up comments.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 51 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_psr.h |  2 +
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  3 files changed, 54 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index b3c7eef53bf3..11d37f96ce71 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 a variable for that.
> 
> > +
> > +	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;
> 
> Could we use crtc_state->dc3co_exitline instead (which we would set only
> whenever we want to enable DC3co)?
> 
> > +
> > +	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);
> > +}
> > +
> >  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);
> 
> This is too late to program the EXITLINE reg, since the transcoder is
> already enabled.
intel_psr_enable/disable called by encoder enable/disable fucntion.
AFAIR u seemed to had opinion call it from intel_psr_enable()
https://patchwork.freedesktop.org/patch/323128/?series=64923&rev=3
How about calling these from encoder pre_enable functions ?
or any other better place apart from haswell_crtc_enable/disable.
> 
> > +
> >  	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);
> 
> So DC5/6 will be reenabled, but I can't see how PSR deep sleep gets
> reenabled. I think we should have a function that we call both from here
> and the idle thread that reenables DC5/6 and also PSR deep sleep.
IMO we don't need to enable PSR2 deep sleep when PSR gets exit, no use of it.
It will always get enabled by intel_psr_activate()->hsw_activate_psr2().
Thanks,
Anshuman Gupta.
> 
> >  		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);
> 
> The transcoder is still active here so we can't program the EXITLINE reg
> at this point.
> 
> Please add tgl_enable/disable_psr2_transcoder_exitline() in this patch
> where the functions are actually used, the review is difficult when I
> have to jump between the two patches.
> 
> OTOH tgl_psr2_deep_sleep_enable/disable() is used in a follow-up patch,
> but you define them already here; those should be also moved where they
> are used for easier review.
> 
> > +
> >  	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__ */
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 68fb732c24c8..4521b9381db3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -494,6 +494,7 @@ struct i915_psr {
> >  	bool link_standby;
> >  	bool colorimetry_support;
> >  	bool psr2_enabled;
> > +	bool psr2_deep_slp_disabled;
> >  	u8 sink_sync_latency;
> >  	ktime_t last_entry_attempt;
> >  	ktime_t last_exit;
> > -- 
> > 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] 22+ messages in thread

* Re: [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.y
  2019-09-23 16:26   ` Imre Deak
@ 2019-09-24  6:13     ` Anshuman Gupta
  2019-09-25  9:10     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
  1 sibling, 0 replies; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-24  6:13 UTC (permalink / raw)
  To: Imre Deak; +Cc: jani.nikula, intel-gfx

On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> On Fri, Sep 13, 2019 at 01:53:36PM +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 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>
> 
> You missed my review comments on v7:
Actually i found this e-mail including earlier v7 couple of review e-mails
received in spam folder, therefore gets unnoticed.
I will fix these comments for next version.
> - Computing the state should be done from a DP encoder compute config
>   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
>   read out), which could be used then when we need to program it.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> >  .../drm/i915/display/intel_display_power.h    |   7 ++
> >  .../drm/i915/display/intel_display_types.h    |   1 +
> >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> >  6 files changed, 120 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a19f8c73f2e0..6b7b8d2112a5 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 7965e07257a0..42eabcdecf00 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,109 @@ 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 (!cstate->has_dc3co_exitline)
> > +		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 (!cstate->has_dc3co_exitline)
> > +		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;
> > +
> > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > +			crtc_state->has_dc3co_exitline = true;
> > +	}
> > +}
> > +
> > +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;
> > +
> > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > +
> > +	if (val & EXITLINE_ENABLE)
> > +		crtc_state->has_dc3co_exitline = true;
> > +}
> > +
> >  static void
> >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 13fc705799fd..bf68f26a5a37 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,
> > @@ -258,6 +260,11 @@ 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);
> > +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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d0ceb272551f..5679d1c1d560 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] 22+ messages in thread

* Re: [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
  2019-09-23 16:26   ` Imre Deak
  2019-09-24  6:13     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.y Anshuman Gupta
@ 2019-09-25  9:10     ` Anshuman Gupta
  2019-09-25 12:26       ` Imre Deak
  1 sibling, 1 reply; 22+ messages in thread
From: Anshuman Gupta @ 2019-09-25  9:10 UTC (permalink / raw)
  To: Imre Deak; +Cc: jani.nikula, intel-gfx

On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> On Fri, Sep 13, 2019 at 01:53:36PM +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 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>
> 
> You missed my review comments on v7:
> - Computing the state should be done from a DP encoder compute config
>   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
>   read out), which could be used then when we need to program it.
Hi Imre,
We need crtc_state pixel rate in order to compute dc3co exitlines,
crtc_state pixel rate is not ready in DP encoder compute config.
It is getting computed in intel_crtc_compute_config().
I have two solution in my mind.
1. compute has_dc3co_exitline in dp compute hook and calculate
   dc3co exitlines in tgl_set_psr2_transcoder_exitline()?
2. call intel_crtc_compute_config() in dp encoder compute config to
   get crtc_state pixel rate?

Please provide your suggestion, will change it accordingly.
Thanks,
Anshuman Gupta.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> >  .../drm/i915/display/intel_display_power.h    |   7 ++
> >  .../drm/i915/display/intel_display_types.h    |   1 +
> >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> >  6 files changed, 120 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a19f8c73f2e0..6b7b8d2112a5 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 7965e07257a0..42eabcdecf00 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,109 @@ 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 (!cstate->has_dc3co_exitline)
> > +		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 (!cstate->has_dc3co_exitline)
> > +		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;
> > +
> > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > +			crtc_state->has_dc3co_exitline = true;
> > +	}
> > +}
> > +
> > +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;
> > +
> > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > +
> > +	if (val & EXITLINE_ENABLE)
> > +		crtc_state->has_dc3co_exitline = true;
> > +}
> > +
> >  static void
> >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 13fc705799fd..bf68f26a5a37 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,
> > @@ -258,6 +260,11 @@ 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);
> > +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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d0ceb272551f..5679d1c1d560 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] 22+ messages in thread

* Re: [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.
  2019-09-25  9:10     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
@ 2019-09-25 12:26       ` Imre Deak
  0 siblings, 0 replies; 22+ messages in thread
From: Imre Deak @ 2019-09-25 12:26 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Wed, Sep 25, 2019 at 02:40:42PM +0530, Anshuman Gupta wrote:
> On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> > On Fri, Sep 13, 2019 at 01:53:36PM +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 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>
> > 
> > You missed my review comments on v7:
> > - Computing the state should be done from a DP encoder compute config
> >   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> > - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
> >   read out), which could be used then when we need to program it.
> Hi Imre,
> We need crtc_state pixel rate in order to compute dc3co exitlines,
> crtc_state pixel rate is not ready in DP encoder compute config.
> It is getting computed in intel_crtc_compute_config().
> I have two solution in my mind.
> 1. compute has_dc3co_exitline in dp compute hook and calculate
>    dc3co exitlines in tgl_set_psr2_transcoder_exitline()?
> 2. call intel_crtc_compute_config() in dp encoder compute config to
>    get crtc_state pixel rate?

intel_get_linetime_us() is the time needed to fetch one line from
memory, while you need the time needed to scan out one line. You need
the inverse of intel_usecs_to_scanlines().

> 
> Please provide your suggestion, will change it accordingly.
> Thanks,
> Anshuman Gupta.
> > 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> > >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> > >  .../drm/i915/display/intel_display_power.h    |   7 ++
> > >  .../drm/i915/display/intel_display_types.h    |   1 +
> > >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> > >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> > >  6 files changed, 120 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index a19f8c73f2e0..6b7b8d2112a5 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 7965e07257a0..42eabcdecf00 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,109 @@ 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 (!cstate->has_dc3co_exitline)
> > > +		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 (!cstate->has_dc3co_exitline)
> > > +		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;
> > > +
> > > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > > +			crtc_state->has_dc3co_exitline = true;
> > > +	}
> > > +}
> > > +
> > > +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;
> > > +
> > > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > > +		return;
> > > +
> > > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > > +
> > > +	if (val & EXITLINE_ENABLE)
> > > +		crtc_state->has_dc3co_exitline = true;
> > > +}
> > > +
> > >  static void
> > >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> > >  {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index 13fc705799fd..bf68f26a5a37 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,
> > > @@ -258,6 +260,11 @@ 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);
> > > +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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index d0ceb272551f..5679d1c1d560 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] 22+ messages in thread

end of thread, other threads:[~2019-09-25 12:27 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13  8:23 [PATCH v8 0/7] DC3CO Support for TGL Anshuman Gupta
2019-09-13  8:23 ` [PATCH v8 1/7] drm/i915/tgl: Add DC3CO required register and bits Anshuman Gupta
2019-09-16 13:24   ` Animesh Manna
2019-09-13  8:23 ` [PATCH v8 2/7] drm/i915/tgl: Add DC3CO mask to allowed_dc_mask and gen9_dc_mask Anshuman Gupta
2019-09-16 13:25   ` Animesh Manna
2019-09-13  8:23 ` [PATCH v8 3/7] drm/i915/tgl: Enable DC3CO state in "DC Off" power well Anshuman Gupta
2019-09-23 16:21   ` Imre Deak
2019-09-13  8:23 ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
2019-09-23 16:26   ` Imre Deak
2019-09-24  6:13     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.y Anshuman Gupta
2019-09-25  9:10     ` [PATCH v8 4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline Anshuman Gupta
2019-09-25 12:26       ` Imre Deak
2019-09-13  8:23 ` [PATCH v8 5/7] drm/i915/tgl: DC3CO PSR2 helper Anshuman Gupta
2019-09-23 16:42   ` Imre Deak
2019-09-23 17:56     ` Anshuman Gupta
2019-09-13  8:23 ` [PATCH v8 6/7] drm/i915/tgl: switch between dc3co and dc5 based on display idleness Anshuman Gupta
2019-09-23 16:46   ` Imre Deak
2019-09-13  8:23 ` [PATCH v8 7/7] drm/i915/tgl: Add DC3CO counter in i915_dmc_info Anshuman Gupta
2019-09-13 12:17 ` ✗ Fi.CI.CHECKPATCH: warning for DC3CO Support for TGL (rev9) Patchwork
2019-09-13 12:20 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-09-13 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-14 10:21 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.