[PATCH 00/10] PSR VLV-Braytrail and CHV-Braswell

[PATCH 00/10] PSR VLV-Braytrail and CHV-Braswell

[Rodrigo Vivi]

This series introduce support for PSR on Valleyview and Cherryview plataforms.

On these platforms the PSR implementation on Hardware was completelly different from what we had on
HSW/BDW so I put more comment and function headers to explain it better to help reviewer.

I had giving up on PSR Baytrail, but when implementing it for Braswell I noticed that I had fixes for my old
issues and when I was fixing it I decided move everything over SW timer control and it worked pretty nice.
At least nice enough.

Unfortunatelly on Baytrail it doesn't look so nice yet. But it seems a frontbuffer tracking issue that I'll debug
more later.

On both BYT and BSW I couldn't validate it with kms_psr_sink_crc because of a known bug:
https://bugs.freedesktop.org/show_bug.cgi?id=84890
That gets even worse with PSR enabled.

Anyway PSR is disabled by default and this patch series is the way to go with PSR on these 2 platforms.
So I decided to submit patches for review while I continue working to track and fix the remaining 2 bugs
that unblock validation more eyes on this code helps a lot.

Also any other idea/suggestion about frontbuffer tracking issue on BYT or this sink CRC issue on both platforms are welcome.

Thanks,
Rodrigo.

Rodrigo Vivi (10):
  drm/i915: Add PSR registers for PSR VLV/CHV.
  drm/i915: VLV PSR: Status/enabled function.
  drm/i915: Rename psr_do_exit/psr_exit.
  drm/i915: PSR VLV/CHV: Introduce setup, enable and disable functions
  drm/i915: PSR activate function.
  drm/i915: VLV/CHV PSR Software timer mode control
  drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
  drm/i915: VLV/CHV PSR debugfs.
  drm/i915: PSR Baytrail: Not available on Pipe C.
  drm/i915: Enable PSR for Baytrail and Braswell.

 drivers/gpu/drm/i915/i915_debugfs.c |  34 +++-
 drivers/gpu/drm/i915/i915_drv.h     |   3 +-
 drivers/gpu/drm/i915/i915_reg.h     |  37 ++++
 drivers/gpu/drm/i915/intel_dp.c     | 331 ++++++++++++++++++++++++++++++++----
 4 files changed, 367 insertions(+), 38 deletions(-)

-- 
1.9.3

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

[PATCH 01/10] drm/i915: Add PSR registers for PSR VLV/CHV.

[Rodrigo Vivi]

Baytrail (Valleyview) and Braswell (Cherryview) uses a complete different
implementation of PSR that we currently have supported for
Haswell and Broadwell. So let's start by adding registers definitions.

I usually don't like commit that adds just registers without using,
but after I put all in one commit I realized that no one would want
to take the AR to review it so I decided to split in order to make
reviewer's life easier. Only last commit in this series will actually
enable the PSR on intel enable panel path.

But as it happens currently with HSW/BDW the plan is to let it
disabled by default (protected by kernel parameter)
while we are able to fully validate it.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 77fce96..771780a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2509,6 +2509,43 @@ enum punit_power_well {
 #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
 #define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
 
+/* VLV eDP PSR registers */
+#define _PSRCTLA				(VLV_DISPLAY_BASE + 0x60090)
+#define _PSRCTLB				(VLV_DISPLAY_BASE + 0x61090)
+#define  VLV_EDP_PSR_ENABLE			(1<<0)
+#define  VLV_EDP_PSR_RESET			(1<<1)
+#define  VLV_EDP_PSR_MODE_MASK			(7<<2)
+#define  VLV_EDP_PSR_MODE_HW_TIMER		(1<<3)
+#define  VLV_EDP_PSR_MODE_SW_TIMER		(1<<2)
+#define  VLV_EDP_PSR_SINGLE_FRAME_UPDATE	(1<<7)
+#define  VLV_EDP_PSR_ACTIVE_ENTRY		(1<<8)
+#define  VLV_EDP_PSR_SRC_TRANSMITTER_STATE	(1<<9)
+#define  VLV_EDP_PSR_DBL_FRAME			(1<<10)
+#define  VLV_EDP_PSR_FRAME_COUNT_MASK		(0xff<<16)
+#define  VLV_EDP_PSR_IDLE_FRAME_SHIFT		16
+#define  VLV_EDP_PSR_INT_TRANSITION		(1<<24)
+#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB)
+
+#define _VSCSDPA			(VLV_DISPLAY_BASE + 0x600a0)
+#define _VSCSDPB			(VLV_DISPLAY_BASE + 0x610a0)
+#define  VLV_EDP_PSR_SDP_FREQ_MASK	(3<<30)
+#define  VLV_EDP_PSR_SDP_FREQ_ONCE	(1<<31)
+#define  VLV_EDP_PSR_SDP_FREQ_EVFRAME	(1<<30)
+#define VLV_VSCSDP(pipe)	_PIPE(pipe, _VSCSDPA, _VSCSDPB)
+
+#define _PSRSTATA			(VLV_DISPLAY_BASE + 0x60094)
+#define _PSRSTATB			(VLV_DISPLAY_BASE + 0x61094)
+#define  VLV_EDP_PSR_LAST_STATE_MASK	(7<<3)
+#define  VLV_EDP_PSR_CURR_STATE_MASK	7
+#define  VLV_EDP_PSR_DISABLED		(0<<0)
+#define  VLV_EDP_PSR_INACTIVE		(1<<0)
+#define  VLV_EDP_PSR_IN_TRANS_TO_ACTIVE	(2<<0)
+#define  VLV_EDP_PSR_ACTIVE_NORFB_UP	(3<<0)
+#define  VLV_EDP_PSR_ACTIVE_SF_UPDATE	(4<<0)
+#define  VLV_EDP_PSR_EXIT		(5<<0)
+#define  VLV_EDP_PSR_IN_TRANS		(1<<7)
+#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB)
+
 /* HSW+ eDP PSR registers */
 #define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
 #define EDP_PSR_CTL(dev)			(EDP_PSR_BASE(dev) + 0)
-- 
1.9.3

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

[PATCH 02/10] drm/i915: VLV PSR: Status/enabled function.

[Rodrigo Vivi]

VLV PSR support PSR per pipe, including the status. So we have to check
if it is enabled per pipe on status.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 82e47da..774eb6c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2076,7 +2076,26 @@ 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)
+/**
+ * vlv_edp_is_psr_enabled_on_pipe
+ * @dev: DRM device
+ * @pipe: pipe to check for PSR status.
+ *
+ * VLV PSR implements PSR per pipe. This function allows to check if it is
+ * enabled on given pipe.
+ */
+static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	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 bool hsw_edp_is_psr_enabled(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -5066,7 +5085,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 (IS_HASWELL(dev) && hsw_edp_is_psr_enabled(dev)) {
 		DRM_DEBUG_KMS("DRRS is disabled as PSR is enabled\n");
 		return;
 	}
-- 
1.9.3

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

[PATCH 03/10] drm/i915: Rename psr_do_exit/psr_exit.

[Rodrigo Vivi]

Just a bit of organization and comment.
 In the past we had two functions psr_exit
and psr_do_exit. psr_exit is the propper name now.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 774eb6c..ac70b3a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2388,7 +2388,14 @@ unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-static void intel_edp_psr_do_exit(struct drm_device *dev)
+/**
+ * intel_edp_is_psr_exit
+ * @dev: DRM device
+ *
+ * This function immediately inactivate PSR. In other words it forces a
+ * transition from PSR Entry state to PSR Exit/Inactive.
+ */
+static void intel_edp_psr_exit(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -2420,7 +2427,7 @@ void intel_edp_psr_invalidate(struct drm_device *dev,
 	crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
 	pipe = to_intel_crtc(crtc)->pipe;
 
-	intel_edp_psr_do_exit(dev);
+	intel_edp_psr_exit(dev);
 
 	frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
 
@@ -2453,7 +2460,7 @@ void intel_edp_psr_flush(struct drm_device *dev,
 	 */
 	if (IS_HASWELL(dev) &&
 	    (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
-		intel_edp_psr_do_exit(dev);
+		intel_edp_psr_exit(dev);
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
 		schedule_delayed_work(&dev_priv->psr.work,
-- 
1.9.3

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

[PATCH 04/10] drm/i915: PSR VLV/CHV: Introduce setup, enable and disable functions

[Rodrigo Vivi]

The biggest difference from HSW/BDW PSR here is that VLV enable_source
function enables PSR but let it in Inactive state. So it might be called
on early stage along with setup and enable_sink ones.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ac70b3a..335a711 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2134,7 +2134,44 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp,
 	POSTING_READ(ctl_reg);
 }
 
-static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp)
+/**
+ * vlv_edp_psr_setup_vsc - Setup PSR related VSC aux registers on VLV+.
+ * @intel_dp: DP struct
+ *
+ * On VLV we don't need to generate VSC. It auto generates according
+ * EDP 1.3 spec, Table 3.10.
+ * HB0   - Secondary Data Packet ID = 0
+ * HB1   - Secondary Data Packet Type = 07h
+ * HB2   - Bits 4:0 = Revision Number = 02h
+ *         Bits 7:5 = Reserved (all 0s)
+ * HB3   - Bits 4:0 Number of Valid Data Bytes = 08h
+ *         Bits 7:5 Reserved
+ * DB0   - Bits 7:4 Stereo Interface Method Specific Paramenter
+ *         Bits 3:0 Stereo Interface Method Code
+ * DB1   - Bit 0 - PSR State (0 when Inactive / 1 when Active)
+ *         Bit 1 - Update RFB. (0 do not update RFB / 1 update RFB)
+ *         Bit 2 - CRC Valid. VLV always send 0 which
+ *                            means CRC value in DB7:2 invalid
+ *         Bits 7:3 - Reserved
+ * DB2-7 - CRC - don't care because DB1[2] always tells this is invalid.
+ */
+static void vlv_edp_psr_setup_vsc(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;
+	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	uint32_t val;
+
+	/* VLV auto-generate VSC package as per EDP 1.3 spec, Table 3.10 */
+	val  = I915_READ(VLV_VSCSDP(pipe));
+	val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
+	val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
+	I915_WRITE(VLV_VSCSDP(pipe), val);
+}
+
+static void hsw_edp_psr_setup_vsc(struct intel_dp *intel_dp)
 {
 	struct edp_vsc_psr psr_vsc;
 
@@ -2147,7 +2184,20 @@ static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp)
 	intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
 }
 
-static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
+/**
+ * vlv_edp_psr_enable_sink - Enable PSR on Panel for VLV.
+ * @intel_dp: DP struct
+ *
+ * This function enable PSR on Panel (Sink) side over DPCD write.
+ */
+static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp)
+{
+	/* Enable PSR in sink */
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+			   DP_PSR_ENABLE);
+}
+
+static void hsw_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;
@@ -2191,7 +2241,28 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
 }
 
-static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
+/**
+ * vlv_edp_psr_enable_source - Enable PSR on VLV without activate it.
+ * @intel_dp: DP struct
+ *
+ * This function do the transition from PSR_state 0 to PSR_state 1 that is
+ * PSR Inactive one..
+ */
+static void vlv_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;
+	struct drm_crtc *crtc = dig_port->base.base.crtc;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	I915_WRITE(VLV_PSRCTL(pipe),
+		   VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
+		   VLV_EDP_PSR_MODE_SW_TIMER |
+		   VLV_EDP_PSR_ENABLE);
+}
+
+static void hsw_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;
@@ -2276,7 +2347,7 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	lockdep_assert_held(&dev_priv->psr.lock);
 
 	/* Enable/Re-enable PSR on the host */
