[PATCH 1/4] drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1

[PATCH 1/4] drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1

[Rodrigo Vivi]

Let's use VBT + 1 now we parse it.

v2: fix subject and consider that idle_frame = 2 is another fix.

Cc: Arthur Runyan <arthur.j.runyan@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 20beb59..41df221 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2174,7 +2174,12 @@ static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t max_sleep_time = 0x1f;
-	uint32_t idle_frames = 2; /* 2 is the minimum allowed */
+	/* Lately it was identified that depending on panel idle frame count
+	 * calculated at HW can be off by 1. So let's use what came
+	 * from VBT + 1 and at minimum 2 to stay on the safest side.
+	 */
+	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames ?
+			       dev_priv->vbt.psr.idle_frames + 1 : 2;
 	uint32_t val = 0x0;
 	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
 	bool only_standby = false;
-- 
1.9.3

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

[PATCH 2/4] drm/i915: Make dp aux pack/unpack public outside intel_dp.c

[Rodrigo Vivi]

No functional change. Just making it public for use outside intel_dp.c
Allowing split psr functions.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41df221..cab57e2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -227,8 +227,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static uint32_t
-pack_aux(const uint8_t *src, int src_bytes)
+uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes)
 {
 	int	i;
 	uint32_t v = 0;
@@ -240,8 +239,7 @@ pack_aux(const uint8_t *src, int src_bytes)
 	return v;
 }
 
-static void
-unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes)
+void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes)
 {
 	int i;
 	if (dst_bytes > 4)
@@ -863,7 +861,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 			/* Load the send data into the aux channel data registers */
 			for (i = 0; i < send_bytes; i += 4)
 				I915_WRITE(ch_data + i,
-					   pack_aux(send + i, send_bytes - i));
+					   intel_dp_pack_aux(send + i,
+							     send_bytes - i));
 
 			/* Send the command and wait for it to complete */
 			I915_WRITE(ch_ctl, send_ctl);
@@ -917,8 +916,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		recv_bytes = recv_size;
 
 	for (i = 0; i < recv_bytes; i += 4)
-		unpack_aux(I915_READ(ch_data + i),
-			   recv + i, recv_bytes - i);
+		intel_dp_unpack_aux(I915_READ(ch_data + i),
+				    recv + i, recv_bytes - i);
 
 	ret = recv_bytes;
 out:
@@ -2159,7 +2158,7 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 	/* Setup AUX registers */
 	for (i = 0; i < sizeof(aux_msg); i += 4)
 		I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
-			   pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
+			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
 
 	I915_WRITE(EDP_PSR_AUX_CTL(dev),
 		   DP_AUX_CH_CTL_TIME_OUT_400us |
-- 
1.9.3

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

[PATCH 3/4] drm/i915: Introduce intel_psr.c

[Rodrigo Vivi]

No functional changes. Just cleaning and reorganizing it.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/Makefile            |   1 +
 drivers/gpu/drm/i915/intel_ddi.c         |   4 +-
 drivers/gpu/drm/i915/intel_display.c     |   2 +-
 drivers/gpu/drm/i915/intel_dp.c          | 386 +----------------------------
 drivers/gpu/drm/i915/intel_drv.h         |  21 +-
 drivers/gpu/drm/i915/intel_frontbuffer.c |   4 +-
 drivers/gpu/drm/i915/intel_psr.c         | 413 +++++++++++++++++++++++++++++++
 7 files changed, 433 insertions(+), 398 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_psr.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 891e584..e4083e4 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -51,6 +51,7 @@ i915-y += intel_audio.o \
 	  intel_frontbuffer.o \
 	  intel_modes.o \
 	  intel_overlay.o \
+	  intel_psr.o \
 	  intel_sideband.o \
 	  intel_sprite.o
 i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 68703ce..a5a8acd 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1228,7 +1228,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 			intel_dp_stop_link_train(intel_dp);
 
 		intel_edp_backlight_on(intel_dp);
-		intel_edp_psr_enable(intel_dp);
+		intel_psr_enable(intel_dp);
 	}
 
 	if (intel_crtc->config.has_audio) {
@@ -1254,7 +1254,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
-		intel_edp_psr_disable(intel_dp);
+		intel_psr_disable(intel_dp);
 		intel_edp_backlight_off(intel_dp);
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ff071a7..3bcdf87 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12231,7 +12231,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 	if (SUPPORTS_TV(dev))
 		intel_tv_init(dev);
 
-	intel_edp_psr_init(dev);
+	intel_psr_init(dev);
 
 	for_each_intel_encoder(dev, encoder) {
 		encoder->base.possible_crtcs = encoder->crtc_mask;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cab57e2..1ad1fbb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2066,390 +2066,6 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	}
 }
 
