drm/i915/psr: vbt change for psr

Message ID	1527044741-4756-1-git-send-email-vathsala.nagaraju@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: vathsala nagaraju <vathsala.nagaraju@intel.com> To: intel-gfx@lists.freedesktop.org Date: Wed, 23 May 2018 08:35:41 +0530 Message-Id: <1527044741-4756-1-git-send-email-vathsala.nagaraju@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH] drm/i915/psr: vbt change for psr Precedence: list Cc: Jani Nikula <jani.nikula@intel.com>, Maulik V Vaghela <maulik.v.vaghela@intel.com>, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>, Rodrigo Vivi <rodrigo.vivi@intel.com>, Puthikorn Voravootivat <puthik@chromium.org> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Message ID

1527044741-4756-1-git-send-email-vathsala.nagaraju@intel.com (mailing list archive)

State

New, archived

Headers

From: vathsala nagaraju <vathsala.nagaraju@intel.com>
To: intel-gfx@lists.freedesktop.org
Date: Wed, 23 May 2018 08:35:41 +0530
Message-Id: <1527044741-4756-1-git-send-email-vathsala.nagaraju@intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH] drm/i915/psr: vbt change for psr
Precedence: list
Cc: Jani Nikula <jani.nikula@intel.com>,
	Maulik V Vaghela <maulik.v.vaghela@intel.com>,
	Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Puthikorn Voravootivat <puthik@chromium.org>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Commit Message

vathsala nagaraju May 23, 2018, 3:05 a.m. UTC

From: Vathsala Nagaraju <vathsala.nagaraju@intel.com>

For psr block #9, the vbt description has moved to options [0-3] for
TP1,TP2,TP3 Wakeup time from decimal value without any change to vbt
structure. Since spec does not  mention from which VBT version this
change was added to vbt.bsf file, we cannot depend on bdb->version check
to change for all the platforms.

There is RCR inplace for GOP team to  provide the version number
to make generic change. Since Kabylake with bdb version 209 is having this
change, limiting this change to gen9_bc and version 209+ to unblock google.

Tested on skl(bdb version 203,without options) and
kabylake(bdb version 209,212) having new options.

bspec 20131

v2: (Jani and Rodrigo)
    move the 165 version check to intel_bios.c
v3: Jani
    Move the abstraction to intel_bios.
v4: Jani
    Rename tp*_wakeup_time to have "us" suffix.
    For values outside range[0-3],default to max 2500us.
    Old decimal value was wake up time in multiples of 100us.
v5: Jani and Rodrigo
    Handle option 2 in default condition.
    Print oustide range value.
    For negetive values default to 2500us.
v6: Jani
    Handle default first and then fall through for case 2.
v7: Rodrigo
    Apply this change for IS_GEN9_BC and vbt version > 209
v8: Puthik
    Add new function vbt_psr_to_us.
v9: Jani
    Change to v7 version as it's more readable.
    DK
    add comment /*fall through*/ after case2.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Puthikorn Voravootivat <puthik@chromium.org>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>

Signed-off-by: Maulik V Vaghela <maulik.v.vaghela@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  4 ++--
 drivers/gpu/drm/i915/i915_reg.h   |  8 +++----
 drivers/gpu/drm/i915/intel_bios.c | 48 +++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_psr.c  | 39 +++++++++++++++----------------
 4 files changed, 72 insertions(+), 27 deletions(-)

Comments

Jani Nikula May 23, 2018, 10:03 a.m. UTC | #1

On Wed, 23 May 2018, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
> From: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>
> For psr block #9, the vbt description has moved to options [0-3] for
> TP1,TP2,TP3 Wakeup time from decimal value without any change to vbt
> structure. Since spec does not  mention from which VBT version this
> change was added to vbt.bsf file, we cannot depend on bdb->version check
> to change for all the platforms.
>
> There is RCR inplace for GOP team to  provide the version number
> to make generic change. Since Kabylake with bdb version 209 is having this
> change, limiting this change to gen9_bc and version 209+ to unblock google.
>
> Tested on skl(bdb version 203,without options) and
> kabylake(bdb version 209,212) having new options.
>
> bspec 20131
>
> v2: (Jani and Rodrigo)
>     move the 165 version check to intel_bios.c
> v3: Jani
>     Move the abstraction to intel_bios.
> v4: Jani
>     Rename tp*_wakeup_time to have "us" suffix.
>     For values outside range[0-3],default to max 2500us.
>     Old decimal value was wake up time in multiples of 100us.
> v5: Jani and Rodrigo
>     Handle option 2 in default condition.
>     Print oustide range value.
>     For negetive values default to 2500us.
> v6: Jani
>     Handle default first and then fall through for case 2.
> v7: Rodrigo
>     Apply this change for IS_GEN9_BC and vbt version > 209
> v8: Puthik
>     Add new function vbt_psr_to_us.
> v9: Jani
>     Change to v7 version as it's more readable.
>     DK
>     add comment /*fall through*/ after case2.

Why did you send this again? Did you change something? I already
reviewed the version from yesterday.

BR,
Jani.

vathsala nagaraju May 23, 2018, 12:55 p.m. UTC | #2

On 5/23/2018 3:33 PM, Jani Nikula wrote:
> On Wed, 23 May 2018, vathsala nagaraju <vathsala.nagaraju@intel.com> wrote:
>> From: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>>
>> For psr block #9, the vbt description has moved to options [0-3] for
>> TP1,TP2,TP3 Wakeup time from decimal value without any change to vbt
>> structure. Since spec does not  mention from which VBT version this
>> change was added to vbt.bsf file, we cannot depend on bdb->version check
>> to change for all the platforms.
>>
>> There is RCR inplace for GOP team to  provide the version number
>> to make generic change. Since Kabylake with bdb version 209 is having this
>> change, limiting this change to gen9_bc and version 209+ to unblock google.
>>
>> Tested on skl(bdb version 203,without options) and
>> kabylake(bdb version 209,212) having new options.
>>
>> bspec 20131
>>
>> v2: (Jani and Rodrigo)
>>      move the 165 version check to intel_bios.c
>> v3: Jani
>>      Move the abstraction to intel_bios.
>> v4: Jani
>>      Rename tp*_wakeup_time to have "us" suffix.
>>      For values outside range[0-3],default to max 2500us.
>>      Old decimal value was wake up time in multiples of 100us.
>> v5: Jani and Rodrigo
>>      Handle option 2 in default condition.
>>      Print oustide range value.
>>      For negetive values default to 2500us.
>> v6: Jani
>>      Handle default first and then fall through for case 2.
>> v7: Rodrigo
>>      Apply this change for IS_GEN9_BC and vbt version > 209
>> v8: Puthik
>>      Add new function vbt_psr_to_us.
>> v9: Jani
>>      Change to v7 version as it's more readable.
>>      DK
>>      add comment /*fall through*/ after case2.
> Why did you send this again? Did you change something? I already
> reviewed the version from yesterday.
no code change, added only  rb tag by you and resent it.
> BR,
> Jani.
>
>

Jani Nikula May 23, 2018, 1:10 p.m. UTC | #3

On Wed, 23 May 2018, "Nagaraju, Vathsala" <vathsala.nagaraju@intel.com> wrote:
> On 5/23/2018 3:33 PM, Jani Nikula wrote:
>> Why did you send this again? Did you change something? I already
>> reviewed the version from yesterday.
> no code change, added only  rb tag by you and resent it.

For future reference, you should *not* resend for that.

BR,
Jani.

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e33c380..dcfa791 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1078,8 +1078,8 @@  struct intel_vbt_data {
 		bool require_aux_wakeup;
 		int idle_frames;
 		enum psr_lines_to_wait lines_to_wait;
-		int tp1_wakeup_time;
-		int tp2_tp3_wakeup_time;
+		int tp1_wakeup_time_us;
+		int tp2_tp3_wakeup_time_us;
 	} psr;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 196a0eb..513b4a4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4088,10 +4088,10 @@  enum {
 #define   EDP_Y_COORDINATE_ENABLE	(1<<25) /* GLK and CNL+ */
 #define   EDP_MAX_SU_DISABLE_TIME(t)	((t)<<20)
 #define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f<<20)
-#define   EDP_PSR2_TP2_TIME_500		(0<<8)
-#define   EDP_PSR2_TP2_TIME_100		(1<<8)
-#define   EDP_PSR2_TP2_TIME_2500	(2<<8)
-#define   EDP_PSR2_TP2_TIME_50		(3<<8)
+#define   EDP_PSR2_TP2_TIME_500us	(0<<8)
+#define   EDP_PSR2_TP2_TIME_100us	(1<<8)
+#define   EDP_PSR2_TP2_TIME_2500us	(2<<8)
+#define   EDP_PSR2_TP2_TIME_50us	(3<<8)
 #define   EDP_PSR2_TP2_TIME_MASK	(3<<8)
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 54270bd..417f656 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -688,8 +688,52 @@  static int intel_bios_ssc_frequency(struct drm_i915_private *dev_priv,
 		break;
 	}
 
-	dev_priv->vbt.psr.tp1_wakeup_time = psr_table->tp1_wakeup_time;
-	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
+	/*
+	 * New psr options 0=500us, 1=100us, 2=2500us, 3=0us
+	 * Old decimal value is wake up time in multiples of 100 us.
+	 */
+	if (bdb->version >= 209 && IS_GEN9_BC(dev_priv)) {
+		switch (psr_table->tp1_wakeup_time) {
+		case 0:
+			dev_priv->vbt.psr.tp1_wakeup_time_us = 500;
+			break;
+		case 1:
+			dev_priv->vbt.psr.tp1_wakeup_time_us = 100;
+			break;
+		case 3:
+			dev_priv->vbt.psr.tp1_wakeup_time_us = 0;
+			break;
+		default:
+			DRM_DEBUG_KMS("VBT tp1 wakeup time value %d is outside range[0-3], defaulting to max value 2500us\n",
+					psr_table->tp1_wakeup_time);
+			/* fallthrough */
+		case 2:
+			dev_priv->vbt.psr.tp1_wakeup_time_us = 2500;
+			break;
+		}
+
+		switch (psr_table->tp2_tp3_wakeup_time) {
+		case 0:
+			dev_priv->vbt.psr.tp2_tp3_wakeup_time_us = 500;
+			break;
+		case 1:
+			dev_priv->vbt.psr.tp2_tp3_wakeup_time_us = 100;
+			break;
+		case 3:
+			dev_priv->vbt.psr.tp1_wakeup_time_us = 0;
+			break;
+		default:
+			DRM_DEBUG_KMS("VBT tp2_tp3 wakeup time value %d is outside range[0-3], defaulting to max value 2500us\n",
+					psr_table->tp2_tp3_wakeup_time);
+			/* fallthrough */
+		case 2:
+			dev_priv->vbt.psr.tp2_tp3_wakeup_time_us = 2500;
+		break;
+		}
+	} else {
+		dev_priv->vbt.psr.tp1_wakeup_time_us = psr_table->tp1_wakeup_time * 100;
+		dev_priv->vbt.psr.tp2_tp3_wakeup_time_us = psr_table->tp2_tp3_wakeup_time * 100;
+	}
 }
 
 static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index db27f2f..d64f039 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -461,23 +461,23 @@  static void hsw_activate_psr1(struct intel_dp *intel_dp)
 	if (dev_priv->psr.link_standby)
 		val |= EDP_PSR_LINK_STANDBY;
 
-	if (dev_priv->vbt.psr.tp1_wakeup_time > 5)
-		val |= EDP_PSR_TP1_TIME_2500us;
-	else if (dev_priv->vbt.psr.tp1_wakeup_time > 1)
-		val |= EDP_PSR_TP1_TIME_500us;
-	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
+	if (dev_priv->vbt.psr.tp1_wakeup_time_us == 0)
+		val |=  EDP_PSR_TP1_TIME_0us;
+	else if (dev_priv->vbt.psr.tp1_wakeup_time_us <= 100)
 		val |= EDP_PSR_TP1_TIME_100us;
+	else if (dev_priv->vbt.psr.tp1_wakeup_time_us <= 500)
+		val |= EDP_PSR_TP1_TIME_500us;
 	else
-		val |= EDP_PSR_TP1_TIME_0us;
+		val |= EDP_PSR_TP1_TIME_2500us;
 
-	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
-		val |= EDP_PSR_TP2_TP3_TIME_2500us;
-	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
-		val |= EDP_PSR_TP2_TP3_TIME_500us;
-	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
+	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us == 0)
+		val |=  EDP_PSR_TP2_TP3_TIME_0us;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 100)
 		val |= EDP_PSR_TP2_TP3_TIME_100us;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 500)
+		val |= EDP_PSR_TP2_TP3_TIME_500us;
 	else
-		val |= EDP_PSR_TP2_TP3_TIME_0us;
+		val |= EDP_PSR_TP2_TP3_TIME_2500us;
 
 	if (intel_dp_source_supports_hbr2(intel_dp) &&
 	    drm_dp_tps3_supported(intel_dp->dpcd))
@@ -513,14 +513,15 @@  static void hsw_activate_psr2(struct intel_dp *intel_dp)
 
 	val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
 
-	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
-		val |= EDP_PSR2_TP2_TIME_2500;
-	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
-		val |= EDP_PSR2_TP2_TIME_500;
-	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
-		val |= EDP_PSR2_TP2_TIME_100;
+	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 &&
+	    dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
+		val |= EDP_PSR2_TP2_TIME_50us;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 100)
+		val |= EDP_PSR2_TP2_TIME_100us;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 500)
+		val |= EDP_PSR2_TP2_TIME_500us;
 	else
-		val |= EDP_PSR2_TP2_TIME_50;
+		val |= EDP_PSR2_TP2_TIME_2500us;
 
 	I915_WRITE(EDP_PSR2_CTL, val);
 }

drm/i915/psr: vbt change for psr

Commit Message

Comments

Patch