-	intel_edp_psr_enable_source(intel_dp);
+	hsw_edp_psr_enable_source(intel_dp);
 
 	dev_priv->psr.active = true;
 }
@@ -2307,30 +2378,71 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
 
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
-	intel_edp_psr_setup_vsc(intel_dp);
+	if (HAS_DDI(dev)) {
+		hsw_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);
 
-	/* 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 */
+		hsw_edp_psr_enable_sink(intel_dp);
+	} else {
+		vlv_edp_psr_setup_vsc(intel_dp);
 
-	/* Enable PSR on the panel */
-	intel_edp_psr_enable_sink(intel_dp);
+		/* Enable PSR on the panel */
+		vlv_edp_psr_enable_sink(intel_dp);
+
+		/* On HSW/BDW enable_source also means go to PSR entry/active
+		 * state as soon as idle_frame achieved and here would be
+		 * to soon. However on VLV enable_source just enable PSR
+		 * but let it on inactive state. So we might do this prior
+		 * to active transition, i.e. here.
+		 */
+		vlv_edp_psr_enable_source(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)
+/**
+ * vlv_edp_psr_disable - Disable PSR on VLV
+ * @intel_dp: DP struct
+ *
+ * This function puts PSR VLV back to PSR_state 0 that is PSR Disabled.
+ */
+static void vlv_edp_psr_disable(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(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;
+	struct intel_crtc *intel_crtc =
+		to_intel_crtc(intel_dig_port->base.base.crtc);
+	uint32_t val;
 
-	mutex_lock(&dev_priv->psr.lock);
-	if (!dev_priv->psr.enabled) {
-		mutex_unlock(&dev_priv->psr.lock);
-		return;
+	if (dev_priv->psr.active) {
+		if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
+			      VLV_EDP_PSR_IN_TRANS) == 0, 250))
+			WARN(1, "PSR transition took longer than expected\n");
+
+		val = I915_READ(VLV_PSRCTL(intel_crtc->pipe));
+		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
+		val &= ~VLV_EDP_PSR_ENABLE;
+		val &= ~VLV_EDP_PSR_MODE_MASK;
+		I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val);
+
+		dev_priv->psr.active = false;
+	} else {
+		WARN_ON(vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe));
 	}
+}
+
+static void hsw_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;
 
 	if (dev_priv->psr.active) {
 		I915_WRITE(EDP_PSR_CTL(dev),
@@ -2345,6 +2457,23 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
 	} else {
 		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
 	}
+}
+
+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 (HAS_DDI(dev))
+		hsw_edp_psr_disable(intel_dp);
+	else
+		vlv_edp_psr_disable(intel_dp);
 
 	dev_priv->psr.enabled = NULL;
 	mutex_unlock(&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 05/10] drm/i915: PSR activate function.

[Rodrigo Vivi]

Let's clear a bit the difference between enable_source and activate
and explain it on comments and docs.

After that we will be able to introduce inactivate/activate VLV+
functions

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 335a711..813537a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2197,6 +2197,14 @@ static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp)
 			   DP_PSR_ENABLE);
 }
 
+/**
+ * hsw_edp_psr_enable_sink - Enable PSR on Panel for HSW+.
+ * @intel_dp: DP struct
+ *
+ * This function enable PSR on Panel (Sink) side.
+ * On HSW+ platform when enabling PSR on Panel we need also to setup aux
+ * registers.
+ */
 static void hsw_edp_psr_enable_sink(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -2262,6 +2270,16 @@ static void vlv_edp_psr_enable_source(struct intel_dp *intel_dp)
 		   VLV_EDP_PSR_ENABLE);
 }
 
+/**
+ * hsw_edp_psr_enable_source - Enable PSR on Source.
+ * @intel_dp: DP struct
+ *
+ * This function enable PSR on Platform (Source) side.
+ * On HSW+ platform when enabling PSR on Source it also allow Hardware to
+ * immediately transitioning to PSR Entry/Active state as soon as it matches
+ * configure idle_frame count. So this function might be called only when
+ * we really can go to activate state.
+ */
 static void hsw_edp_psr_enable_source(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -2336,7 +2354,13 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
-static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
+/**
+ * intel_edp_psr_activate - Activate PSR
+ * @intel_dp: DP struct
+ *
+ * This function allows PSR to go to Entry/Active state as soon as possible.
+ */
+static void intel_edp_psr_activate(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;
@@ -2346,7 +2370,11 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
-	/* Enable/Re-enable PSR on the host */
+	/* Enable/Re-enable PSR on the host
+	 * On HSW/BDW after we enable PSR on source it will activate it
+	 * as soon as it match configure idle_frame count. So
+	 * we just actually enable it here on activation time.
+	 */
 	hsw_edp_psr_enable_source(intel_dp);
 
 	dev_priv->psr.active = true;
@@ -2512,7 +2540,7 @@ static void intel_edp_psr_work(struct work_struct *work)
 	if (dev_priv->psr.busy_frontbuffer_bits)
 		goto unlock;
 
-	intel_edp_psr_do_enable(intel_dp);
+	intel_edp_psr_activate(intel_dp);
 unlock:
 	mutex_unlock(&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 06/10] drm/i915: VLV/CHV PSR Software timer mode control

[Rodrigo Vivi]

This patch introduces exit/activate functions for PSR
on VLV+. Since on VLV+ HW cannot track frame updates and force PSR
exit let's use fully SW tracking available.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 813537a..6829ecb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2271,6 +2271,27 @@ static void vlv_edp_psr_enable_source(struct intel_dp *intel_dp)
 }
 
 /**
+ * vlv_edp_psr_activate - Activate PSR on VLV
+ * @intel_dp: DP struct
+ *
+ * This function do the transition from PSR_state 1 to PSR_state 2 that is
+ * PSR transition to active - static frame transmission.
+ * Then Hardware is responsible for the transition to PSR_state 3 that is
+ * PSR active - no Remote Frame Buffer (RFB) update.
+ */
+static void vlv_edp_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 = dev->dev_private;
+	struct drm_crtc *crtc = dig_port->base.base.crtc;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
+		   VLV_EDP_PSR_ACTIVE_ENTRY);
+}
+
+/**
  * hsw_edp_psr_enable_source - Enable PSR on Source.
  * @intel_dp: DP struct
  *
@@ -2370,12 +2391,15 @@ static void intel_edp_psr_activate(struct intel_dp *intel_dp)
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
-	/* Enable/Re-enable PSR on the host
-	 * On HSW/BDW after we enable PSR on source it will activate it
-	 * as soon as it match configure idle_frame count. So
-	 * we just actually enable it here on activation time.
-	 */
-	hsw_edp_psr_enable_source(intel_dp);
+	/* Enable/Re-enable PSR on the host */
+	if (HAS_DDI(dev))
+		/* On HSW/BDW after we enable PSR on source it will activate it
+		 * as soon as it match configure idle_frame count. So
+		 * we just actually enable it here on activation time.
+		 */
+		hsw_edp_psr_enable_source(intel_dp);
+	else
+		vlv_edp_psr_activate(intel_dp);
 
 	dev_priv->psr.active = true;
 }
@@ -2514,18 +2538,30 @@ 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;
+	struct drm_crtc *crtc = dp_to_dig_port(
+		dev_priv->psr.enabled)->base.base.crtc;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 
 	/* 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;
+	if (HAS_DDI(dev_priv->dev)) {
+		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;
+		}
+	} else {
+		if (wait_for((I915_READ(VLV_PSRSTAT(pipe)) &
+			      VLV_EDP_PSR_IN_TRANS) == 0, 250)) {
+			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;
 
@@ -2555,17 +2591,49 @@ unlock:
 static void intel_edp_psr_exit(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	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) {
-		u32 val = I915_READ(EDP_PSR_CTL(dev));
+	if (!dev_priv->psr.active)
+		return;
+
+	if (HAS_DDI(dev)) {
+		val = I915_READ(EDP_PSR_CTL(dev));
 
 		WARN_ON(!(val & EDP_PSR_ENABLE));
 
 		I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
+	} else {
+		val = I915_READ(VLV_PSRCTL(pipe));
 
-		dev_priv->psr.active = false;
+		/* Here we do the transition from PSR_state 3 to PSR_state 4
+		 * that is PSR active - Single frame update
+		 */
+		val |= VLV_EDP_PSR_SINGLE_FRAME_UPDATE;
+		I915_WRITE(VLV_PSRCTL(pipe), val);
+
+		/* Now we are able to proceed to PSR_state 5 that is PSR exit
+		 * then Hardware is responsible to transition back to
+		 * PSR_state 1 that is PSR inactive. Same state after
+		 * vlv_edp_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);
 	}
 
+	dev_priv->psr.active = false;
 }
 
 void intel_edp_psr_invalidate(struct drm_device *dev,
-- 
1.9.3

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

[PATCH 07/10] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Rodrigo Vivi]

Since active function on VLV immediately activate PSR let's give more
time for idleness.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6829ecb..ded73ae 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2666,6 +2666,11 @@ void intel_edp_psr_flush(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 	enum pipe pipe;
+	/* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
+	* However on VLV we go to PSR active state with psr work. So let's
+	* wait more time and let the user experience smoth enough.
+	*/
+	int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
@@ -2688,8 +2693,7 @@ void intel_edp_psr_flush(struct drm_device *dev,
 		intel_edp_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));
+		schedule_delayed_work(&dev_priv->psr.work, delay);
 	mutex_unlock(&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 08/10] drm/i915: VLV/CHV PSR debugfs.

[Rodrigo Vivi]

Add debugfs support for Valleyview and Cherryview considering that
we have PSR per pipe and  we don't have any kind of
performance counter as we have on other platforms that support PSR.

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

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a79f83c..3edc20f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2126,6 +2126,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 psrperf = 0;
+	u32 stat[3];
+	enum pipe pipe;
 	bool enabled = false;
 
 	intel_runtime_pm_get(dev_priv);
@@ -2140,14 +2142,36 @@ 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)));
 
-	enabled = HAS_PSR(dev) &&
-		I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
-	seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
+	if (HAS_PSR(dev)) {
+		if (HAS_DDI(dev))
+			enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
+		else {
+			for_each_pipe(dev_priv, pipe) {
+				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;
+			}
+		}
+	}
+	seq_printf(m, "HW Enabled & Active bit: %s", yesno(enabled));
 
-	if (HAS_PSR(dev))
+	if (!HAS_DDI(dev))
+		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");
+
+	/* CHV PSR has no kind of performance counter */
+	if (HAS_PSR(dev) && HAS_DDI(dev)) {
 		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
 			EDP_PSR_PERF_CNT_MASK;
-	seq_printf(m, "Performance_Counter: %u\n", psrperf);
+
+		seq_printf(m, "Performance_Counter: %u\n", psrperf);
+	}
 	mutex_unlock(&dev_priv->psr.lock);
 
 	intel_runtime_pm_put(dev_priv);
-- 
1.9.3

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

[PATCH 09/10] drm/i915: PSR Baytrail: Not available on Pipe C.

[Rodrigo Vivi]

Altought VLV/CHV PSR supports per pipe PSR on VLV it isn't available
on pipe C.
Cherryview supports on all 3 pipes.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ded73ae..b93eb40 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2370,6 +2370,14 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	/* Baytrail supports per-pipe PSR configuration, however it is
+	 * available only on pipes A and B.
+	 */
+	if (IS_VALLEYVIEW(dev) && intel_crtc->pipe == PIPE_C) {
+		DRM_DEBUG_KMS("PSR on BYT isn't available on pipe C.\n");
+		return false;
+	}
+
  out:
 	dev_priv->psr.source_ok = true;
 	return true;
-- 
1.9.3

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

[PATCH 10/10] drm/i915: Enable PSR for Baytrail and Braswell.

[Rodrigo Vivi]

This patch is the last in series of VLV/CHV PSR,
that finnaly enable psr by adding it to HAS_PSR
and calling the propper enable and disable
functions on the right places.

Although it is still disabled by default.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 drivers/gpu/drm/i915/intel_dp.c | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a73803..119977b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2184,7 +2184,8 @@ struct drm_i915_cmd_table {
 
 #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
-#define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev))
+#define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
+				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
 #define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
 				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
 #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b93eb40..3cddf49 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2718,6 +2718,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
 
+	if (HAS_PSR(dev) & !HAS_DDI(dev))
+		intel_edp_psr_disable(intel_dp);
+
 	/* Make sure the panel is off before trying to change the mode. But also
 	 * ensure that we have vdd while we switch off the panel. */
 	intel_edp_panel_vdd_on(intel_dp);
@@ -2935,6 +2938,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder)
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
 	intel_edp_backlight_on(intel_dp);
+	intel_edp_psr_enable(intel_dp);
 }
 
 static void g4x_pre_enable_dp(struct intel_encoder *encoder)
-- 
1.9.3

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

Re: [PATCH 09/10] drm/i915: PSR Baytrail: Not available on Pipe C.

[Imre Deak]

On Tue, 2014-10-28 at 11:45 -0700, Rodrigo Vivi wrote:
> Altought VLV/CHV PSR supports per pipe PSR on VLV it isn't available
> on pipe C.
> Cherryview supports on all 3 pipes.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ded73ae..b93eb40 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2370,6 +2370,14 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  		return false;
>  	}
>  
> +	/* Baytrail supports per-pipe PSR configuration, however it is
> +	 * available only on pipes A and B.
> +	 */
> +	if (IS_VALLEYVIEW(dev) && intel_crtc->pipe == PIPE_C) {
> +		DRM_DEBUG_KMS("PSR on BYT isn't available on pipe C.\n");
> +		return false;
> +	}

VLV/BYT has only 2 pipes.

> +
>   out:
>  	dev_priv->psr.source_ok = true;
>  	return true;


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

Re: [PATCH 02/10] drm/i915: VLV PSR: Status/enabled function.

[Daniel Vetter]

On Tue, Oct 28, 2014 at 11:45:35AM -0700, Rodrigo Vivi wrote:
> VLV PSR support PSR per pipe, including the status. So we have to check
> if it is enabled per pipe on status.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 82e47da..774eb6c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2076,7 +2076,26 @@ 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)
> +/**
> + * vlv_edp_is_psr_enabled_on_pipe
> + * @dev: DRM device
> + * @pipe: pipe to check for PSR status.
> + *
> + * VLV PSR implements PSR per pipe. This function allows to check if it is
> + * enabled on given pipe.
> + */

It's nice that people now go overboard with kerneldoc, but I think we need
to strike a good balance. And in general I think documenting static inline
functions isn't worth it - they really should be self-explanatory as-is.

Documentation is imo only really useful for the bigger stuff, which
usually means it's used in a few places all over. So non-static functions.

If you feel like documenting tricky stuff in static functions, then imo
that should be done as in-line code comments, where appropriate.
-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

Re: [PATCH 01/10] drm/i915: Add PSR registers for PSR VLV/CHV.

[R, Durgadoss]

Hi Rodrigo,

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 01/10] drm/i915: Add PSR registers for PSR VLV/CHV.
>
>Baytrail (Valleyview) and Braswell (Cherryview) uses a complete different
>implementation of PSR that we currently have supported for
>Haswell and Broadwell. So let's start by adding registers definitions.
>
>I usually don't like commit that adds just registers without using,
>but after I put all in one commit I realized that no one would want
>to take the AR to review it so I decided to split in order to make
>reviewer's life easier. Only last commit in this series will actually
>enable the PSR on intel enable panel path.
>
>But as it happens currently with HSW/BDW the plan is to let it
>disabled by default (protected by kernel parameter)
>while we are able to fully validate it.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h | 37
>+++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 77fce96..771780a 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -2509,6 +2509,43 @@ enum punit_power_well {
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> #define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
>
>+/* VLV eDP PSR registers */
>+#define _PSRCTLA				(VLV_DISPLAY_BASE + 0x60090)
>+#define _PSRCTLB				(VLV_DISPLAY_BASE + 0x61090)
>+#define  VLV_EDP_PSR_ENABLE			(1<<0)
>+#define  VLV_EDP_PSR_RESET			(1<<1)
>+#define  VLV_EDP_PSR_MODE_MASK			(7<<2)
>+#define  VLV_EDP_PSR_MODE_HW_TIMER		(1<<3)
>+#define  VLV_EDP_PSR_MODE_SW_TIMER		(1<<2)
>+#define  VLV_EDP_PSR_SINGLE_FRAME_UPDATE	(1<<7)
>+#define  VLV_EDP_PSR_ACTIVE_ENTRY		(1<<8)
>+#define  VLV_EDP_PSR_SRC_TRANSMITTER_STATE	(1<<9)
>+#define  VLV_EDP_PSR_DBL_FRAME			(1<<10)
>+#define  VLV_EDP_PSR_FRAME_COUNT_MASK		(0xff<<16)
>+#define  VLV_EDP_PSR_IDLE_FRAME_SHIFT		16
>+#define  VLV_EDP_PSR_INT_TRANSITION		(1<<24)

Bit 24 is Reserved for CHV.
The PSR interrupt bits for CHV are moved to pipe control registers.

[Looks like we do not use this bit anywhere in our code]

>+#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB)
>+
>+#define _VSCSDPA			(VLV_DISPLAY_BASE + 0x600a0)
>+#define _VSCSDPB			(VLV_DISPLAY_BASE + 0x610a0)
>+#define  VLV_EDP_PSR_SDP_FREQ_MASK	(3<<30)
>+#define  VLV_EDP_PSR_SDP_FREQ_ONCE	(1<<31)
>+#define  VLV_EDP_PSR_SDP_FREQ_EVFRAME	(1<<30)
>+#define VLV_VSCSDP(pipe)	_PIPE(pipe, _VSCSDPA, _VSCSDPB)
>+
>+#define _PSRSTATA			(VLV_DISPLAY_BASE + 0x60094)
>+#define _PSRSTATB			(VLV_DISPLAY_BASE + 0x61094)
>+#define  VLV_EDP_PSR_LAST_STATE_MASK	(7<<3)
>+#define  VLV_EDP_PSR_CURR_STATE_MASK	7
>+#define  VLV_EDP_PSR_DISABLED		(0<<0)
>+#define  VLV_EDP_PSR_INACTIVE		(1<<0)
>+#define  VLV_EDP_PSR_IN_TRANS_TO_ACTIVE	(2<<0)
>+#define  VLV_EDP_PSR_ACTIVE_NORFB_UP	(3<<0)
>+#define  VLV_EDP_PSR_ACTIVE_SF_UPDATE	(4<<0)
>+#define  VLV_EDP_PSR_EXIT		(5<<0)
>+#define  VLV_EDP_PSR_IN_TRANS		(1<<7)
>+#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB)
>+
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> #define EDP_PSR_CTL(dev)			(EDP_PSR_BASE(dev) + 0)
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: VLV PSR: Status/enabled function.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 02/10] drm/i915: VLV PSR: Status/enabled function.
>
>VLV PSR support PSR per pipe, including the status. So we have to check
>if it is enabled per pipe on status.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index 82e47da..774eb6c 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -2076,7 +2076,26 @@ 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)
>+/**
>+ * vlv_edp_is_psr_enabled_on_pipe
>+ * @dev: DRM device
>+ * @pipe: pipe to check for PSR status.
>+ *
>+ * VLV PSR implements PSR per pipe. This function allows to check if it is
>+ * enabled on given pipe.
>+ */
>+static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe)
>+{
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+	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);
>+}

I feel *_is_psr_active() would be a better name for this function,
given that we check for State 3 or 4.

(and not just bit 0 in control register)

Thanks,
Durga

>+
>+static bool hsw_edp_is_psr_enabled(struct drm_device *dev)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
>
>@@ -5066,7 +5085,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 (IS_HASWELL(dev) && hsw_edp_is_psr_enabled(dev)) {
> 		DRM_DEBUG_KMS("DRRS is disabled as PSR is enabled\n");
> 		return;
> 	}
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/10] drm/i915: PSR VLV/CHV: Introduce setup, enable and disable functions

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 04/10] drm/i915: PSR VLV/CHV: Introduce setup,
>enable and disable functions
>
>The biggest difference from HSW/BDW PSR here is that VLV enable_source
>function enables PSR but let it in Inactive state. So it might be called
>on early stage along with setup and enable_sink ones.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_dp.c | 161
>++++++++++++++++++++++++++++++++++++----
> 1 file changed, 145 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index ac70b3a..335a711 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -2134,7 +2134,44 @@ static void intel_edp_psr_write_vsc(struct intel_dp
>*intel_dp,
> 	POSTING_READ(ctl_reg);
> }
>
>-static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp)
>+/**
>+ * vlv_edp_psr_setup_vsc - Setup PSR related VSC aux registers on VLV+.
>+ * @intel_dp: DP struct
>+ *
>+ * On VLV we don't need to generate VSC. It auto generates according
>+ * EDP 1.3 spec, Table 3.10.
>+ * HB0   - Secondary Data Packet ID = 0
>+ * HB1   - Secondary Data Packet Type = 07h
>+ * HB2   - Bits 4:0 = Revision Number = 02h
>+ *         Bits 7:5 = Reserved (all 0s)
>+ * HB3   - Bits 4:0 Number of Valid Data Bytes = 08h
>+ *         Bits 7:5 Reserved
>+ * DB0   - Bits 7:4 Stereo Interface Method Specific Paramenter
>+ *         Bits 3:0 Stereo Interface Method Code
>+ * DB1   - Bit 0 - PSR State (0 when Inactive / 1 when Active)
>+ *         Bit 1 - Update RFB. (0 do not update RFB / 1 update RFB)
>+ *         Bit 2 - CRC Valid. VLV always send 0 which
>+ *                            means CRC value in DB7:2 invalid
>+ *         Bits 7:3 - Reserved
>+ * DB2-7 - CRC - don't care because DB1[2] always tells this is invalid.
>+ */
>+static void vlv_edp_psr_setup_vsc(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;
>+	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>+	uint32_t val;
>+
>+	/* VLV auto-generate VSC package as per EDP 1.3 spec, Table 3.10 */
>+	val  = I915_READ(VLV_VSCSDP(pipe));
>+	val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
>+	val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
>+	I915_WRITE(VLV_VSCSDP(pipe), val);
>+}
>+
>+static void hsw_edp_psr_setup_vsc(struct intel_dp *intel_dp)
> {
> 	struct edp_vsc_psr psr_vsc;
>
>@@ -2147,7 +2184,20 @@ static void intel_edp_psr_setup_vsc(struct intel_dp
>*intel_dp)
> 	intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
> }
>
>-static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
>+/**
>+ * vlv_edp_psr_enable_sink - Enable PSR on Panel for VLV.
>+ * @intel_dp: DP struct
>+ *
>+ * This function enable PSR on Panel (Sink) side over DPCD write.
>+ */
>+static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp)
>+{
>+	/* Enable PSR in sink */
>+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>+			   DP_PSR_ENABLE);
>+}
>+
>+static void hsw_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;
>@@ -2191,7 +2241,28 @@ static void intel_edp_psr_enable_sink(struct intel_dp
>*intel_dp)
> 		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
> }
>
>-static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
>+/**
>+ * vlv_edp_psr_enable_source - Enable PSR on VLV without activate it.
>+ * @intel_dp: DP struct
>+ *
>+ * This function do the transition from PSR_state 0 to PSR_state 1 that is
>+ * PSR Inactive one..
>+ */
>+static void vlv_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;
>+	struct drm_crtc *crtc = dig_port->base.base.crtc;
>+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>+
>+	I915_WRITE(VLV_PSRCTL(pipe),
>+		   VLV_EDP_PSR_SRC_TRANSMITTER_STATE |

I see we are keeping the source active during PSR.
Making this '0' may provide better power savings,
but we need to manage a lot of hassle during PSR exit..
(like Link training, DPIO/PLL bring up etc..)

>+		   VLV_EDP_PSR_MODE_SW_TIMER |
>+		   VLV_EDP_PSR_ENABLE);
>+}
>+
>+static void hsw_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;
>@@ -2276,7 +2347,7 @@ static void intel_edp_psr_do_enable(struct intel_dp
>*intel_dp)
> 	lockdep_assert_held(&dev_priv->psr.lock);
>
> 	/* Enable/Re-enable PSR on the host */
>-	intel_edp_psr_enable_source(intel_dp);
>+	hsw_edp_psr_enable_source(intel_dp);
>
> 	dev_priv->psr.active = true;
> }
>@@ -2307,30 +2378,71 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
>
> 	dev_priv->psr.busy_frontbuffer_bits = 0;
>
>-	intel_edp_psr_setup_vsc(intel_dp);
>+	if (HAS_DDI(dev)) {
>+		hsw_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);
>
>-	/* 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 */
>+		hsw_edp_psr_enable_sink(intel_dp);
>+	} else {
>+		vlv_edp_psr_setup_vsc(intel_dp);
>
>-	/* Enable PSR on the panel */
>-	intel_edp_psr_enable_sink(intel_dp);
>+		/* Enable PSR on the panel */
>+		vlv_edp_psr_enable_sink(intel_dp);
>+
>+		/* On HSW/BDW enable_source also means go to PSR entry/active
>+		 * state as soon as idle_frame achieved and here would be
>+		 * to soon. However on VLV enable_source just enable PSR
>+		 * but let it on inactive state. So we might do this prior
>+		 * to active transition, i.e. here.
>+		 */
>+		vlv_edp_psr_enable_source(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)
>+/**
>+ * vlv_edp_psr_disable - Disable PSR on VLV
>+ * @intel_dp: DP struct
>+ *
>+ * This function puts PSR VLV back to PSR_state 0 that is PSR Disabled.

I hope you meant 'puts VLV PSR'..

>+ */
>+static void vlv_edp_psr_disable(struct intel_dp *intel_dp)
> {
>-	struct drm_device *dev = intel_dp_to_dev(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;
>+	struct intel_crtc *intel_crtc =
>+		to_intel_crtc(intel_dig_port->base.base.crtc);
>+	uint32_t val;
>
>-	mutex_lock(&dev_priv->psr.lock);
>-	if (!dev_priv->psr.enabled) {
>-		mutex_unlock(&dev_priv->psr.lock);
>-		return;
>+	if (dev_priv->psr.active) {
>+		if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
>+			      VLV_EDP_PSR_IN_TRANS) == 0, 250))
>+			WARN(1, "PSR transition took longer than expected\n");

The spec claims this waiting time as 250 micro seconds.
Looks like we are waiting for 250 ms.. No ?

Just curious, did you see this transition during testing ?

Thanks,
Durga

>+
>+		val = I915_READ(VLV_PSRCTL(intel_crtc->pipe));
>+		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
>+		val &= ~VLV_EDP_PSR_ENABLE;
>+		val &= ~VLV_EDP_PSR_MODE_MASK;
>+		I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val);
>+
>+		dev_priv->psr.active = false;
>+	} else {
>+		WARN_ON(vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc-
>>pipe));
> 	}
>+}
>+
>+static void hsw_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;
>
> 	if (dev_priv->psr.active) {
> 		I915_WRITE(EDP_PSR_CTL(dev),
>@@ -2345,6 +2457,23 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
> 	} else {
> 		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
> 	}
>+}
>+
>+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 (HAS_DDI(dev))
>+		hsw_edp_psr_disable(intel_dp);
>+	else
>+		vlv_edp_psr_disable(intel_dp);
>
> 	dev_priv->psr.enabled = NULL;
> 	mutex_unlock(&dev_priv->psr.lock);
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: VLV/CHV PSR Software timer mode control

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 06/10] drm/i915: VLV/CHV PSR Software timer mode
>control
>
>This patch introduces exit/activate functions for PSR
>on VLV+. Since on VLV+ HW cannot track frame updates and force PSR
>exit let's use fully SW tracking available.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_dp.c | 94
>+++++++++++++++++++++++++++++++++++------
> 1 file changed, 81 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index 813537a..6829ecb 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -2271,6 +2271,27 @@ static void vlv_edp_psr_enable_source(struct intel_dp
>*intel_dp)
> }
>
> /**
>+ * vlv_edp_psr_activate - Activate PSR on VLV
>+ * @intel_dp: DP struct
>+ *
>+ * This function do the transition from PSR_state 1 to PSR_state 2 that is
>+ * PSR transition to active - static frame transmission.
>+ * Then Hardware is responsible for the transition to PSR_state 3 that is
>+ * PSR active - no Remote Frame Buffer (RFB) update.
>+ */
>+static void vlv_edp_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 = dev->dev_private;
>+	struct drm_crtc *crtc = dig_port->base.base.crtc;
>+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>+
>+	I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
>+		   VLV_EDP_PSR_ACTIVE_ENTRY);
>+}
>+
>+/**
>  * hsw_edp_psr_enable_source - Enable PSR on Source.
>  * @intel_dp: DP struct
>  *
>@@ -2370,12 +2391,15 @@ static void intel_edp_psr_activate(struct intel_dp
>*intel_dp)
> 	WARN_ON(dev_priv->psr.active);
> 	lockdep_assert_held(&dev_priv->psr.lock);
>
>-	/* Enable/Re-enable PSR on the host
>-	 * On HSW/BDW after we enable PSR on source it will activate it
>-	 * as soon as it match configure idle_frame count. So
>-	 * we just actually enable it here on activation time.
>-	 */
>-	hsw_edp_psr_enable_source(intel_dp);
>+	/* Enable/Re-enable PSR on the host */
>+	if (HAS_DDI(dev))
>+		/* On HSW/BDW after we enable PSR on source it will activate it
>+		 * as soon as it match configure idle_frame count. So
>+		 * we just actually enable it here on activation time.
>+		 */
>+		hsw_edp_psr_enable_source(intel_dp);
>+	else
>+		vlv_edp_psr_activate(intel_dp);
>
> 	dev_priv->psr.active = true;
> }
>@@ -2514,18 +2538,30 @@ 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;
>+	struct drm_crtc *crtc = dp_to_dig_port(
>+		dev_priv->psr.enabled)->base.base.crtc;
>+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>
> 	/* 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;
>+	if (HAS_DDI(dev_priv->dev)) {
>+		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;
>+		}
>+	} else {
>+		if (wait_for((I915_READ(VLV_PSRSTAT(pipe)) &
>+			      VLV_EDP_PSR_IN_TRANS) == 0, 250)) {
>+			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;
>
>@@ -2555,17 +2591,49 @@ unlock:
> static void intel_edp_psr_exit(struct drm_device *dev)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
>+	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) {
>-		u32 val = I915_READ(EDP_PSR_CTL(dev));
>+	if (!dev_priv->psr.active)
>+		return;
>+
>+	if (HAS_DDI(dev)) {
>+		val = I915_READ(EDP_PSR_CTL(dev));
>
> 		WARN_ON(!(val & EDP_PSR_ENABLE));
>
> 		I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
>+	} else {
>+		val = I915_READ(VLV_PSRCTL(pipe));
>
>-		dev_priv->psr.active = false;
>+		/* Here we do the transition from PSR_state 3 to PSR_state 4
>+		 * that is PSR active - Single frame update
>+		 */
>+		val |= VLV_EDP_PSR_SINGLE_FRAME_UPDATE;
>+		I915_WRITE(VLV_PSRCTL(pipe), val);

Do we really need this step ?
i.e The state machine can go to state 5 from state 3 directly.
So, this should not be really needed. Can we double check once ?

>+
>+		/* Now we are able to proceed to PSR_state 5 that is PSR exit
>+		 * then Hardware is responsible to transition back to
>+		 * PSR_state 1 that is PSR inactive. Same state after
>+		 * vlv_edp_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);

Any specific reason for not using 'intel_sink_dpms_on(dp, MODE_ON) ?

Thanks,
Durga

> 	}
>
>+	dev_priv->psr.active = false;
> }
>
> void intel_edp_psr_invalidate(struct drm_device *dev,
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/10] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 07/10] drm/i915: VLV/CHV PSR: Increase wait delay
>time before active PSR.
>
>Since active function on VLV immediately activate PSR let's give more
>time for idleness.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_dp.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index 6829ecb..ded73ae 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -2666,6 +2666,11 @@ void intel_edp_psr_flush(struct drm_device *dev,
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct drm_crtc *crtc;
> 	enum pipe pipe;
>+	/* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
>+	* However on VLV we go to PSR active state with psr work. So let's
>+	* wait more time and let the user experience smoth enough.

s/smoth/smooth

otherwise looks fine..

>+	*/
>+	int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
>
> 	mutex_lock(&dev_priv->psr.lock);
> 	if (!dev_priv->psr.enabled) {
>@@ -2688,8 +2693,7 @@ void intel_edp_psr_flush(struct drm_device *dev,
> 		intel_edp_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));
>+		schedule_delayed_work(&dev_priv->psr.work, delay);
> 	mutex_unlock(&dev_priv->psr.lock);
> }
>
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: VLV/CHV PSR debugfs.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 08/10] drm/i915: VLV/CHV PSR debugfs.
>
>Add debugfs support for Valleyview and Cherryview considering that
>we have PSR per pipe and  we don't have any kind of
>performance counter as we have on other platforms that support PSR.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/i915_debugfs.c | 34 +++++++++++++++++++++++++++++-----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>index a79f83c..3edc20f 100644
>--- a/drivers/gpu/drm/i915/i915_debugfs.c
>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
>@@ -2126,6 +2126,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> 	struct drm_device *dev = node->minor->dev;
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	u32 psrperf = 0;
>+	u32 stat[3];
>+	enum pipe pipe;
> 	bool enabled = false;
>
> 	intel_runtime_pm_get(dev_priv);
>@@ -2140,14 +2142,36 @@ 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)));
>
>-	enabled = HAS_PSR(dev) &&
>-		I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
>-	seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
>+	if (HAS_PSR(dev)) {
>+		if (HAS_DDI(dev))
>+			enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
>+		else {
>+			for_each_pipe(dev_priv, pipe) {
>+				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))

I think it is better to include 'state 2' (transition to active) also here.
We almost always go to state3 from state 2, before exiting (to state 5)
Unless it is an 'abort' case.

>+					enabled = true;
>+			}
>+		}
>+	}
>+	seq_printf(m, "HW Enabled & Active bit: %s", yesno(enabled));
>
>-	if (HAS_PSR(dev))
>+	if (!HAS_DDI(dev))
>+		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");
>+
>+	/* CHV PSR has no kind of performance counter */
>+	if (HAS_PSR(dev) && HAS_DDI(dev)) {
> 		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
> 			EDP_PSR_PERF_CNT_MASK;
>-	seq_printf(m, "Performance_Counter: %u\n", psrperf);
>+
>+		seq_printf(m, "Performance_Counter: %u\n", psrperf);
>+	}
> 	mutex_unlock(&dev_priv->psr.lock);
>
> 	intel_runtime_pm_put(dev_priv);
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Enable PSR for Baytrail and Braswell.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Wednesday, October 29, 2014 12:16 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 10/10] drm/i915: Enable PSR for Baytrail and Braswell.
>
>This patch is the last in series of VLV/CHV PSR,
>that finnaly enable psr by adding it to HAS_PSR
>and calling the propper enable and disable

s/propper/proper

>functions on the right places.
>
>Although it is still disabled by default.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 6a73803..119977b 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -2184,7 +2184,8 @@ struct drm_i915_cmd_table {
>
> #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
>-#define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev))
>+#define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>+				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> #define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
> 				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index b93eb40..3cddf49 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -2718,6 +2718,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
> 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> 	struct drm_device *dev = encoder->base.dev;
>
>+	if (HAS_PSR(dev) & !HAS_DDI(dev))

I think you really meant an '&&' here.. No ??
[although this may work]

Thanks,
Durga

>+		intel_edp_psr_disable(intel_dp);
>+
> 	/* Make sure the panel is off before trying to change the mode. But also
> 	 * ensure that we have vdd while we switch off the panel. */
> 	intel_edp_panel_vdd_on(intel_dp);
>@@ -2935,6 +2938,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder)
> 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>
> 	intel_edp_backlight_on(intel_dp);
>+	intel_edp_psr_enable(intel_dp);
> }
>
> static void g4x_pre_enable_dp(struct intel_encoder *encoder)
>--
>1.9.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: VLV PSR: Status/enabled function.

[Daniel Vetter]

On Tue, Oct 28, 2014 at 11:45:35AM -0700, Rodrigo Vivi wrote:
> VLV PSR support PSR per pipe, including the status. So we have to check
> if it is enabled per pipe on status.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 82e47da..774eb6c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2076,7 +2076,26 @@ 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)
> +/**
> + * vlv_edp_is_psr_enabled_on_pipe
> + * @dev: DRM device
> + * @pipe: pipe to check for PSR status.
> + *
> + * VLV PSR implements PSR per pipe. This function allows to check if it is
> + * enabled on given pipe.
> + */
> +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe)

Adding functions without users makes imo patch review harder.

> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	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 bool hsw_edp_is_psr_enabled(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> @@ -5066,7 +5085,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 (IS_HASWELL(dev) && hsw_edp_is_psr_enabled(dev)) {

Or is there something missing here? Otherwise this hunk here looks like a
separate patch ...
-Daniel

>  		DRM_DEBUG_KMS("DRRS is disabled as PSR is enabled\n");
>  		return;
>  	}
> -- 
> 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 08/10] drm/i915: VLV/CHV PSR debugfs.

[Rodrigo Vivi]

On Thu, Nov 6, 2014 at 1:45 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>>Sent: Wednesday, October 29, 2014 12:16 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 08/10] drm/i915: VLV/CHV PSR debugfs.
>>
>>Add debugfs support for Valleyview and Cherryview considering that
>>we have PSR per pipe and  we don't have any kind of
>>performance counter as we have on other platforms that support PSR.
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_debugfs.c | 34 +++++++++++++++++++++++++++++-----
>> 1 file changed, 29 insertions(+), 5 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>index a79f83c..3edc20f 100644
>>--- a/drivers/gpu/drm/i915/i915_debugfs.c
>>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>@@ -2126,6 +2126,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>>       struct drm_device *dev = node->minor->dev;
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       u32 psrperf = 0;
>>+      u32 stat[3];
>>+      enum pipe pipe;
>>       bool enabled = false;
>>
>>       intel_runtime_pm_get(dev_priv);
>>@@ -2140,14 +2142,36 @@ 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)));
>>
>>-      enabled = HAS_PSR(dev) &&
>>-              I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
>>-      seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
>>+      if (HAS_PSR(dev)) {
>>+              if (HAS_DDI(dev))
>>+                      enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
>>+              else {
>>+                      for_each_pipe(dev_priv, pipe) {
>>+                              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))
>
> I think it is better to include 'state 2' (transition to active) also here.
> We almost always go to state3 from state 2, before exiting (to state 5)
> Unless it is an 'abort' case.

This one I don´t agree. On test cases we wait until we are for sure in
an active state already so we can update state and check if we got
proper screen updates.
If it is yet transitioning we will probably get many false positives
in test cases...

(Considering that I can get sink crc working again! :/ )

>
>>+                                      enabled = true;
>>+                      }
>>+              }
>>+      }
>>+      seq_printf(m, "HW Enabled & Active bit: %s", yesno(enabled));
>>
>>-      if (HAS_PSR(dev))
>>+      if (!HAS_DDI(dev))
>>+              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");
>>+
>>+      /* CHV PSR has no kind of performance counter */
>>+      if (HAS_PSR(dev) && HAS_DDI(dev)) {
>>               psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
>>                       EDP_PSR_PERF_CNT_MASK;
>>-      seq_printf(m, "Performance_Counter: %u\n", psrperf);
>>+
>>+              seq_printf(m, "Performance_Counter: %u\n", psrperf);
>>+      }
>>       mutex_unlock(&dev_priv->psr.lock);
>>
>>       intel_runtime_pm_put(dev_priv);
>>--
>>1.9.3
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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 06/10] drm/i915: VLV/CHV PSR Software timer mode control

[Rodrigo Vivi]

On Thu, Nov 6, 2014 at 1:39 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>Rodrigo Vivi
>>Sent: Wednesday, October 29, 2014 12:16 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 06/10] drm/i915: VLV/CHV PSR Software timer mode
>>control
>>
>>This patch introduces exit/activate functions for PSR
>>on VLV+. Since on VLV+ HW cannot track frame updates and force PSR
>>exit let's use fully SW tracking available.
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/intel_dp.c | 94
>>+++++++++++++++++++++++++++++++++++------
>> 1 file changed, 81 insertions(+), 13 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>index 813537a..6829ecb 100644
>>--- a/drivers/gpu/drm/i915/intel_dp.c
>>+++ b/drivers/gpu/drm/i915/intel_dp.c
>>@@ -2271,6 +2271,27 @@ static void vlv_edp_psr_enable_source(struct intel_dp
>>*intel_dp)
>> }
>>
>> /**
>>+ * vlv_edp_psr_activate - Activate PSR on VLV
>>+ * @intel_dp: DP struct
>>+ *
>>+ * This function do the transition from PSR_state 1 to PSR_state 2 that is
>>+ * PSR transition to active - static frame transmission.
>>+ * Then Hardware is responsible for the transition to PSR_state 3 that is
>>+ * PSR active - no Remote Frame Buffer (RFB) update.
>>+ */
>>+static void vlv_edp_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 = dev->dev_private;
>>+      struct drm_crtc *crtc = dig_port->base.base.crtc;
>>+      enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>+
>>+      I915_WRITE(VLV_PSRCTL(pipe), I915_READ(VLV_PSRCTL(pipe)) |
>>+                 VLV_EDP_PSR_ACTIVE_ENTRY);
>>+}
>>+
>>+/**
>>  * hsw_edp_psr_enable_source - Enable PSR on Source.
>>  * @intel_dp: DP struct
>>  *
>>@@ -2370,12 +2391,15 @@ static void intel_edp_psr_activate(struct intel_dp
>>*intel_dp)
>>       WARN_ON(dev_priv->psr.active);
>>       lockdep_assert_held(&dev_priv->psr.lock);
>>
>>-      /* Enable/Re-enable PSR on the host
>>-       * On HSW/BDW after we enable PSR on source it will activate it
>>-       * as soon as it match configure idle_frame count. So
>>-       * we just actually enable it here on activation time.
>>-       */
>>-      hsw_edp_psr_enable_source(intel_dp);
>>+      /* Enable/Re-enable PSR on the host */
>>+      if (HAS_DDI(dev))
>>+              /* On HSW/BDW after we enable PSR on source it will activate it
>>+               * as soon as it match configure idle_frame count. So
>>+               * we just actually enable it here on activation time.
>>+               */
>>+              hsw_edp_psr_enable_source(intel_dp);
>>+      else
>>+              vlv_edp_psr_activate(intel_dp);
>>
>>       dev_priv->psr.active = true;
>> }
>>@@ -2514,18 +2538,30 @@ 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;
>>+      struct drm_crtc *crtc = dp_to_dig_port(
>>+              dev_priv->psr.enabled)->base.base.crtc;
>>+      enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>
>>       /* 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;
>>+      if (HAS_DDI(dev_priv->dev)) {
>>+              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;
>>+              }
>>+      } else {
>>+              if (wait_for((I915_READ(VLV_PSRSTAT(pipe)) &
>>+                            VLV_EDP_PSR_IN_TRANS) == 0, 250)) {
>>+                      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;
>>
>>@@ -2555,17 +2591,49 @@ unlock:
>> static void intel_edp_psr_exit(struct drm_device *dev)
>> {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>+      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) {
>>-              u32 val = I915_READ(EDP_PSR_CTL(dev));
>>+      if (!dev_priv->psr.active)
>>+              return;
>>+
>>+      if (HAS_DDI(dev)) {
>>+              val = I915_READ(EDP_PSR_CTL(dev));
>>
>>               WARN_ON(!(val & EDP_PSR_ENABLE));
>>
>>               I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
>>+      } else {
>>+              val = I915_READ(VLV_PSRCTL(pipe));
>>
>>-              dev_priv->psr.active = false;
>>+              /* Here we do the transition from PSR_state 3 to PSR_state 4
>>+               * that is PSR active - Single frame update
>>+               */
>>+              val |= VLV_EDP_PSR_SINGLE_FRAME_UPDATE;
>>+              I915_WRITE(VLV_PSRCTL(pipe), val);
>
> Do we really need this step ?
> i.e The state machine can go to state 5 from state 3 directly.
> So, this should not be really needed. Can we double check once ?

