[PATCH 1/9] drm/i915: Disable PSR in Apple panels

[PATCH 1/9] drm/i915: Disable PSR in Apple panels

[José Roberto de Souza]

i915 yet don't support PSR in Apple panels, so lets keep it disabled
while we work on that.

Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c  | 2 ++
 drivers/gpu/drm/i915/intel_psr.c | 6 ++++++
 include/drm/drm_dp_helper.h      | 1 +
 3 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 6d483487f2b4..6b5a19d3e347 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
 	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
 	/* LG LP140WF6-SPM1 eDP panel */
 	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
+	/* Apple panels needs some additional handling to support PSR */
+	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) }
 };
 
 #undef OUI
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 572e626eadff..f5d27a02eb28 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 		DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n");
 		return;
 	}
+
+	if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
+		DRM_DEBUG_KMS("PSR support not currently available for this panel\n");
+		return;
+	}
+
 	dev_priv->psr.sink_support = true;
 	dev_priv->psr.sink_sync_latency =
 		intel_dp_get_sink_sync_latency(intel_dp);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 3314e91f6eb3..db516c48cda3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1364,6 +1364,7 @@ enum drm_dp_quirk {
 	 * to 16 bits. So will give a constant value (0x8000) for compatability.
 	 */
 	DP_DPCD_QUIRK_CONSTANT_N,
+	DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
 };
 
 /**
-- 
2.19.2

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

[PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2

[José Roberto de Souza]

For PSR2 there is no register to tell HW to keep main link enabled
while PSR2 is active, so don't configure sink DPCD with a
misleading value.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f5d27a02eb28..888e348cc1b4 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE);
 		dpcd_val |= DP_PSR_ENABLE_PSR2;
+	} else {
+		if (dev_priv->psr.link_standby)
+			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+
+		if (INTEL_GEN(dev_priv) >= 8)
+			dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	}
 
-	if (dev_priv->psr.link_standby)
-		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
-	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
-		dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
 
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[José Roberto de Souza]

eDP spec states 2 different bits to enable sink to trigger a
interruption when there is a CRC mismatch.
DP_PSR_CRC_VERIFICATION is for PSR only and
DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 888e348cc1b4..607c3ec41679 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 	if (dev_priv->psr.psr2_enabled) {
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE);
-		dpcd_val |= DP_PSR_ENABLE_PSR2;
+		dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
 	} else {
 		if (dev_priv->psr.link_standby)
 			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2

[José Roberto de Souza]

For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not
touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already
unset in gen10 + GLK we can just drop it and fix for both gens.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 607c3ec41679..7607a58a6ec0 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hsw_psr_setup_aux(intel_dp);
 
-	if (dev_priv->psr.psr2_enabled) {
+	if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
+					   !IS_GEMINILAKE(dev_priv))) {
 		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
 							cpu_transcoder);
 		u32 chicken = I915_READ(reg);
 
-		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
-			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
-				   | PSR2_ADD_VERTICAL_LINE_COUNT);
-
-		else
-			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
+		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
+			   PSR2_ADD_VERTICAL_LINE_COUNT;
 		I915_WRITE(reg, chicken);
 	}
 
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm: Add offset of PSR2 SU X granularity value

[José Roberto de Souza]

Source is required to comply to sink SU granularity when
DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS,
so adding the register here.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 include/drm/drm_dp_helper.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index db516c48cda3..acc7ccfd2044 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -314,6 +314,9 @@
 # define DP_PSR_SETUP_TIME_SHIFT            1
 # define DP_PSR2_SU_Y_COORDINATE_REQUIRED   (1 << 4)  /* eDP 1.4a */
 # define DP_PSR2_SU_GRANULARITY_REQUIRED    (1 << 5)  /* eDP 1.4b */
+
+#define DP_PSR2_SU_X_GRANULARITY	    0x072 /* eDP 1.4b */
+
 /*
  * 0x80-0x8f describe downstream port capabilities, but there are two layouts
  * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/9] drm/i915/psr: Check if source supports sink specific SU granularity

[José Roberto de Souza]

According to eDP spec, sink can required specific selective update
granularity that source must comply.
Here caching the value if required and checking source supports it.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f763b30f98d9..cbcd85af95bf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -506,6 +506,7 @@ struct i915_psr {
 	ktime_t last_exit;
 	bool sink_not_reliable;
 	bool irq_aux_error;
+	u16 su_x_granularity;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 7607a58a6ec0..9215c9052381 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -257,6 +257,21 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
 	return val;
 }
 
+static u16 intel_dp_get_su_x_granulartiy(struct intel_dp *intel_dp)
+{
+	u16 val = 0;
+	ssize_t r;
+
+	if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
+		return val;
+
+	r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &val, 2);
+	if (r != 2)
+		DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n");
+
+	return val;
+}
+
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv =
@@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 		if (dev_priv->psr.sink_psr2_support) {
 			dev_priv->psr.colorimetry_support =
 				intel_dp_get_colorimetry_status(intel_dp);
+			dev_priv->psr.su_x_granularity =
+				intel_dp_get_su_x_granulartiy(intel_dp);
 		}
 	}
 }
@@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 		return false;
 	}
 
+	if (dev_priv->psr.su_x_granularity) {
+		/*
+		 * HW will always send full lines in SU blocks, so X will
+		 * always be 0 and we only need to check the width to validate
+		 * horizontal granularity.
+		 * About vertical granularity HW works by SU blocks starting
+		 * at each 4 lines with height of 4 lines, what eDP states
+		 * that sink should support.
+		 */
+		if (crtc_hdisplay % dev_priv->psr.su_x_granularity) {
+			DRM_DEBUG_KMS("PSR2 not enabled, HW can not match sink SU granularity requirement\n");
+			return false;
+		}
+	}
+
 	return true;
 }
 
-- 
2.19.2

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

[PATCH 7/9] drm/i915/psr: Rename PSR2 macros to better match meaning

[José Roberto de Souza]

The first 8 bits of PSR2_CTL have 2 fields to set frames count, the
first one is to set how many idle frames PSR2 HW needs to wait before
enter in deep sleep and the second one it is how many frames(it don't
need to be idle frames) PSR2 HW will wait before start the PSR
activation sequence.
The previous names was really misleading and caused wrong values being
set so better rename to make it clear.

