[PATCH v3 1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[PATCH v3 1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[José Roberto de Souza]

From: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

PSR hardware and hence the driver code for VLV and CHV deviates a lot from
their DDI counterparts. While the feature has been disabled for a long time
now, retaining support for these platforms is a maintenance burden. There
have been multiple refactoring commits to just keep the existing code for
these platforms in line with the rest. There are known issues that need to
be fixed to enable PSR on these platforms, and there is no PSR capable
platform in CI to ensure the code does not break again if we get around to
fixing the existing issues. On account of all these reasons, let's nuke
this code for now and bring it back if a need arises in the future.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> 
---

Please review this one here: https://patchwork.freedesktop.org/patch/222230/

 drivers/gpu/drm/i915/i915_debugfs.c      |  42 +---
 drivers/gpu/drm/i915/i915_drv.h          |   1 -
 drivers/gpu/drm/i915/i915_pci.c          |   2 -
 drivers/gpu/drm/i915/intel_drv.h         |   2 -
 drivers/gpu/drm/i915/intel_frontbuffer.c |   2 -
 drivers/gpu/drm/i915/intel_psr.c         | 248 ++---------------------
 6 files changed, 27 insertions(+), 270 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 13e7b9e4a6e6..0096e209fe04 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2630,8 +2630,6 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	u32 psrperf = 0;
-	u32 stat[3];
-	enum pipe pipe;
 	bool enabled = false;
 	bool sink_support;
 
@@ -2652,47 +2650,17 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 	seq_printf(m, "Re-enable work scheduled: %s\n",
 		   yesno(work_busy(&dev_priv->psr.work.work)));
 
-	if (HAS_DDI(dev_priv)) {
-		if (dev_priv->psr.psr2_enabled)
-			enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
-		else
-			enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
-	} else {
-		for_each_pipe(dev_priv, pipe) {
-			enum transcoder cpu_transcoder =
-				intel_pipe_to_cpu_transcoder(dev_priv, pipe);
-			enum intel_display_power_domain power_domain;
-
-			power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
-			if (!intel_display_power_get_if_enabled(dev_priv,
-								power_domain))
-				continue;
-
-			stat[pipe] = I915_READ(VLV_PSRSTAT(pipe)) &
-				VLV_EDP_PSR_CURR_STATE_MASK;
-			if ((stat[pipe] == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
-			    (stat[pipe] == VLV_EDP_PSR_ACTIVE_SF_UPDATE))
-				enabled = true;
-
-			intel_display_power_put(dev_priv, power_domain);
-		}
-	}
+	if (dev_priv->psr.psr2_enabled)
+		enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
+	else
+		enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
 
 	seq_printf(m, "Main link in standby mode: %s\n",
 		   yesno(dev_priv->psr.link_standby));
 
-	seq_printf(m, "HW Enabled & Active bit: %s", yesno(enabled));
-
-	if (!HAS_DDI(dev_priv))
-		for_each_pipe(dev_priv, pipe) {
-			if ((stat[pipe] == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
-			    (stat[pipe] == VLV_EDP_PSR_ACTIVE_SF_UPDATE))
-				seq_printf(m, " pipe %c", pipe_name(pipe));
-		}
-	seq_puts(m, "\n");
+	seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
 
 	/*
-	 * VLV/CHV PSR has no kind of performance counter
 	 * SKL+ Perf counter is reset to 0 everytime DC state is entered
 	 */
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34c125e2d90c..c58c5dae4424 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -607,7 +607,6 @@ struct i915_psr {
 	bool link_standby;
 	bool colorimetry_support;
 	bool alpm;
-	bool has_hw_tracking;
 	bool psr2_enabled;
 	u8 sink_sync_latency;
 	bool debug;
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 4364922e935d..97a91e6af7e3 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -340,7 +340,6 @@ static const struct intel_device_info intel_valleyview_info = {
 	GEN(7),
 	.is_lp = 1,
 	.num_pipes = 2,
-	.has_psr = 1,
 	.has_runtime_pm = 1,
 	.has_rc6 = 1,
 	.has_gmch_display = 1,
@@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info = {
 	.is_lp = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
 	.has_64bit_reloc = 1,
-	.has_psr = 1,
 	.has_runtime_pm = 1,
 	.has_resource_streamer = 1,
 	.has_rc6 = 1,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 12002fc77235..4508be628450 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1917,8 +1917,6 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 		     unsigned frontbuffer_bits,
 		     enum fb_op_origin origin);
 void intel_psr_init(struct drm_i915_private *dev_priv);
-void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
-				   unsigned frontbuffer_bits);
 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);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 7fff0a0eceb4..c3379bde266f 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -153,8 +153,6 @@ void intel_frontbuffer_flip_prepare(struct drm_i915_private *dev_priv,
 	/* Remove stale busy bits due to the old buffer. */
 	dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits;
 	spin_unlock(&dev_priv->fb_tracking.lock);
-
-	intel_psr_single_frame_update(dev_priv, frontbuffer_bits);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index db27f2faa1de..29443d2c35bb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -97,10 +97,6 @@ void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
 {
 	u32 debug_mask, mask;
 
-	/* No PSR interrupts on VLV/CHV */
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		return;
-
 	mask = EDP_PSR_ERROR(TRANSCODER_EDP);
 	debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
 		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
@@ -284,31 +280,6 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 	}
 }
 
-static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	uint32_t val;
-
-	val = I915_READ(VLV_PSRSTAT(pipe)) &
-	      VLV_EDP_PSR_CURR_STATE_MASK;
-	return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
-	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
-}
-
-static void vlv_psr_setup_vsc(struct intel_dp *intel_dp,
-			      const struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	uint32_t val;
-
-	/* VLV auto-generate VSC package as per EDP 1.3 spec, Table 3.10 */
-	val  = I915_READ(VLV_VSCSDP(crtc->pipe));
-	val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
-	val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
-	I915_WRITE(VLV_VSCSDP(crtc->pipe), val);
-}
-
 static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
 			      const struct intel_crtc_state *crtc_state)
 {
@@ -341,12 +312,6 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
 					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
 }
 
-static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
-{
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-			   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
-}
-
 static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -403,38 +368,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
 }
 
-static void vlv_psr_enable_source(struct intel_dp *intel_dp,
-				  const 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 *crtc = to_intel_crtc(crtc_state->base.crtc);
-
-	/* Transition from PSR_state 0 (disabled) to PSR_state 1 (inactive) */
-	I915_WRITE(VLV_PSRCTL(crtc->pipe),
-		   VLV_EDP_PSR_MODE_SW_TIMER |
-		   VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
-		   VLV_EDP_PSR_ENABLE);
-}
-
-static void vlv_psr_activate(struct intel_dp *intel_dp)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
-
-	/*
-	 * Let's do the transition from PSR_state 1 (inactive) to
-	 * PSR_state 2 (transition to active - static frame transmission).
-	 * Then Hardware is responsible for the transition to
-	 * PSR_state 3 (active - no Remote Frame Buffer (RFB) update).
-	 */
-	I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
-		   VLV_EDP_PSR_ACTIVE_ENTRY);
-}
-
 static void hsw_activate_psr1(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -602,17 +535,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 	 * ones. Since by Display design transcoder EDP is tied to port A
 	 * we can safely escape based on the port A.
 	 */
-	if (HAS_DDI(dev_priv) && dig_port->base.port != PORT_A) {
+	if (dig_port->base.port != PORT_A) {
 		DRM_DEBUG_KMS("PSR condition failed: Port not supported\n");
 		return;
 	}
 
-	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	    !dev_priv->psr.link_standby) {
-		DRM_ERROR("PSR condition failed: Link off requested but not supported on this platform\n");
-		return;
-	}
-
 	if (IS_HASWELL(dev_priv) &&
 	    I915_READ(HSW_STEREO_3D_CTL(crtc_state->cpu_transcoder)) &
 		      S3D_ENABLE) {
@@ -760,7 +687,6 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 		 * enabled.
 		 * However on some platforms we face issues when first
 		 * activation follows a modeset so quickly.
-		 *     - On VLV/CHV we get bank screen on first activation
 		 *     - On HSW/BDW we get a recoverable frozen screen until
 		 *       next exit-activate sequence.
 		 */
@@ -772,36 +698,6 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-static void vlv_psr_disable(struct intel_dp *intel_dp,
-			    const struct intel_crtc_state *old_crtc_state)
-{
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
-	uint32_t val;
-
-	if (dev_priv->psr.active) {
-		/* Put VLV PSR back to PSR_state 0 (disabled). */
-		if (intel_wait_for_register(dev_priv,
-					    VLV_PSRSTAT(crtc->pipe),
-					    VLV_EDP_PSR_IN_TRANS,
-					    0,
-					    1))
-			WARN(1, "PSR transition took longer than expected\n");
-
-		val = I915_READ(VLV_PSRCTL(crtc->pipe));
-		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
-		val &= ~VLV_EDP_PSR_ENABLE;
-		val &= ~VLV_EDP_PSR_MODE_MASK;
-		I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
-
-		dev_priv->psr.active = false;
-	} else {
-		WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
-	}
-}
-
 static void hsw_psr_disable(struct intel_dp *intel_dp,
 			    const struct intel_crtc_state *old_crtc_state)
 {
@@ -894,21 +790,12 @@ static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
 	if (!intel_dp)
 		return false;
 
-	if (HAS_DDI(dev_priv)) {
-		if (dev_priv->psr.psr2_enabled) {
-			reg = EDP_PSR2_STATUS;
-			mask = EDP_PSR2_STATUS_STATE_MASK;
-		} else {
-			reg = EDP_PSR_STATUS;
-			mask = EDP_PSR_STATUS_STATE_MASK;
-		}
+	if (dev_priv->psr.psr2_enabled) {
+		reg = EDP_PSR2_STATUS;
+		mask = EDP_PSR2_STATUS_STATE_MASK;
 	} else {
-		struct drm_crtc *crtc =
-			dp_to_dig_port(intel_dp)->base.base.crtc;
-		enum pipe pipe = to_intel_crtc(crtc)->pipe;
-
-		reg = VLV_PSRSTAT(pipe);
-		mask = VLV_EDP_PSR_IN_TRANS;
+		reg = EDP_PSR_STATUS;
+		mask = EDP_PSR_STATUS_STATE_MASK;
 	}
 
 	mutex_unlock(&dev_priv->psr.lock);
@@ -953,102 +840,23 @@ static void intel_psr_work(struct work_struct *work)
 
 static void intel_psr_exit(struct drm_i915_private *dev_priv)
 {
-	struct intel_dp *intel_dp = dev_priv->psr.enabled;
-	struct drm_crtc *crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	u32 val;
 
 	if (!dev_priv->psr.active)
 		return;
 
-	if (HAS_DDI(dev_priv)) {
-		if (dev_priv->psr.psr2_enabled) {
-			val = I915_READ(EDP_PSR2_CTL);
-			WARN_ON(!(val & EDP_PSR2_ENABLE));
-			I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
-		} else {
-			val = I915_READ(EDP_PSR_CTL);
-			WARN_ON(!(val & EDP_PSR_ENABLE));
-			I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
-		}
+	if (dev_priv->psr.psr2_enabled) {
+		val = I915_READ(EDP_PSR2_CTL);
+		WARN_ON(!(val & EDP_PSR2_ENABLE));
+		I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
 	} else {
-		val = I915_READ(VLV_PSRCTL(pipe));
-
-		/*
-		 * Here we do the transition drirectly from
-		 * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
-		 * PSR_state 5 (exit).
-		 * PSR State 4 (active with single frame update) can be skipped.
-		 * On PSR_state 5 (exit) Hardware is responsible to transition
-		 * back to PSR_state 1 (inactive).
-		 * Now we are at Same state after vlv_psr_enable_source.
-		 */
-		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
-		I915_WRITE(VLV_PSRCTL(pipe), val);
-
-		/*
-		 * Send AUX wake up - Spec says after transitioning to PSR
-		 * active we have to send AUX wake up by writing 01h in DPCD
-		 * 600h of sink device.
-		 * XXX: This might slow down the transition, but without this
-		 * HW doesn't complete the transition to PSR_state 1 and we
-		 * never get the screen updated.
-		 */
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
-				   DP_SET_POWER_D0);
+		val = I915_READ(EDP_PSR_CTL);
+		WARN_ON(!(val & EDP_PSR_ENABLE));
+		I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
 	}
-
 	dev_priv->psr.active = false;
 }
 
-/**
- * intel_psr_single_frame_update - Single Frame Update
- * @dev_priv: i915 device
- * @frontbuffer_bits: frontbuffer plane tracking bits
- *
- * Some platforms support a single frame update feature that is used to
- * send and update only one frame on Remote Frame Buffer.
- * So far it is only implemented for Valleyview and Cherryview because
- * hardware requires this to be done before a page flip.
- */
-void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
-				   unsigned frontbuffer_bits)
-{
-	struct drm_crtc *crtc;
-	enum pipe pipe;
-	u32 val;
-
-	if (!CAN_PSR(dev_priv))
-		return;
-
-	/*
-	 * Single frame update is already supported on BDW+ but it requires
-	 * many W/A and it isn't really needed.
-	 */
-	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
-		return;
-
-	mutex_lock(&dev_priv->psr.lock);
-	if (!dev_priv->psr.enabled) {
-		mutex_unlock(&dev_priv->psr.lock);
-		return;
-	}
-
-	crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
-	pipe = to_intel_crtc(crtc)->pipe;
-
-	if (frontbuffer_bits & INTEL_FRONTBUFFER_ALL_MASK(pipe)) {
-		val = I915_READ(VLV_PSRCTL(pipe));
-
-		/*
-		 * We need to set this bit before writing registers for a flip.
-		 * This bit will be self-clear when it gets to the PSR active state.
-		 */
-		I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
-	}
-	mutex_unlock(&dev_priv->psr.lock);
-}
-
 /**
  * intel_psr_invalidate - Invalidade PSR
  * @dev_priv: i915 device
@@ -1071,7 +879,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (origin == ORIGIN_FLIP)
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
@@ -1114,7 +922,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (origin == ORIGIN_FLIP)
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
@@ -1131,8 +939,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 
 	/* By definition flush = invalidate + flush */
 	if (frontbuffer_bits) {
-		if (dev_priv->psr.psr2_enabled ||
-		    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		if (dev_priv->psr.psr2_enabled) {
 			intel_psr_exit(dev_priv);
 		} else {
 			/*
@@ -1184,9 +991,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		/* HSW and BDW require workarounds that we don't implement. */
 		dev_priv->psr.link_standby = false;
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		/* On VLV and CHV only standby mode is supported. */
-		dev_priv->psr.link_standby = true;
 	else
 		/* For new platforms let's respect VBT back again */
 		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
@@ -1204,18 +1008,10 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
 	mutex_init(&dev_priv->psr.lock);
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		dev_priv->psr.enable_source = vlv_psr_enable_source;
-		dev_priv->psr.disable_source = vlv_psr_disable;
-		dev_priv->psr.enable_sink = vlv_psr_enable_sink;
-		dev_priv->psr.activate = vlv_psr_activate;
-		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
-	} else {
-		dev_priv->psr.has_hw_tracking = true;
-		dev_priv->psr.enable_source = hsw_psr_enable_source;
-		dev_priv->psr.disable_source = hsw_psr_disable;
-		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
-		dev_priv->psr.activate = hsw_psr_activate;
-		dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
-	}
+	dev_priv->psr.enable_source = hsw_psr_enable_source;
+	dev_priv->psr.disable_source = hsw_psr_disable;
+	dev_priv->psr.enable_sink = hsw_psr_enable_sink;
+	dev_priv->psr.activate = hsw_psr_activate;
+	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
+
 }
-- 
2.17.0

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

[PATCH v3 2/7] drm/i915/dp: Use intel_dp_aux_wait_done() to wait for previous aux xfer

[José Roberto de Souza]

This reduces the spaghetti that intel_dp_aux_xfer() and reuses code.
The only difference is that now it will wait up to 10ms instead of
3ms.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2cc58596ff5a..b86da48fd38e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1102,23 +1102,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
 	intel_dp_check_edp(intel_dp);
 
 	/* Try to wait for any previous AUX channel activity */
-	for (try = 0; try < 3; try++) {
-		status = I915_READ_NOTRACE(ch_ctl);
-		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
-			break;
-		msleep(1);
-	}
-
-	if (try == 3) {
-		static u32 last_status = -1;
-		const u32 status = I915_READ(ch_ctl);
-
-		if (status != last_status) {
-			WARN(1, "dp_aux_ch not started status 0x%08x\n",
-			     status);
-			last_status = status;
-		}
-
+	status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+	if (status & DP_AUX_CH_CTL_SEND_BUSY) {
+		DRM_WARN("dp_aux_ch not started status 0x%08x\n", status);
 		ret = -EBUSY;
 		goto out;
 	}
-- 
2.17.0

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

Re: [PATCH v3 2/7] drm/i915/dp: Use intel_dp_aux_wait_done() to wait for previous aux xfer

[Dhinakaran Pandiyan]

On Thursday, May 17, 2018 3:21:13 PM PDT José Roberto de Souza wrote:
> This reduces the spaghetti that intel_dp_aux_xfer() and reuses code.
> The only difference is that now it will wait up to 10ms instead of
> 3ms.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 20 +++-----------------
>  1 file changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index 2cc58596ff5a..b86da48fd38e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1102,23 +1102,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	intel_dp_check_edp(intel_dp);
> 
>  	/* Try to wait for any previous AUX channel activity */
> -	for (try = 0; try < 3; try++) {
> -		status = I915_READ_NOTRACE(ch_ctl);
> -		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> -			break;
> -		msleep(1);
> -	}
> -
> -	if (try == 3) {
> -		static u32 last_status = -1;
> -		const u32 status = I915_READ(ch_ctl);
> -
> -		if (status != last_status) {
> -			WARN(1, "dp_aux_ch not started status 0x%08x\n",
> -			     status);
> -			last_status = status;
> -		}
> -
> +	status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
Isn't intel_dp_aux_wait_done(intel_dp, false) better? Assuming busy bit is set 
for HW initiated AUX transactions, the current behavior of polling for idle is 
better. 

> +	if (status & DP_AUX_CH_CTL_SEND_BUSY) {
> +		DRM_WARN("dp_aux_ch not started status 0x%08x\n", status);

Retain the current code to avoid log spam and fix it in a second patch with a 
per AUX channel flag?

>  		ret = -EBUSY;
>  		goto out;
>  	}


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

[PATCH v3 3/7] drm/i915/psr: Remove intel_crtc_state parameter from disable()

[José Roberto de Souza]

It was only used in VLV/CHV so after the removal of the PSR support
for those platforms it is not necessary any more.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c58c5dae4424..34e3449ea182 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -615,8 +615,7 @@ struct i915_psr {
 
 	void (*enable_source)(struct intel_dp *,
 			      const struct intel_crtc_state *);
-	void (*disable_source)(struct intel_dp *,
-			       const struct intel_crtc_state *);
+	void (*disable_source)(struct intel_dp *intel_dp);
 	void (*enable_sink)(struct intel_dp *);
 	void (*activate)(struct intel_dp *);
 	void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 29443d2c35bb..d88799482875 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -698,8 +698,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-static void hsw_psr_disable(struct intel_dp *intel_dp,
-			    const struct intel_crtc_state *old_crtc_state)
+static void hsw_psr_disable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -768,7 +767,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
 		return;
 	}
 
-	dev_priv->psr.disable_source(intel_dp, old_crtc_state);
+	dev_priv->psr.disable_source(intel_dp);
 
 	/* Disable PSR on Sink */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
-- 
2.17.0

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

Re: [PATCH v3 3/7] drm/i915/psr: Remove intel_crtc_state parameter from disable()

[Dhinakaran Pandiyan]

On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> It was only used in VLV/CHV so after the removal of the PSR support
> for those platforms it is not necessary any more.
Right, Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 3 +--
>  drivers/gpu/drm/i915/intel_psr.c | 5 ++---
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index c58c5dae4424..34e3449ea182 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -615,8 +615,7 @@ struct i915_psr {
>  
>  	void (*enable_source)(struct intel_dp *,
>  			      const struct intel_crtc_state *);
> -	void (*disable_source)(struct intel_dp *,
> -			       const struct intel_crtc_state *);
> +	void (*disable_source)(struct intel_dp *intel_dp);
>  	void (*enable_sink)(struct intel_dp *);
>  	void (*activate)(struct intel_dp *);
>  	void (*setup_vsc)(struct intel_dp *, const struct
> intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 29443d2c35bb..d88799482875 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -698,8 +698,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>  	mutex_unlock(&dev_priv->psr.lock);
>  }
>  
> -static void hsw_psr_disable(struct intel_dp *intel_dp,
> -			    const struct intel_crtc_state
> *old_crtc_state)
> +static void hsw_psr_disable(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -768,7 +767,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> -	dev_priv->psr.disable_source(intel_dp, old_crtc_state);
> +	dev_priv->psr.disable_source(intel_dp);
>  
>  	/* Disable PSR on Sink */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[José Roberto de Souza]

eDP spec states that sink device will do a short pulse in HPD
line when there is a PSR/PSR2 error that needs to be handled by
source, this is handling the first and most simples error:
DP_PSR_SINK_INTERNAL_ERROR.

Here taking the safest approach and disabling PSR(at least until
the next modeset), to avoid multiple rendering issues due to
bad pannels.

v3:
disabling PSR instead of exiting on error

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b86da48fd38e..fa2851d4fb36 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 	if (intel_dp_needs_link_retrain(intel_dp))
 		return false;
 
+	intel_psr_short_pulse(intel_dp);
+
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
 		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
 		/* Send a Hotplug Uevent to userspace to start modeset */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4508be628450..892da65358e9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1921,6 +1921,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);
+void intel_psr_short_pulse(struct intel_dp *intel_dp);
 
 /* 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 d88799482875..60797c8f9f0e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -741,6 +741,23 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 	psr_aux_io_power_put(intel_dp);
 }
 
+static void psr_disable(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (!dev_priv->psr.enabled)
+		return;
+
+	dev_priv->psr.disable_source(intel_dp);
+
+	/* Disable PSR on Sink */
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
+	dev_priv->psr.enabled = NULL;
+	cancel_delayed_work_sync(&dev_priv->psr.work);
+}
+
 /**
  * intel_psr_disable - Disable PSR
  * @intel_dp: Intel DP
@@ -762,20 +779,8 @@ void intel_psr_disable(struct intel_dp *intel_dp,
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
-	if (!dev_priv->psr.enabled) {
-		mutex_unlock(&dev_priv->psr.lock);
-		return;
-	}
-
-	dev_priv->psr.disable_source(intel_dp);
-
-	/* Disable PSR on Sink */
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
-
-	dev_priv->psr.enabled = NULL;
+	psr_disable(intel_dp);
 	mutex_unlock(&dev_priv->psr.lock);
-
-	cancel_delayed_work_sync(&dev_priv->psr.work);
 }
 
 static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
@@ -1014,3 +1019,34 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
 
 }
+
+void intel_psr_short_pulse(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct i915_psr *psr = &dev_priv->psr;
+	uint8_t val;
+
+	if (!HAS_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
+		return;
+
+	mutex_lock(&psr->lock);
+
+	if (psr->enabled != intel_dp)
+		goto exit;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) != 1) {
+		DRM_ERROR("PSR_STATUS dpcd read failed\n");
+		goto exit;
+	}
+
+	if ((val & DP_PSR_SINK_STATE_MASK) == DP_PSR_SINK_INTERNAL_ERROR) {
+		DRM_DEBUG_KMS("PSR sink internal error, disabling PSR\n");
+		psr_disable(intel_dp);
+	}
+
+	/* TODO: handle other PSR/PSR2 errors */
+exit:
+	mutex_unlock(&psr->lock);
+}
-- 
2.17.0

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

Re: [PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[Dhinakaran Pandiyan]

On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> eDP spec states that sink device will do a short pulse in HPD
> line when there is a PSR/PSR2 error that needs to be handled by
> source, this is handling the first and most simples error:
> DP_PSR_SINK_INTERNAL_ERROR.
> 
> Here taking the safest approach and disabling PSR(at least until
> the next modeset), to avoid multiple rendering issues due to
> bad pannels.
> 
> v3:
> disabling PSR instead of exiting on error
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 62 +++++++++++++++++++++++++-----
> --
>  3 files changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index b86da48fd38e..fa2851d4fb36 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  	if (intel_dp_needs_link_retrain(intel_dp))
>  		return false;
>  
> +	intel_psr_short_pulse(intel_dp);
> +
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING)
> {
>  		DRM_DEBUG_KMS("Link Training Compliance Test
> requested\n");
>  		/* Send a Hotplug Uevent to userspace to start
> modeset */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 4508be628450..892da65358e9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1921,6 +1921,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);
> +void intel_psr_short_pulse(struct intel_dp *intel_dp);
>  
>  /* 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 d88799482875..60797c8f9f0e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -741,6 +741,23 @@ static void hsw_psr_disable(struct intel_dp
> *intel_dp)
>  	psr_aux_io_power_put(intel_dp);
>  }
>  
> +static void psr_disable(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (!dev_priv->psr.enabled)
> +		return;
> +
> +	dev_priv->psr.disable_source(intel_dp);
> +
> +	/* Disable PSR on Sink */
> +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> +	dev_priv->psr.enabled = NULL;
> +	cancel_delayed_work_sync(&dev_priv->psr.work);
> +}
> +
>  /**
>   * intel_psr_disable - Disable PSR
>   * @intel_dp: Intel DP
> @@ -762,20 +779,8 @@ void intel_psr_disable(struct intel_dp
> *intel_dp,
>  		return;
>  
>  	mutex_lock(&dev_priv->psr.lock);
> -	if (!dev_priv->psr.enabled) {
> -		mutex_unlock(&dev_priv->psr.lock);
> -		return;
> -	}
> -
> -	dev_priv->psr.disable_source(intel_dp);
> -
> -	/* Disable PSR on Sink */
> -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> -
> -	dev_priv->psr.enabled = NULL;
> +	psr_disable(intel_dp);
>  	mutex_unlock(&dev_priv->psr.lock);
> -
> -	cancel_delayed_work_sync(&dev_priv->psr.work);
>  }
>  
>  static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
> @@ -1014,3 +1019,34 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
>  	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
>  
>  }
> +
> +void intel_psr_short_pulse(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct i915_psr *psr = &dev_priv->psr;
> +	uint8_t val;
> +
> +	if (!HAS_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
> +		return;
	CAN_PSR(dev_priv) should take care of this.

> +
> +	mutex_lock(&psr->lock);
Do we really need to acquire the mutex here? How about
> +
> +	if (psr->enabled != intel_dp)
not doing this check?

> +		goto exit;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val)
> != 1) {
> +		DRM_ERROR("PSR_STATUS dpcd read failed\n");
> +		goto exit;
> +	}
> +
> +	if ((val & DP_PSR_SINK_STATE_MASK) ==
> DP_PSR_SINK_INTERNAL_ERROR) {
> +		DRM_DEBUG_KMS("PSR sink internal error, disabling
> PSR\n");
> +		psr_disable(intel_dp);
And calling intel_psr_disable() here?

By doing this, we can minimize the time for which the lock is held.

> +	}
> +
> +	/* TODO: handle other PSR/PSR2 errors */
> +exit:
> +	mutex_unlock(&psr->lock);
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[Souza, Jose]

On Tue, 2018-05-22 at 16:58 -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> > eDP spec states that sink device will do a short pulse in HPD
> > line when there is a PSR/PSR2 error that needs to be handled by
> > source, this is handling the first and most simples error:
> > DP_PSR_SINK_INTERNAL_ERROR.
> > 
> > Here taking the safest approach and disabling PSR(at least until
> > the next modeset), to avoid multiple rendering issues due to
> > bad pannels.
> > 
> > v3:
> > disabling PSR instead of exiting on error
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  |  2 ++
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_psr.c | 62 +++++++++++++++++++++++++---
> > --
> > --
> >  3 files changed, 52 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b86da48fd38e..fa2851d4fb36 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp
> > *intel_dp)
> >  	if (intel_dp_needs_link_retrain(intel_dp))
> >  		return false;
> >  
> > +	intel_psr_short_pulse(intel_dp);
> > +
> >  	if (intel_dp->compliance.test_type ==
> > DP_TEST_LINK_TRAINING)
> > {
> >  		DRM_DEBUG_KMS("Link Training Compliance Test
> > requested\n");
> >  		/* Send a Hotplug Uevent to userspace to start
> > modeset */
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 4508be628450..892da65358e9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1921,6 +1921,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);
> > +void intel_psr_short_pulse(struct intel_dp *intel_dp);
> >  
> >  /* 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 d88799482875..60797c8f9f0e 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -741,6 +741,23 @@ static void hsw_psr_disable(struct intel_dp
> > *intel_dp)
> >  	psr_aux_io_power_put(intel_dp);
> >  }
> >  
> > +static void psr_disable(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > +	struct drm_device *dev = intel_dig_port->base.base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +
> > +	if (!dev_priv->psr.enabled)
> > +		return;
> > +
> > +	dev_priv->psr.disable_source(intel_dp);
> > +
> > +	/* Disable PSR on Sink */
> > +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > +	dev_priv->psr.enabled = NULL;
> > +	cancel_delayed_work_sync(&dev_priv->psr.work);
> > +}
> > +
> >  /**
> >   * intel_psr_disable - Disable PSR
> >   * @intel_dp: Intel DP
> > @@ -762,20 +779,8 @@ void intel_psr_disable(struct intel_dp
> > *intel_dp,
> >  		return;
> >  
> >  	mutex_lock(&dev_priv->psr.lock);
> > -	if (!dev_priv->psr.enabled) {
> > -		mutex_unlock(&dev_priv->psr.lock);
> > -		return;
> > -	}
> > -
> > -	dev_priv->psr.disable_source(intel_dp);
> > -
> > -	/* Disable PSR on Sink */
> > -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > -
> > -	dev_priv->psr.enabled = NULL;
> > +	psr_disable(intel_dp);
> >  	mutex_unlock(&dev_priv->psr.lock);
> > -
> > -	cancel_delayed_work_sync(&dev_priv->psr.work);
> >  }
> >  
> >  static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
> > @@ -1014,3 +1019,34 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
> >  
> >  }
> > +
> > +void intel_psr_short_pulse(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > +	struct drm_device *dev = intel_dig_port->base.base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct i915_psr *psr = &dev_priv->psr;
> > +	uint8_t val;
> > +
> > +	if (!HAS_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
> > +		return;
> 
> 	CAN_PSR(dev_priv) should take care of this.

CAN_PSR is better and I will use that, but to remove the lock and 'if
(psr->enabled != intel_dp)' we would also need to check
i915_modparams.enable_psr. Even although we could end up doing the aux
transactions bellow and PSR is disabled(because of one of the errors
bellow), what do you think it is still worthy do it lockless?

> 
> > +
> > +	mutex_lock(&psr->lock);
> 
> Do we really need to acquire the mutex here? How about
> > +
> > +	if (psr->enabled != intel_dp)
> 
> not doing this check?
> 
> > +		goto exit;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val)
> > != 1) {
> > +		DRM_ERROR("PSR_STATUS dpcd read failed\n");
> > +		goto exit;
> > +	}
> > +
> > +	if ((val & DP_PSR_SINK_STATE_MASK) ==
> > DP_PSR_SINK_INTERNAL_ERROR) {
> > +		DRM_DEBUG_KMS("PSR sink internal error, disabling
> > PSR\n");
> > +		psr_disable(intel_dp);
> 
> And calling intel_psr_disable() here?
> 
> By doing this, we can minimize the time for which the lock is held.
> 
> > +	}
> > +
> > +	/* TODO: handle other PSR/PSR2 errors */
> > +exit:
> > +	mutex_unlock(&psr->lock);
> > +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[Dhinakaran Pandiyan]

On Tue, 2018-06-05 at 22:45 +0000, Souza, Jose wrote:
> On Tue, 2018-05-22 at 16:58 -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> > > 
> > > eDP spec states that sink device will do a short pulse in HPD
> > > line when there is a PSR/PSR2 error that needs to be handled by
> > > source, this is handling the first and most simples error:
> > > DP_PSR_SINK_INTERNAL_ERROR.
> > > 
> > > Here taking the safest approach and disabling PSR(at least until
> > > the next modeset), to avoid multiple rendering issues due to
> > > bad pannels.
> > > 
> > > v3:
> > > disabling PSR instead of exiting on error
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c  |  2 ++
> > >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> > >  drivers/gpu/drm/i915/intel_psr.c | 62 +++++++++++++++++++++++++-
> > > --
> > > --
> > > --
> > >  3 files changed, 52 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index b86da48fd38e..fa2851d4fb36 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp
> > > *intel_dp)
> > >  	if (intel_dp_needs_link_retrain(intel_dp))
> > >  		return false;
> > >  
> > > +	intel_psr_short_pulse(intel_dp);
> > > +
> > >  	if (intel_dp->compliance.test_type ==
> > > DP_TEST_LINK_TRAINING)
> > > {
> > >  		DRM_DEBUG_KMS("Link Training Compliance Test
> > > requested\n");
> > >  		/* Send a Hotplug Uevent to userspace to start
> > > modeset */
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 4508be628450..892da65358e9 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1921,6 +1921,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);
> > > +void intel_psr_short_pulse(struct intel_dp *intel_dp);
> > >  
> > >  /* 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 d88799482875..60797c8f9f0e 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -741,6 +741,23 @@ static void hsw_psr_disable(struct intel_dp
> > > *intel_dp)
> > >  	psr_aux_io_power_put(intel_dp);
> > >  }
> > >  
> > > +static void psr_disable(struct intel_dp *intel_dp)
nit: How about __psr_disable()? Might be worth checking other files
what the right convention is.

> > > +{+	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct drm_device *dev = intel_dig_port->base.base.dev;
> > > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > > +
> > > +	if (!dev_priv->psr.enabled)
> > > +		return;
> > > +
> > > +	dev_priv->psr.disable_source(intel_dp);
> > > +
> > > +	/* Disable PSR on Sink */
> > > +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > > +	dev_priv->psr.enabled = NULL;
> > > +	cancel_delayed_work_sync(&dev_priv->psr.work);
> > > +}
> > > +
> > >  /**
> > >   * intel_psr_disable - Disable PSR
> > >   * @intel_dp: Intel DP
> > > @@ -762,20 +779,8 @@ void intel_psr_disable(struct intel_dp
> > > *intel_dp,
> > >  		return;
> > >  
> > >  	mutex_lock(&dev_priv->psr.lock);
> > > -	if (!dev_priv->psr.enabled) {
> > > -		mutex_unlock(&dev_priv->psr.lock);
> > > -		return;
> > > -	}
> > > -
> > > -	dev_priv->psr.disable_source(intel_dp);
> > > -
> > > -	/* Disable PSR on Sink */
> > > -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > > -
> > > -	dev_priv->psr.enabled = NULL;
> > > +	psr_disable(intel_dp);
> > >  	mutex_unlock(&dev_priv->psr.lock);
> > > -
> > > -	cancel_delayed_work_sync(&dev_priv->psr.work);
> > >  }
> > >  
> > >  static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
> > > @@ -1014,3 +1019,34 @@ void intel_psr_init(struct
> > > drm_i915_private
> > > *dev_priv)
> > >  	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
> > >  
> > >  }
> > > +
> > > +void intel_psr_short_pulse(struct intel_dp *intel_dp)
> > > +{
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct drm_device *dev = intel_dig_port->base.base.dev;
> > > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > > +	struct i915_psr *psr = &dev_priv->psr;
> > > +	uint8_t val;
> > > +
> > > +	if (!HAS_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
> > > +		return;
> > 	CAN_PSR(dev_priv) should take care of this.
> CAN_PSR is better and I will use that, but to remove the lock and 'if
> (psr->enabled != intel_dp)' we would also need to check
> i915_modparams.enable_psr. Even although we could end up doing the
> aux
> transactions bellow and PSR is disabled(because of one of the errors
> bellow), what do you think it is still worthy do it lockless?

That is a good point, avoiding DPCD read in case PSR is disabled is
indeed better.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[Souza, Jose]

On Wed, 2018-06-13 at 13:17 -0700, Dhinakaran Pandiyan wrote:
> On Tue, 2018-06-05 at 22:45 +0000, Souza, Jose wrote:
> > On Tue, 2018-05-22 at 16:58 -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> > > > 
> > > > eDP spec states that sink device will do a short pulse in HPD
> > > > line when there is a PSR/PSR2 error that needs to be handled by
> > > > source, this is handling the first and most simples error:
> > > > DP_PSR_SINK_INTERNAL_ERROR.
> > > > 
> > > > Here taking the safest approach and disabling PSR(at least
> > > > until
> > > > the next modeset), to avoid multiple rendering issues due to
> > > > bad pannels.
> > > > 
> > > > v3:
> > > > disabling PSR instead of exiting on error
> > > > 
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c  |  2 ++
> > > >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> > > >  drivers/gpu/drm/i915/intel_psr.c | 62
> > > > +++++++++++++++++++++++++-
> > > > --
> > > > --
> > > > --
> > > >  3 files changed, 52 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index b86da48fd38e..fa2851d4fb36 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp
> > > > *intel_dp)
> > > >  	if (intel_dp_needs_link_retrain(intel_dp))
> > > >  		return false;
> > > >  
> > > > +	intel_psr_short_pulse(intel_dp);
> > > > +
> > > >  	if (intel_dp->compliance.test_type ==
> > > > DP_TEST_LINK_TRAINING)
> > > > {
> > > >  		DRM_DEBUG_KMS("Link Training Compliance Test
> > > > requested\n");
> > > >  		/* Send a Hotplug Uevent to userspace to start
> > > > modeset */
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 4508be628450..892da65358e9 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1921,6 +1921,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);
> > > > +void intel_psr_short_pulse(struct intel_dp *intel_dp);
> > > >  
> > > >  /* 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 d88799482875..60797c8f9f0e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -741,6 +741,23 @@ static void hsw_psr_disable(struct
> > > > intel_dp
> > > > *intel_dp)
> > > >  	psr_aux_io_power_put(intel_dp);
> > > >  }
> > > >  
> > > > +static void psr_disable(struct intel_dp *intel_dp)
> 
> nit: How about __psr_disable()? Might be worth checking other files
> what the right convention is.

It varies from file to file but inside of intel_psr.c we are not adding
 "__" so better keep that, right?

> 
> > > > +{+	struct intel_digital_port *intel_dig_port =
> > > > dp_to_dig_port(intel_dp);
> > > > +	struct drm_device *dev = intel_dig_port-
> > > > >base.base.dev;
> > > > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > > > +
> > > > +	if (!dev_priv->psr.enabled)
> > > > +		return;
> > > > +
> > > > +	dev_priv->psr.disable_source(intel_dp);
> > > > +
> > > > +	/* Disable PSR on Sink */
> > > > +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > > > +	dev_priv->psr.enabled = NULL;
> > > > +	cancel_delayed_work_sync(&dev_priv->psr.work);
> > > > +}
> > > > +
> > > >  /**
> > > >   * intel_psr_disable - Disable PSR
> > > >   * @intel_dp: Intel DP
> > > > @@ -762,20 +779,8 @@ void intel_psr_disable(struct intel_dp
> > > > *intel_dp,
> > > >  		return;
> > > >  
> > > >  	mutex_lock(&dev_priv->psr.lock);
> > > > -	if (!dev_priv->psr.enabled) {
> > > > -		mutex_unlock(&dev_priv->psr.lock);
> > > > -		return;
> > > > -	}
> > > > -
> > > > -	dev_priv->psr.disable_source(intel_dp);
> > > > -
> > > > -	/* Disable PSR on Sink */
> > > > -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> > > > -
> > > > -	dev_priv->psr.enabled = NULL;
> > > > +	psr_disable(intel_dp);
> > > >  	mutex_unlock(&dev_priv->psr.lock);
> > > > -
> > > > -	cancel_delayed_work_sync(&dev_priv->psr.work);
> > > >  }
> > > >  
> > > >  static bool psr_wait_for_idle(struct drm_i915_private
> > > > *dev_priv)
> > > > @@ -1014,3 +1019,34 @@ void intel_psr_init(struct
> > > > drm_i915_private
> > > > *dev_priv)
> > > >  	dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
> > > >  
> > > >  }
> > > > +
> > > > +void intel_psr_short_pulse(struct intel_dp *intel_dp)
> > > > +{
> > > > +	struct intel_digital_port *intel_dig_port =
> > > > dp_to_dig_port(intel_dp);
> > > > +	struct drm_device *dev = intel_dig_port-
> > > > >base.base.dev;
> > > > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > > > +	struct i915_psr *psr = &dev_priv->psr;
> > > > +	uint8_t val;
> > > > +
> > > > +	if (!HAS_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
> > > > +		return;
> > > 
> > > 	CAN_PSR(dev_priv) should take care of this.
> > 
> > CAN_PSR is better and I will use that, but to remove the lock and
> > 'if
> > (psr->enabled != intel_dp)' we would also need to check
> > i915_modparams.enable_psr. Even although we could end up doing the
> > aux
> > transactions bellow and PSR is disabled(because of one of the
> > errors
> > bellow), what do you think it is still worthy do it lockless?
> 
> That is a good point, avoiding DPCD read in case PSR is disabled is
> indeed better.


Cool, I will send the new version soon.

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

Re: [PATCH v3 4/7] drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink

[Dhinakaran Pandiyan]

On Wed, 2018-06-13 at 20:02 +0000, Souza, Jose wrote:
> On Wed, 2018-06-13 at 13:17 -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Tue, 2018-06-05 at 22:45 +0000, Souza, Jose wrote:
> > > 
> > > On Tue, 2018-05-22 at 16:58 -0700, Dhinakaran Pandiyan wrote:
> > > > 
> > > > 
> > > > On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> > > > > 
> > > > > 
> > > > > eDP spec states that sink device will do a short pulse in HPD
> > > > > line when there is a PSR/PSR2 error that needs to be handled
> > > > > by
> > > > > source, this is handling the first and most simples error:
> > > > > DP_PSR_SINK_INTERNAL_ERROR.
> > > > > 
> > > > > Here taking the safest approach and disabling PSR(at least
> > > > > until
> > > > > the next modeset), to avoid multiple rendering issues due to
> > > > > bad pannels.
> > > > > 
> > > > > v3:
> > > > > disabling PSR instead of exiting on error
> > > > > 
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dp.c  |  2 ++
> > > > >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> > > > >  drivers/gpu/drm/i915/intel_psr.c | 62
> > > > > +++++++++++++++++++++++++-
> > > > > --
> > > > > --
> > > > > --
> > > > >  3 files changed, 52 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index b86da48fd38e..fa2851d4fb36 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -4479,6 +4479,8 @@ intel_dp_short_pulse(struct intel_dp
> > > > > *intel_dp)
> > > > >  	if (intel_dp_needs_link_retrain(intel_dp))
> > > > >  		return false;
> > > > >  
> > > > > +	intel_psr_short_pulse(intel_dp);
> > > > > +
> > > > >  	if (intel_dp->compliance.test_type ==
> > > > > DP_TEST_LINK_TRAINING)
> > > > > {
> > > > >  		DRM_DEBUG_KMS("Link Training Compliance Test
> > > > > requested\n");
> > > > >  		/* Send a Hotplug Uevent to userspace to
> > > > > start
> > > > > modeset */
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index 4508be628450..892da65358e9 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -1921,6 +1921,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);
> > > > > +void intel_psr_short_pulse(struct intel_dp *intel_dp);
> > > > >  
> > > > >  /* 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 d88799482875..60797c8f9f0e 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -741,6 +741,23 @@ static void hsw_psr_disable(struct
> > > > > intel_dp
> > > > > *intel_dp)
> > > > >  	psr_aux_io_power_put(intel_dp);
> > > > >  }
> > > > >  
> > > > > +static void psr_disable(struct intel_dp *intel_dp)
> > nit: How about __psr_disable()? Might be worth checking other files
> > what the right convention is.
> It varies from file to file but inside of intel_psr.c we are not
> adding
>  "__" so better keep that, right?
Sure, either way is okay with me.



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

[PATCH v3 5/7] drm/i915/psr: Handle PSR RFB storage error

[José Roberto de Souza]

Sink will interrupt source when it have any problem saving or reading
the remote frame buffer.

v3:
disabling PSR instead of exiting on error

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

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 60797c8f9f0e..f72e3f91809f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1046,6 +1046,20 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 		psr_disable(intel_dp);
 	}
 
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_ERROR_STATUS, &val) != 1) {
+		DRM_ERROR("PSR_ERROR_STATUS dpcd read failed\n");
+		goto exit;
+	}
+
+	if (val & DP_PSR_RFB_STORAGE_ERROR) {
+		DRM_DEBUG_KMS("PSR RFB storage error, exiting PSR\n");
+		psr_disable(intel_dp);
+	}
+	if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR | DP_PSR_LINK_CRC_ERROR))
+		DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
+	/* clear status register */
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
+
 	/* TODO: handle other PSR/PSR2 errors */
 exit:
 	mutex_unlock(&psr->lock);
-- 
2.17.0

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

Re: [PATCH v3 5/7] drm/i915/psr: Handle PSR RFB storage error

[Dhinakaran Pandiyan]

On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> Sink will interrupt source when it have any problem saving or reading
> the remote frame buffer.
> 
> v3:
> disabling PSR instead of exiting on error
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 60797c8f9f0e..f72e3f91809f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1046,6 +1046,20 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  		psr_disable(intel_dp);
>  	}
>  
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> &val) != 1) {
> +		DRM_ERROR("PSR_ERROR_STATUS dpcd read failed\n");
> +		goto exit;
> +	}
> +
> +	if (val & DP_PSR_RFB_STORAGE_ERROR) {
> +		DRM_DEBUG_KMS("PSR RFB storage error, exiting
> PSR\n");
> +		psr_disable(intel_dp);
> +	}
> +	if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> DP_PSR_LINK_CRC_ERROR))
> +		DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
> +	/* clear status register */
> +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> val);
> +
>  	/* TODO: handle other PSR/PSR2 errors */
>  exit:
>  	mutex_unlock(&psr->lock);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 6/7] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side

[José Roberto de Souza]

Sink can be configured to calculate the CRC over the static frame and
compare with the CRC calculated and transmited in the VSC SDP by
source, if there is a mismatch sink will do a short pulse in HPD
and set DP_PSR_LINK_CRC_ERROR on DP_PSR_ERROR_STATUS.

Also spec recommends to disable MAX_SLEEP as a trigger to exit PSR when
CRC check is enabled to improve power savings.

Spec: 7723

v3:
disabling PSR instead of exiting on error

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 196a0eb79272..1add22e664ea 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4020,6 +4020,7 @@ enum {
 #define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
 #define   EDP_PSR_TP1_TP2_SEL			(0<<11)
 #define   EDP_PSR_TP1_TP3_SEL			(1<<11)
+#define   EDP_PSR_CRC_ENABLE			(1<<10) /* BDW+ */
 #define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
 #define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
 #define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f72e3f91809f..2f29dcd6f69e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -363,6 +363,8 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		dpcd_val |= DP_PSR_ENABLE_PSR2;
 	if (dev_priv->psr.link_standby)
 		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
+		dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
 
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
@@ -418,6 +420,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
 	else
 		val |= EDP_PSR_TP1_TP2_SEL;
 
+	if (INTEL_GEN(dev_priv) >= 8)
+		val |= EDP_PSR_CRC_ENABLE;
+
 	val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
 	I915_WRITE(EDP_PSR_CTL, val);
 }
@@ -635,11 +640,14 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
 		 * preventing  other hw tracking issues now we can rely
 		 * on frontbuffer tracking.
 		 */
-		I915_WRITE(EDP_PSR_DEBUG,
-			   EDP_PSR_DEBUG_MASK_MEMUP |
-			   EDP_PSR_DEBUG_MASK_HPD |
-			   EDP_PSR_DEBUG_MASK_LPSP |
-			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
+		u32 val = EDP_PSR_DEBUG_MASK_MEMUP |
+			  EDP_PSR_DEBUG_MASK_HPD |
+			  EDP_PSR_DEBUG_MASK_LPSP |
+			  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
+
+		if (INTEL_GEN(dev_priv) >= 8)
+			val |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;
+		I915_WRITE(EDP_PSR_DEBUG, val);
 	}
 }
 
@@ -1051,16 +1059,19 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 		goto exit;
 	}
 
-	if (val & DP_PSR_RFB_STORAGE_ERROR) {
-		DRM_DEBUG_KMS("PSR RFB storage error, exiting PSR\n");
+	if (val & (DP_PSR_RFB_STORAGE_ERROR | DP_PSR_LINK_CRC_ERROR)) {
+		if (val & DP_PSR_RFB_STORAGE_ERROR)
+			DRM_DEBUG_KMS("PSR RFB storage error, disabling PSR\n");
+		if (val & DP_PSR_LINK_CRC_ERROR)
+			DRM_DEBUG_KMS("PSR Link CRC error, disabling PSR\n");
 		psr_disable(intel_dp);
 	}
-	if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR | DP_PSR_LINK_CRC_ERROR))
+	if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
 		DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
 	/* clear status register */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
 
-	/* TODO: handle other PSR/PSR2 errors */
+	/* TODO: handle PSR2 errors */
 exit:
 	mutex_unlock(&psr->lock);
 }
-- 
2.17.0

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

Re: [PATCH v3 6/7] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side

[Dhinakaran Pandiyan]

On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> Sink can be configured to calculate the CRC over the static frame and
> compare with the CRC calculated and transmited in the VSC SDP by
> source, if there is a mismatch sink will do a short pulse in HPD
> and set DP_PSR_LINK_CRC_ERROR on DP_PSR_ERROR_STATUS.
> 
> Also spec recommends to disable MAX_SLEEP as a trigger to exit PSR
> when
> CRC check is enabled to improve power savings.
> 
> Spec: 7723
> 
> v3:
> disabling PSR instead of exiting on error
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 29 ++++++++++++++++++++---------
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 196a0eb79272..1add22e664ea 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4020,6 +4020,7 @@ enum {
>  #define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
>  #define   EDP_PSR_TP1_TP2_SEL			(0<<11)
>  #define   EDP_PSR_TP1_TP3_SEL			(1<<11)
> +#define   EDP_PSR_CRC_ENABLE			(1<<10) /* BDW+
> */
>  #define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index f72e3f91809f..2f29dcd6f69e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -363,6 +363,8 @@ static void hsw_psr_enable_sink(struct intel_dp
> *intel_dp)
>  		dpcd_val |= DP_PSR_ENABLE_PSR2;
>  	if (dev_priv->psr.link_standby)
>  		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> +		dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
>  
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> @@ -418,6 +420,9 @@ static void hsw_activate_psr1(struct intel_dp
> *intel_dp)
>  	else
>  		val |= EDP_PSR_TP1_TP2_SEL;
>  
> +	if (INTEL_GEN(dev_priv) >= 8)
> +		val |= EDP_PSR_CRC_ENABLE;
> +
>  	val |= I915_READ(EDP_PSR_CTL) &
> EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
>  	I915_WRITE(EDP_PSR_CTL, val);
>  }
> @@ -635,11 +640,14 @@ static void hsw_psr_enable_source(struct
> intel_dp *intel_dp,
>  		 * preventing  other hw tracking issues now we can
> rely
>  		 * on frontbuffer tracking.
>  		 */
> -		I915_WRITE(EDP_PSR_DEBUG,
> -			   EDP_PSR_DEBUG_MASK_MEMUP |
> -			   EDP_PSR_DEBUG_MASK_HPD |
> -			   EDP_PSR_DEBUG_MASK_LPSP |
> -			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> +		u32 val = EDP_PSR_DEBUG_MASK_MEMUP |
> +			  EDP_PSR_DEBUG_MASK_HPD |
> +			  EDP_PSR_DEBUG_MASK_LPSP |
> +			  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
> +
> +		if (INTEL_GEN(dev_priv) >= 8)
> +			val |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;
Move Patch 7/7 ahead of this one to avoid removing this check again?

> +		I915_WRITE(EDP_PSR_DEBUG, val);
>  	}
>  }
>  
> @@ -1051,16 +1059,19 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  		goto exit;
>  	}
>  
> -	if (val & DP_PSR_RFB_STORAGE_ERROR) {
> -		DRM_DEBUG_KMS("PSR RFB storage error, exiting
> PSR\n");
> +	if (val & (DP_PSR_RFB_STORAGE_ERROR |
> DP_PSR_LINK_CRC_ERROR)) {
> +		if (val & DP_PSR_RFB_STORAGE_ERROR)
> +			DRM_DEBUG_KMS("PSR RFB storage error,
> disabling PSR\n");
> +		if (val & DP_PSR_LINK_CRC_ERROR)
> +			DRM_DEBUG_KMS("PSR Link CRC error, disabling
> PSR\n");
>  		psr_disable(intel_dp);
>  	}
> -	if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> DP_PSR_LINK_CRC_ERROR))
> +	if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
>  		DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> val);
>  
> -	/* TODO: handle other PSR/PSR2 errors */
> +	/* TODO: handle PSR2 errors */
>  exit:
>  	mutex_unlock(&psr->lock);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 7/7] drm/i915/psr: Avoid PSR exit max time timeout

[José Roberto de Souza]

Also masking max time as a trigger to exit PSR for the remaning
platforms that do not support sink CRC check(gen <= 8).
This will make PSR exits more deterministic and only when really
needed. If this was used to fix a issue in some pannel than can
only self-refresh for a few seconds, that panel will interrupt
and assert one of the PSR errors handled in:
'drm/i915/psr: Handle PSR RFB storage error' and
'drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink'

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

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2f29dcd6f69e..8e2e90d139b5 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -376,7 +376,6 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	uint32_t max_sleep_time = 0x1f;
 	/*
 	 * Let's respect VBT in case VBT asks a higher idle_frame value.
 	 * Let's use 6 as the minimum to cover all known cases including
@@ -387,7 +386,6 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val = EDP_PSR_ENABLE;
 
-	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
 	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
 	if (IS_HASWELL(dev_priv))
@@ -640,14 +638,12 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
 		 * preventing  other hw tracking issues now we can rely
 		 * on frontbuffer tracking.
 		 */
-		u32 val = EDP_PSR_DEBUG_MASK_MEMUP |
+		I915_WRITE(EDP_PSR_DEBUG,
+			  EDP_PSR_DEBUG_MASK_MEMUP |
 			  EDP_PSR_DEBUG_MASK_HPD |
 			  EDP_PSR_DEBUG_MASK_LPSP |
-			  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
-
-		if (INTEL_GEN(dev_priv) >= 8)
-			val |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;
-		I915_WRITE(EDP_PSR_DEBUG, val);
+			  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
+			  EDP_PSR_DEBUG_MASK_MAX_SLEEP);
 	}
 }
 
-- 
2.17.0

_______________________________________________
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 [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV
URL   : https://patchwork.freedesktop.org/series/43368/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6e561e557b3d drm/i915/psr: Nuke PSR support for VLV and CHV
76eeb06056e8 drm/i915/dp: Use intel_dp_aux_wait_done() to wait for previous aux xfer
03733f0d0e8c drm/i915/psr: Remove intel_crtc_state parameter from disable()
704b28a727be drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
3a64512fbe43 drm/i915/psr: Handle PSR RFB storage error
1e477b3f6103 drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
-:35: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#35: FILE: drivers/gpu/drm/i915/i915_reg.h:4023:
+#define   EDP_PSR_CRC_ENABLE			(1<<10) /* BDW+ */
                             			  ^

total: 0 errors, 0 warnings, 1 checks, 66 lines checked
b3fc76974022 drm/i915/psr: Avoid PSR exit max time timeout
-:48: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#48: FILE: drivers/gpu/drm/i915/intel_psr.c:642:
+		I915_WRITE(EDP_PSR_DEBUG,
+			  EDP_PSR_DEBUG_MASK_MEMUP |

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

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

✗ Fi.CI.SPARSE: warning for series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV
URL   : https://patchwork.freedesktop.org/series/43368/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/psr: Nuke PSR support for VLV and CHV
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3663:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3662:16: warning: expression using sizeof(void)

Commit: drm/i915/dp: Use intel_dp_aux_wait_done() to wait for previous aux xfer
Okay!

Commit: drm/i915/psr: Remove intel_crtc_state parameter from disable()
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3662:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3661:16: warning: expression using sizeof(void)

Commit: drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
Okay!

Commit: drm/i915/psr: Handle PSR RFB storage error
Okay!

Commit: drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
Okay!

Commit: drm/i915/psr: Avoid PSR exit max time timeout
-O:drivers/gpu/drm/i915/intel_psr.c:387:32: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_psr.c:386:32: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV
URL   : https://patchwork.freedesktop.org/series/43368/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4201 -> Patchwork_9039 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)

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

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-cnl-y3:          FAIL (fdo#104724, fdo#103167) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (43 -> 39) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4201 -> Patchwork_9039

  CI_DRM_4201: 5e68b6c9b2d2781edec45e84460517704f557126 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4487: eccae1360d6d01e73c6af2bd97122cef708207ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9039: b3fc769740221068d91eb1879d9daa75ef108dd3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4487: 6ab75f7eb5e1dccbb773e1739beeb2d7cbd6ad0d @ git://anongit.freedesktop.org/piglit


== Linux commits ==

b3fc76974022 drm/i915/psr: Avoid PSR exit max time timeout
1e477b3f6103 drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side
3a64512fbe43 drm/i915/psr: Handle PSR RFB storage error
704b28a727be drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink
03733f0d0e8c drm/i915/psr: Remove intel_crtc_state parameter from disable()
76eeb06056e8 drm/i915/dp: Use intel_dp_aux_wait_done() to wait for previous aux xfer
6e561e557b3d drm/i915/psr: Nuke PSR support for VLV and CHV

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/7] drm/i915/psr: Nuke PSR support for VLV and CHV
URL   : https://patchwork.freedesktop.org/series/43368/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4201_full -> Patchwork_9039_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9039_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9039_full, 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/43368/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          SKIP -> PASS

    igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy:
      shard-apl:          PASS -> SKIP +24

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_eio@in-flight-suspend:
      shard-kbl:          PASS -> FAIL (fdo#105957)

    igt@kms_atomic_transition@2x-modeset-transitions-nonblocking-fencing:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          PASS -> DMESG-WARN (fdo#103558)

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip@flip-vs-blocking-wf-vblank:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#104724)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (9 -> 9) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4201 -> Patchwork_9039

  CI_DRM_4201: 5e68b6c9b2d2781edec45e84460517704f557126 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4487: eccae1360d6d01e73c6af2bd97122cef708207ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9039: b3fc769740221068d91eb1879d9daa75ef108dd3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4487: 6ab75f7eb5e1dccbb773e1739beeb2d7cbd6ad0d @ git://anongit.freedesktop.org/piglit

== Logs ==

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