I think we really need, otherwise I was getting some strange blank screen.
But I'll double check tomorrow before sending the new series.

>
>>+
>>+              /* Now we are able to proceed to PSR_state 5 that is PSR exit
>>+               * then Hardware is responsible to transition back to
>>+               * PSR_state 1 that is PSR inactive. Same state after
>>+               * vlv_edp_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);
>
> Any specific reason for not using 'intel_sink_dpms_on(dp, MODE_ON) ?
>
> Thanks,
> Durga
>
>>       }
>>
>>+      dev_priv->psr.active = false;
>> }
>>
>> void intel_edp_psr_invalidate(struct drm_device *dev,
>>--
>>1.9.3
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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 07/10] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Rodrigo Vivi]

On Thu, Nov 6, 2014 at 1:40 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>Rodrigo Vivi
>>Sent: Wednesday, October 29, 2014 12:16 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 07/10] drm/i915: VLV/CHV PSR: Increase wait delay
>>time before active PSR.
>>
>>Since active function on VLV immediately activate PSR let's give more
>>time for idleness.
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/intel_dp.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>index 6829ecb..ded73ae 100644
>>--- a/drivers/gpu/drm/i915/intel_dp.c
>>+++ b/drivers/gpu/drm/i915/intel_dp.c
>>@@ -2666,6 +2666,11 @@ void intel_edp_psr_flush(struct drm_device *dev,
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct drm_crtc *crtc;
>>       enum pipe pipe;
>>+      /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
>>+      * However on VLV we go to PSR active state with psr work. So let's
>>+      * wait more time and let the user experience smoth enough.
>
> s/smoth/smooth

thanks
>
> otherwise looks fine..
>
>>+      */
>>+      int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
>>
>>       mutex_lock(&dev_priv->psr.lock);
>>       if (!dev_priv->psr.enabled) {
>>@@ -2688,8 +2693,7 @@ void intel_edp_psr_flush(struct drm_device *dev,
>>               intel_edp_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));
>>+              schedule_delayed_work(&dev_priv->psr.work, delay);
>>       mutex_unlock(&dev_priv->psr.lock);
>> }
>>
>>--
>>1.9.3
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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 04/10] drm/i915: PSR VLV/CHV: Introduce setup, enable and disable functions

[Rodrigo Vivi]

On Thu, Nov 6, 2014 at 1:32 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>Rodrigo Vivi
>>Sent: Wednesday, October 29, 2014 12:16 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 04/10] drm/i915: PSR VLV/CHV: Introduce setup,
>>enable and disable functions
>>
>>The biggest difference from HSW/BDW PSR here is that VLV enable_source
>>function enables PSR but let it in Inactive state. So it might be called
>>on early stage along with setup and enable_sink ones.
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/intel_dp.c | 161
>>++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 145 insertions(+), 16 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>index ac70b3a..335a711 100644
>>--- a/drivers/gpu/drm/i915/intel_dp.c
>>+++ b/drivers/gpu/drm/i915/intel_dp.c
>>@@ -2134,7 +2134,44 @@ static void intel_edp_psr_write_vsc(struct intel_dp
>>*intel_dp,
>>       POSTING_READ(ctl_reg);
>> }
>>
>>-static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp)
>>+/**
>>+ * vlv_edp_psr_setup_vsc - Setup PSR related VSC aux registers on VLV+.
>>+ * @intel_dp: DP struct
>>+ *
>>+ * On VLV we don't need to generate VSC. It auto generates according
>>+ * EDP 1.3 spec, Table 3.10.
>>+ * HB0   - Secondary Data Packet ID = 0
>>+ * HB1   - Secondary Data Packet Type = 07h
>>+ * HB2   - Bits 4:0 = Revision Number = 02h
>>+ *         Bits 7:5 = Reserved (all 0s)
>>+ * HB3   - Bits 4:0 Number of Valid Data Bytes = 08h
>>+ *         Bits 7:5 Reserved
>>+ * DB0   - Bits 7:4 Stereo Interface Method Specific Paramenter
>>+ *         Bits 3:0 Stereo Interface Method Code
>>+ * DB1   - Bit 0 - PSR State (0 when Inactive / 1 when Active)
>>+ *         Bit 1 - Update RFB. (0 do not update RFB / 1 update RFB)
>>+ *         Bit 2 - CRC Valid. VLV always send 0 which
>>+ *                            means CRC value in DB7:2 invalid
>>+ *         Bits 7:3 - Reserved
>>+ * DB2-7 - CRC - don't care because DB1[2] always tells this is invalid.
>>+ */
>>+static void vlv_edp_psr_setup_vsc(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;
>>+      struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>>+      enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>+      uint32_t val;
>>+
>>+      /* VLV auto-generate VSC package as per EDP 1.3 spec, Table 3.10 */
>>+      val  = I915_READ(VLV_VSCSDP(pipe));
>>+      val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
>>+      val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
>>+      I915_WRITE(VLV_VSCSDP(pipe), val);
>>+}
>>+
>>+static void hsw_edp_psr_setup_vsc(struct intel_dp *intel_dp)
>> {
>>       struct edp_vsc_psr psr_vsc;
>>
>>@@ -2147,7 +2184,20 @@ static void intel_edp_psr_setup_vsc(struct intel_dp
>>*intel_dp)
>>       intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
>> }
>>
>>-static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
>>+/**
>>+ * vlv_edp_psr_enable_sink - Enable PSR on Panel for VLV.
>>+ * @intel_dp: DP struct
>>+ *
>>+ * This function enable PSR on Panel (Sink) side over DPCD write.
>>+ */
>>+static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp)
>>+{
>>+      /* Enable PSR in sink */
>>+      drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>>+                         DP_PSR_ENABLE);
>>+}
>>+
>>+static void hsw_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;
>>@@ -2191,7 +2241,28 @@ static void intel_edp_psr_enable_sink(struct intel_dp
>>*intel_dp)
>>                  (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>> }
>>
>>-static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
>>+/**
>>+ * vlv_edp_psr_enable_source - Enable PSR on VLV without activate it.
>>+ * @intel_dp: DP struct
>>+ *
>>+ * This function do the transition from PSR_state 0 to PSR_state 1 that is
>>+ * PSR Inactive one..
>>+ */
>>+static void vlv_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;
>>+      struct drm_crtc *crtc = dig_port->base.base.crtc;
>>+      enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>+
>>+      I915_WRITE(VLV_PSRCTL(pipe),
>>+                 VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
>
> I see we are keeping the source active during PSR.
> Making this '0' may provide better power savings,
> but we need to manage a lot of hassle during PSR exit..
> (like Link training, DPIO/PLL bring up etc..)

Also I think this blocked sink crc to work. So we wouldn't be able to
validate it.
Although there are other things blocking that tests :(
But I can double check that anyway.

>
>>+                 VLV_EDP_PSR_MODE_SW_TIMER |
>>+                 VLV_EDP_PSR_ENABLE);
>>+}
>>+
>>+static void hsw_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;
>>@@ -2276,7 +2347,7 @@ static void intel_edp_psr_do_enable(struct intel_dp
>>*intel_dp)
>>       lockdep_assert_held(&dev_priv->psr.lock);
>>
>>       /* Enable/Re-enable PSR on the host */
>>-      intel_edp_psr_enable_source(intel_dp);
>>+      hsw_edp_psr_enable_source(intel_dp);
>>
>>       dev_priv->psr.active = true;
>> }
>>@@ -2307,30 +2378,71 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
>>
>>       dev_priv->psr.busy_frontbuffer_bits = 0;
>>
>>-      intel_edp_psr_setup_vsc(intel_dp);
>>+      if (HAS_DDI(dev)) {
>>+              hsw_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);
>>
>>-      /* 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 */
>>+              hsw_edp_psr_enable_sink(intel_dp);
>>+      } else {
>>+              vlv_edp_psr_setup_vsc(intel_dp);
>>
>>-      /* Enable PSR on the panel */
>>-      intel_edp_psr_enable_sink(intel_dp);
>>+              /* Enable PSR on the panel */
>>+              vlv_edp_psr_enable_sink(intel_dp);
>>+
>>+              /* On HSW/BDW enable_source also means go to PSR entry/active
>>+               * state as soon as idle_frame achieved and here would be
>>+               * to soon. However on VLV enable_source just enable PSR
>>+               * but let it on inactive state. So we might do this prior
>>+               * to active transition, i.e. here.
>>+               */
>>+              vlv_edp_psr_enable_source(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)
>>+/**
>>+ * vlv_edp_psr_disable - Disable PSR on VLV
>>+ * @intel_dp: DP struct
>>+ *
>>+ * This function puts PSR VLV back to PSR_state 0 that is PSR Disabled.
>
> I hope you meant 'puts VLV PSR'..

Yes thanks!

>
>>+ */
>>+static void vlv_edp_psr_disable(struct intel_dp *intel_dp)
>> {
>>-      struct drm_device *dev = intel_dp_to_dev(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;
>>+      struct intel_crtc *intel_crtc =
>>+              to_intel_crtc(intel_dig_port->base.base.crtc);
>>+      uint32_t val;
>>
>>-      mutex_lock(&dev_priv->psr.lock);
>>-      if (!dev_priv->psr.enabled) {
>>-              mutex_unlock(&dev_priv->psr.lock);
>>-              return;
>>+      if (dev_priv->psr.active) {
>>+              if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
>>+                            VLV_EDP_PSR_IN_TRANS) == 0, 250))
>>+                      WARN(1, "PSR transition took longer than expected\n");
>
> The spec claims this waiting time as 250 micro seconds.
> Looks like we are waiting for 250 ms.. No ?

This is true. I'll fix that and test tomorrow.

>
> Just curious, did you see this transition during testing ?

No, I never checked.

>
> Thanks,
> Durga
>
>>+
>>+              val = I915_READ(VLV_PSRCTL(intel_crtc->pipe));
>>+              val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
>>+              val &= ~VLV_EDP_PSR_ENABLE;
>>+              val &= ~VLV_EDP_PSR_MODE_MASK;
>>+              I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val);
>>+
>>+              dev_priv->psr.active = false;
>>+      } else {
>>+              WARN_ON(vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc-
>>>pipe));
>>       }
>>+}
>>+
>>+static void hsw_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;
>>
>>       if (dev_priv->psr.active) {
>>               I915_WRITE(EDP_PSR_CTL(dev),
>>@@ -2345,6 +2457,23 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
>>       } else {
>>               WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
>>       }
>>+}
>>+
>>+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 (HAS_DDI(dev))
>>+              hsw_edp_psr_disable(intel_dp);
>>+      else
>>+              vlv_edp_psr_disable(intel_dp);
>>
>>       dev_priv->psr.enabled = NULL;
>>       mutex_unlock(&dev_priv->psr.lock);
>>--
>>1.9.3
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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 01/10] drm/i915: Add PSR registers for PSR VLV/CHV.

[Rodrigo Vivi]

On Thu, Nov 6, 2014 at 1:17 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
> Hi Rodrigo,
>
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>Rodrigo Vivi
>>Sent: Wednesday, October 29, 2014 12:16 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 01/10] drm/i915: Add PSR registers for PSR VLV/CHV.
>>
>>Baytrail (Valleyview) and Braswell (Cherryview) uses a complete different
>>implementation of PSR that we currently have supported for
>>Haswell and Broadwell. So let's start by adding registers definitions.
>>
>>I usually don't like commit that adds just registers without using,
>>but after I put all in one commit I realized that no one would want
>>to take the AR to review it so I decided to split in order to make
>>reviewer's life easier. Only last commit in this series will actually
>>enable the PSR on intel enable panel path.
>>
>>But as it happens currently with HSW/BDW the plan is to let it
>>disabled by default (protected by kernel parameter)
>>while we are able to fully validate it.
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_reg.h | 37
>>+++++++++++++++++++++++++++++++++++++
>> 1 file changed, 37 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index 77fce96..771780a 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -2509,6 +2509,43 @@ enum punit_power_well {
>> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
>> #define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
>>
>>+/* VLV eDP PSR registers */
>>+#define _PSRCTLA                              (VLV_DISPLAY_BASE + 0x60090)
>>+#define _PSRCTLB                              (VLV_DISPLAY_BASE + 0x61090)
>>+#define  VLV_EDP_PSR_ENABLE                   (1<<0)
>>+#define  VLV_EDP_PSR_RESET                    (1<<1)
>>+#define  VLV_EDP_PSR_MODE_MASK                        (7<<2)
>>+#define  VLV_EDP_PSR_MODE_HW_TIMER            (1<<3)
>>+#define  VLV_EDP_PSR_MODE_SW_TIMER            (1<<2)
>>+#define  VLV_EDP_PSR_SINGLE_FRAME_UPDATE      (1<<7)
>>+#define  VLV_EDP_PSR_ACTIVE_ENTRY             (1<<8)
>>+#define  VLV_EDP_PSR_SRC_TRANSMITTER_STATE    (1<<9)
>>+#define  VLV_EDP_PSR_DBL_FRAME                        (1<<10)
>>+#define  VLV_EDP_PSR_FRAME_COUNT_MASK         (0xff<<16)
>>+#define  VLV_EDP_PSR_IDLE_FRAME_SHIFT         16
>>+#define  VLV_EDP_PSR_INT_TRANSITION           (1<<24)
>
> Bit 24 is Reserved for CHV.
> The PSR interrupt bits for CHV are moved to pipe control registers.
>
> [Looks like we do not use this bit anywhere in our code]

Removed. Thanks.
>
>>+#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB)
>>+
>>+#define _VSCSDPA                      (VLV_DISPLAY_BASE + 0x600a0)
>>+#define _VSCSDPB                      (VLV_DISPLAY_BASE + 0x610a0)
>>+#define  VLV_EDP_PSR_SDP_FREQ_MASK    (3<<30)
>>+#define  VLV_EDP_PSR_SDP_FREQ_ONCE    (1<<31)
>>+#define  VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30)
>>+#define VLV_VSCSDP(pipe)      _PIPE(pipe, _VSCSDPA, _VSCSDPB)
>>+
>>+#define _PSRSTATA                     (VLV_DISPLAY_BASE + 0x60094)
>>+#define _PSRSTATB                     (VLV_DISPLAY_BASE + 0x61094)
>>+#define  VLV_EDP_PSR_LAST_STATE_MASK  (7<<3)
>>+#define  VLV_EDP_PSR_CURR_STATE_MASK  7
>>+#define  VLV_EDP_PSR_DISABLED         (0<<0)
>>+#define  VLV_EDP_PSR_INACTIVE         (1<<0)
>>+#define  VLV_EDP_PSR_IN_TRANS_TO_ACTIVE       (2<<0)
>>+#define  VLV_EDP_PSR_ACTIVE_NORFB_UP  (3<<0)
>>+#define  VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0)
>>+#define  VLV_EDP_PSR_EXIT             (5<<0)
>>+#define  VLV_EDP_PSR_IN_TRANS         (1<<7)
>>+#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB)
>>+
>> /* HSW+ eDP PSR registers */
>> #define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
>> #define EDP_PSR_CTL(dev)                      (EDP_PSR_BASE(dev) + 0)
>>--
>>1.9.3
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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 04/10] drm/i915: PSR VLV/CHV: Introduce setup, enable and disable functions

[Daniel Vetter]

On Thu, Nov 13, 2014 at 07:33:21PM -0500, Rodrigo Vivi wrote:
> On Thu, Nov 6, 2014 at 1:32 PM, R, Durgadoss <durgadoss.r@intel.com> wrote:
> > I see we are keeping the source active during PSR.
> > Making this '0' may provide better power savings,
> > but we need to manage a lot of hassle during PSR exit..
> > (like Link training, DPIO/PLL bring up etc..)
> 
> Also I think this blocked sink crc to work. So we wouldn't be able to
> validate it.
> Although there are other things blocking that tests :(
> But I can double check that anyway.

Well we only need to wake up dp aux to get the sink crc, which should just
be the power well. There shouldn't be any need to wake up the source port
at all. So I don't expect big issues here if we do this. Yeah, I'm an
optimist as ever ;-)
-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