-static bool is_edp_psr(struct intel_dp *intel_dp)
-{
-	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
-}
-
-static bool intel_edp_is_psr_enabled(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!HAS_PSR(dev))
-		return false;
-
-	return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
-}
-
-static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp,
-				    struct edp_vsc_psr *vsc_psr)
-{
-	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 = dev->dev_private;
-	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
-	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(crtc->config.cpu_transcoder);
-	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(crtc->config.cpu_transcoder);
-	uint32_t *data = (uint32_t *) vsc_psr;
-	unsigned int i;
-
-	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
-	   the video DIP being updated before program video DIP data buffer
-	   registers for DIP being updated. */
-	I915_WRITE(ctl_reg, 0);
-	POSTING_READ(ctl_reg);
-
-	for (i = 0; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4) {
-		if (i < sizeof(struct edp_vsc_psr))
-			I915_WRITE(data_reg + i, *data++);
-		else
-			I915_WRITE(data_reg + i, 0);
-	}
-
-	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
-	POSTING_READ(ctl_reg);
-}
-
-static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp)
-{
-	struct edp_vsc_psr psr_vsc;
-
-	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
-	memset(&psr_vsc, 0, sizeof(psr_vsc));
-	psr_vsc.sdp_header.HB0 = 0;
-	psr_vsc.sdp_header.HB1 = 0x7;
-	psr_vsc.sdp_header.HB2 = 0x2;
-	psr_vsc.sdp_header.HB3 = 0x8;
-	intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
-}
-
-static void intel_edp_psr_enable_sink(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 = dev->dev_private;
-	uint32_t aux_clock_divider;
-	int precharge = 0x3;
-	bool only_standby = false;
-	static const uint8_t aux_msg[] = {
-		[0] = DP_AUX_NATIVE_WRITE << 4,
-		[1] = DP_SET_POWER >> 8,
-		[2] = DP_SET_POWER & 0xff,
-		[3] = 1 - 1,
-		[4] = DP_SET_POWER_D0,
-	};
-	int i;
-
-	BUILD_BUG_ON(sizeof(aux_msg) > 20);
-
-	aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
-
-	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
-		only_standby = true;
-
-	/* Enable PSR in sink */
-	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
-	else
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
-
-	/* Setup AUX registers */
-	for (i = 0; i < sizeof(aux_msg); i += 4)
-		I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
-			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
-
-	I915_WRITE(EDP_PSR_AUX_CTL(dev),
-		   DP_AUX_CH_CTL_TIME_OUT_400us |
-		   (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
-}
-
-static void intel_edp_psr_enable_source(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 = dev->dev_private;
-	uint32_t max_sleep_time = 0x1f;
-	/* Lately it was identified that depending on panel idle frame count
-	 * calculated at HW can be off by 1. So let's use what came
-	 * from VBT + 1 and at minimum 2 to stay on the safest side.
-	 */
-	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames ?
-			       dev_priv->vbt.psr.idle_frames + 1 : 2;
-	uint32_t val = 0x0;
-	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
-	bool only_standby = false;
-
-	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
-		only_standby = true;
-
-	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby) {
-		val |= EDP_PSR_LINK_STANDBY;
-		val |= EDP_PSR_TP2_TP3_TIME_0us;
-		val |= EDP_PSR_TP1_TIME_0us;
-		val |= EDP_PSR_SKIP_AUX_EXIT;
-		val |= IS_BROADWELL(dev) ? BDW_PSR_SINGLE_FRAME : 0;
-	} else
-		val |= EDP_PSR_LINK_DISABLE;
-
-	I915_WRITE(EDP_PSR_CTL(dev), val |
-		   (IS_BROADWELL(dev) ? 0 : link_entry_time) |
-		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
-		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
-		   EDP_PSR_ENABLE);
-}
-
-static bool intel_edp_psr_match_conditions(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 = dev->dev_private;
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	lockdep_assert_held(&dev_priv->psr.lock);
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
-	dev_priv->psr.source_ok = false;
-
-	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
-		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
-		return false;
-	}
-
-	if (!i915.enable_psr) {
-		DRM_DEBUG_KMS("PSR disable by flag\n");
-		return false;
-	}
-
-	/* Below limitations aren't valid for Broadwell */
-	if (IS_BROADWELL(dev))
-		goto out;
-
-	if (I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config.cpu_transcoder)) &
-	    S3D_ENABLE) {
-		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
-		return false;
-	}
-
-	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
-		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
-		return false;
-	}
-
- out:
-	dev_priv->psr.source_ok = true;
-	return true;
-}
-
-static void intel_edp_psr_do_enable(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 = dev->dev_private;
-
-	WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
-	WARN_ON(dev_priv->psr.active);
-	lockdep_assert_held(&dev_priv->psr.lock);
-
-	/* Enable/Re-enable PSR on the host */
-	intel_edp_psr_enable_source(intel_dp);
-
-	dev_priv->psr.active = true;
-}
-
-void intel_edp_psr_enable(struct intel_dp *intel_dp)
-{
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
-		return;
-	}
-
-	mutex_lock(&dev_priv->psr.lock);
-	if (dev_priv->psr.enabled) {
-		DRM_DEBUG_KMS("PSR already in use\n");
-		goto unlock;
-	}
-
-	if (!intel_edp_psr_match_conditions(intel_dp))
-		goto unlock;
-
-	dev_priv->psr.busy_frontbuffer_bits = 0;
-
-	intel_edp_psr_setup_vsc(intel_dp);
-
-	/* Avoid continuous PSR exit by masking memup and hpd */
-	I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
-		   EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
-
-	/* Enable PSR on the panel */
-	intel_edp_psr_enable_sink(intel_dp);
-
-	dev_priv->psr.enabled = intel_dp;
-unlock:
-	mutex_unlock(&dev_priv->psr.lock);
-}
-
-void intel_edp_psr_disable(struct intel_dp *intel_dp)
-{
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	mutex_lock(&dev_priv->psr.lock);
-	if (!dev_priv->psr.enabled) {
-		mutex_unlock(&dev_priv->psr.lock);
-		return;
-	}
-
-	if (dev_priv->psr.active) {
-		I915_WRITE(EDP_PSR_CTL(dev),
-			   I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE);
-
-		/* Wait till PSR is idle */
-		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
-			       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
-			DRM_ERROR("Timed out waiting for PSR Idle State\n");
-
-		dev_priv->psr.active = false;
-	} else {
-		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
-	}
-
-	dev_priv->psr.enabled = NULL;
-	mutex_unlock(&dev_priv->psr.lock);
-
-	cancel_delayed_work_sync(&dev_priv->psr.work);
-}
-
-static void intel_edp_psr_work(struct work_struct *work)
-{
-	struct drm_i915_private *dev_priv =
-		container_of(work, typeof(*dev_priv), psr.work.work);
-	struct intel_dp *intel_dp = dev_priv->psr.enabled;
-
-	/* We have to make sure PSR is ready for re-enable
-	 * otherwise it keeps disabled until next full enable/disable cycle.
-	 * PSR might take some time to get fully disabled
-	 * and be ready for re-enable.
-	 */
-	if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev_priv->dev)) &
-		      EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
-		DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
-		return;
-	}
-
-	mutex_lock(&dev_priv->psr.lock);
-	intel_dp = dev_priv->psr.enabled;
-
-	if (!intel_dp)
-		goto unlock;
-
-	/*
-	 * The delayed work can race with an invalidate hence we need to
-	 * recheck. Since psr_flush first clears this and then reschedules we
-	 * won't ever miss a flush when bailing out here.
-	 */
-	if (dev_priv->psr.busy_frontbuffer_bits)
-		goto unlock;
-
-	intel_edp_psr_do_enable(intel_dp);
-unlock:
-	mutex_unlock(&dev_priv->psr.lock);
-}
-
-static void intel_edp_psr_do_exit(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (dev_priv->psr.active) {
-		u32 val = I915_READ(EDP_PSR_CTL(dev));
-
-		WARN_ON(!(val & EDP_PSR_ENABLE));
-
-		I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
-
-		dev_priv->psr.active = false;
-	}
-
-}
-
-void intel_edp_psr_invalidate(struct drm_device *dev,
-			      unsigned frontbuffer_bits)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc;
-	enum pipe pipe;
-
-	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;
-
-	intel_edp_psr_do_exit(dev);
-
-	frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
-
-	dev_priv->psr.busy_frontbuffer_bits |= frontbuffer_bits;
-	mutex_unlock(&dev_priv->psr.lock);
-}
-
-void intel_edp_psr_flush(struct drm_device *dev,
-			 unsigned frontbuffer_bits)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc;
-	enum pipe pipe;
-
-	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;
-	dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
-
-	/*
-	 * On Haswell sprite plane updates don't result in a psr invalidating
-	 * signal in the hardware. Which means we need to manually fake this in
-	 * software for all flushes, not just when we've seen a preceding
-	 * invalidation through frontbuffer rendering.
-	 */
-	if (IS_HASWELL(dev) &&
-	    (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
-		intel_edp_psr_do_exit(dev);
-
-	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
-		schedule_delayed_work(&dev_priv->psr.work,
-				      msecs_to_jiffies(100));
-	mutex_unlock(&dev_priv->psr.lock);
-}
-
-void intel_edp_psr_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work);
-	mutex_init(&dev_priv->psr.lock);
-}
-
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -5120,7 +4736,7 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 	 * hard to tell without seeing the user of this function of this code.
 	 * Check locking and ordering once that lands.
 	 */
-	if (INTEL_INFO(dev)->gen < 8 && intel_edp_is_psr_enabled(dev)) {
+	if (INTEL_INFO(dev)->gen < 8 && intel_psr_is_enabled(dev)) {
 		DRM_DEBUG_KMS("DRRS is disabled as PSR is enabled\n");
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5c622ad..df25a09 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -992,21 +992,16 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp);
 void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
 void intel_edp_panel_on(struct intel_dp *intel_dp);
 void intel_edp_panel_off(struct intel_dp *intel_dp);
-void intel_edp_psr_enable(struct intel_dp *intel_dp);
-void intel_edp_psr_disable(struct intel_dp *intel_dp);
 void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate);
-void intel_edp_psr_invalidate(struct drm_device *dev,
-			      unsigned frontbuffer_bits);
-void intel_edp_psr_flush(struct drm_device *dev,
-			 unsigned frontbuffer_bits);
-void intel_edp_psr_init(struct drm_device *dev);
-
 void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
 void intel_dp_mst_suspend(struct drm_device *dev);
 void intel_dp_mst_resume(struct drm_device *dev);
 int intel_dp_max_link_bw(struct intel_dp *intel_dp);
 void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
 void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
+uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
+void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes);
+
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
 void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
@@ -1107,6 +1102,16 @@ extern struct drm_display_mode *intel_find_panel_downclock(
 				struct drm_display_mode *fixed_mode,
 				struct drm_connector *connector);
 
+/* intel_psr.c */
+bool intel_psr_is_enabled(struct drm_device *dev);
+void intel_psr_enable(struct intel_dp *intel_dp);
+void intel_psr_disable(struct intel_dp *intel_dp);
+void intel_psr_invalidate(struct drm_device *dev,
+			      unsigned frontbuffer_bits);
+void intel_psr_flush(struct drm_device *dev,
+			 unsigned frontbuffer_bits);
+void intel_psr_init(struct drm_device *dev);
+
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
 void intel_power_domains_fini(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 58cf2e6..79f6d72 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -156,7 +156,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
 
 	intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
 
-	intel_edp_psr_invalidate(dev, obj->frontbuffer_bits);
+	intel_psr_invalidate(dev, obj->frontbuffer_bits);
 }
 
 /**
@@ -182,7 +182,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
 
 	intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
 
-	intel_edp_psr_flush(dev, frontbuffer_bits);
+	intel_psr_flush(dev, frontbuffer_bits);
 
 	/*
 	 * FIXME: Unconditional fbc flushing here is a rather gross hack and
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
new file mode 100644
index 0000000..793f491
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -0,0 +1,413 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drmP.h>
+
+#include "intel_drv.h"
+#include "i915_drv.h"
+
+static bool is_edp_psr(struct intel_dp *intel_dp)
+{
+	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
+}
+
+bool intel_psr_is_enabled(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!HAS_PSR(dev))
+		return false;
+
+	return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
+}
+
+static void intel_psr_write_vsc(struct intel_dp *intel_dp,
+				    struct edp_vsc_psr *vsc_psr)
+{
+	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 = dev->dev_private;
+	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
+	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(crtc->config.cpu_transcoder);
+	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(crtc->config.cpu_transcoder);
+	uint32_t *data = (uint32_t *) vsc_psr;
+	unsigned int i;
+
+	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
+	   the video DIP being updated before program video DIP data buffer
+	   registers for DIP being updated. */
+	I915_WRITE(ctl_reg, 0);
+	POSTING_READ(ctl_reg);
+
+	for (i = 0; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4) {
+		if (i < sizeof(struct edp_vsc_psr))
+			I915_WRITE(data_reg + i, *data++);
+		else
+			I915_WRITE(data_reg + i, 0);
+	}
+
+	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
+	POSTING_READ(ctl_reg);
+}
+
+static void intel_psr_setup_vsc(struct intel_dp *intel_dp)
+{
+	struct edp_vsc_psr psr_vsc;
+
+	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
+	memset(&psr_vsc, 0, sizeof(psr_vsc));
+	psr_vsc.sdp_header.HB0 = 0;
+	psr_vsc.sdp_header.HB1 = 0x7;
+	psr_vsc.sdp_header.HB2 = 0x2;
+	psr_vsc.sdp_header.HB3 = 0x8;
+	intel_psr_write_vsc(intel_dp, &psr_vsc);
+}
+
+static void intel_psr_enable_sink(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 = dev->dev_private;
+	uint32_t aux_clock_divider;
+	int precharge = 0x3;
+	bool only_standby = false;
+	static const uint8_t aux_msg[] = {
+		[0] = DP_AUX_NATIVE_WRITE << 4,
+		[1] = DP_SET_POWER >> 8,
+		[2] = DP_SET_POWER & 0xff,
+		[3] = 1 - 1,
+		[4] = DP_SET_POWER_D0,
+	};
+	int i;
+
+	BUILD_BUG_ON(sizeof(aux_msg) > 20);
+
+	aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
+
+	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
+		only_standby = true;
+
+	/* Enable PSR in sink */
+	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
+	else
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
+
+	/* Setup AUX registers */
+	for (i = 0; i < sizeof(aux_msg); i += 4)
+		I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
+			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
+
+	I915_WRITE(EDP_PSR_AUX_CTL(dev),
+		   DP_AUX_CH_CTL_TIME_OUT_400us |
+		   (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
+		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
+}
+
+static void intel_psr_enable_source(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 = dev->dev_private;
+	uint32_t max_sleep_time = 0x1f;
+	/* Lately it was identified that depending on panel idle frame count
+	 * calculated at HW can be off by 1. So let's use what came
+	 * from VBT + 1 and at minimum 2 to be on the safe side.
+	 */
+	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames ?
+			       dev_priv->vbt.psr.idle_frames + 1 : 2;
+	uint32_t val = 0x0;
+	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
+	bool only_standby = false;
+
+	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
+		only_standby = true;
+
+	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby) {
+		val |= EDP_PSR_LINK_STANDBY;
+		val |= EDP_PSR_TP2_TP3_TIME_0us;
+		val |= EDP_PSR_TP1_TIME_0us;
+		val |= EDP_PSR_SKIP_AUX_EXIT;
+		val |= IS_BROADWELL(dev) ? BDW_PSR_SINGLE_FRAME : 0;
+	} else
+		val |= EDP_PSR_LINK_DISABLE;
+
+	I915_WRITE(EDP_PSR_CTL(dev), val |
+		   (IS_BROADWELL(dev) ? 0 : link_entry_time) |
+		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
+		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
+		   EDP_PSR_ENABLE);
+}
+
+static bool intel_psr_match_conditions(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 = dev->dev_private;
+	struct drm_crtc *crtc = dig_port->base.base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	lockdep_assert_held(&dev_priv->psr.lock);
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
+	dev_priv->psr.source_ok = false;
+
+	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
+		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
+		return false;
+	}
+
+	if (!i915.enable_psr) {
+		DRM_DEBUG_KMS("PSR disable by flag\n");
+		return false;
+	}
+
+	/* Below limitations aren't valid for Broadwell */
+	if (IS_BROADWELL(dev))
+		goto out;
+
+	if (I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config.cpu_transcoder)) &
+	    S3D_ENABLE) {
+		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
+		return false;
+	}
+
+	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
+		return false;
+	}
+
+ out:
+	dev_priv->psr.source_ok = true;
+	return true;
+}
+
+static void intel_psr_do_enable(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 = dev->dev_private;
+
+	WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
+	WARN_ON(dev_priv->psr.active);
+	lockdep_assert_held(&dev_priv->psr.lock);
+
+	/* Enable/Re-enable PSR on the host */
+	intel_psr_enable_source(intel_dp);
+
+	dev_priv->psr.active = true;
+}
+
+void intel_psr_enable(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 = dev->dev_private;
+
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return;
+	}
+
+	if (!is_edp_psr(intel_dp)) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return;
+	}
+
+	mutex_lock(&dev_priv->psr.lock);
+	if (dev_priv->psr.enabled) {
+		DRM_DEBUG_KMS("PSR already in use\n");
+		goto unlock;
+	}
+
+	if (!intel_psr_match_conditions(intel_dp))
+		goto unlock;
+
+	dev_priv->psr.busy_frontbuffer_bits = 0;
+
+	intel_psr_setup_vsc(intel_dp);
+
+	/* Avoid continuous PSR exit by masking memup and hpd */
+	I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
+		   EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
+
+	/* Enable PSR on the panel */
+	intel_psr_enable_sink(intel_dp);
+
+	dev_priv->psr.enabled = intel_dp;
+unlock:
+	mutex_unlock(&dev_priv->psr.lock);
+}
+
+void intel_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 = dev->dev_private;
+
+	mutex_lock(&dev_priv->psr.lock);
+	if (!dev_priv->psr.enabled) {
+		mutex_unlock(&dev_priv->psr.lock);
+		return;
+	}
+
+	if (dev_priv->psr.active) {
+		I915_WRITE(EDP_PSR_CTL(dev),
+			   I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE);
+
+		/* Wait till PSR is idle */
+		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
+			       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
+			DRM_ERROR("Timed out waiting for PSR Idle State\n");
+
+		dev_priv->psr.active = false;
+	} else {
+		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
+	}
+
+	dev_priv->psr.enabled = NULL;
+	mutex_unlock(&dev_priv->psr.lock);
+
+	cancel_delayed_work_sync(&dev_priv->psr.work);
+}
+
+static void intel_psr_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv), psr.work.work);
+	struct intel_dp *intel_dp = dev_priv->psr.enabled;
+
+	/* We have to make sure PSR is ready for re-enable
+	 * otherwise it keeps disabled until next full enable/disable cycle.
+	 * PSR might take some time to get fully disabled
+	 * and be ready for re-enable.
+	 */
+	if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev_priv->dev)) &
+		      EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
+		DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
+		return;
+	}
+
+	mutex_lock(&dev_priv->psr.lock);
+	intel_dp = dev_priv->psr.enabled;
+
+	if (!intel_dp)
+		goto unlock;
+
+	/*
+	 * The delayed work can race with an invalidate hence we need to
+	 * recheck. Since psr_flush first clears this and then reschedules we
+	 * won't ever miss a flush when bailing out here.
+	 */
+	if (dev_priv->psr.busy_frontbuffer_bits)
+		goto unlock;
+
+	intel_psr_do_enable(intel_dp);
+unlock:
+	mutex_unlock(&dev_priv->psr.lock);
+}
+
+static void intel_psr_exit(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (dev_priv->psr.active) {
+		u32 val = I915_READ(EDP_PSR_CTL(dev));
+
+		WARN_ON(!(val & EDP_PSR_ENABLE));
+
+		I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
+
+		dev_priv->psr.active = false;
+	}
+
+}
+
+void intel_psr_invalidate(struct drm_device *dev,
+			      unsigned frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	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;
+
+	intel_psr_exit(dev);
+
+	frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
+
+	dev_priv->psr.busy_frontbuffer_bits |= frontbuffer_bits;
+	mutex_unlock(&dev_priv->psr.lock);
+}
+
+void intel_psr_flush(struct drm_device *dev,
+			 unsigned frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	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;
+	dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
+
+	/*
+	 * On Haswell sprite plane updates don't result in a psr invalidating
+	 * signal in the hardware. Which means we need to manually fake this in
+	 * software for all flushes, not just when we've seen a preceding
+	 * invalidation through frontbuffer rendering.
+	 */
+	if (IS_HASWELL(dev) &&
+	    (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
+		intel_psr_exit(dev);
+
+	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
+		schedule_delayed_work(&dev_priv->psr.work,
+				      msecs_to_jiffies(100));
+	mutex_unlock(&dev_priv->psr.lock);
+}
+
+void intel_psr_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
+	mutex_init(&dev_priv->psr.lock);
+}
-- 
1.9.3

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

[PATCH 4/4] drm/i915: Add PSR docbook

[Rodrigo Vivi]

Let's document PSR a bit. No functional changes.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 58 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 793f491..5f350f0 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -21,6 +21,26 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+/**
+ * DOC: Panel Self Refresh (PSR/SRD)
+ *
+ * Since 4th Generation Intel® Core™ processors with Intel® HD Graphics
+ * (Haswell) Display controller supports Panel Self-Refresh on display panels
+ * witch have a remote frame buffer (RFB) implemented according to PSR spec in
+ * eDP1.3. PSR feature allows the display to go to lower standby states when
+ * system is idle but display is on as it eliminates diplay refresh request
+ * to DDR memory completely as long as the frame buffer for that display is
+ * unchanged.
+ *
+ * Panel Self Refresh must be supported by both Hardware (source) and
+ * Panel (sink).
+ *
+ * The PSR feature enables system-level power savings when the displayed image
+ * remains static for multiple display frames. The Sink device stores a static
+ * image locally in the RFB within the Sink device, and displays this image
+ * from the RFB, while the eDP Main-Link can be turned OFF.
+ */
+
 #include <drm/drmP.h>
 
 #include "intel_drv.h"
@@ -222,6 +242,13 @@ static void intel_psr_do_enable(struct intel_dp *intel_dp)
 	dev_priv->psr.active = true;
 }
 
