drm/i915: Kill sink_crc for good

rodrigovivi · rodrigovivi · commit 5fd9df6ac6ee · 2018-07-18T12:53:08.000-07:00
It was originally introduced following the VESA spec in order to validate PSR. However we found so many issues around sink_crc that instead of helping PSR development it only brought another layer of trouble to the table. So, sink_crc has been a black whole for us in question of time, effort and hope. First of the problems is that HW statement is clear: "Do not attempt to use aux communication with PSR enabled". So the main reason behind sink_crc is already compromised. For a while we had hope on the aux-mutex could workaround this problem on SKL+ platforms, but that mutex was not reliable, not tested, and we shouldn't use according to HW engineers. Also, nor source, nor sink designed and implemented the sink_crc to be used like we are trying to use here. Well, the sink side of things is also apparently not prepared for this case. Each panel that we tried seemed to have a different behavior with same code and same source. So, for all the time we lost on trying to ducktape all these different issues I believe it is now time to move PSR to a more reliable validation. Maybe not a perfect one as we dreamed for this sink_crc, but at least more reliable. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180705192528.30515-1-rodrigo.vivi@intel.com
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2767,86 +2767,6 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
 			i915_edp_psr_debug_get, i915_edp_psr_debug_set,
 			"%llu\n");
 
-static int i915_sink_crc(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_connector *connector;
-	struct drm_connector_list_iter conn_iter;
-	struct intel_dp *intel_dp = NULL;
-	struct drm_modeset_acquire_ctx ctx;
-	int ret;
-	u8 crc[6];
-
-	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
-
-	drm_connector_list_iter_begin(dev, &conn_iter);
-
-	for_each_intel_connector_iter(connector, &conn_iter) {
-		struct drm_crtc *crtc;
-		struct drm_connector_state *state;
-		struct intel_crtc_state *crtc_state;
-
-		if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
-			continue;
-
-retry:
-		ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
-		if (ret)
-			goto err;
-
-		state = connector->base.state;
-		if (!state->best_encoder)
-			continue;
-
-		crtc = state->crtc;
-		ret = drm_modeset_lock(&crtc->mutex, &ctx);
-		if (ret)
-			goto err;
-
-		crtc_state = to_intel_crtc_state(crtc->state);
-		if (!crtc_state->base.active)
-			continue;
-
-		/*
-		 * We need to wait for all crtc updates to complete, to make
-		 * sure any pending modesets and plane updates are completed.
-		 */
-		if (crtc_state->base.commit) {
-			ret = wait_for_completion_interruptible(&crtc_state->base.commit->hw_done);
-
-			if (ret)
-				goto err;
-		}
-
-		intel_dp = enc_to_intel_dp(state->best_encoder);
-
-		ret = intel_dp_sink_crc(intel_dp, crtc_state, crc);
-		if (ret)
-			goto err;
-
-		seq_printf(m, "%02x%02x%02x%02x%02x%02x\n",
-			   crc[0], crc[1], crc[2],
-			   crc[3], crc[4], crc[5]);
-		goto out;
-
-err:
-		if (ret == -EDEADLK) {
-			ret = drm_modeset_backoff(&ctx);
-			if (!ret)
-				goto retry;
-		}
-		goto out;
-	}
-	ret = -ENODEV;
-out:
-	drm_connector_list_iter_end(&conn_iter);
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
-	return ret;
-}
-
 static int i915_energy_uJ(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4792,7 +4712,6 @@ 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_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
@@ -3874,129 +3874,6 @@ 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)
-{
-	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;
-
-	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;
-	}
-
-	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);
-
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_TEST_SINK_MISC, &buf) < 0) {
-			ret = -EIO;
-			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");
-		ret = -ETIMEDOUT;
-		goto stop;
-	}
-
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
-		ret = -EIO;
-		goto stop;
-	}
-
-stop:
-	intel_dp_sink_crc_stop(intel_dp, crtc_state, true);
-	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
@@ -1682,8 +1682,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,
-		      struct intel_crtc_state *crtc_state, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
