From mboxrd@z Thu Jan 1 00:00:00 1970 From: vathsala nagaraju Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2 Date: Mon, 25 Sep 2017 14:40:28 +0530 Message-ID: <59C8C804.6040002@intel.com> References: <1506126883-22800-1-git-send-email-vathsala.nagaraju@intel.com> <1506126883-22800-2-git-send-email-vathsala.nagaraju@intel.com> <87bmlz5gfn.fsf@nikula.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2082482554==" Return-path: In-Reply-To: <87bmlz5gfn.fsf@nikula.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jani Nikula , rodrigo.vivi@intel.com, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: Puthikorn Voravootivat List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============2082482554== Content-Type: multipart/alternative; boundary="------------010303030506030402020000" This is a multi-part message in MIME format. --------------010303030506030402020000 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On Monday 25 September 2017 02:00 PM, Jani Nikula wrote: > On Sat, 23 Sep 2017, vathsala nagaraju wrote: >> Set frames before SU entry value for max resync frame count of >> dpcd register 2009, bit field 0:3. >> >> v2 : >> - add macro EDP_PSR2_FRAME_BEFORE_SU (Rodrigo) >> - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo) >> - add check ==1 for dpcd_read call (ville) >> >> v3 : (Rodrigo) >> - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU >> - replace with &= >> >> Cc: Rodrigo Vivi >> CC: Puthikorn Voravootivat >> Reviewed-by: Rodrigo Vivi >> Signed-off-by: Vathsala Nagaraju >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 +- >> drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++-- >> 2 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 82f36dd..b880c84 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -4047,7 +4047,7 @@ enum { >> #define EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4 >> #define EDP_PSR2_FRAME_BEFORE_SU_MASK (0xf<<4) >> #define EDP_PSR2_IDLE_MASK 0xf >> -#define EDP_FRAMES_BEFORE_SU_ENTRY (1<<4) >> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT) > In the register definitions we use the shift values directly, not the > macro. That's the style we've adopted. Please stick to it. Macro was suggested by Rodrigo. > > Ditto for the indent, why do you remove it? BR, Jani. >> >> #define EDP_PSR2_STATUS_CTL _MMIO(0x6f940) >> #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28) >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c >> index 0a17d1f..adf7abc 100644 >> --- a/drivers/gpu/drm/i915/intel_psr.c >> +++ b/drivers/gpu/drm/i915/intel_psr.c >> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) >> */ >> uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames); >> uint32_t val; >> + uint8_t sink_latency; >> >> val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; >> >> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) >> * mesh at all with our frontbuffer tracking. And the hw alone isn't >> * good enough. */ >> val |= EDP_PSR2_ENABLE | >> - EDP_SU_TRACK_ENABLE | >> - EDP_FRAMES_BEFORE_SU_ENTRY; >> + EDP_SU_TRACK_ENABLE; >> + >> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY, >> + &sink_latency) == 1) { >> + sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK; >> + } else { >> + sink_latency = 0; >> + } >> + val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1); >> >> if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) >> val |= EDP_PSR2_TP2_TIME_2500; --------------010303030506030402020000 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 7bit
On Monday 25 September 2017 02:00 PM, Jani Nikula wrote:
On Sat, 23 Sep 2017, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

v3 : (Rodrigo)
 - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
 - replace with &=

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..b880c84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,7 +4047,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK		0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
+#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)

In the register definitions we use the shift values directly, not the
macro. That's the style we've adopted. Please stick to it.
Macro was suggested by Rodrigo. 

Ditto for the indent, why do you remove it?
BR, Jani. 
 
 #define EDP_PSR2_STATUS_CTL            _MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK     (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..adf7abc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
 	uint32_t val;
+	uint8_t sink_latency;
 
 	val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 	 * good enough. */
 	val |= EDP_PSR2_ENABLE |
-		EDP_SU_TRACK_ENABLE |
-		EDP_FRAMES_BEFORE_SU_ENTRY;
+		EDP_SU_TRACK_ENABLE;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+				&sink_latency) == 1) {
+		sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
+	} else {
+		sink_latency = 0;
+	}
+	val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR2_TP2_TIME_2500;


    

    

  


--------------010303030506030402020000--

--===============2082482554==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline

X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs
IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz
dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==

--===============2082482554==--