+/**
+ * intel_psr_enable - Enable PSR
+ * @intel_dp: Intel DP
+ *
+ * This function gets called every time eDP panel is turned on. Right after
+ * turning backlight on.
+ */
 void intel_psr_enable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -263,6 +290,13 @@ unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
+/**
+ * intel_psr_disable - Disable PSR
+ * @intel_dp: Intel DP
+ *
+ * This function gets called every time eDP panel is turned off. Right before
+ * turning backlight off.
+ */
 void intel_psr_disable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -347,6 +381,16 @@ static void intel_psr_exit(struct drm_device *dev)
 
 }
 
+/**
+ * intel_psr_invalidate - Invalidade PSR
+ * @dev: DRM device
+ * @frontbuffer_bits: frontbuffer plane tracking bits
+ *
+ * Hardware (source) is usually the responsible for identifying when the screen
+ * changed and Remote Frame Buffer must be updated. However this HW tracking
+ * doesn't cover all cases. So we can take advantage of frontbuffer tracking
+ * to force a psr_exit when driver knows RFB must be updated.
+ */
 void intel_psr_invalidate(struct drm_device *dev,
 			      unsigned frontbuffer_bits)
 {
@@ -371,6 +415,14 @@ void intel_psr_invalidate(struct drm_device *dev,
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
+/**
+ * intel_psr_flush - Flush PSR
+ * @dev: DRM device
+ * @frontbuffer_bits: frontbuffer plane tracking bits
+ *
+ * After PSR got invalidated forcing PSR exit and RFB update driver must also
+ * take care to flush it allowing PSR to come back to regular work.
+ */
 void intel_psr_flush(struct drm_device *dev,
 			 unsigned frontbuffer_bits)
 {
@@ -404,6 +456,12 @@ void intel_psr_flush(struct drm_device *dev,
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
+/**
+ * intel_psr_init - Init basic PSR work and mutex.
+ * @dev: DRM device
+ *
+ * This function is to be called only once to initialize basic PSR stuff.
+ */
 void intel_psr_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.9.3

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

Re: [PATCH 1/4] drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1

[Daniel Vetter]

On Fri, Nov 07, 2014 at 03:55:15PM -0800, Rodrigo Vivi wrote:
> Let's use VBT + 1 now we parse it.

This patch completely lacks the justification for why we need this. Please
add a short summary of our previous discussion with Art.

> 
> v2: fix subject and consider that idle_frame = 2 is another fix.
> 
> Cc: Arthur Runyan <arthur.j.runyan@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 20beb59..41df221 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2174,7 +2174,12 @@ static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
>  	struct drm_device *dev = dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t max_sleep_time = 0x1f;
> -	uint32_t idle_frames = 2; /* 2 is the minimum allowed */

Doesn't apply because the v1 series seems to be missing. I tried to fix
that but then it wouldn't compile because I'm missing the vbt patch.

Please always send the full patch series or mention unmerged depencies in
the cover letter.
-Daniel
> +	/* Lately it was identified that depending on panel idle frame count
> +	 * calculated at HW can be off by 1. So let's use what came
> +	 * from VBT + 1 and at minimum 2 to stay on the safest side.
> +	 */
> +	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames ?
> +			       dev_priv->vbt.psr.idle_frames + 1 : 2;
>  	uint32_t val = 0x0;
>  	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>  	bool only_standby = false;
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Add PSR docbook

[Daniel Vetter]

On Fri, Nov 07, 2014 at 03:55:18PM -0800, Rodrigo Vivi wrote:
> Let's document PSR a bit. No functional changes.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

I've tried to merge the preceeding patches to avoid rebase pain, but
there's conflicts. I think it'd be best if we get those patches into shape
first. A few comments below to clarify the kerneldoc.

Thanks for doing this, docs are always highly appreciated!
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 58 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 793f491..5f350f0 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -21,6 +21,26 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>  
> +/**
> + * DOC: Panel Self Refresh (PSR/SRD)
> + *
> + * Since 4th Generation Intel® Core™ processors with Intel® HD Graphics
> + * (Haswell) Display controller supports Panel Self-Refresh on display panels
> + * witch have a remote frame buffer (RFB) implemented according to PSR spec in
> + * eDP1.3. PSR feature allows the display to go to lower standby states when
> + * system is idle but display is on as it eliminates diplay refresh request
> + * to DDR memory completely as long as the frame buffer for that display is
> + * unchanged.
> + *
> + * Panel Self Refresh must be supported by both Hardware (source) and
> + * Panel (sink).
> + *
> + * The PSR feature enables system-level power savings when the displayed image
> + * remains static for multiple display frames. The Sink device stores a static
> + * image locally in the RFB within the Sink device, and displays this image
> + * from the RFB, while the eDP Main-Link can be turned OFF.

This smells a lot like a marketing copy. Generally in code we don't use
marketing names but e.g. hsw.

Also it misses to explain the tricky implementation details, which is
what's actually important for the implementation. That psr has nice power
saving benefits is important, but I wouldn't put more than a sentence into
that, e.g.

"PSR saves power by caching the framebuffer in the panel, which allows us
to power down the link and memory controller. For DSI panels the same idea
is called "manual mode".

"The implementation uses the hardware-based PSR support which automatically
enters/exits self-refresh mode. The hardware takes care of sending the
required DP aux message and could even retrain the link (that part isn't
enabled yet though). The hardware also keeps track of any frontbuffer
changes to know when to exit self-refresh mode again. Unfortunately that
part doesn't work too well, hence why the i915 PSR support uses the
software frontbuffer tracking to make sure it doesn't miss a screen
update. For this integration intel_psr_invalidate() and intel_psr_flush()
get called by the frontbuffer tracking code. Note that because of locking
issues the self-refresh re-enable code is done from a work queue, which
must be correctly synchronized/cancelled when shutting down the pipe."

This way docs raise the tricky parts of the implementation and how/where
this integrates with the driver.
> + */
> +
>  #include <drm/drmP.h>
>  
>  #include "intel_drv.h"
> @@ -222,6 +242,13 @@ static void intel_psr_do_enable(struct intel_dp *intel_dp)
>  	dev_priv->psr.active = true;
>  }
>  
> +/**
> + * intel_psr_enable - Enable PSR
> + * @intel_dp: Intel DP
> + *
> + * This function gets called every time eDP panel is turned on. Right after
> + * turning backlight on.

Does the backlight really matter? Should this be: "This can only be called
after the pipe is fully trained and enabled."?
> + */
>  void intel_psr_enable(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -263,6 +290,13 @@ unlock:
>  	mutex_unlock(&dev_priv->psr.lock);
>  }
>  
> +/**
> + * intel_psr_disable - Disable PSR
> + * @intel_dp: Intel DP
> + *
> + * This function gets called every time eDP panel is turned off. Right before
> + * turning backlight off.

Same here.

> + */
>  void intel_psr_disable(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -347,6 +381,16 @@ static void intel_psr_exit(struct drm_device *dev)
>  
>  }
>  
> +/**
> + * intel_psr_invalidate - Invalidade PSR
> + * @dev: DRM device
> + * @frontbuffer_bits: frontbuffer plane tracking bits
> + *
> + * Hardware (source) is usually the responsible for identifying when the screen
> + * changed and Remote Frame Buffer must be updated. However this HW tracking
> + * doesn't cover all cases. So we can take advantage of frontbuffer tracking
> + * to force a psr_exit when driver knows RFB must be updated.

Source hardware isn't responsible for tracking, this is just how it's
implemented on intel hw. You can do psr in software. What about:

"Since the hardware frontbuffer tracking has gaps we need to integrate
with the software frontbuffer tracking. This function gets called every
time frontbuffer rendering has completed and flushed out to memory. PSR
can be enabled again if no other frontbuffer relevant to PSR is dirty.

Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."

> + */
>  void intel_psr_invalidate(struct drm_device *dev,
>  			      unsigned frontbuffer_bits)
>  {
> @@ -371,6 +415,14 @@ void intel_psr_invalidate(struct drm_device *dev,
>  	mutex_unlock(&dev_priv->psr.lock);
>  }
>  
> +/**
> + * intel_psr_flush - Flush PSR
> + * @dev: DRM device
> + * @frontbuffer_bits: frontbuffer plane tracking bits
> + *
> + * After PSR got invalidated forcing PSR exit and RFB update driver must also
> + * take care to flush it allowing PSR to come back to regular work.

"Since the hardware frontbuffer tracking has gaps we need to integrate
with the software frontbuffer tracking. This function gets called every
time frontbuffer rendering starts and a buffer gets dirtied. PSR must be
disabled if the frontbuffer mask contains a buffer relevant to PSR.

Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."
> + */
>  void intel_psr_flush(struct drm_device *dev,
>  			 unsigned frontbuffer_bits)
>  {
> @@ -404,6 +456,12 @@ void intel_psr_flush(struct drm_device *dev,
>  	mutex_unlock(&dev_priv->psr.lock);
>  }
>  
> +/**
> + * intel_psr_init - Init basic PSR work and mutex.
> + * @dev: DRM device
> + *
> + * This function is to be called only once to initialize basic PSR stuff.

s/to be// and maybe s/once/once at driver load/

> + */
>  void intel_psr_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Add PSR docbook

[Rodrigo Vivi]

On Wed, Nov 12, 2014 at 1:16 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 07, 2014 at 03:55:18PM -0800, Rodrigo Vivi wrote:
>> Let's document PSR a bit. No functional changes.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> I've tried to merge the preceeding patches to avoid rebase pain, but
> there's conflicts. I think it'd be best if we get those patches into shape
> first. A few comments below to clarify the kerneldoc.


Thanks. I started the rebase yesterday putting it on begin of the
series than vbt work and at least 1 or 2 more coming.
Also I'll send along with vlv/chv rebased... Putting everything
organized together.

>
> Thanks for doing this, docs are always highly appreciated!
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 58 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 58 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 793f491..5f350f0 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -21,6 +21,26 @@
>>   * DEALINGS IN THE SOFTWARE.
>>   */
>>
>> +/**
>> + * DOC: Panel Self Refresh (PSR/SRD)
>> + *
>> + * Since 4th Generation Intel® Core™ processors with Intel® HD Graphics
>> + * (Haswell) Display controller supports Panel Self-Refresh on display panels
>> + * witch have a remote frame buffer (RFB) implemented according to PSR spec in
>> + * eDP1.3. PSR feature allows the display to go to lower standby states when
>> + * system is idle but display is on as it eliminates diplay refresh request
>> + * to DDR memory completely as long as the frame buffer for that display is
>> + * unchanged.
>> + *
>> + * Panel Self Refresh must be supported by both Hardware (source) and
>> + * Panel (sink).
>> + *
>> + * The PSR feature enables system-level power savings when the displayed image
>> + * remains static for multiple display frames. The Sink device stores a static
>> + * image locally in the RFB within the Sink device, and displays this image
>> + * from the RFB, while the eDP Main-Link can be turned OFF.
>
> This smells a lot like a marketing copy. Generally in code we don't use
> marketing names but e.g. hsw.

I got vesa spec + our pm enabling guide and put the market names by
myself. Used to use market names on stack releases.

I believe that on docs that is visible to end users we could use more
market names besides the codenames.

>
> Also it misses to explain the tricky implementation details, which is
> what's actually important for the implementation. That psr has nice power
> saving benefits is important, but I wouldn't put more than a sentence into
> that, e.g.
>
> "PSR saves power by caching the framebuffer in the panel, which allows us
> to power down the link and memory controller. For DSI panels the same idea
> is called "manual mode".
>
> "The implementation uses the hardware-based PSR support which automatically
> enters/exits self-refresh mode. The hardware takes care of sending the
> required DP aux message and could even retrain the link (that part isn't
> enabled yet though). The hardware also keeps track of any frontbuffer
> changes to know when to exit self-refresh mode again. Unfortunately that
> part doesn't work too well, hence why the i915 PSR support uses the
> software frontbuffer tracking to make sure it doesn't miss a screen
> update. For this integration intel_psr_invalidate() and intel_psr_flush()
> get called by the frontbuffer tracking code. Note that because of locking
> issues the self-refresh re-enable code is done from a work queue, which
> must be correctly synchronized/cancelled when shutting down the pipe."
>
> This way docs raise the tricky parts of the implementation and how/where
> this integrates with the driver.
>> + */

Good. Thanks!

>> +
>>  #include <drm/drmP.h>
>>
>>  #include "intel_drv.h"
>> @@ -222,6 +242,13 @@ static void intel_psr_do_enable(struct intel_dp *intel_dp)
>>       dev_priv->psr.active = true;
>>  }
>>
>> +/**
>> + * intel_psr_enable - Enable PSR
>> + * @intel_dp: Intel DP
>> + *
>> + * This function gets called every time eDP panel is turned on. Right after
>> + * turning backlight on.
>
> Does the backlight really matter? Should this be: "This can only be called
> after the pipe is fully trained and enabled."?

definitely better. Thanks.

>> + */
>>  void intel_psr_enable(struct intel_dp *intel_dp)
>>  {
>>       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> @@ -263,6 +290,13 @@ unlock:
>>       mutex_unlock(&dev_priv->psr.lock);
>>  }
>>
>> +/**
>> + * intel_psr_disable - Disable PSR
>> + * @intel_dp: Intel DP
>> + *
>> + * This function gets called every time eDP panel is turned off. Right before
>> + * turning backlight off.
>
> Same here.

noted.
>
>> + */
>>  void intel_psr_disable(struct intel_dp *intel_dp)
>>  {
>>       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> @@ -347,6 +381,16 @@ static void intel_psr_exit(struct drm_device *dev)
>>
>>  }
>>
>> +/**
>> + * intel_psr_invalidate - Invalidade PSR
>> + * @dev: DRM device
>> + * @frontbuffer_bits: frontbuffer plane tracking bits
>> + *
>> + * Hardware (source) is usually the responsible for identifying when the screen
>> + * changed and Remote Frame Buffer must be updated. However this HW tracking
>> + * doesn't cover all cases. So we can take advantage of frontbuffer tracking
>> + * to force a psr_exit when driver knows RFB must be updated.
>
> Source hardware isn't responsible for tracking, this is just how it's
> implemented on intel hw. You can do psr in software. What about:
>
> "Since the hardware frontbuffer tracking has gaps we need to integrate
> with the software frontbuffer tracking. This function gets called every
> time frontbuffer rendering has completed and flushed out to memory. PSR
> can be enabled again if no other frontbuffer relevant to PSR is dirty.
>
> Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."

yeah. I'd have to change that for vlv/chv anyway where hw can never track.
thanks.

>
>> + */
>>  void intel_psr_invalidate(struct drm_device *dev,
>>                             unsigned frontbuffer_bits)
>>  {
>> @@ -371,6 +415,14 @@ void intel_psr_invalidate(struct drm_device *dev,
>>       mutex_unlock(&dev_priv->psr.lock);
>>  }
>>
>> +/**
>> + * intel_psr_flush - Flush PSR
>> + * @dev: DRM device
>> + * @frontbuffer_bits: frontbuffer plane tracking bits
>> + *
>> + * After PSR got invalidated forcing PSR exit and RFB update driver must also
>> + * take care to flush it allowing PSR to come back to regular work.
>
> "Since the hardware frontbuffer tracking has gaps we need to integrate
> with the software frontbuffer tracking. This function gets called every
> time frontbuffer rendering starts and a buffer gets dirtied. PSR must be
> disabled if the frontbuffer mask contains a buffer relevant to PSR.
>
> Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."

noted

>> + */
>>  void intel_psr_flush(struct drm_device *dev,
>>                        unsigned frontbuffer_bits)
>>  {
>> @@ -404,6 +456,12 @@ void intel_psr_flush(struct drm_device *dev,
>>       mutex_unlock(&dev_priv->psr.lock);
>>  }
>>
>> +/**
>> + * intel_psr_init - Init basic PSR work and mutex.
>> + * @dev: DRM device
>> + *
>> + * This function is to be called only once to initialize basic PSR stuff.
>
> s/to be// and maybe s/once/once at driver load/

noted

thank you very much for all suggestions.

>
>> + */
>>  void intel_psr_init(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> --
>> 1.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Add PSR docbook

[Daniel Vetter]

On Wed, Nov 12, 2014 at 08:00:26AM -0800, Rodrigo Vivi wrote:
> I believe that on docs that is visible to end users we could use more
> market names besides the codenames.

This isn't documentation for end users but developers working on i915. So
aligning with the codenames used in the driver is more important imo than
aligning with marketing. We could do an overview section in the docs to
match up marketing names with codenames in the driver (or explain stuff
like what IS_VALLEYVIEW means ...). Or just do a link to wikipedia, since
that page is usually really accurate ;-)

But imo that should be pulled together into a separate section.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx