[PATCH 1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.

[PATCH 1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.

[Dhinakaran Pandiyan]

We attempt to read 6 bytes, make sure we have.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4298ac..7dcc874b7d8f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3989,7 +3989,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_s
 		goto stop;
 	}
 
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
 		ret = -EIO;
 		goto stop;
 	}
-- 
2.14.1

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

[PATCH 2/3] drm/i915/dp: Fix sink-crc reads.

[Dhinakaran Pandiyan]

Sink crc is calculated by the sink for static frames irrespective of
what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
case for sink crc, we don't really need the sink_crc_{start, stop} code.

The second problem with the current implementation is vblank waits.
Enabling vblank interrupts triggers PSR exit, which means we aren't
really reading the correct CRC values for PSR tests. vblank waits are
replaced by delays.

With the changes made in this patch, sink CRC is available only for
static frames. I have tested this on a SKL laptop with PSR panel.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
 drivers/gpu/drm/i915/intel_dp.c     | 114 ++++--------------------------------
 drivers/gpu/drm/i915/intel_drv.h    |   3 +-
 3 files changed, 15 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2f05f5262bba..35fa1418cc07 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
 
 		intel_dp = enc_to_intel_dp(state->best_encoder);
 
-		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
+		ret = intel_dp_sink_crc(intel_dp, crc);
 		if (ret)
 			goto err;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7dcc874b7d8f..7352ab631ea8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3876,32 +3876,18 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 					intel_dp->is_mst);
 }
 
-static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
-				  struct intel_crtc_state *crtc_state, bool disable_wa)
+int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int ret = 0;
-	int count = 0;
-	int attempts = 10;
+	int count = 0, ret = 0, attempts;
 
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
-		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
-		ret = -EIO;
-		goto out;
-	}
+	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
+		u8 buf;
 
-	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
-			       buf & ~DP_TEST_SINK_START) < 0) {
-		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
-		ret = -EIO;
-		goto out;
-	}
-
-	do {
-		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
+		/* Wait for approximately half a frame, we cannot wait for a
+		 * vblank interrupt as it triggers PSR exit.
+		 */
+		if (attempts)
+			usleep_range(8000, 8500);
 
 		if (drm_dp_dpcd_readb(&intel_dp->aux,
 				      DP_TEST_SINK_MISC, &buf) < 0) {
@@ -3909,93 +3895,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
 			goto out;
 		}
 		count = buf & DP_TEST_COUNT_MASK;
-	} while (--attempts && count);
-
-	if (attempts == 0) {
-		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after calculation is stopped\n");
-		ret = -ETIMEDOUT;
 	}
 
- out:
-	if (disable_wa)
-		hsw_enable_ips(crtc_state);
-	return ret;
-}
-
-static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
-				   struct intel_crtc_state *crtc_state)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int ret;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
-		return -EIO;
-
-	if (!(buf & DP_TEST_CRC_SUPPORTED))
-		return -ENOTTY;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
-		return -EIO;
-
-	if (buf & DP_TEST_SINK_START) {
-		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state, false);
-		if (ret)
-			return ret;
-	}
-
-	hsw_disable_ips(crtc_state);
-
-	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
-			       buf | DP_TEST_SINK_START) < 0) {
-		hsw_enable_ips(crtc_state);
-		return -EIO;
-	}
-
-	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
-	return 0;
-}
-
-int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, u8 *crc)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int count, ret;
-	int attempts = 6;
-
-	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
-	if (ret)
-		return ret;
-
-	do {
-		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
-
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_TEST_SINK_MISC, &buf) < 0) {
-			ret = -EIO;
-			goto stop;
-		}
-		count = buf & DP_TEST_COUNT_MASK;
-
-	} while (--attempts && count == 0);
-
-	if (attempts == 0) {
-		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
+	if (attempts == 3) {
 		ret = -ETIMEDOUT;
-		goto stop;
+		goto out;
 	}
 
 	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
 		ret = -EIO;
-		goto stop;
+		goto out;
 	}
 
-stop:
-	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
+out:
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44ed248f1fe9..cacee94749e2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-int intel_dp_sink_crc(struct intel_dp *intel_dp,
-		      struct intel_crtc_state *crtc_state, u8 *crc);
+int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
-- 
2.14.1

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

[PATCH 3/3] drm/i915/psr: Move sink-crc to intel_psr.c

[Dhinakaran Pandiyan]

With sink-crc now being relevant only for PSR static frames, move the
code to intel_psr.c and rename the function.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
 drivers/gpu/drm/i915/intel_dp.c     | 35 -----------------------------------
 drivers/gpu/drm/i915/intel_drv.h    |  2 +-
 drivers/gpu/drm/i915/intel_psr.c    | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 35fa1418cc07..9913258e20ed 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
 
 		intel_dp = enc_to_intel_dp(state->best_encoder);
 
-		ret = intel_dp_sink_crc(intel_dp, crc);
+		ret = intel_psr_sink_crc(intel_dp, crc);
 		if (ret)
 			goto err;
 
@@ -4854,7 +4854,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_ppgtt_info", i915_ppgtt_info, 0},
 	{"i915_llc", i915_llc, 0},
 	{"i915_edp_psr_status", i915_edp_psr_status, 0},
-	{"i915_sink_crc_eDP1", i915_sink_crc, 0},
+	{"i915_edp_psr_sink_crc", i915_sink_crc, 0},
 	{"i915_energy_uJ", i915_energy_uJ, 0},
 	{"i915_runtime_pm_status", i915_runtime_pm_status, 0},
 	{"i915_power_domain_info", i915_power_domain_info, 0},
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7352ab631ea8..ff7522d3632e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3876,41 +3876,6 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 					intel_dp->is_mst);
 }
 
-int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
-{
-	int count = 0, ret = 0, attempts;
-
-	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
-		u8 buf;
-
-		/* Wait for approximately half a frame, we cannot wait for a
-		 * vblank interrupt as it triggers PSR exit.
-		 */
-		if (attempts)
-			usleep_range(8000, 8500);
-
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_TEST_SINK_MISC, &buf) < 0) {
-			ret = -EIO;
-			goto out;
-		}
-		count = buf & DP_TEST_COUNT_MASK;
-	}
-
-	if (attempts == 3) {
-		ret = -ETIMEDOUT;
-		goto out;
-	}
-
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
-		ret = -EIO;
-		goto out;
-	}
-
-out:
-	return ret;
-}
-
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cacee94749e2..0da7be3349cc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1644,7 +1644,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
@@ -1900,6 +1899,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
 void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
+int intel_psr_sink_crc(struct intel_dp *intel_dp, u8 *crc);
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0d548292dd09..4ebd05b6e86d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -197,6 +197,41 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
 	return val;
 }
 
+int intel_psr_sink_crc(struct intel_dp *intel_dp, u8 *crc)
+{
+	int count = 0, ret = 0, attempts;
+
+	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
+		u8 buf;
+
+		/* Wait for approximately half a frame, we cannot wait for a
+		 * vblank interrupt as it triggers PSR exit.
+		 */
+		if (attempts)
+			usleep_range(8000, 8500);
+
+		if (drm_dp_dpcd_readb(&intel_dp->aux,
+				      DP_TEST_SINK_MISC, &buf) < 0) {
+			ret = -EIO;
+			goto out;
+		}
+		count = buf & DP_TEST_COUNT_MASK;
+	}
+
+	if (attempts == 3) {
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
+		ret = -EIO;
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv =
-- 
2.14.1

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.
URL   : https://patchwork.freedesktop.org/series/42154/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4082 -> Patchwork_8783 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8783 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8783, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42154/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_frontbuffer_tracking@basic:
      fi-cnl-y3:          PASS -> FAIL
      fi-cnl-psr:         PASS -> FAIL
      fi-cfl-s3:          PASS -> FAIL
      {fi-kbl-7560u}:     PASS -> FAIL
      fi-skl-6600u:       PASS -> FAIL
      fi-kbl-r:           PASS -> FAIL
      fi-cfl-u:           PASS -> FAIL

    
    ==== Warnings ====

    igt@kms_force_connector_basic@force-load-detect:
      fi-ivb-3520m:       PASS -> SKIP +3

    igt@kms_sink_crc_basic:
      fi-cfl-u:           PASS -> SKIP
      fi-cnl-psr:         PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-skl-guc:         PASS -> FAIL (fdo#103191)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-ilk-650:         DMESG-WARN -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

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

  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008


== Participating hosts (36 -> 33) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4082 -> Patchwork_8783

  CI_DRM_4082: 0600266ef9b8407e0dc281acb6b754eba8178c91 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8783: c0e07f8b9d9412e9d41da72654ec408658913b94 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit


== Linux commits ==

c0e07f8b9d94 drm/i915/psr: Move sink-crc to intel_psr.c
676d279f8a86 drm/i915/dp: Fix sink-crc reads.
7fc2b33053e5 drm/i915/dp: Check if the sink crc we read is 6 bytes.

== Logs ==

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

Re: ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.

[Dhinakaran Pandiyan]

On Monday, April 23, 2018 8:25:02 PM PDT Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915/dp: Check if the sink crc we
> read is 6 bytes. URL   : https://patchwork.freedesktop.org/series/42154/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4082 -> Patchwork_8783 =
> 
> == Summary - FAILURE ==
> 
>   Serious unknown changes coming with Patchwork_8783 absolutely need to be
>   verified manually.
> 
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_8783, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in
> CI.
> 
>   External URL:
> https://patchwork.freedesktop.org/api/1.0/series/42154/revisions/1/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in
> Patchwork_8783:
> 
>   === IGT changes ===
> 
>     ==== Possible regressions ====
> 
>     igt@kms_frontbuffer_tracking@basic:
>       fi-cnl-y3:          PASS -> FAIL
>       fi-cnl-psr:         PASS -> FAIL
>       fi-cfl-s3:          PASS -> FAIL
>       {fi-kbl-7560u}:     PASS -> FAIL
>       fi-skl-6600u:       PASS -> FAIL
>       fi-kbl-r:           PASS -> FAIL
>       fi-cfl-u:           PASS -> FAIL
> 

These failures are expected, tests need to modified if this series is accepted.

> 
>     ==== Warnings ====
> 
>     igt@kms_force_connector_basic@force-load-detect:
>       fi-ivb-3520m:       PASS -> SKIP +3
> 
>     igt@kms_sink_crc_basic:
>       fi-cfl-u:           PASS -> SKIP
>       fi-cnl-psr:         PASS -> SKIP
> 

So are these.

> 
> == Known issues ==
> 
>   Here are the changes found in Patchwork_8783 that come from known issues:
> 
>   === IGT changes ===
> 
>     ==== Issues hit ====
> 
>     igt@gem_mmap_gtt@basic-small-bo-tiledx:
>       fi-gdg-551:         PASS -> FAIL (fdo#102575)
> 
>     igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
>       fi-skl-guc:         PASS -> FAIL (fdo#103191)
> 
> 
>     ==== Possible fixes ====
> 
>     igt@gem_exec_suspend@basic-s4-devices:
>       fi-ilk-650:         DMESG-WARN -> PASS
> 
>     igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
>       fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS
> 
>     igt@prime_vgem@basic-fence-flip:
>       fi-ilk-650:         FAIL (fdo#104008) -> PASS
> 
> 
>   {name}: This element is suppressed. This means it is ignored when
> computing the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
>   fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
>   fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
>   fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
> 
> 
> == Participating hosts (36 -> 33) ==
> 
>   Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq
> 
> 
> == Build changes ==
> 
>     * Linux: CI_DRM_4082 -> Patchwork_8783
> 
>   CI_DRM_4082: 0600266ef9b8407e0dc281acb6b754eba8178c91 @
> git://anongit.freedesktop.org/gfx-ci/linux IGT_4444:
> dcc44347494231feabc588c2a76998cbc9afdf8c @
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_8783:
> c0e07f8b9d9412e9d41da72654ec408658913b94 @
> git://anongit.freedesktop.org/gfx-ci/linux piglit_4444:
> a2f486679f467cd6e82578384f56d4aabaa8cf2e @
> git://anongit.freedesktop.org/piglit
> 
> 
> == Linux commits ==
> 
> c0e07f8b9d94 drm/i915/psr: Move sink-crc to intel_psr.c
> 676d279f8a86 drm/i915/dp: Fix sink-crc reads.
> 7fc2b33053e5 drm/i915/dp: Check if the sink crc we read is 6 bytes.
> 
> == Logs ==
> 
> For more details see:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8783/issues.html
> _______________________________________________
> 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 2/3] drm/i915/dp: Fix sink-crc reads.

[Mika Kahola]

On Mon, 2018-04-23 at 19:56 -0700, Dhinakaran Pandiyan wrote:
> Sink crc is calculated by the sink for static frames irrespective of
> what the driver sets in TEST_SINK_START dpcd. Since PSR is the only
> use
> case for sink crc, we don't really need the sink_crc_{start, stop}
> code.
> 
> The second problem with the current implementation is vblank waits.
> Enabling vblank interrupts triggers PSR exit, which means we aren't
> really reading the correct CRC values for PSR tests. vblank waits are
> replaced by delays.
> 
> With the changes made in this patch, sink CRC is available only for
> static frames. I have tested this on a SKL laptop with PSR panel.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
>  drivers/gpu/drm/i915/intel_dp.c     | 114 ++++--------------------
> ------------
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
>  3 files changed, 15 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2f05f5262bba..35fa1418cc07 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m,
> void *data)
>  
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
>  
> -		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
> +		ret = intel_dp_sink_crc(intel_dp, crc);
>  		if (ret)
>  			goto err;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 7dcc874b7d8f..7352ab631ea8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3876,32 +3876,18 @@ intel_dp_configure_mst(struct intel_dp
> *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> -				  struct intel_crtc_state
> *crtc_state, bool disable_wa)
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  {
> -	struct intel_digital_port *dig_port =
> dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port-
> >base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> >base.crtc);
> -	u8 buf;
> -	int ret = 0;
> -	int count = 0;
> -	int attempts = 10;
> +	int count = 0, ret = 0, attempts;
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) <
> 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped
> properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
A question. How did you end up with three attempts or one and a half
frames? We used to try 6 times or 6 vblanks until we gave up trying.
Well, I don't really know the history why we have ended up with 6
attempts in the first place. 


> +		u8 buf;
>  
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf & ~DP_TEST_SINK_START) < 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped
> properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> +		/* Wait for approximately half a frame, we cannot
> wait for a
> +		 * vblank interrupt as it triggers PSR exit.
> +		 */
> +		if (attempts)
> +			usleep_range(8000, 8500);
>  
>  		if (drm_dp_dpcd_readb(&intel_dp->aux,
>  				      DP_TEST_SINK_MISC, &buf) < 0)
> {
> @@ -3909,93 +3895,19 @@ static int intel_dp_sink_crc_stop(struct
> intel_dp *intel_dp,
>  			goto out;
>  		}
>  		count = buf & DP_TEST_COUNT_MASK;
> -	} while (--attempts && count);
> -
> -	if (attempts == 0) {
> -		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not
> zeroed after calculation is stopped\n");
> -		ret = -ETIMEDOUT;
>  	}
>  
> - out:
> -	if (disable_wa)
> -		hsw_enable_ips(crtc_state);
> -	return ret;
> -}
> -
> -static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
> -				   struct intel_crtc_state
> *crtc_state)
> -{
> -	struct intel_digital_port *dig_port =
> dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port-
> >base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> >base.crtc);
> -	u8 buf;
> -	int ret;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC,
> &buf) < 0)
> -		return -EIO;
> -
> -	if (!(buf & DP_TEST_CRC_SUPPORTED))
> -		return -ENOTTY;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) <
> 0)
> -		return -EIO;
> -
> -	if (buf & DP_TEST_SINK_START) {
> -		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state,
> false);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	hsw_disable_ips(crtc_state);
> -
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf | DP_TEST_SINK_START) < 0) {
> -		hsw_enable_ips(crtc_state);
> -		return -EIO;
> -	}
> -
> -	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -	return 0;
> -}
> -
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, struct
> intel_crtc_state *crtc_state, u8 *crc)
> -{
> -	struct intel_digital_port *dig_port =
> dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port-
> >base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> >base.crtc);
> -	u8 buf;
> -	int count, ret;
> -	int attempts = 6;
> -
> -	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
> -	if (ret)
> -		return ret;
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0)
> {
> -			ret = -EIO;
> -			goto stop;
> -		}
> -		count = buf & DP_TEST_COUNT_MASK;
> -
> -	} while (--attempts && count == 0);
> -
> -	if (attempts == 0) {
> -		DRM_ERROR("Panel is unable to calculate any CRC
> after 6 vblanks\n");
> +	if (attempts == 3) {
>  		ret = -ETIMEDOUT;
> -		goto stop;
> +		goto out;
>  	}
>  
>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc,
> 6) != 6) {
>  		ret = -EIO;
> -		goto stop;
> +		goto out;
>  	}
>  
> -stop:
> -	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
> +out:
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 44ed248f1fe9..cacee94749e2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp
> *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> -int intel_dp_sink_crc(struct intel_dp *intel_dp,
> -		      struct intel_crtc_state *crtc_state, u8 *crc);
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state
> *conn_state);
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH 2/3] drm/i915/dp: Fix sink-crc reads.

[Dhinakaran Pandiyan]

On Tue, 2018-04-24 at 16:26 +0300, Mika Kahola wrote:
> On Mon, 2018-04-23 at 19:56 -0700, Dhinakaran Pandiyan wrote:
> > Sink crc is calculated by the sink for static frames irrespective of
> > what the driver sets in TEST_SINK_START dpcd. Since PSR is the only
> > use
> > case for sink crc, we don't really need the sink_crc_{start, stop}
> > code.
> > 
> > The second problem with the current implementation is vblank waits.
> > Enabling vblank interrupts triggers PSR exit, which means we aren't
> > really reading the correct CRC values for PSR tests. vblank waits are
> > replaced by delays.
> > 
> > With the changes made in this patch, sink CRC is available only for
> > static frames. I have tested this on a SKL laptop with PSR panel.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
> >  drivers/gpu/drm/i915/intel_dp.c     | 114 ++++--------------------
> > ------------
> >  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
> >  3 files changed, 15 insertions(+), 104 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 2f05f5262bba..35fa1418cc07 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m,
> > void *data)
> >  
> >  		intel_dp = enc_to_intel_dp(state->best_encoder);
> >  
> > -		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
> > +		ret = intel_dp_sink_crc(intel_dp, crc);
> >  		if (ret)
> >  			goto err;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 7dcc874b7d8f..7352ab631ea8 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3876,32 +3876,18 @@ intel_dp_configure_mst(struct intel_dp
> > *intel_dp)
> >  					intel_dp->is_mst);
> >  }
> >  
> > -static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> > -				  struct intel_crtc_state
> > *crtc_state, bool disable_wa)
> > +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
> >  {
> > -	struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > -	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> > >base.crtc);
> > -	u8 buf;
> > -	int ret = 0;
> > -	int count = 0;
> > -	int attempts = 10;
> > +	int count = 0, ret = 0, attempts;
> >  
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) <
> > 0) {
> > -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped
> > properly\n");
> > -		ret = -EIO;
> > -		goto out;
> > -	}
> > +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> A question. How did you end up with three attempts or one and a half
> frames? We used to try 6 times or 6 vblanks until we gave up trying.
> Well, I don't really know the history why we have ended up with 6
> attempts in the first place. 

CRC should be available within the next static frame. So the idea for a
tests is to do, for example, a page flip and immediately attempt to read
the CRC. Since we cannot exactly synchronize the flip getting committed,
debugfs read and CRC generation, all that the kernel can do correctly is
check if the CRC is available within a frame and read it.

I think the reason for trying 6 times was to somehow get a non-zero CRC.
But that means tests end up reading CRC of a different frame.


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

Re: [PATCH 1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes.

[Rodrigo Vivi]

On Mon, Apr 23, 2018 at 07:56:57PM -0700, Dhinakaran Pandiyan wrote:
> We attempt to read 6 bytes, make sure we have.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 62f82c4298ac..7dcc874b7d8f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3989,7 +3989,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_s
>  		goto stop;
>  	}
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
>  		ret = -EIO;
>  		goto stop;
>  	}
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/dp: Fix sink-crc reads.

[Rodrigo Vivi]

On Mon, Apr 23, 2018 at 07:56:58PM -0700, Dhinakaran Pandiyan wrote:
> Sink crc is calculated by the sink for static frames irrespective of
> what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
> case for sink crc, we don't really need the sink_crc_{start, stop} code.
> 
> The second problem with the current implementation is vblank waits.
> Enabling vblank interrupts triggers PSR exit, which means we aren't
> really reading the correct CRC values for PSR tests. vblank waits are
> replaced by delays.
> 
> With the changes made in this patch, sink CRC is available only for
> static frames. I have tested this on a SKL laptop with PSR panel.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
>  drivers/gpu/drm/i915/intel_dp.c     | 114 ++++--------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
>  3 files changed, 15 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2f05f5262bba..35fa1418cc07 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
>  
> -		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
> +		ret = intel_dp_sink_crc(intel_dp, crc);
>  		if (ret)
>  			goto err;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7dcc874b7d8f..7352ab631ea8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3876,32 +3876,18 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> -				  struct intel_crtc_state *crtc_state, bool disable_wa)
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  {
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int ret = 0;
> -	int count = 0;
> -	int attempts = 10;
> +	int count = 0, ret = 0, attempts;
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> +		u8 buf;
>  
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf & ~DP_TEST_SINK_START) < 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> +		/* Wait for approximately half a frame, we cannot wait for a

that should be dependend on the mode timings right?!
I mean, I'm not against some random value that matches most used cases,
but I believe the statement should be different.

> +		 * vblank interrupt as it triggers PSR exit.
> +		 */
> +		if (attempts)
> +			usleep_range(8000, 8500);
>  
>  		if (drm_dp_dpcd_readb(&intel_dp->aux,
>  				      DP_TEST_SINK_MISC, &buf) < 0) {
> @@ -3909,93 +3895,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
>  			goto out;
>  		}
>  		count = buf & DP_TEST_COUNT_MASK;
> -	} while (--attempts && count);
> -
> -	if (attempts == 0) {
> -		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after calculation is stopped\n");
> -		ret = -ETIMEDOUT;
>  	}
>  
> - out:
> -	if (disable_wa)
> -		hsw_enable_ips(crtc_state);
> -	return ret;
> -}
> -
> -static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
> -				   struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int ret;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> -		return -EIO;
> -
> -	if (!(buf & DP_TEST_CRC_SUPPORTED))
> -		return -ENOTTY;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
> -		return -EIO;
> -
> -	if (buf & DP_TEST_SINK_START) {
> -		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state, false);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	hsw_disable_ips(crtc_state);
> -
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf | DP_TEST_SINK_START) < 0) {
> -		hsw_enable_ips(crtc_state);
> -		return -EIO;
> -	}
> -
> -	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -	return 0;
> -}
> -
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, u8 *crc)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int count, ret;
> -	int attempts = 6;
> -
> -	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
> -	if (ret)
> -		return ret;
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0) {
> -			ret = -EIO;
> -			goto stop;
> -		}
> -		count = buf & DP_TEST_COUNT_MASK;
> -
> -	} while (--attempts && count == 0);
> -
> -	if (attempts == 0) {
> -		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
> +	if (attempts == 3) {
>  		ret = -ETIMEDOUT;
> -		goto stop;
> +		goto out;
>  	}
>  
>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
>  		ret = -EIO;
> -		goto stop;
> +		goto out;
>  	}
>  
> -stop:
> -	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
> +out:
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 44ed248f1fe9..cacee94749e2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> -int intel_dp_sink_crc(struct intel_dp *intel_dp,
> -		      struct intel_crtc_state *crtc_state, u8 *crc);
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state);
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/psr: Move sink-crc to intel_psr.c

[Rodrigo Vivi]

On Mon, Apr 23, 2018 at 07:56:59PM -0700, Dhinakaran Pandiyan wrote:
> With sink-crc now being relevant only for PSR static frames, move the
> code to intel_psr.c and rename the function.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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


> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 35 -----------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c    | 35 +++++++++++++++++++++++++++++++++++
>  4 files changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 35fa1418cc07..9913258e20ed 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
>  
> -		ret = intel_dp_sink_crc(intel_dp, crc);
> +		ret = intel_psr_sink_crc(intel_dp, crc);
>  		if (ret)
>  			goto err;
>  
> @@ -4854,7 +4854,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_ppgtt_info", i915_ppgtt_info, 0},
>  	{"i915_llc", i915_llc, 0},
>  	{"i915_edp_psr_status", i915_edp_psr_status, 0},
> -	{"i915_sink_crc_eDP1", i915_sink_crc, 0},
> +	{"i915_edp_psr_sink_crc", i915_sink_crc, 0},
>  	{"i915_energy_uJ", i915_energy_uJ, 0},
>  	{"i915_runtime_pm_status", i915_runtime_pm_status, 0},
>  	{"i915_power_domain_info", i915_power_domain_info, 0},
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7352ab631ea8..ff7522d3632e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3876,41 +3876,6 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
> -{
> -	int count = 0, ret = 0, attempts;
> -
> -	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> -		u8 buf;
> -
> -		/* Wait for approximately half a frame, we cannot wait for a
> -		 * vblank interrupt as it triggers PSR exit.
> -		 */
> -		if (attempts)
> -			usleep_range(8000, 8500);
> -
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0) {
> -			ret = -EIO;
> -			goto out;
> -		}
> -		count = buf & DP_TEST_COUNT_MASK;
> -	}
> -
> -	if (attempts == 3) {
> -		ret = -ETIMEDOUT;
> -		goto out;
> -	}
> -
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -out:
> -	return ret;
> -}
> -
>  static bool
>  intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cacee94749e2..0da7be3349cc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1644,7 +1644,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state);
> @@ -1900,6 +1899,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
>  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
> +int intel_psr_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  
>  /* intel_runtime_pm.c */
>  int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0d548292dd09..4ebd05b6e86d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -197,6 +197,41 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
>  	return val;
>  }
>  
> +int intel_psr_sink_crc(struct intel_dp *intel_dp, u8 *crc)
> +{
> +	int count = 0, ret = 0, attempts;
> +
> +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> +		u8 buf;
> +
> +		/* Wait for approximately half a frame, we cannot wait for a
> +		 * vblank interrupt as it triggers PSR exit.
> +		 */
> +		if (attempts)
> +			usleep_range(8000, 8500);
> +
> +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> +				      DP_TEST_SINK_MISC, &buf) < 0) {
> +			ret = -EIO;
> +			goto out;
> +		}
> +		count = buf & DP_TEST_COUNT_MASK;
> +	}
> +
> +	if (attempts == 3) {
> +		ret = -ETIMEDOUT;
> +		goto out;
> +	}
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> +
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv =
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/dp: Fix sink-crc reads.

[Dhinakaran Pandiyan]

On Tue, 2018-04-24 at 13:55 -0700, Rodrigo Vivi wrote:
> On Mon, Apr 23, 2018 at 07:56:58PM -0700, Dhinakaran Pandiyan wrote:
> > Sink crc is calculated by the sink for static frames irrespective of
> > what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
> > case for sink crc, we don't really need the sink_crc_{start, stop} code.
> > 
> > The second problem with the current implementation is vblank waits.
> > Enabling vblank interrupts triggers PSR exit, which means we aren't
> > really reading the correct CRC values for PSR tests. vblank waits are
> > replaced by delays.
> > 
> > With the changes made in this patch, sink CRC is available only for
> > static frames. I have tested this on a SKL laptop with PSR panel.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
> >  drivers/gpu/drm/i915/intel_dp.c     | 114 ++++--------------------------------
> >  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
> >  3 files changed, 15 insertions(+), 104 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 2f05f5262bba..35fa1418cc07 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2784,7 +2784,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
> >  
> >  		intel_dp = enc_to_intel_dp(state->best_encoder);
> >  
> > -		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
> > +		ret = intel_dp_sink_crc(intel_dp, crc);
> >  		if (ret)
> >  			goto err;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 7dcc874b7d8f..7352ab631ea8 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3876,32 +3876,18 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
> >  					intel_dp->is_mst);
> >  }
> >  
> > -static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> > -				  struct intel_crtc_state *crtc_state, bool disable_wa)
> > +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
> >  {
> > -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	u8 buf;
> > -	int ret = 0;
> > -	int count = 0;
> > -	int attempts = 10;
> > +	int count = 0, ret = 0, attempts;
> >  
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
> > -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> > -		ret = -EIO;
> > -		goto out;
> > -	}
> > +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> > +		u8 buf;
> >  
> > -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> > -			       buf & ~DP_TEST_SINK_START) < 0) {
> > -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> > -		ret = -EIO;
> > -		goto out;
> > -	}
> > -
> > -	do {
> > -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> > +		/* Wait for approximately half a frame, we cannot wait for a
> 
> that should be dependend on the mode timings right?!
> I mean, I'm not against some random value that matches most used cases,
> but I believe the statement should be different.
> 

I'll modify the comment, thanks!

Is there an existing function that provides this number in
milliseconds? 



> > +		 * vblank interrupt as it triggers PSR exit.
> > +		 */
> > +		if (attempts)
> > +			usleep_range(8000, 8500);
> >  
> >  		if (drm_dp_dpcd_readb(&intel_dp->aux,
> >  				      DP_TEST_SINK_MISC, &buf) < 0) {
> > @@ -3909,93 +3895,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> >  			goto out;
> >  		}
> >  		count = buf & DP_TEST_COUNT_MASK;
> > -	} while (--attempts && count);
> > -
> > -	if (attempts == 0) {
> > -		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after calculation is stopped\n");
> > -		ret = -ETIMEDOUT;
> >  	}
> >  
> > - out:
> > -	if (disable_wa)
> > -		hsw_enable_ips(crtc_state);
> > -	return ret;
> > -}
> > -
> > -static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
> > -				   struct intel_crtc_state *crtc_state)
> > -{
> > -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	u8 buf;
> > -	int ret;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> > -		return -EIO;
> > -
> > -	if (!(buf & DP_TEST_CRC_SUPPORTED))
> > -		return -ENOTTY;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
> > -		return -EIO;
> > -
> > -	if (buf & DP_TEST_SINK_START) {
> > -		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state, false);
> > -		if (ret)
> > -			return ret;
> > -	}
> > -
> > -	hsw_disable_ips(crtc_state);
> > -
> > -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> > -			       buf | DP_TEST_SINK_START) < 0) {
> > -		hsw_enable_ips(crtc_state);
> > -		return -EIO;
> > -	}
> > -
> > -	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> > -	return 0;
> > -}
> > -
> > -int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, u8 *crc)
> > -{
> > -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> > -	u8 buf;
> > -	int count, ret;
> > -	int attempts = 6;
> > -
> > -	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
> > -	if (ret)
> > -		return ret;
> > -
> > -	do {
> > -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> > -
> > -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> > -				      DP_TEST_SINK_MISC, &buf) < 0) {
> > -			ret = -EIO;
> > -			goto stop;
> > -		}
> > -		count = buf & DP_TEST_COUNT_MASK;
> > -
> > -	} while (--attempts && count == 0);
> > -
> > -	if (attempts == 0) {
> > -		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
> > +	if (attempts == 3) {
> >  		ret = -ETIMEDOUT;
> > -		goto stop;
> > +		goto out;
> >  	}
> >  
> >  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
> >  		ret = -EIO;
> > -		goto stop;
> > +		goto out;
> >  	}
> >  
> > -stop:
> > -	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
> > +out:
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 44ed248f1fe9..cacee94749e2 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
> >  void intel_dp_encoder_reset(struct drm_encoder *encoder);
> >  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> >  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> > -int intel_dp_sink_crc(struct intel_dp *intel_dp,
> > -		      struct intel_crtc_state *crtc_state, u8 *crc);
> > +int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
> >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> >  			     struct intel_crtc_state *pipe_config,
> >  			     struct drm_connector_state *conn_state);
> > -- 
> > 2.14.1
> > 

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

[PATCH v2 2/3] drm/i915/dp: Fix sink-crc reads.

[Dhinakaran Pandiyan]

Sink crc is calculated by the sink for static frames irrespective of
what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
case for sink crc, we don't really need the sink_crc_{start, stop} code.

The second problem with the current implementation is vblank waits.
Enabling vblank interrupts triggers PSR exit, which means we aren't
really reading the correct CRC values for PSR tests. vblank waits are
replaced by delays.

With the changes made in this patch, sink CRC is available only for
static frames. I have tested this on a SKL laptop with PSR panel.

v2: Use refresh rate to calculate frame time.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
 drivers/gpu/drm/i915/intel_dp.c     | 115 +++++-------------------------------
 drivers/gpu/drm/i915/intel_drv.h    |   3 +-
 3 files changed, 18 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2f05f5262bba..e4ba6527c16e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2749,6 +2749,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
 		struct drm_crtc *crtc;
 		struct drm_connector_state *state;
 		struct intel_crtc_state *crtc_state;
+		int vrefresh;
 
 		if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
 			continue;
@@ -2783,8 +2784,9 @@ static int i915_sink_crc(struct seq_file *m, void *data)
 		}
 
 		intel_dp = enc_to_intel_dp(state->best_encoder);
+		vrefresh = crtc_state->base.adjusted_mode.vrefresh;
 
-		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
+		ret = intel_dp_sink_crc(intel_dp, vrefresh, crc);
 		if (ret)
 			goto err;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7dcc874b7d8f..3ab3f82e33f6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3876,32 +3876,19 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 					intel_dp->is_mst);
 }
 
-static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
-				  struct intel_crtc_state *crtc_state, bool disable_wa)
+int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int ret = 0;
-	int count = 0;
-	int attempts = 10;
+	int count = 0, ret = 0, attempts;
+	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
 
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
-		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
-		ret = -EIO;
-		goto out;
-	}
+	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
+		u8 buf;
 
-	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
-			       buf & ~DP_TEST_SINK_START) < 0) {
-		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
-		ret = -EIO;
-		goto out;
-	}
-
-	do {
-		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
+		/* Wait for approximately half a frame, we cannot wait for a
+		 * vblank interrupt as it triggers PSR exit.
+		 */
+		if (attempts)
+			usleep_range(half_frame_us, half_frame_us + 100);
 
 		if (drm_dp_dpcd_readb(&intel_dp->aux,
 				      DP_TEST_SINK_MISC, &buf) < 0) {
@@ -3909,93 +3896,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
 			goto out;
 		}
 		count = buf & DP_TEST_COUNT_MASK;
-	} while (--attempts && count);
-
-	if (attempts == 0) {
-		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after calculation is stopped\n");
-		ret = -ETIMEDOUT;
 	}
 
- out:
-	if (disable_wa)
-		hsw_enable_ips(crtc_state);
-	return ret;
-}
-
-static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
-				   struct intel_crtc_state *crtc_state)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int ret;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
-		return -EIO;
-
-	if (!(buf & DP_TEST_CRC_SUPPORTED))
-		return -ENOTTY;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
-		return -EIO;
-
-	if (buf & DP_TEST_SINK_START) {
-		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state, false);
-		if (ret)
-			return ret;
-	}
-
-	hsw_disable_ips(crtc_state);
-
-	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
-			       buf | DP_TEST_SINK_START) < 0) {
-		hsw_enable_ips(crtc_state);
-		return -EIO;
-	}
-
-	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
-	return 0;
-}
-
-int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, u8 *crc)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	u8 buf;
-	int count, ret;
-	int attempts = 6;
-
-	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
-	if (ret)
-		return ret;
-
-	do {
-		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
-
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_TEST_SINK_MISC, &buf) < 0) {
-			ret = -EIO;
-			goto stop;
-		}
-		count = buf & DP_TEST_COUNT_MASK;
-
-	} while (--attempts && count == 0);
-
-	if (attempts == 0) {
-		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
+	if (attempts == 3) {
 		ret = -ETIMEDOUT;
-		goto stop;
+		goto out;
 	}
 
 	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
 		ret = -EIO;
-		goto stop;
+		goto out;
 	}
 
-stop:
-	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
+out:
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44ed248f1fe9..48073be8a023 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-int intel_dp_sink_crc(struct intel_dp *intel_dp,
-		      struct intel_crtc_state *crtc_state, u8 *crc);
+int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
-- 
2.14.1

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

[PATCH v2 3/3] drm/i915/psr: Move sink-crc to intel_psr.c

[Dhinakaran Pandiyan]

With sink-crc now being relevant only for PSR static frames, move the
code to intel_psr.c and rename the function.

v2: Rebased.
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
 drivers/gpu/drm/i915/intel_dp.c     | 36 ------------------------------------
 drivers/gpu/drm/i915/intel_drv.h    |  2 +-
 drivers/gpu/drm/i915/intel_psr.c    | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e4ba6527c16e..a84f410ad722 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2786,7 +2786,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
 		intel_dp = enc_to_intel_dp(state->best_encoder);
 		vrefresh = crtc_state->base.adjusted_mode.vrefresh;
 
-		ret = intel_dp_sink_crc(intel_dp, vrefresh, crc);
+		ret = intel_psr_sink_crc(intel_dp, vrefresh, crc);
 		if (ret)
 			goto err;
 
@@ -4856,7 +4856,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_ppgtt_info", i915_ppgtt_info, 0},
 	{"i915_llc", i915_llc, 0},
 	{"i915_edp_psr_status", i915_edp_psr_status, 0},
-	{"i915_sink_crc_eDP1", i915_sink_crc, 0},
+	{"i915_edp_psr_sink_crc", i915_sink_crc, 0},
 	{"i915_energy_uJ", i915_energy_uJ, 0},
 	{"i915_runtime_pm_status", i915_runtime_pm_status, 0},
 	{"i915_power_domain_info", i915_power_domain_info, 0},
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ab3f82e33f6..ff7522d3632e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3876,42 +3876,6 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 					intel_dp->is_mst);
 }
 
-int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
-{
-	int count = 0, ret = 0, attempts;
-	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
-
-	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
-		u8 buf;
-
-		/* Wait for approximately half a frame, we cannot wait for a
-		 * vblank interrupt as it triggers PSR exit.
-		 */
-		if (attempts)
-			usleep_range(half_frame_us, half_frame_us + 100);
-
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_TEST_SINK_MISC, &buf) < 0) {
-			ret = -EIO;
-			goto out;
-		}
-		count = buf & DP_TEST_COUNT_MASK;
-	}
-
-	if (attempts == 3) {
-		ret = -ETIMEDOUT;
-		goto out;
-	}
-
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
-		ret = -EIO;
-		goto out;
-	}
-
-out:
-	return ret;
-}
-
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 48073be8a023..d4e5a407777f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1644,7 +1644,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_destroy(struct drm_encoder *encoder);
-int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
@@ -1900,6 +1899,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
 void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
+int intel_psr_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0d548292dd09..8b3a3459eae5 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -197,6 +197,42 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
 	return val;
 }
 
+int intel_psr_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
+{
+	int count = 0, ret = 0, attempts;
+	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
+
+	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
+		u8 buf;
+
+		/* Wait for approximately half a frame, we cannot wait for a
+		 * vblank interrupt as it triggers PSR exit.
+		 */
+		if (attempts)
+			usleep_range(half_frame_us, half_frame_us + 100);
+
+		if (drm_dp_dpcd_readb(&intel_dp->aux,
+				      DP_TEST_SINK_MISC, &buf) < 0) {
+			ret = -EIO;
+			goto out;
+		}
+		count = buf & DP_TEST_COUNT_MASK;
+	}
+
+	if (attempts == 3) {
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
+		ret = -EIO;
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv =
-- 
2.14.1

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)
URL   : https://patchwork.freedesktop.org/series/42154/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
89124b64449f drm/i915/dp: Check if the sink crc we read is 6 bytes.
d8db909cc13f drm/i915/dp: Fix sink-crc reads.
-:65: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#65: FILE: drivers/gpu/drm/i915/intel_dp.c:3882:
+	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
 	                                        ^

total: 0 errors, 0 warnings, 1 checks, 165 lines checked
3288f3762198 drm/i915/psr: Move sink-crc to intel_psr.c
-:112: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#112: FILE: drivers/gpu/drm/i915/intel_psr.c:203:
+	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
 	                                        ^

total: 0 errors, 0 warnings, 1 checks, 114 lines checked

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)
URL   : https://patchwork.freedesktop.org/series/42154/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4099 -> Patchwork_8804 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8804 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8804, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42154/revisions/3/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_frontbuffer_tracking@basic:
      fi-cnl-y3:          PASS -> FAIL
      fi-cnl-psr:         PASS -> FAIL
      fi-cfl-s3:          PASS -> FAIL
      fi-kbl-7560u:       PASS -> FAIL
      fi-skl-6600u:       PASS -> FAIL
      {fi-hsw-4200u}:     PASS -> FAIL
      fi-kbl-r:           PASS -> FAIL
      fi-cfl-u:           PASS -> FAIL

    
    ==== Warnings ====

    igt@kms_sink_crc_basic:
      fi-cfl-u:           PASS -> SKIP
      {fi-hsw-4200u}:     PASS -> SKIP
      fi-cnl-psr:         PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

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

  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713


== Participating hosts (38 -> 35) ==

  Missing    (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 


== Build changes ==

    * Linux: CI_DRM_4099 -> Patchwork_8804

  CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8804: 3288f3762198964ac51b630273a5b3cbbee2db4d @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit


== Linux commits ==

3288f3762198 drm/i915/psr: Move sink-crc to intel_psr.c
d8db909cc13f drm/i915/dp: Fix sink-crc reads.
89124b64449f drm/i915/dp: Check if the sink crc we read is 6 bytes.

== Logs ==

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

Re: [PATCH v2 2/3] drm/i915/dp: Fix sink-crc reads.

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 02:57:57PM -0700, Dhinakaran Pandiyan wrote:
> Sink crc is calculated by the sink for static frames irrespective of
> what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
> case for sink crc, we don't really need the sink_crc_{start, stop} code.
> 
> The second problem with the current implementation is vblank waits.
> Enabling vblank interrupts triggers PSR exit, which means we aren't
> really reading the correct CRC values for PSR tests. vblank waits are
> replaced by delays.
> 
> With the changes made in this patch, sink CRC is available only for
> static frames. I have tested this on a SKL laptop with PSR panel.
> 
> v2: Use refresh rate to calculate frame time.

Cool! :)

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
>  drivers/gpu/drm/i915/intel_dp.c     | 115 +++++-------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
>  3 files changed, 18 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2f05f5262bba..e4ba6527c16e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2749,6 +2749,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  		struct drm_crtc *crtc;
>  		struct drm_connector_state *state;
>  		struct intel_crtc_state *crtc_state;
> +		int vrefresh;
>  
>  		if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
>  			continue;
> @@ -2783,8 +2784,9 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  		}
>  
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
> +		vrefresh = crtc_state->base.adjusted_mode.vrefresh;
>  
> -		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
> +		ret = intel_dp_sink_crc(intel_dp, vrefresh, crc);
>  		if (ret)
>  			goto err;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7dcc874b7d8f..3ab3f82e33f6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3876,32 +3876,19 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
> -				  struct intel_crtc_state *crtc_state, bool disable_wa)
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
>  {
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int ret = 0;
> -	int count = 0;
> -	int attempts = 10;
> +	int count = 0, ret = 0, attempts;
> +	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> +		u8 buf;
>  
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf & ~DP_TEST_SINK_START) < 0) {
> -		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> +		/* Wait for approximately half a frame, we cannot wait for a
> +		 * vblank interrupt as it triggers PSR exit.
> +		 */
> +		if (attempts)
> +			usleep_range(half_frame_us, half_frame_us + 100);
>  
>  		if (drm_dp_dpcd_readb(&intel_dp->aux,
>  				      DP_TEST_SINK_MISC, &buf) < 0) {
> @@ -3909,93 +3896,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp,
>  			goto out;
>  		}
>  		count = buf & DP_TEST_COUNT_MASK;
> -	} while (--attempts && count);
> -
> -	if (attempts == 0) {
> -		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after calculation is stopped\n");
> -		ret = -ETIMEDOUT;
>  	}
>  
> - out:
> -	if (disable_wa)
> -		hsw_enable_ips(crtc_state);
> -	return ret;
> -}
> -
> -static int intel_dp_sink_crc_start(struct intel_dp *intel_dp,
> -				   struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int ret;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> -		return -EIO;
> -
> -	if (!(buf & DP_TEST_CRC_SUPPORTED))
> -		return -ENOTTY;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
> -		return -EIO;
> -
> -	if (buf & DP_TEST_SINK_START) {
> -		ret = intel_dp_sink_crc_stop(intel_dp, crtc_state, false);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	hsw_disable_ips(crtc_state);
> -
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> -			       buf | DP_TEST_SINK_START) < 0) {
> -		hsw_enable_ips(crtc_state);
> -		return -EIO;
> -	}
> -
> -	intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -	return 0;
> -}
> -
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, u8 *crc)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	u8 buf;
> -	int count, ret;
> -	int attempts = 6;
> -
> -	ret = intel_dp_sink_crc_start(intel_dp, crtc_state);
> -	if (ret)
> -		return ret;
> -
> -	do {
> -		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> -
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0) {
> -			ret = -EIO;
> -			goto stop;
> -		}
> -		count = buf & DP_TEST_COUNT_MASK;
> -
> -	} while (--attempts && count == 0);
> -
> -	if (attempts == 0) {
> -		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
> +	if (attempts == 3) {
>  		ret = -ETIMEDOUT;
> -		goto stop;
> +		goto out;
>  	}
>  
>  	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
>  		ret = -EIO;
> -		goto stop;
> +		goto out;
>  	}
>  
> -stop:
> -	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
> +out:
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 44ed248f1fe9..48073be8a023 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1644,8 +1644,7 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> -int intel_dp_sink_crc(struct intel_dp *intel_dp,
> -		      struct intel_crtc_state *crtc_state, u8 *crc);
> +int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state);
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/psr: Move sink-crc to intel_psr.c

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 02:58:34PM -0700, Dhinakaran Pandiyan wrote:
> With sink-crc now being relevant only for PSR static frames, move the
> code to intel_psr.c and rename the function.
> 
> v2: Rebased.
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 36 ------------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c    | 36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e4ba6527c16e..a84f410ad722 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2786,7 +2786,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
>  		vrefresh = crtc_state->base.adjusted_mode.vrefresh;
>  
> -		ret = intel_dp_sink_crc(intel_dp, vrefresh, crc);
> +		ret = intel_psr_sink_crc(intel_dp, vrefresh, crc);
>  		if (ret)
>  			goto err;
>  
> @@ -4856,7 +4856,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_ppgtt_info", i915_ppgtt_info, 0},
>  	{"i915_llc", i915_llc, 0},
>  	{"i915_edp_psr_status", i915_edp_psr_status, 0},
> -	{"i915_sink_crc_eDP1", i915_sink_crc, 0},
> +	{"i915_edp_psr_sink_crc", i915_sink_crc, 0},
>  	{"i915_energy_uJ", i915_energy_uJ, 0},
>  	{"i915_runtime_pm_status", i915_runtime_pm_status, 0},
>  	{"i915_power_domain_info", i915_power_domain_info, 0},
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3ab3f82e33f6..ff7522d3632e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3876,42 +3876,6 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
> -{
> -	int count = 0, ret = 0, attempts;
> -	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
> -
> -	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> -		u8 buf;
> -
> -		/* Wait for approximately half a frame, we cannot wait for a
> -		 * vblank interrupt as it triggers PSR exit.
> -		 */
> -		if (attempts)
> -			usleep_range(half_frame_us, half_frame_us + 100);
> -
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0) {
> -			ret = -EIO;
> -			goto out;
> -		}
> -		count = buf & DP_TEST_COUNT_MASK;
> -	}
> -
> -	if (attempts == 3) {
> -		ret = -ETIMEDOUT;
> -		goto out;
> -	}
> -
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -out:
> -	return ret;
> -}
> -
>  static bool
>  intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 48073be8a023..d4e5a407777f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1644,7 +1644,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> -int intel_dp_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state);
> @@ -1900,6 +1899,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
>  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
> +int intel_psr_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc);
>  
>  /* intel_runtime_pm.c */
>  int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0d548292dd09..8b3a3459eae5 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -197,6 +197,42 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
>  	return val;
>  }
>  
> +int intel_psr_sink_crc(struct intel_dp *intel_dp, int vrefresh, u8 *crc)
> +{
> +	int count = 0, ret = 0, attempts;
> +	int half_frame_us = DIV_ROUND_UP(1000000/2, vrefresh);
> +
> +	for (attempts = 0; attempts < 3 && count == 0; attempts++) {
> +		u8 buf;
> +
> +		/* Wait for approximately half a frame, we cannot wait for a
> +		 * vblank interrupt as it triggers PSR exit.
> +		 */
> +		if (attempts)
> +			usleep_range(half_frame_us, half_frame_us + 100);
> +
> +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> +				      DP_TEST_SINK_MISC, &buf) < 0) {
> +			ret = -EIO;
> +			goto out;
> +		}
> +		count = buf & DP_TEST_COUNT_MASK;
> +	}
> +
> +	if (attempts == 3) {
> +		ret = -ETIMEDOUT;
> +		goto out;
> +	}
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) != 6) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> +
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv =
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 10:48:10PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915/dp: Check if the sink crc we read is 6 bytes. (rev3)
> URL   : https://patchwork.freedesktop.org/series/42154/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4099 -> Patchwork_8804 =
> 
> == Summary - FAILURE ==
> 
>   Serious unknown changes coming with Patchwork_8804 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_8804, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/42154/revisions/3/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in Patchwork_8804:
> 
>   === IGT changes ===
> 
>     ==== Possible regressions ====
> 
>     igt@kms_frontbuffer_tracking@basic:
>       fi-cnl-y3:          PASS -> FAIL
>       fi-cnl-psr:         PASS -> FAIL
>       fi-cfl-s3:          PASS -> FAIL
>       fi-kbl-7560u:       PASS -> FAIL
>       fi-skl-6600u:       PASS -> FAIL
>       {fi-hsw-4200u}:     PASS -> FAIL
>       fi-kbl-r:           PASS -> FAIL
>       fi-cfl-u:           PASS -> FAIL

it seems that we need to kill sink_crc test case first...

> 
>     
>     ==== Warnings ====
> 
>     igt@kms_sink_crc_basic:
>       fi-cfl-u:           PASS -> SKIP
>       {fi-hsw-4200u}:     PASS -> SKIP
>       fi-cnl-psr:         PASS -> SKIP
> 
>     
> == Known issues ==
> 
>   Here are the changes found in Patchwork_8804 that come from known issues:
> 
>   === IGT changes ===
> 
>     ==== Possible fixes ====
> 
>     igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
>       fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS
> 
>     
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
> 
> 
> == Participating hosts (38 -> 35) ==
> 
>   Missing    (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 
> 
> 
> == Build changes ==
> 
>     * Linux: CI_DRM_4099 -> Patchwork_8804
> 
>   CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_8804: 3288f3762198964ac51b630273a5b3cbbee2db4d @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit
> 
> 
> == Linux commits ==
> 
> 3288f3762198 drm/i915/psr: Move sink-crc to intel_psr.c
> d8db909cc13f drm/i915/dp: Fix sink-crc reads.
> 89124b64449f drm/i915/dp: Check if the sink crc we read is 6 bytes.
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8804/issues.html
> _______________________________________________
> 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 v2 2/3] drm/i915/dp: Fix sink-crc reads.

[Ville Syrjälä]

On Wed, Apr 25, 2018 at 02:57:57PM -0700, Dhinakaran Pandiyan wrote:
> Sink crc is calculated by the sink for static frames irrespective of
> what the driver sets in TEST_SINK_START dpcd. Since PSR is the only use
> case for sink crc, we don't really need the sink_crc_{start, stop} code.
> 
> The second problem with the current implementation is vblank waits.
> Enabling vblank interrupts triggers PSR exit, which means we aren't
> really reading the correct CRC values for PSR tests. vblank waits are
> replaced by delays.
> 
> With the changes made in this patch, sink CRC is available only for
> static frames. I have tested this on a SKL laptop with PSR panel.
> 
> v2: Use refresh rate to calculate frame time.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
>  drivers/gpu/drm/i915/intel_dp.c     | 115 +++++-------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +-
>  3 files changed, 18 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2f05f5262bba..e4ba6527c16e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2749,6 +2749,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  		struct drm_crtc *crtc;
>  		struct drm_connector_state *state;
>  		struct intel_crtc_state *crtc_state;
> +		int vrefresh;
>  
>  		if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
>  			continue;
> @@ -2783,8 +2784,9 @@ static int i915_sink_crc(struct seq_file *m, void *data)
>  		}
>  
>  		intel_dp = enc_to_intel_dp(state->best_encoder);
> +		vrefresh = crtc_state->base.adjusted_mode.vrefresh;

Hmm. Is vrefresh always populated correctly for us?

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