Also taking the oportunity to improve those macros.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 35 ++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_psr.c |  7 ++++---
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47baf2fe8f71..73046bb9ec7c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4203,23 +4203,24 @@ enum {
 #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
 #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
 
-#define EDP_PSR2_CTL			_MMIO(0x6f900)
-#define   EDP_PSR2_ENABLE		(1 << 31)
-#define   EDP_SU_TRACK_ENABLE		(1 << 30)
-#define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
-#define   EDP_Y_COORDINATE_ENABLE	(1 << 25) /* GLK and CNL+ */
-#define   EDP_MAX_SU_DISABLE_TIME(t)	((t) << 20)
-#define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f << 20)
-#define   EDP_PSR2_TP2_TIME_500us	(0 << 8)
-#define   EDP_PSR2_TP2_TIME_100us	(1 << 8)
-#define   EDP_PSR2_TP2_TIME_2500us	(2 << 8)
-#define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
-#define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
-#define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
-#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
-#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
-#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
-#define   EDP_PSR2_IDLE_FRAME_SHIFT	0
+#define EDP_PSR2_CTL					_MMIO(0x6f900)
+#define   EDP_PSR2_ENABLE				(1 << 31)
+#define   EDP_SU_TRACK_ENABLE				(1 << 30)
+#define   EDP_Y_COORDINATE_VALID			(1 << 26) /* GLK and CNL+ */
+#define   EDP_Y_COORDINATE_ENABLE			(1 << 25) /* GLK and CNL+ */
+#define   EDP_MAX_SU_DISABLE_TIME(t)			((t) << 20)
+#define   EDP_MAX_SU_DISABLE_TIME_MASK			(0x1f << 20)
+#define   EDP_PSR2_TP2_TIME_500us			(0 << 8)
+#define   EDP_PSR2_TP2_TIME_100us			(1 << 8)
+#define   EDP_PSR2_TP2_TIME_2500us			(2 << 8)
+#define   EDP_PSR2_TP2_TIME_50us			(3 << 8)
+#define   EDP_PSR2_TP2_TIME_MASK			(3 << 8)
+#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT		(4)
+#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK		(0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT)
+#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n)		(((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK)
+#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK	(0xf)
+#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT	(0)
+#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n)		(((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
 
 #define _PSR_EVENT_TRANS_A			0x60848
 #define _PSR_EVENT_TRANS_B			0x61848
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 9215c9052381..ba7bbe3f8df2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
 static void hsw_activate_psr2(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct i915_psr *psr = &dev_priv->psr;
 	u32 val;
 
 	/* Let's use 6 as the minimum to cover all known cases including the
@@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 
-	idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1);
-	val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
+	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
+	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
 
 	/* FIXME: selective update is probably totally broken because it doesn't
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
@@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 		val |= EDP_Y_COORDINATE_ENABLE;
 
-	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
+	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
 	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] drm/i915/psr: Set the right frames values

[José Roberto de Souza]

EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of
frames that it should wait to enter PSR, what is wrong.
Here it is setting this field with the highest value to avoid PSR2
exits frequently, as when HW exit deep sleep it needs to go to idle
state causing a PSR exit for then waiting a few frames before
activate PSR2 again.
This will result in more power saving as the sleep state also provide
some power savings by doing selective updates instead of full screen
updates.

About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames
(not idle frames) that PSR2 hardware will wait to activate PSR2, so
lets keep using the sink sync latency.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index ba7bbe3f8df2..6fd793fec5e9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	struct i915_psr *psr = &dev_priv->psr;
 	u32 val;
 
-	/* Let's use 6 as the minimum to cover all known cases including the
-	 * off-by-one issue that HW has in some cases.
+	/* sink_sync_latency of 8 means source has to wait for more than 8
+	 * frames, we'll go with 9 frames for now
 	 */
-	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
+	val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
 
-	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
-	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
+	/* Avoid deep sleep as much as possible to avoid PSR2 idle state */
+	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
 
 	/* FIXME: selective update is probably totally broken because it doesn't
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
@@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 		val |= EDP_Y_COORDINATE_ENABLE;
 
-	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
-
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
 	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
 		val |= EDP_PSR2_TP2_TIME_50us;
-- 
2.19.2

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

[PATCH 9/9] drm/i915: Remove old PSR2 FIXME about frontbuffer tracking

[José Roberto de Souza]

Our frontbuffer tracking improved over the years + the WA #0884
helped us keep PSR2 enabled while triggering screen updates when
necessary so this FIXME is not valid anymore.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 6fd793fec5e9..a1bde8bbd85b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	/* Avoid deep sleep as much as possible to avoid PSR2 idle state */
 	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
 
-	/* FIXME: selective update is probably totally broken because it doesn't
-	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
-	 * good enough. */
 	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 		val |= EDP_Y_COORDINATE_ENABLE;
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/9] drm/i915: Disable PSR in Apple panels

[Ville Syrjälä]

On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza wrote:
> i915 yet don't support PSR in Apple panels, so lets keep it disabled
> while we work on that.
> 
> Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c  | 2 ++
>  drivers/gpu/drm/i915/intel_psr.c | 6 ++++++
>  include/drm/drm_dp_helper.h      | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 6d483487f2b4..6b5a19d3e347 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>  	/* LG LP140WF6-SPM1 eDP panel */
>  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> +	/* Apple panels needs some additional handling to support PSR */
> +	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) }
>  };
>  
>  #undef OUI
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 572e626eadff..f5d27a02eb28 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  		DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n");
>  		return;
>  	}
> +
> +	if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
> +		DRM_DEBUG_KMS("PSR support not currently available for this panel\n");
> +		return;
> +	}
> +
>  	dev_priv->psr.sink_support = true;
>  	dev_priv->psr.sink_sync_latency =
>  		intel_dp_get_sink_sync_latency(intel_dp);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 3314e91f6eb3..db516c48cda3 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1364,6 +1364,7 @@ enum drm_dp_quirk {
>  	 * to 16 bits. So will give a constant value (0x8000) for compatability.
>  	 */
>  	DP_DPCD_QUIRK_CONSTANT_N,
> +	DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,

Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?

>  };
>  
>  /**
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/9] drm/i915: Disable PSR in Apple panels

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 24045 bytes --]

Hi José,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.20-rc4 next-20181126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-Disable-PSR-in-Apple-panels/20181127-085802
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params '
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info'
   kernel/rcu/tree.c:685: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:375: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/slimbus/stream.c:1: warning: no structured comments found
   include/linux/spi/spi.h:177: warning: Function parameter or member 'driver_override' not described in 'spi_device'
   drivers/target/target_core_device.c:1: warning: no structured comments found
   drivers/usb/typec/bus.c:1: warning: no structured comments found
   drivers/usb/typec/class.c:1: warning: no structured comments found
   include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
   fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/file_table.c:1: warning: no structured comments found
   fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:254: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_gfx'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:302: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:382: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:383: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'end' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:848: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3098: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'backlight_link' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'freesync_module' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'fw_dmcu' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'dmcu_fw_version' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1: warning: no structured comments found
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_atomic_state_helper.h:1: warning: no structured comments found
>> include/drm/drm_dp_helper.h:1369: warning: Enum value 'DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED' not described in enum 'drm_dp_quirk'
   drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'dsc_dpcd' not described in 'drm_dp_dsc_sink_max_slice_count'
   drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'is_edp' not described in 'drm_dp_dsc_sink_max_slice_count'
   Error: Cannot open file drivers/gpu/drm/drm_global.c
   Error: Cannot open file drivers/gpu/drm/drm_global.c
   WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export drivers/gpu/drm/drm_global.c' failed with return code 2
   drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma '
   drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
   drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
   include/linux/skbuff.h:862: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:862: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2052: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   Documentation/admin-guide/cgroup-v2.rst:1507: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1509: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1510: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/net/mac80211.h:1211: ERROR: Unexpected indentation.
   include/net/mac80211.h:1218: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:110: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:113: ERROR: Unexpected indentation.
   include/linux/wait.h:115: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/time/hrtimer.c:1129: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/signal.c:344: WARNING: Inline literal start-string without end-string.
   include/linux/kernel.h:137: WARNING: Inline interpreted text or phrase reference start-string without end-string.
   include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string.
   Documentation/driver-api/gpio/board.rst:209: ERROR: Unexpected indentation.
   drivers/ata/libata-core.c:5958: ERROR: Unknown target name: "hw".
   drivers/message/fusion/mptbase.c:5057: WARNING: Definition list ends without a blank line; unexpected unindent.
   drivers/tty/serial/serial_core.c:1938: WARNING: Definition list ends without a blank line; unexpected unindent.
   include/linux/mtd/rawnand.h:1189: WARNING: Inline strong start-string without end-string.
   include/linux/mtd/rawnand.h:1191: WARNING: Inline strong start-string without end-string.
   include/linux/regulator/driver.h:286: ERROR: Unknown target name: "regulator_regmap_x_voltage".
   Documentation/driver-api/soundwire/locking.rst:50: ERROR: Inconsistent literal block quoting.
   Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line.
   Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string.
   Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line.
   include/linux/spi/spi.h:365: ERROR: Unexpected indentation.
   Documentation/driver-api/usb/typec_bus.rst:76: WARNING: Definition list ends without a blank line; unexpected unindent.
   block/bio.c:883: WARNING: Inline emphasis start-string without end-string.
   fs/posix_acl.c:635: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1573: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1575: WARNING: Inline emphasis start-string without end-string.
   include/drm/drm_drv.h:656: ERROR: Unknown target name: "driver".
   Documentation/laptops/lg-laptop.rst:2: WARNING: Explicit markup ends without a blank line; unexpected unindent.
   Documentation/laptops/lg-laptop.rst:16: ERROR: Unexpected indentation.
   Documentation/laptops/lg-laptop.rst:17: WARNING: Block quote ends without a blank line; unexpected unindent.

vim +1369 include/drm/drm_dp_helper.h

76fa998a Jani Nikula 2017-05-18 @1369  

:::::: The code at line 1369 was first introduced by commit
:::::: 76fa998acd86b6b40d0217e12af39c2406bdcd2b drm/dp: start a DPCD based DP sink/branch device quirk database

:::::: TO: Jani Nikula <jani.nikula@intel.com>
:::::: CC: Jani Nikula <jani.nikula@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6597 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 1/9] drm/i915: Disable PSR in Apple panels

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 2997 bytes --]

On Tue, 2018-11-27 at 15:38 +0200, Ville Syrjälä wrote:
> On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza
> wrote:
> > i915 yet don't support PSR in Apple panels, so lets keep it
> > disabled
> > while we work on that.
> > 
> > Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c  | 2 ++
> >  drivers/gpu/drm/i915/intel_psr.c | 6 ++++++
> >  include/drm/drm_dp_helper.h      | 1 +
> >  3 files changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 6d483487f2b4..6b5a19d3e347 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk
> > dpcd_quirk_list[] = {
> >  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true,
> > BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> >  	/* LG LP140WF6-SPM1 eDP panel */
> >  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r',
> > 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> > +	/* Apple panels needs some additional handling to support PSR
> > */
> > +	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
> > BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) }
> >  };
> >  
> >  #undef OUI
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 572e626eadff..f5d27a02eb28 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp
> > *intel_dp)
> >  		DRM_DEBUG_KMS("Panel lacks power state control, PSR
> > cannot be enabled\n");
> >  		return;
> >  	}
> > +
> > +	if (drm_dp_has_quirk(&intel_dp->desc,
> > DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
> > +		DRM_DEBUG_KMS("PSR support not currently available for
> > this panel\n");
> > +		return;
> > +	}
> > +
> >  	dev_priv->psr.sink_support = true;
> >  	dev_priv->psr.sink_sync_latency =
> >  		intel_dp_get_sink_sync_latency(intel_dp);
> > diff --git a/include/drm/drm_dp_helper.h
> > b/include/drm/drm_dp_helper.h
> > index 3314e91f6eb3..db516c48cda3 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1364,6 +1364,7 @@ enum drm_dp_quirk {
> >  	 * to 16 bits. So will give a constant value (0x8000) for
> > compatability.
> >  	 */
> >  	DP_DPCD_QUIRK_CONSTANT_N,
> > +	DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
> 
> Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?

Okay changing to DP_DPCD_QUIRK_NO_PSR.

> 
> >  };
> >  
> >  /**
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza wrote:
> For PSR2 there is no register to tell HW to keep main link enabled
> while PSR2 is active, so don't configure sink DPCD with a
> misleading value.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index f5d27a02eb28..888e348cc1b4 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
>  		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE);
>  		dpcd_val |= DP_PSR_ENABLE_PSR2;
> +	} else {
> +		if (dev_priv->psr.link_standby)
> +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +
> +		if (INTEL_GEN(dev_priv) >= 8)
> +			dpcd_val |= DP_PSR_CRC_VERIFICATION;

commit message only mention the link stand-by...
could you please do this in a separated patch?

>  	}
>  
> -	if (dev_priv->psr.link_standby)
> -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
>  
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
> -- 
> 2.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 1964 bytes --]

On Wed, 2018-11-28 at 11:02 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza
> wrote:
> > For PSR2 there is no register to tell HW to keep main link enabled
> > while PSR2 is active, so don't configure sink DPCD with a
> > misleading value.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index f5d27a02eb28..888e348cc1b4 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct
> > intel_dp *intel_dp)
> >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CONFIG,
> >  				   DP_ALPM_ENABLE);
> >  		dpcd_val |= DP_PSR_ENABLE_PSR2;
> > +	} else {
> > +		if (dev_priv->psr.link_standby)
> > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > +
> > +		if (INTEL_GEN(dev_priv) >= 8)
> > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> 
> commit message only mention the link stand-by...
> could you please do this in a separated patch?

We were already doing it for PSR1, I just grouped all the PSR1 checks
inside of this else block, so there is no functional change in CRC
verification but I can move it to a separated patch if you want.


> 
> >  	}
> >  
> > -	if (dev_priv->psr.link_standby)
> > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> >  
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -- 
> > 2.19.2
> > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/9] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:04PM -0800, José Roberto de Souza wrote:
> eDP spec states 2 different bits to enable sink to trigger a
> interruption when there is a CRC mismatch.
> DP_PSR_CRC_VERIFICATION is for PSR only and
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 888e348cc1b4..607c3ec41679 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
>  	if (dev_priv->psr.psr2_enabled) {
>  		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE);
> -		dpcd_val |= DP_PSR_ENABLE_PSR2;
> +		dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;

good catch!


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



>  	} else {
>  		if (dev_priv->psr.link_standby)
>  			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> -- 
> 2.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza wrote:
> For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not
> touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already
> unset in gen10 + GLK we can just drop it and fix for both gens.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Ok, this patch seems right according to spec.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

But I wonder now if we need intel_psr_setup_vsc() at all for
platforms different than gen9.

Because description of this bit is:
This field enables the programmable header for the PSR2 VSC packet.

Without the programmable version I would assume display
engine is now responsible for setting header entirely?

Art?

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 607c3ec41679..7607a58a6ec0 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_psr_setup_aux(intel_dp);
>  
> -	if (dev_priv->psr.psr2_enabled) {
> +	if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
> +					   !IS_GEMINILAKE(dev_priv))) {
>  		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
>  							cpu_transcoder);
>  		u32 chicken = I915_READ(reg);
>  
> -		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> -			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> -				   | PSR2_ADD_VERTICAL_LINE_COUNT);
> -
> -		else
> -			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> +		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> +			   PSR2_ADD_VERTICAL_LINE_COUNT;
>  		I915_WRITE(reg, chicken);
>  	}
>  
> -- 
> 2.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm: Add offset of PSR2 SU X granularity value

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:06PM -0800, José Roberto de Souza wrote:
> Source is required to comply to sink SU granularity when
> DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS,
> so adding the register here.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  include/drm/drm_dp_helper.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index db516c48cda3..acc7ccfd2044 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -314,6 +314,9 @@
>  # define DP_PSR_SETUP_TIME_SHIFT            1
>  # define DP_PSR2_SU_Y_COORDINATE_REQUIRED   (1 << 4)  /* eDP 1.4a */
>  # define DP_PSR2_SU_GRANULARITY_REQUIRED    (1 << 5)  /* eDP 1.4b */
> +
> +#define DP_PSR2_SU_X_GRANULARITY	    0x072 /* eDP 1.4b */
> +
>  /*
>   * 0x80-0x8f describe downstream port capabilities, but there are two layouts
>   * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
> -- 
> 2.19.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/9] drm/i915/psr: Check if source supports sink specific SU granularity

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:07PM -0800, José Roberto de Souza wrote:
> According to eDP spec, sink can required specific selective update
> granularity that source must comply.
> Here caching the value if required and checking source supports it.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f763b30f98d9..cbcd85af95bf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -506,6 +506,7 @@ struct i915_psr {
>  	ktime_t last_exit;
>  	bool sink_not_reliable;
>  	bool irq_aux_error;
> +	u16 su_x_granularity;
>  };
>  
>  enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 7607a58a6ec0..9215c9052381 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -257,6 +257,21 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
>  	return val;
>  }
>  
> +static u16 intel_dp_get_su_x_granulartiy(struct intel_dp *intel_dp)
> +{
> +	u16 val = 0;
> +	ssize_t r;
> +
> +	if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
> +		return val;
> +
> +	r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &val, 2);
> +	if (r != 2)
> +		DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n");
> +
> +	return val;
> +}
> +
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv =
> @@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  		if (dev_priv->psr.sink_psr2_support) {
>  			dev_priv->psr.colorimetry_support =
>  				intel_dp_get_colorimetry_status(intel_dp);
> +			dev_priv->psr.su_x_granularity =
> +				intel_dp_get_su_x_granulartiy(intel_dp);
>  		}
>  	}
>  }
> @@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  		return false;
>  	}
>  
> +	if (dev_priv->psr.su_x_granularity) {
> +		/*
> +		 * HW will always send full lines in SU blocks, so X will
> +		 * always be 0 and we only need to check the width to validate
> +		 * horizontal granularity.
> +		 * About vertical granularity HW works by SU blocks starting
> +		 * at each 4 lines with height of 4 lines, what eDP states
> +		 * that sink should support.
> +		 */
> +		if (crtc_hdisplay % dev_priv->psr.su_x_granularity) {
> +			DRM_DEBUG_KMS("PSR2 not enabled, HW can not match sink SU granularity requirement\n");
> +			return false;


I wonder if regardless this bit we still need to do a sort of
check anyway because spec states:

"
Sets the grid pattern granularity in the X direction.
A value of 0 indicates that no X-coordinate granularity requirement exists other than
the standard restrictions, wherein the:
•
 Starting X-coordinate must be evenly divisible by 16
 •
 Rectangle width must be evenly divisible by 4
"

Also, why we are just checking X granularity and not Y? (0074h)

(maybe it would be useful to introduce along with previous patch
that I had just reviewed)

> +		}
> +	}
> +
>  	return true;
>  }
>  
> -- 
> 2.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/9] drm/i915/psr: Rename PSR2 macros to better match meaning

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza wrote:
> The first 8 bits of PSR2_CTL have 2 fields to set frames count, the
> first one is to set how many idle frames PSR2 HW needs to wait before
> enter in deep sleep and the second one it is how many frames(it don't
> need to be idle frames) PSR2 HW will wait before start the PSR
> activation sequence.
> The previous names was really misleading and caused wrong values being
> set so better rename to make it clear.

I honestly prefer the old names for 2 reasons:

- they are shorter
- they follow the exact name we have on spec

> 
> Also taking the oportunity to improve those macros.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 35 ++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_psr.c |  7 ++++---
>  2 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 47baf2fe8f71..73046bb9ec7c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4203,23 +4203,24 @@ enum {
>  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
>  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
>  
> -#define EDP_PSR2_CTL			_MMIO(0x6f900)
> -#define   EDP_PSR2_ENABLE		(1 << 31)
> -#define   EDP_SU_TRACK_ENABLE		(1 << 30)
> -#define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
> -#define   EDP_Y_COORDINATE_ENABLE	(1 << 25) /* GLK and CNL+ */
> -#define   EDP_MAX_SU_DISABLE_TIME(t)	((t) << 20)
> -#define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f << 20)
> -#define   EDP_PSR2_TP2_TIME_500us	(0 << 8)
> -#define   EDP_PSR2_TP2_TIME_100us	(1 << 8)
> -#define   EDP_PSR2_TP2_TIME_2500us	(2 << 8)
> -#define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
> -#define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
> -#define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> -#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
> -#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
> -#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> -#define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> +#define EDP_PSR2_CTL					_MMIO(0x6f900)
> +#define   EDP_PSR2_ENABLE				(1 << 31)
> +#define   EDP_SU_TRACK_ENABLE				(1 << 30)
> +#define   EDP_Y_COORDINATE_VALID			(1 << 26) /* GLK and CNL+ */
> +#define   EDP_Y_COORDINATE_ENABLE			(1 << 25) /* GLK and CNL+ */
> +#define   EDP_MAX_SU_DISABLE_TIME(t)			((t) << 20)
> +#define   EDP_MAX_SU_DISABLE_TIME_MASK			(0x1f << 20)
> +#define   EDP_PSR2_TP2_TIME_500us			(0 << 8)
> +#define   EDP_PSR2_TP2_TIME_100us			(1 << 8)
> +#define   EDP_PSR2_TP2_TIME_2500us			(2 << 8)
> +#define   EDP_PSR2_TP2_TIME_50us			(3 << 8)
> +#define   EDP_PSR2_TP2_TIME_MASK			(3 << 8)
> +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT		(4)
> +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK		(0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT)
> +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n)		(((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK)
> +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK	(0xf)
> +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT	(0)
> +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n)		(((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
>  
>  #define _PSR_EVENT_TRANS_A			0x60848
>  #define _PSR_EVENT_TRANS_B			0x61848
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 9215c9052381..ba7bbe3f8df2 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
>  static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct i915_psr *psr = &dev_priv->psr;
>  	u32 val;
>  
>  	/* Let's use 6 as the minimum to cover all known cases including the
> @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	 */
>  	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>  
> -	idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1);
> -	val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
> +	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
> +	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
>  
>  	/* FIXME: selective update is probably totally broken because it doesn't
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>  		val |= EDP_Y_COORDINATE_ENABLE;
>  
> -	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
> +	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
>  
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
>  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
> -- 
> 2.19.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] drm/i915/psr: Set the right frames values

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:09PM -0800, José Roberto de Souza wrote:
> EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of
> frames that it should wait to enter PSR, what is wrong.
> Here it is setting this field with the highest value to avoid PSR2
> exits frequently, as when HW exit deep sleep it needs to go to idle
> state causing a PSR exit for then waiting a few frames before
> activate PSR2 again.
> This will result in more power saving as the sleep state also provide
> some power savings by doing selective updates instead of full screen
> updates.
> 
> About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames
> (not idle frames) that PSR2 hardware will wait to activate PSR2, so
> lets keep using the sink sync latency.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index ba7bbe3f8df2..6fd793fec5e9 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	struct i915_psr *psr = &dev_priv->psr;
>  	u32 val;
>  
> -	/* Let's use 6 as the minimum to cover all known cases including the
> -	 * off-by-one issue that HW has in some cases.
> +	/* sink_sync_latency of 8 means source has to wait for more than 8
> +	 * frames, we'll go with 9 frames for now
>  	 */
> -	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);

Too many changes in a single patch that I couldn't understand why we
are removing the minimal of 6 that was our safe net.

> +	val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
>  
> -	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
> -	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
> +	/* Avoid deep sleep as much as possible to avoid PSR2 idle state */
> +	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
>  
>  	/* FIXME: selective update is probably totally broken because it doesn't
>  	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> @@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>  		val |= EDP_Y_COORDINATE_ENABLE;
>  
> -	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
> -
>  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
>  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
>  		val |= EDP_PSR2_TP2_TIME_50us;
> -- 
> 2.19.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 9/9] drm/i915: Remove old PSR2 FIXME about frontbuffer tracking

[Rodrigo Vivi]

On Mon, Nov 26, 2018 at 04:37:10PM -0800, José Roberto de Souza wrote:
> Our frontbuffer tracking improved over the years + the WA #0884
> helped us keep PSR2 enabled while triggering screen updates when
> necessary so this FIXME is not valid anymore.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 6fd793fec5e9..a1bde8bbd85b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	/* Avoid deep sleep as much as possible to avoid PSR2 idle state */
>  	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
>  
> -	/* FIXME: selective update is probably totally broken because it doesn't
> -	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> -	 * good enough. */
>  	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
>  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>  		val |= EDP_Y_COORDINATE_ENABLE;
> -- 
> 2.19.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/9] drm/i915/psr: Rename PSR2 macros to better match meaning

[Dhinakaran Pandiyan]

On Thu, 2018-11-29 at 15:07 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza
> wrote:
> > The first 8 bits of PSR2_CTL have 2 fields to set frames count, the
> > first one is to set how many idle frames PSR2 HW needs to wait
> > before
> > enter in deep sleep and the second one it is how many frames(it
> > don't
> > need to be idle frames) PSR2 HW will wait before start the PSR
> > activation sequence.
> > The previous names was really misleading and caused wrong values 
The idea was to setup a conservative configuration for PSR2 until we
were ready to enable the feature and some testing was done. Not sure
why you think the values are wrong.

> > being
> > set so better rename to make it clear.
> 
> I honestly prefer the old names for 2 reasons:
> 
> - they are shorter
> - they follow the exact name we have on spec
+1 for the above reason.

> 
> > 
> > Also taking the oportunity to improve those macros.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  | 35 ++++++++++++++++----------
> > ------
> >  drivers/gpu/drm/i915/intel_psr.c |  7 ++++---
> >  2 files changed, 22 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 47baf2fe8f71..73046bb9ec7c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4203,23 +4203,24 @@ enum {
> >  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /*
> > Reserved in ICL+ */
> >  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+
> > */
> >  
> > -#define EDP_PSR2_CTL			_MMIO(0x6f900)
> > -#define   EDP_PSR2_ENABLE		(1 << 31)
> > -#define   EDP_SU_TRACK_ENABLE		(1 << 30)
> > -#define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
> > -#define   EDP_Y_COORDINATE_ENABLE	(1 << 25) /* GLK and CNL+ */
> > -#define   EDP_MAX_SU_DISABLE_TIME(t)	((t) << 20)
> > -#define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f << 20)
> > -#define   EDP_PSR2_TP2_TIME_500us	(0 << 8)
> > -#define   EDP_PSR2_TP2_TIME_100us	(1 << 8)
> > -#define   EDP_PSR2_TP2_TIME_2500us	(2 << 8)
> > -#define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
> > -#define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
> > -#define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> > -#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
> > -#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
> > -#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > -#define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > +#define EDP_PSR2_CTL					_MMIO(0
> > x6f900)
> > +#define   EDP_PSR2_ENABLE				(1 << 31)
> > +#define   EDP_SU_TRACK_ENABLE				(1 <<
> > 30)
> > +#define   EDP_Y_COORDINATE_VALID			(1 << 26) /*
> > GLK and CNL+ */
> > +#define   EDP_Y_COORDINATE_ENABLE			(1 << 25) /*
> > GLK and CNL+ */
> > +#define   EDP_MAX_SU_DISABLE_TIME(t)			((t) <<
> > 20)
> > +#define   EDP_MAX_SU_DISABLE_TIME_MASK			(0x1f
> > << 20)
> > +#define   EDP_PSR2_TP2_TIME_500us			(0 << 8)
> > +#define   EDP_PSR2_TP2_TIME_100us			(1 << 8)
> > +#define   EDP_PSR2_TP2_TIME_2500us			(2 << 8)
> > +#define   EDP_PSR2_TP2_TIME_50us			(3 << 8)
> > +#define   EDP_PSR2_TP2_TIME_MASK			(3 << 8)
> > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT		(4)
> > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK		(0xf <<
> > EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT)
> > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n)		(((n)
> > << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) &
> > EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK)
> > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK	(0xf)
> > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT	(0)
> > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n)		(((n)
> > << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) &
> > EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
> >  
> >  #define _PSR_EVENT_TRANS_A			0x60848
> >  #define _PSR_EVENT_TRANS_B			0x61848
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 9215c9052381..ba7bbe3f8df2 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp
> > *intel_dp)
> >  static void hsw_activate_psr2(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	struct i915_psr *psr = &dev_priv->psr;
> >  	u32 val;
> >  
> >  	/* Let's use 6 as the minimum to cover all known cases
> > including the
> > @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> >  	 */
> >  	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> >  
> > -	idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency
> > + 1);
> > -	val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
> > +	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
> > +	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
> >  
> >  	/* FIXME: selective update is probably totally broken because
> > it doesn't
> >  	 * mesh at all with our frontbuffer tracking. And the hw alone
> > isn't
> > @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> >  		val |= EDP_Y_COORDINATE_ENABLE;
> >  
> > -	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency 
> > + 1);
> > +	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
> > 1);
> >  
> >  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
> >  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
> > -- 
> > 2.19.2
> > 

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

Re: [PATCH 9/9] drm/i915: Remove old PSR2 FIXME about frontbuffer tracking

[Dhinakaran Pandiyan]

On Thu, 2018-11-29 at 15:11 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:10PM -0800, José Roberto de Souza
> wrote:
> > Our frontbuffer tracking improved over the years + the WA #0884
> > helped us keep PSR2 enabled while triggering screen updates when
> > necessary so this FIXME is not valid anymore.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


> 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 6fd793fec5e9..a1bde8bbd85b 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> >  	/* Avoid deep sleep as much as possible to avoid PSR2 idle
> > state */
> >  	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
> >  
> > -	/* FIXME: selective update is probably totally broken because
> > it doesn't
> > -	 * mesh at all with our frontbuffer tracking. And the hw alone
> > isn't
> > -	 * good enough. */
> >  	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> >  		val |= EDP_Y_COORDINATE_ENABLE;
> > -- 
> > 2.19.2
> > 

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

Re: [PATCH 3/9] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[Dhinakaran Pandiyan]

On Thu, 2018-11-29 at 14:04 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:04PM -0800, José Roberto de Souza
> wrote:
> > eDP spec states 2 different bits to enable sink to trigger a
> > interruption when there is a CRC mismatch.
> > DP_PSR_CRC_VERIFICATION is for PSR only and
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 888e348cc1b4..607c3ec41679 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct
> > intel_dp *intel_dp)
> >  	if (dev_priv->psr.psr2_enabled) {
> >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CONFIG,
> >  				   DP_ALPM_ENABLE);
> > -		dpcd_val |= DP_PSR_ENABLE_PSR2;
> > +		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> 
> good catch!
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Is there a commit that this patch Fixes?

> 
> 
> 
> >  	} else {
> >  		if (dev_priv->psr.link_standby)
> >  			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > -- 
> > 2.19.2
> > 

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

Re: [PATCH 1/9] drm/i915: Disable PSR in Apple panels

[Dhinakaran Pandiyan]

On Tue, 2018-11-27 at 13:55 -0800, Souza, Jose wrote:
> On Tue, 2018-11-27 at 15:38 +0200, Ville Syrjälä wrote:
> > On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza
> > wrote:
> > > i915 yet don't support PSR in Apple panels, so lets keep
Replace "Apple" with specific model name?

> > > disabled
> > > while we work on that.
> > > 
> > > Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
Bugzilla please. Also Cc the bug reporter?


> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c  | 2 ++
> > >  drivers/gpu/drm/i915/intel_psr.c | 6 ++++++
> > >  include/drm/drm_dp_helper.h      | 1 +
> > >  3 files changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > > b/drivers/gpu/drm/drm_dp_helper.c
> > > index 6d483487f2b4..6b5a19d3e347 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk
> > > dpcd_quirk_list[] = {
> > >  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true,
> > > BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> > >  	/* LG LP140WF6-SPM1 eDP panel */
> > >  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r',
> > > 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> > > +	/* Apple panels needs some additional handling to support PSR
> > > */
> > > +	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
> > > BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) }
> > >  };
> > >  
> > >  #undef OUI
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 572e626eadff..f5d27a02eb28 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp
> > > *intel_dp)
> > >  		DRM_DEBUG_KMS("Panel lacks power state control, PSR
> > > cannot be enabled\n");
> > >  		return;
> > >  	}
> > > +
> > > +	if (drm_dp_has_quirk(&intel_dp->desc,
> > > DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
> > > +		DRM_DEBUG_KMS("PSR support not currently available for
> > > this panel\n");
> > > +		return;
> > > +	}
> > > +
> > >  	dev_priv->psr.sink_support = true;
> > >  	dev_priv->psr.sink_sync_latency =
> > >  		intel_dp_get_sink_sync_latency(intel_dp);
> > > diff --git a/include/drm/drm_dp_helper.h
> > > b/include/drm/drm_dp_helper.h
> > > index 3314e91f6eb3..db516c48cda3 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -1364,6 +1364,7 @@ enum drm_dp_quirk {
> > >  	 * to 16 bits. So will give a constant value (0x8000) for
> > > compatability.
> > >  	 */
> > >  	DP_DPCD_QUIRK_CONSTANT_N,
> > > +	DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
> > 
> > Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?
> 
> Okay changing to DP_DPCD_QUIRK_NO_PSR.
> 
> > 
> > >  };
> > >  
> > >  /**
> > > -- 
> > > 2.19.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 2414 bytes --]

On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza
> wrote:
> > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not
> > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already
> > unset in gen10 + GLK we can just drop it and fix for both gens.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> 
> Ok, this patch seems right according to spec.
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> But I wonder now if we need intel_psr_setup_vsc() at all for
> platforms different than gen9.
> 
> Because description of this bit is:
> This field enables the programmable header for the PSR2 VSC packet.
> 
> Without the programmable version I would assume display
> engine is now responsible for setting header entirely?

As this was in a chicken register in gen 9 my guess is that it was
fixed on newer gens and as it is required for PSR2 we don't need to
manually enable it in driver but Art could confirm it.

> 
> Art?
> 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 607c3ec41679..7607a58a6ec0 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp,
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> >  		hsw_psr_setup_aux(intel_dp);
> >  
> > -	if (dev_priv->psr.psr2_enabled) {
> > +	if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
> > +					   !IS_GEMINILAKE(dev_priv))) {
> >  		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> >  							cpu_transcoder)
> > ;
> >  		u32 chicken = I915_READ(reg);
> >  
> > -		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> > -			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> > -				   | PSR2_ADD_VERTICAL_LINE_COUNT);
> > -
> > -		else
> > -			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> > +		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > +			   PSR2_ADD_VERTICAL_LINE_COUNT;
> >  		I915_WRITE(reg, chicken);
> >  	}
> >  
> > -- 
> > 2.19.2
> > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 6/9] drm/i915/psr: Check if source supports sink specific SU granularity

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 4340 bytes --]

On Thu, 2018-11-29 at 15:03 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:07PM -0800, José Roberto de Souza
> wrote:
> > According to eDP spec, sink can required specific selective update
> > granularity that source must comply.
> > Here caching the value if required and checking source supports it.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_psr.c | 32
> > ++++++++++++++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index f763b30f98d9..cbcd85af95bf 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -506,6 +506,7 @@ struct i915_psr {
> >  	ktime_t last_exit;
> >  	bool sink_not_reliable;
> >  	bool irq_aux_error;
> > +	u16 su_x_granularity;
> >  };
> >  
> >  enum intel_pch {
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 7607a58a6ec0..9215c9052381 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -257,6 +257,21 @@ static u8
> > intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
> >  	return val;
> >  }
> >  
> > +static u16 intel_dp_get_su_x_granulartiy(struct intel_dp
> > *intel_dp)
> > +{
> > +	u16 val = 0;
> > +	ssize_t r;
> > +
> > +	if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
> > +		return val;
> > +
> > +	r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY,
> > &val, 2);
> > +	if (r != 2)
> > +		DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n");
> > +
> > +	return val;
> > +}
> > +
> >  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *dev_priv =
> > @@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > *intel_dp)
> >  		if (dev_priv->psr.sink_psr2_support) {
> >  			dev_priv->psr.colorimetry_support =
> >  				intel_dp_get_colorimetry_status(intel_d
> > p);
> > +			dev_priv->psr.su_x_granularity =
> > +				intel_dp_get_su_x_granulartiy(intel_dp)
> > ;
> >  		}
> >  	}
> >  }
> > @@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct
> > intel_dp *intel_dp,
> >  		return false;
> >  	}
> >  
> > +	if (dev_priv->psr.su_x_granularity) {
> > +		/*
> > +		 * HW will always send full lines in SU blocks, so X
> > will
> > +		 * always be 0 and we only need to check the width to
> > validate
> > +		 * horizontal granularity.
> > +		 * About vertical granularity HW works by SU blocks
> > starting
> > +		 * at each 4 lines with height of 4 lines, what eDP
> > states
> > +		 * that sink should support.
> > +		 */
> > +		if (crtc_hdisplay % dev_priv->psr.su_x_granularity) {
> > +			DRM_DEBUG_KMS("PSR2 not enabled, HW can not
> > match sink SU granularity requirement\n");
> > +			return false;
> 
> I wonder if regardless this bit we still need to do a sort of
> check anyway because spec states:
> 
> "
> Sets the grid pattern granularity in the X direction.
> A value of 0 indicates that no X-coordinate granularity requirement
> exists other than
> the standard restrictions, wherein the:
> •
>  Starting X-coordinate must be evenly divisible by 16
>  •
>  Rectangle width must be evenly divisible by 4
> "

Hum, the x will always be 0 so it will always be divisible by 16 but
the width could not be divisible by 4 in the odd resolutions. I will
add this check, thanks.


> 
> Also, why we are just checking X granularity and not Y? (0074h)
> 
> (maybe it would be useful to introduce along with previous patch
> that I had just reviewed)


It is in the comment above but coping from eDP spec:

A value of 00h, 01h, 02h, or 04h should be supported by the Sink device
to ensure
interoperability with various Source devices. A value of 08h or 10h may
be considered
for system-specific implementations.

Our height is multiple of 4 so we are safe even if sink have a y
granularity set.

> 
> > +		}
> > +	}
> > +
> >  	return true;
> >  }
> >  
> > -- 
> > 2.19.2
> > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/9] drm/i915/psr: Rename PSR2 macros to better match meaning

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 6199 bytes --]

On Thu, 2018-11-29 at 15:25 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2018-11-29 at 15:07 -0800, Rodrigo Vivi wrote:
> > On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza
> > wrote:
> > > The first 8 bits of PSR2_CTL have 2 fields to set frames count,
> > > the
> > > first one is to set how many idle frames PSR2 HW needs to wait
> > > before
> > > enter in deep sleep and the second one it is how many frames(it
> > > don't
> > > need to be idle frames) PSR2 HW will wait before start the PSR
> > > activation sequence.
> > > The previous names was really misleading and caused wrong values 
> The idea was to setup a conservative configuration for PSR2 until we
> were ready to enable the feature and some testing was done. Not sure
> why you think the values are wrong.
> 
> > > being
> > > set so better rename to make it clear.
> > 
> > I honestly prefer the old names for 2 reasons:
> > 
> > - they are shorter
> > - they follow the exact name we have on spec
> +1 for the above reason.

Okay droping this patch.

> 
> > > Also taking the oportunity to improve those macros.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  | 35 ++++++++++++++++----------
> > > ------
> > >  drivers/gpu/drm/i915/intel_psr.c |  7 ++++---
> > >  2 files changed, 22 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 47baf2fe8f71..73046bb9ec7c 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4203,23 +4203,24 @@ enum {
> > >  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /*
> > > Reserved in ICL+ */
> > >  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+
> > > */
> > >  
> > > -#define EDP_PSR2_CTL			_MMIO(0x6f900)
> > > -#define   EDP_PSR2_ENABLE		(1 << 31)
> > > -#define   EDP_SU_TRACK_ENABLE		(1 << 30)
> > > -#define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
> > > -#define   EDP_Y_COORDINATE_ENABLE	(1 << 25) /* GLK and
> > > CNL+ */
> > > -#define   EDP_MAX_SU_DISABLE_TIME(t)	((t) << 20)
> > > -#define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f << 20)
> > > -#define   EDP_PSR2_TP2_TIME_500us	(0 << 8)
> > > -#define   EDP_PSR2_TP2_TIME_100us	(1 << 8)
> > > -#define   EDP_PSR2_TP2_TIME_2500us	(2 << 8)
> > > -#define   EDP_PSR2_TP2_TIME_50us	(3 << 8)
> > > -#define   EDP_PSR2_TP2_TIME_MASK	(3 << 8)
> > > -#define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> > > -#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf << 4)
> > > -#define   EDP_PSR2_FRAME_BEFORE_SU(a)	((a) << 4)
> > > -#define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > > -#define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > > +#define EDP_PSR2_CTL					_MMIO(0
> > > x6f900)
> > > +#define   EDP_PSR2_ENABLE				(1 <<
> > > 31)
> > > +#define   EDP_SU_TRACK_ENABLE				(1 <<
> > > 30)
> > > +#define   EDP_Y_COORDINATE_VALID			(1 << 26) /*
> > > GLK and CNL+ */
> > > +#define   EDP_Y_COORDINATE_ENABLE			(1 <<
> > > 25) /*
> > > GLK and CNL+ */
> > > +#define   EDP_MAX_SU_DISABLE_TIME(t)			((t) <<
> > > 20)
> > > +#define   EDP_MAX_SU_DISABLE_TIME_MASK			(0x1f
> > > << 20)
> > > +#define   EDP_PSR2_TP2_TIME_500us			(0 <<
> > > 8)
> > > +#define   EDP_PSR2_TP2_TIME_100us			(1 <<
> > > 8)
> > > +#define   EDP_PSR2_TP2_TIME_2500us			(2 <<
> > > 8)
> > > +#define   EDP_PSR2_TP2_TIME_50us			(3 << 8)
> > > +#define   EDP_PSR2_TP2_TIME_MASK			(3 << 8)
> > > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT		(4)
> > > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK		(0xf <<
> > > EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT)
> > > +#define   EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n)		(((n)
> > > << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) &
> > > EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK)
> > > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK	(0xf)
> > > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT	(0)
> > > +#define   EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n)		(((n)
> > > << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) &
> > > EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
> > >  
> > >  #define _PSR_EVENT_TRANS_A			0x60848
> > >  #define _PSR_EVENT_TRANS_B			0x61848
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 9215c9052381..ba7bbe3f8df2 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp
> > > *intel_dp)
> > >  static void hsw_activate_psr2(struct intel_dp *intel_dp)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +	struct i915_psr *psr = &dev_priv->psr;
> > >  	u32 val;
> > >  
> > >  	/* Let's use 6 as the minimum to cover all known cases
> > > including the
> > > @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp
> > > *intel_dp)
> > >  	 */
> > >  	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> > >  
> > > -	idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency
> > > + 1);
> > > -	val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
> > > +	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
> > > +	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
> > >  
> > >  	/* FIXME: selective update is probably totally broken because
> > > it doesn't
> > >  	 * mesh at all with our frontbuffer tracking. And the hw alone
> > > isn't
> > > @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp
> > > *intel_dp)
> > >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > >  		val |= EDP_Y_COORDINATE_ENABLE;
> > >  
> > > -	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv-
> > > >psr.sink_sync_latency 
> > > + 1);
> > > +	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
> > > 1);
> > >  
> > >  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
> > >  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
> > > -- 
> > > 2.19.2
> > > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 8/9] drm/i915/psr: Set the right frames values

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 3413 bytes --]

On Thu, 2018-11-29 at 15:10 -0800, Rodrigo Vivi wrote:
> On Mon, Nov 26, 2018 at 04:37:09PM -0800, José Roberto de Souza
> wrote:
> > EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number
> > of
> > frames that it should wait to enter PSR, what is wrong.
> > Here it is setting this field with the highest value to avoid PSR2
> > exits frequently, as when HW exit deep sleep it needs to go to idle
> > state causing a PSR exit for then waiting a few frames before
> > activate PSR2 again.
> > This will result in more power saving as the sleep state also
> > provide
> > some power savings by doing selective updates instead of full
> > screen
> > updates.
> > 
> > About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames
> > (not idle frames) that PSR2 hardware will wait to activate PSR2, so
> > lets keep using the sink sync latency.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index ba7bbe3f8df2..6fd793fec5e9 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> >  	struct i915_psr *psr = &dev_priv->psr;
> >  	u32 val;
> >  
> > -	/* Let's use 6 as the minimum to cover all known cases
> > including the
> > -	 * off-by-one issue that HW has in some cases.
> > +	/* sink_sync_latency of 8 means source has to wait for more
> > than 8
> > +	 * frames, we'll go with 9 frames for now
> >  	 */
> > -	int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> 
> Too many changes in a single patch that I couldn't understand why we
> are removing the minimal of 6 that was our safe net.


The vbt idle_frames should not be used for PSR2 as PSR2 just wait for X
frames(idle or not) to activate PSR2 so just rely in sink sync_latency
should be enough but I can bring it back for safety.

> 
> > +	val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
> > 1);
> >  
> > -	idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
> > -	val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
> > +	/* Avoid deep sleep as much as possible to avoid PSR2 idle
> > state */
> > +	val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
> >  
> >  	/* FIXME: selective update is probably totally broken because
> > it doesn't
> >  	 * mesh at all with our frontbuffer tracking. And the hw alone
> > isn't
> > @@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> >  		val |= EDP_Y_COORDINATE_ENABLE;
> >  
> > -	val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
> > 1);
> > -
> >  	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
> >  	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
> >  		val |= EDP_PSR2_TP2_TIME_50us;
> > -- 
> > 2.19.2
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2

[Rodrigo Vivi]

On Wed, Nov 28, 2018 at 12:13:30PM -0800, Souza, Jose wrote:
> On Wed, 2018-11-28 at 11:02 -0800, Rodrigo Vivi wrote:
> > On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza
> > wrote:
> > > For PSR2 there is no register to tell HW to keep main link enabled
> > > while PSR2 is active, so don't configure sink DPCD with a
> > > misleading value.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index f5d27a02eb28..888e348cc1b4 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct
> > > intel_dp *intel_dp)
> > >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > > DP_RECEIVER_ALPM_CONFIG,
> > >  				   DP_ALPM_ENABLE);
> > >  		dpcd_val |= DP_PSR_ENABLE_PSR2;
> > > +	} else {
> > > +		if (dev_priv->psr.link_standby)
> > > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > +
> > > +		if (INTEL_GEN(dev_priv) >= 8)
> > > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > 
> > commit message only mention the link stand-by...
> > could you please do this in a separated patch?
> 
> We were already doing it for PSR1, I just grouped all the PSR1 checks
> inside of this else block, so there is no functional change in CRC
> verification but I can move it to a separated patch if you want.

I prefer the separated patch to make things clear.

The other option would be change the commit message and
subject to mention this clean-up here.

but up to you.

> 
> 
> > 
> > >  	}
> > >  
> > > -	if (dev_priv->psr.link_standby)
> > > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> > >  
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > DP_SET_POWER_D0);
> > > -- 
> > > 2.19.2
> > > 


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

Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2

[Runyan, Arthur J]

I'll check on it.

> -----Original Message-----
> From: Souza, Jose
> Sent: Thursday, 29 November, 2018 3:47 PM
> To: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> Runyan, Arthur J <arthur.j.runyan@intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>
> Subject: Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers
> related to PSR2
> 
> On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote:
> > On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza
> > wrote:
> > > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not
> > > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is
> already
> > > unset in gen10 + GLK we can just drop it and fix for both gens.
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> >
> > Ok, this patch seems right according to spec.
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > But I wonder now if we need intel_psr_setup_vsc() at all for
> > platforms different than gen9.
> >
> > Because description of this bit is:
> > This field enables the programmable header for the PSR2 VSC packet.
> >
> > Without the programmable version I would assume display
> > engine is now responsible for setting header entirely?
> 
> As this was in a chicken register in gen 9 my guess is that it was
> fixed on newer gens and as it is required for PSR2 we don't need to
> manually enable it in driver but Art could confirm it.
> 
> >
> > Art?
> >
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 607c3ec41679..7607a58a6ec0 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > >  		hsw_psr_setup_aux(intel_dp);
> > >
> > > -	if (dev_priv->psr.psr2_enabled) {
> > > +	if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
> > > +					   !IS_GEMINILAKE(dev_priv))) {
> > >  		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > >  							cpu_transcoder)
> > > ;
> > >  		u32 chicken = I915_READ(reg);
> > >
> > > -		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> > > -			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> > > -				   | PSR2_ADD_VERTICAL_LINE_COUNT);
> > > -
> > > -		else
> > > -			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> > > +		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > > +			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > >  		I915_WRITE(reg, chicken);
> > >  	}
> > >
> > > --
> > > 2.19.2
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx