* [PATCH] drm/i915/psr: vbt change for psr
@ 2018-05-03 11:36 vathsala nagaraju
2018-05-03 12:52 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: vbt change for psr (rev6) Patchwork
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: vathsala nagaraju @ 2018-05-03 11:36 UTC (permalink / raw)
To: rodrigo.vivi, jani.nikula, intel-gfx
Cc: Puthikorn Voravootivat, Maulik V Vaghela
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 kbl 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.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
CC: Puthikorn Voravootivat <puthik@chromium.org>
Signed-off-by: Maulik V Vaghela <maulik.v.vaghela@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@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 | 46 +++++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_psr.c | 39 +++++++++++++++++----------------
4 files changed, 70 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6268a51..a189382 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1077,8 +1077,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 197c966..6bbd0b4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4084,10 +4084,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 702d3fa..166f704 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -687,8 +687,50 @@ 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_KABYLAKE(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);
+ 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);
+ 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 6233a32..f03dfba 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);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: vbt change for psr (rev6)
2018-05-03 11:36 [PATCH] drm/i915/psr: vbt change for psr vathsala nagaraju
@ 2018-05-03 12:52 ` Patchwork
2018-05-03 13:07 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-03 12:52 UTC (permalink / raw)
To: vathsala nagaraju; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: vbt change for psr (rev6)
URL : https://patchwork.freedesktop.org/series/41289/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
824c9eb50c54 drm/i915/psr: vbt change for psr
-:69: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#69: FILE: drivers/gpu/drm/i915/i915_reg.h:4087:
+#define EDP_PSR2_TP2_TIME_500us (0<<8)
^
-:70: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#70: FILE: drivers/gpu/drm/i915/i915_reg.h:4088:
+#define EDP_PSR2_TP2_TIME_100us (1<<8)
^
-:71: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#71: FILE: drivers/gpu/drm/i915/i915_reg.h:4089:
+#define EDP_PSR2_TP2_TIME_2500us (2<<8)
^
-:72: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#72: FILE: drivers/gpu/drm/i915/i915_reg.h:4090:
+#define EDP_PSR2_TP2_TIME_50us (3<<8)
^
-:103: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#103: FILE: drivers/gpu/drm/i915/intel_bios.c:707:
+ 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);
-:120: WARNING:LINE_CONTINUATIONS: Avoid line continuations in quoted strings
#120: FILE: drivers/gpu/drm/i915/intel_bios.c:724:
+ DRM_DEBUG_KMS("VBT tp2_tp3 wakeup time value %d is outside range[0-3], \
-:121: WARNING:LONG_LINE: line over 100 characters
#121: FILE: drivers/gpu/drm/i915/intel_bios.c:725:
+ defaulting to max value 2500us\n", psr_table->tp2_tp3_wakeup_time);
total: 0 errors, 2 warnings, 5 checks, 133 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/psr: vbt change for psr (rev6)
2018-05-03 11:36 [PATCH] drm/i915/psr: vbt change for psr vathsala nagaraju
2018-05-03 12:52 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: vbt change for psr (rev6) Patchwork
@ 2018-05-03 13:07 ` Patchwork
2018-05-03 15:44 ` [PATCH] drm/i915/psr: vbt change for psr Rodrigo Vivi
2018-05-03 17:19 ` ✓ Fi.CI.IGT: success for drm/i915/psr: vbt change for psr (rev6) Patchwork
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-03 13:07 UTC (permalink / raw)
To: vathsala nagaraju; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: vbt change for psr (rev6)
URL : https://patchwork.freedesktop.org/series/41289/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4131 -> Patchwork_8897 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8897 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8897, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/41289/revisions/6/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8897:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_8897 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-ivb-3520m: PASS -> DMESG-WARN (fdo#106084)
==== Possible fixes ====
igt@drv_module_reload@basic-reload:
fi-bsw-n3050: DMESG-FAIL (fdo#106373) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-skl-6770hq: FAIL (fdo#100368) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-skl-6770hq: FAIL (fdo#103481) -> PASS +1
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
fdo#106373 https://bugs.freedesktop.org/show_bug.cgi?id=106373
== Participating hosts (39 -> 36) ==
Additional (1): fi-byt-j1900
Missing (4): fi-ctg-p8600 fi-kbl-7560u fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4131 -> Patchwork_8897
CI_DRM_4131: 46d3a67e7a5611ef8af00cb7adebf03817856645 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4459: 1b8977e08031253d61b4641bc21e5c7a990d4a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8897: 824c9eb50c54372c51f1b4bf6f417256fe6fb1bd @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4459: f74d92e704849610364b4474a2c67ea2008c14e0 @ git://anongit.freedesktop.org/piglit
== Linux commits ==
824c9eb50c54 drm/i915/psr: vbt change for psr
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8897/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/psr: vbt change for psr
2018-05-03 11:36 [PATCH] drm/i915/psr: vbt change for psr vathsala nagaraju
2018-05-03 12:52 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: vbt change for psr (rev6) Patchwork
2018-05-03 13:07 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-05-03 15:44 ` Rodrigo Vivi
2018-05-03 17:13 ` Nagaraju, Vathsala
2018-05-03 17:19 ` ✓ Fi.CI.IGT: success for drm/i915/psr: vbt change for psr (rev6) Patchwork
3 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2018-05-03 15:44 UTC (permalink / raw)
To: vathsala nagaraju; +Cc: Puthikorn Voravootivat, intel-gfx, Maulik V Vaghela
On Thu, May 03, 2018 at 05:06:09PM +0530, vathsala nagaraju 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 kbl 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.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
>
> Signed-off-by: Maulik V Vaghela <maulik.v.vaghela@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@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 | 46 +++++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_psr.c | 39 +++++++++++++++++----------------
> 4 files changed, 70 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6268a51..a189382 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1077,8 +1077,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 197c966..6bbd0b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4084,10 +4084,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 702d3fa..166f704 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -687,8 +687,50 @@ 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_KABYLAKE(dev_priv)) {
Please file a BSpec bug and get that mess cleaned up first before spreading it here.
I'm looking to the same 20131 and it says "wake up time in multiples of 100" and VBT version 165
Ok, I also see on 20131 the mention to:
"
0 = 500 usec
1 = 100 usec
2 = 2.5 msec
3 = 0 usec (Skip TP1)
"
But it has absolutely no mention to "209" or "Kabylake" or any other platform.
I doubt that it would be for Kabylake and not for Coffelake for instance.
Thanks,
Rodrigo.
> + 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);
> + 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);
> + 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 6233a32..f03dfba 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);
> }
> --
> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/psr: vbt change for psr
2018-05-03 15:44 ` [PATCH] drm/i915/psr: vbt change for psr Rodrigo Vivi
@ 2018-05-03 17:13 ` Nagaraju, Vathsala
2018-05-04 23:13 ` Puthikorn Voravootivat
0 siblings, 1 reply; 7+ messages in thread
From: Nagaraju, Vathsala @ 2018-05-03 17:13 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: Puthikorn Voravootivat, intel-gfx, Vaghela, Maulik V
Rodrigo,
The changes are already in place on kabylake 209+ onwards, So limiting this change to only kabylake and confirmed vbt version for now.
RCR is already raised for GOP team to resolve. Once we get the other platform confirmation, we can add those platforms too.
Here is what happen in Skylake VBT description file , this is decimal , no wake options.
[1] $PSR_TP_2_3_WaitTime_01 2 bytes ; TP2/TP3 wake up time in multiples of 100
[2] EditNum $PSR_TP_2_3_WaitTime_01, "TP2/TP3 WakeUp Time:", DEC,
Help "This field selects the link training TP2(Training Pattern2) or TP3(Training Pattern3) time during PSR exit(wake up)\n"
"TP2/TP3 wake up time in multiples of 100us"
Here is what happen in Kaby Lake VBT description file , this is wakeoptions [0-3]
[3] $PSR_TP_2_3_WaitTime_01 2 bytes ; TP2/TP3 wake up time in multiples of 100
[4] Combo $PSR_TP_2_3_WaitTime_01, "TP2/TP3 WakeUp Time:", &PsrWakeupTimeOptions,
Help "This field selects the link training TP2(Training Pattern2) or TP3(Training Pattern3) time during PSR exit(wake up)\n"
[5] List &PsrWakeupTimeOptions↵
Selection 0x00, "500 usec"↵
Selection 0x01, "100 usec"↵
Selection 0x02, "2.5 msec"↵
Selection 0x03, "0 (Skip)"↵
EndList↵
Regards,
Vathsala
-----Original Message-----
From: Vivi, Rodrigo
Sent: Thursday, May 3, 2018 9:15 PM
To: Nagaraju, Vathsala <vathsala.nagaraju@intel.com>
Cc: jani.nikula@linux.intel.com; intel-gfx@lists.freedesktop.org; Puthikorn Voravootivat <puthik@chromium.org>; Vaghela, Maulik V <maulik.v.vaghela@intel.com>
Subject: Re: [PATCH] drm/i915/psr: vbt change for psr
On Thu, May 03, 2018 at 05:06:09PM +0530, vathsala nagaraju 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 kbl 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.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> CC: Puthikorn Voravootivat <puthik@chromium.org>
>
> Signed-off-by: Maulik V Vaghela <maulik.v.vaghela@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@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 | 46
> +++++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_psr.c | 39
> +++++++++++++++++----------------
> 4 files changed, 70 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 6268a51..a189382 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1077,8 +1077,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 197c966..6bbd0b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4084,10 +4084,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 702d3fa..166f704 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -687,8 +687,50 @@ 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_KABYLAKE(dev_priv)) {
Please file a BSpec bug and get that mess cleaned up first before spreading it here.
I'm looking to the same 20131 and it says "wake up time in multiples of 100" and VBT version 165
Ok, I also see on 20131 the mention to:
"
0 = 500 usec
1 = 100 usec
2 = 2.5 msec
3 = 0 usec (Skip TP1)
"
But it has absolutely no mention to "209" or "Kabylake" or any other platform.
I doubt that it would be for Kabylake and not for Coffelake for instance.
Thanks,
Rodrigo.
> + 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);
> + 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);
> + 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 6233a32..f03dfba 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);
> }
> --
> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/psr: vbt change for psr (rev6)
2018-05-03 11:36 [PATCH] drm/i915/psr: vbt change for psr vathsala nagaraju
` (2 preceding siblings ...)
2018-05-03 15:44 ` [PATCH] drm/i915/psr: vbt change for psr Rodrigo Vivi
@ 2018-05-03 17:19 ` Patchwork
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-05-03 17:19 UTC (permalink / raw)
To: Nagaraju, Vathsala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/psr: vbt change for psr (rev6)
URL : https://patchwork.freedesktop.org/series/41289/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4131_full -> Patchwork_8897_full =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/41289/revisions/6/mbox/
== Known issues ==
Here are the changes found in Patchwork_8897_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_atomic@plane_primary_legacy:
shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +10
igt@kms_flip@dpms-vs-vblank-race:
shard-glk: PASS -> FAIL (fdo#103060)
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-hsw: PASS -> FAIL (fdo#100368)
igt@kms_setmode@basic:
shard-glk: PASS -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-apl: PASS -> WARN (fdo#100047)
==== Possible fixes ====
igt@kms_chv_cursor_fail@pipe-c-256x256-right-edge:
shard-apl: FAIL (fdo#104671) -> PASS
igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
shard-hsw: FAIL (fdo#105767) -> PASS
igt@kms_flip@absolute-wf_vblank-interruptible:
shard-glk: FAIL (fdo#106087) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
shard-hsw: FAIL (fdo#103928) -> PASS
igt@kms_flip@plain-flip-fb-recreate:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@wf_vblank-ts-check-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_rotation_crc@primary-rotation-180:
shard-hsw: FAIL (fdo#103925) -> PASS
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4131 -> Patchwork_8897
CI_DRM_4131: 46d3a67e7a5611ef8af00cb7adebf03817856645 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4459: 1b8977e08031253d61b4641bc21e5c7a990d4a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8897: 824c9eb50c54372c51f1b4bf6f417256fe6fb1bd @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4459: f74d92e704849610364b4474a2c67ea2008c14e0 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8897/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/psr: vbt change for psr
2018-05-03 17:13 ` Nagaraju, Vathsala
@ 2018-05-04 23:13 ` Puthikorn Voravootivat
0 siblings, 0 replies; 7+ messages in thread
From: Puthikorn Voravootivat @ 2018-05-04 23:13 UTC (permalink / raw)
To: vathsala nagaraju
Cc: Puthikorn Voravootivat, intel-gfx, maulik.v.vaghela, Rodrigo Vivi
FYI, Here is the the BDB version of the chip that need this change in
Google Pixelbook.
localhost ~ # uname -p
Intel(R) Core(TM) i5-7Y57 CPU @ 1.20GHz
localhost ~ # dmesg | grep VBT.signature
[ 0.298850] VBT signature "$VBT SKYLAKE ", BDB version 211
localhost ~ # uname -p
Intel(R) Core(TM) i7-7Y75 CPU @ 1.30GHz
localhost ~ # dmesg | grep VBT.signature
[ 0.273325] VBT signature "$VBT SKYLAKE ", BDB version 211
Best,
On Thu, May 3, 2018 at 10:14 AM Nagaraju, Vathsala <
vathsala.nagaraju@intel.com> wrote:
> Rodrigo,
> The changes are already in place on kabylake 209+ onwards, So
limiting this change to only kabylake and confirmed vbt version for now.
> RCR is already raised for GOP team to resolve. Once we get the
other platform confirmation, we can add those platforms too.
> Here is what happen in Skylake VBT description file , this is decimal ,
no wake options.
> [1] $PSR_TP_2_3_WaitTime_01 2 bytes ; TP2/TP3 wake up time in multiples
of 100
> [2] EditNum $PSR_TP_2_3_WaitTime_01, "TP2/TP3 WakeUp Time:", DEC,
> Help "This field selects the link training TP2(Training Pattern2) or
TP3(Training Pattern3) time during PSR exit(wake up)\n"
> "TP2/TP3 wake up time in multiples of 100us"
> Here is what happen in Kaby Lake VBT description file , this is
wakeoptions [0-3]
> [3] $PSR_TP_2_3_WaitTime_01 2 bytes ; TP2/TP3 wake up time in multiples
of 100
> [4] Combo $PSR_TP_2_3_WaitTime_01, "TP2/TP3 WakeUp Time:",
&PsrWakeupTimeOptions,
> Help "This field selects the link training TP2(Training Pattern2) or
TP3(Training Pattern3) time during PSR exit(wake up)\n"
> [5] List &PsrWakeupTimeOptions↵
> Selection 0x00, "500 usec"↵
> Selection 0x01, "100 usec"↵
> Selection 0x02, "2.5 msec"↵
> Selection 0x03, "0 (Skip)"↵
> EndList↵
> Regards,
> Vathsala
> -----Original Message-----
> From: Vivi, Rodrigo
> Sent: Thursday, May 3, 2018 9:15 PM
> To: Nagaraju, Vathsala <vathsala.nagaraju@intel.com>
> Cc: jani.nikula@linux.intel.com; intel-gfx@lists.freedesktop.org;
Puthikorn Voravootivat <puthik@chromium.org>; Vaghela, Maulik V <
maulik.v.vaghela@intel.com>
> Subject: Re: [PATCH] drm/i915/psr: vbt change for psr
> On Thu, May 03, 2018 at 05:06:09PM +0530, vathsala nagaraju 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 kbl 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.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > CC: Puthikorn Voravootivat <puthik@chromium.org>
> >
> > Signed-off-by: Maulik V Vaghela <maulik.v.vaghela@intel.com>
> > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@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 | 46
> > +++++++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_psr.c | 39
> > +++++++++++++++++----------------
> > 4 files changed, 70 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 6268a51..a189382 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1077,8 +1077,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 197c966..6bbd0b4 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4084,10 +4084,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 702d3fa..166f704 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -687,8 +687,50 @@ 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_KABYLAKE(dev_priv)) {
> Please file a BSpec bug and get that mess cleaned up first before
spreading it here.
> I'm looking to the same 20131 and it says "wake up time in multiples of
100" and VBT version 165
> Ok, I also see on 20131 the mention to:
> "
> 0 = 500 usec
> 1 = 100 usec
> 2 = 2.5 msec
> 3 = 0 usec (Skip TP1)
> "
> But it has absolutely no mention to "209" or "Kabylake" or any other
platform.
> I doubt that it would be for Kabylake and not for Coffelake for instance.
> Thanks,
> Rodrigo.
> > + 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);
> > + 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);
> > + 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 6233a32..f03dfba 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);
> > }
> > --
> > 1.9.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-04 23:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 11:36 [PATCH] drm/i915/psr: vbt change for psr vathsala nagaraju
2018-05-03 12:52 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: vbt change for psr (rev6) Patchwork
2018-05-03 13:07 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-03 15:44 ` [PATCH] drm/i915/psr: vbt change for psr Rodrigo Vivi
2018-05-03 17:13 ` Nagaraju, Vathsala
2018-05-04 23:13 ` Puthikorn Voravootivat
2018-05-03 17:19 ` ✓ Fi.CI.IGT: success for drm/i915/psr: vbt change for psr (rev6) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.