[Intel-gfx] [PATCH v2 1/3] drm/i915/bios: Parse HOBL parameter

[Intel-gfx] [PATCH v2 1/3] drm/i915/bios: Parse HOBL parameter

[José Roberto de Souza]

HOBL means hours of battery life, it is a power-saving feature
were supported motherboards can use a special voltage swing table
that uses less power.

So here parsing the VBT to check if this feature is supported.
While at it already added the VRR parameter too.

BSpec: 20150
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c     | 3 +++
 drivers/gpu/drm/i915/display/intel_vbt_defs.h | 2 ++
 drivers/gpu/drm/i915/i915_drv.h               | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 839124647202..b3c453aa7623 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -722,6 +722,9 @@ parse_power_conservation_features(struct drm_i915_private *dev_priv,
 	 */
 	if (!(power->drrs & BIT(panel_type)))
 		dev_priv->vbt.drrs_type = DRRS_NOT_SUPPORTED;
+
+	if (bdb->version >= 232)
+		dev_priv->vbt.edp.hobl = power->hobl & BIT(panel_type);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index aef7fe932d1a..65f552f57e06 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -820,6 +820,8 @@ struct bdb_lfp_power {
 	u16 adb;
 	u16 lace_enabled_status;
 	struct agressiveness_profile_entry aggressivenes[16];
+	u16 hobl; /* 232+ */
+	u16 vrr; /* 233+ */
 } __packed;
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e99255e17eb7..2336c9231eef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -690,6 +690,7 @@ struct intel_vbt_data {
 		bool initialized;
 		int bpp;
 		struct edp_power_seq pps;
+		bool hobl;
 	} edp;
 
 	struct {
-- 
2.27.0

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

[Intel-gfx] [PATCH v2 2/3] drm/i915/display: Implement HOBL

[José Roberto de Souza]

Hours Of Battery Life is a new GEN12+ power-saving feature that allows
supported motherboards to use a special voltage swing table for eDP
panels that uses less power.

So here if supported by HW, OEM will set it in VBT and i915 will try
to train link with HOBL vswing table if link training fails it fall
back to the original table.

Just not sure if DP compliance should also use this new voltage swing
table too, cced some folks that worked in DP compliance.

BSpec: 49291
BSpec: 49399
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 48 +++++++++++++++++--
 .../drm/i915/display/intel_display_types.h    |  2 +
 .../drm/i915/display/intel_dp_link_training.c | 20 +++++++-
 drivers/gpu/drm/i915/i915_drv.h               |  2 +
 drivers/gpu/drm/i915/i915_reg.h               |  2 +
 5 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 236f3762b6f9..57174a111976 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -692,6 +692,10 @@ static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_dp_hbr2[] =
 	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
 };
 
+static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_edp_hbr2_hobl[] = {
+	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 }
+};
+
 static const struct ddi_buf_trans *
 bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -2301,14 +2305,51 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
 	intel_de_write(dev_priv, CNL_PORT_TX_DW5_GRP(port), val);
 }
 
+/*
+ * If supported return HOBL vswing table and set registers to enable HOBL
+ * otherwise returns NULL and unset registers to enable HOBL.
+ */
+static const struct cnl_ddi_buf_trans *
+hobl_get_combo_buf_trans(struct drm_i915_private *dev_priv,
+			 struct intel_encoder *encoder, int type, int rate,
+			 u32 level, int *n_entries)
+{
+	const u32 hobl_en = EDP4K2K_MODE_OVRD_EN | EDP4K2K_MODE_OVRD_OPTIMIZED;
+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
+	struct intel_dp *intel_dp;
+
+	if (!HAS_HOBL(dev_priv) || type != INTEL_OUTPUT_EDP)
+		return NULL;
+
+	intel_dp = enc_to_intel_dp(encoder);
+	if (!intel_dp->try_hobl || rate > 540000) {
+		intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, 0);
+		return NULL;
+	}
+
+	drm_dbg_kms(&dev_priv->drm, "Enabling HOBL in PHY %c\n", phy_name(phy));
+	drm_WARN_ON_ONCE(&dev_priv->drm, level > 0);
+
+	intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, hobl_en);
+	/* Same table applies to TGL, RKL and DG1 */
+	*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
+	return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
+}
+
 static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
-					u32 level, enum phy phy, int type,
-					int rate)
+					 struct intel_encoder *encoder,
+					 u32 level, enum phy phy, int type,
+					 int rate)
 {
 	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
 	u32 n_entries, val;
 	int ln;
 
+	ddi_translations = hobl_get_combo_buf_trans(dev_priv, encoder, type,
+						    rate, level, &n_entries);
+	if (ddi_translations)
+		goto hobl_found;
+
 	if (INTEL_GEN(dev_priv) >= 12)
 		ddi_translations = tgl_get_combo_buf_trans(dev_priv, type, rate,
 							   &n_entries);
@@ -2321,6 +2362,7 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
 	if (!ddi_translations)
 		return;
 
+hobl_found:
 	if (level >= n_entries) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "DDI translation not found for level %d. Using %d instead.",
@@ -2428,7 +2470,7 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
 	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
 
 	/* 5. Program swing and de-emphasis */
-	icl_ddi_combo_vswing_program(dev_priv, level, phy, type, rate);
+	icl_ddi_combo_vswing_program(dev_priv, encoder, level, phy, type, rate);
 
 	/* 6. Set training enable to trigger update */
 	val = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN0(phy));
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4b0aaa3081c9..f8943b67819d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1375,6 +1375,8 @@ struct intel_dp {
 
 	/* Display stream compression testing */
 	bool force_dsc_en;
+
+	bool try_hobl;
 };
 
 enum lspcon_vendor {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index b9e4ee2dbddc..88f366bb28d7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -52,12 +52,24 @@ static u8 dp_voltage_max(u8 preemph)
 void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
 			       const u8 link_status[DP_LINK_STATUS_SIZE])
 {
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	u8 v = 0;
 	u8 p = 0;
 	int lane;
 	u8 voltage_max;
 	u8 preemph_max;
 
+	if (intel_dp->try_hobl) {
+		/*
+		 * Do not adjust, try now with the regular table using VSwing 0
+		 * and pre-emp 0
+		 */
+		intel_dp->try_hobl = false;
+		drm_dbg_kms(&dev_priv->drm, "HOBL vswing table failed link "
+			    "training, switching back to regular table\n");
+		return;
+	}
+
 	for (lane = 0; lane < intel_dp->lane_count; lane++) {
 		v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
 		p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
@@ -103,9 +115,13 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 }
 
 static bool
-intel_dp_reset_link_train(struct intel_dp *intel_dp,
-			u8 dp_train_pat)
+intel_dp_reset_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
 {
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+	if (intel_dp_is_edp(intel_dp) && dev_priv->vbt.edp.hobl)
+		intel_dp->try_hobl = true;
+
 	memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
 	intel_dp_set_signal_levels(intel_dp);
 	return intel_dp_set_link_train(intel_dp, dp_train_pat);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2336c9231eef..c7e7df17eef2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1687,6 +1687,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define INTEL_DISPLAY_ENABLED(dev_priv) \
 		(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
 
+#define HAS_HOBL(dev_priv) (INTEL_GEN(dev_priv) >= 12)
+
 static inline bool intel_vtd_active(void)
 {
 #ifdef CONFIG_INTEL_IOMMU
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 578cfe11cbb9..d4611171f075 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1896,6 +1896,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define  PWR_DOWN_LN_3_1_0		(0xb << 4)
 #define  PWR_DOWN_LN_MASK		(0xf << 4)
 #define  PWR_DOWN_LN_SHIFT		4
+#define  EDP4K2K_MODE_OVRD_EN		(1 << 3)
+#define  EDP4K2K_MODE_OVRD_OPTIMIZED	(1 << 2)
 
 #define ICL_PORT_CL_DW12(phy)		_MMIO(_ICL_PORT_CL_DW(12, phy))
 #define   ICL_LANE_ENABLE_AUX		(1 << 0)
-- 
2.27.0

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

[Intel-gfx] [PATCH v2 3/3] drm/i915/display: Enable HOBL regardless the VBT value

[José Roberto de Souza]

HOBL worked in my TGL RVP even without the necessary HW support, also
it worked in more than half of the TGL machines in CI so it is worthy
to enable it by default.
Even if link training fails with this new vswing table it will only
cause one additional link training, that is worthy the try to get the
additional power-savings.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 88f366bb28d7..13f7bc0a4bc0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -119,7 +119,7 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 
-	if (intel_dp_is_edp(intel_dp) && dev_priv->vbt.edp.hobl)
+	if (HAS_HOBL(dev_priv) && intel_dp_is_edp(intel_dp))
 		intel_dp->try_hobl = true;
 
 	memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
-- 
2.27.0

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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter
URL   : https://patchwork.freedesktop.org/series/77966/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/display/intel_display.c:1222:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1225:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1228:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1231:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2274:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2275:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2276:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2277:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2278:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2279:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_reset.c:1310:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/sysfs_engines.c:61:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/sysfs_engines.c:62:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/sysfs_engines.c:66:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1425:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1479:15: warning: memset with byte count of 16777216
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter
URL   : https://patchwork.freedesktop.org/series/77966/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8579 -> Patchwork_17857
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/index.html

Known issues
------------

  Here are the changes found in Patchwork_17857 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-byt-n2820:       [DMESG-WARN][3] ([i915#1982]) -> [PASS][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-byt-n2820/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-byt-n2820/igt@i915_module_load@reload.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-n3050:       [DMESG-WARN][5] ([i915#1982]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
    - {fi-tgl-dsi}:       [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-tgl-dsi/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-tgl-dsi/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * {igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1}:
    - fi-bwr-2160:        [FAIL][9] ([i915#1928]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1.html

  * {igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1}:
    - fi-icl-u2:          [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][14] ([i915#62] / [i915#92]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-kbl-x1275/igt@i915_pm_rpm@basic-pci-d3-state.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-kbl-x1275/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (51 -> 44)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8579 -> Patchwork_17857

  CI-20190529: 20190529
  CI_DRM_8579: 289eb12c88c49a4ac8d325dc457d8878c7f5bdc0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5694: a9b6c4c74bfddf7d3d2da3be08804fe315945cea @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17857: e333b5ede9369eb28a775ce63e45def78b64d4a3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e333b5ede936 drm/i915/display: Enable HOBL regardless the VBT value
35dda9799440 drm/i915/display: Implement HOBL
f4eeb33eb557 drm/i915/bios: Parse HOBL parameter

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/display: Implement HOBL

[Ville Syrjälä]

On Wed, Jun 03, 2020 at 12:43:07PM -0700, José Roberto de Souza wrote:
> Hours Of Battery Life is a new GEN12+ power-saving feature that allows
> supported motherboards to use a special voltage swing table for eDP
> panels that uses less power.
> 
> So here if supported by HW, OEM will set it in VBT and i915 will try
> to train link with HOBL vswing table if link training fails it fall
> back to the original table.
> 
> Just not sure if DP compliance should also use this new voltage swing
> table too, cced some folks that worked in DP compliance.
> 
> BSpec: 49291
> BSpec: 49399
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 48 +++++++++++++++++--
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  .../drm/i915/display/intel_dp_link_training.c | 20 +++++++-
>  drivers/gpu/drm/i915/i915_drv.h               |  2 +
>  drivers/gpu/drm/i915/i915_reg.h               |  2 +
>  5 files changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 236f3762b6f9..57174a111976 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -692,6 +692,10 @@ static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_dp_hbr2[] =
>  	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
>  };
>  
> +static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_edp_hbr2_hobl[] = {
> +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 }
> +};
> +
>  static const struct ddi_buf_trans *
>  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -2301,14 +2305,51 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	intel_de_write(dev_priv, CNL_PORT_TX_DW5_GRP(port), val);
>  }
>  
> +/*
> + * If supported return HOBL vswing table and set registers to enable HOBL
> + * otherwise returns NULL and unset registers to enable HOBL.
> + */
> +static const struct cnl_ddi_buf_trans *
> +hobl_get_combo_buf_trans(struct drm_i915_private *dev_priv,
> +			 struct intel_encoder *encoder, int type, int rate,
> +			 u32 level, int *n_entries)
> +{
> +	const u32 hobl_en = EDP4K2K_MODE_OVRD_EN | EDP4K2K_MODE_OVRD_OPTIMIZED;
> +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> +	struct intel_dp *intel_dp;
> +
> +	if (!HAS_HOBL(dev_priv) || type != INTEL_OUTPUT_EDP)
> +		return NULL;

Not a real fan of the "hobl" name. It just sounds like nonsense. Also
bspec doesn't use that term at all. It only appears in the vbt spec.
Not sure if there's a better one though.

> +
> +	intel_dp = enc_to_intel_dp(encoder);
> +	if (!intel_dp->try_hobl || rate > 540000) {
> +		intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, 0);

I would vote for just doing this programming unconditionally in the normal
sequence.

> +		return NULL;
> +	}
> +
> +	drm_dbg_kms(&dev_priv->drm, "Enabling HOBL in PHY %c\n", phy_name(phy));
> +	drm_WARN_ON_ONCE(&dev_priv->drm, level > 0);
> +
> +	intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, hobl_en);
> +	/* Same table applies to TGL, RKL and DG1 */
> +	*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> +	return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> +}
> +
>  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> -					u32 level, enum phy phy, int type,
> -					int rate)
> +					 struct intel_encoder *encoder,
> +					 u32 level, enum phy phy, int type,
> +					 int rate)

If we're passing in the encoder then a bunch of this other stuff is
redundant.

>  {
>  	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
>  	u32 n_entries, val;
>  	int ln;
>  
> +	ddi_translations = hobl_get_combo_buf_trans(dev_priv, encoder, type,
> +						    rate, level, &n_entries);
> +	if (ddi_translations)
> +		goto hobl_found;

Why not just put it into tgl_get_combo_buf_trans(). Hmm. I guess to not
upset .voltage_max(). This feels a bit hackish, but I don't have better
ideas for now.

> +
>  	if (INTEL_GEN(dev_priv) >= 12)
>  		ddi_translations = tgl_get_combo_buf_trans(dev_priv, type, rate,
>  							   &n_entries);
> @@ -2321,6 +2362,7 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
>  	if (!ddi_translations)
>  		return;
>  
> +hobl_found:
>  	if (level >= n_entries) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "DDI translation not found for level %d. Using %d instead.",
> @@ -2428,7 +2470,7 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
>  
>  	/* 5. Program swing and de-emphasis */
> -	icl_ddi_combo_vswing_program(dev_priv, level, phy, type, rate);
> +	icl_ddi_combo_vswing_program(dev_priv, encoder, level, phy, type, rate);
>  
>  	/* 6. Set training enable to trigger update */
>  	val = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN0(phy));
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 4b0aaa3081c9..f8943b67819d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1375,6 +1375,8 @@ struct intel_dp {
>  
>  	/* Display stream compression testing */
>  	bool force_dsc_en;
> +
> +	bool try_hobl;
>  };
>  
>  enum lspcon_vendor {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index b9e4ee2dbddc..88f366bb28d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -52,12 +52,24 @@ static u8 dp_voltage_max(u8 preemph)
>  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>  			       const u8 link_status[DP_LINK_STATUS_SIZE])
>  {
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  	u8 v = 0;
>  	u8 p = 0;
>  	int lane;
>  	u8 voltage_max;
>  	u8 preemph_max;
>  
> +	if (intel_dp->try_hobl) {
> +		/*
> +		 * Do not adjust, try now with the regular table using VSwing 0
> +		 * and pre-emp 0
> +		 */

What if the sink is still asking for vswing 0 + preemph 0? The spec is
rather ambiguous when it comes to this stuff.

The table also doesn't specify the vswing/preemph for which we should
use this optimized value. Your interpretation of 0+0 seems like the most
sensible thing, but given that the VBT can also specifiy the fast link
training vswing/preemph as something else (and maybe there was also
something like this for normal link training?) I'm not 100% sure.

Hmm. Actually noticed that all the eDP tables are missing the
vswing/preemph levels (they do have the raw mV/dB values but not the
DP spec levels). I filed a few issues in the hopes of clarification.

> +		intel_dp->try_hobl = false;
> +		drm_dbg_kms(&dev_priv->drm, "HOBL vswing table failed link "
> +			    "training, switching back to regular table\n");
> +		return;
> +	}
> +
>  	for (lane = 0; lane < intel_dp->lane_count; lane++) {
>  		v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
>  		p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
> @@ -103,9 +115,13 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
>  }
>  
>  static bool
> -intel_dp_reset_link_train(struct intel_dp *intel_dp,
> -			u8 dp_train_pat)
> +intel_dp_reset_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
>  {
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> +	if (intel_dp_is_edp(intel_dp) && dev_priv->vbt.edp.hobl)
> +		intel_dp->try_hobl = true;

If it failed once does it make sense to keep trying to use it?

> +
>  	memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
>  	intel_dp_set_signal_levels(intel_dp);
>  	return intel_dp_set_link_train(intel_dp, dp_train_pat);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2336c9231eef..c7e7df17eef2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1687,6 +1687,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define INTEL_DISPLAY_ENABLED(dev_priv) \
>  		(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
>  
> +#define HAS_HOBL(dev_priv) (INTEL_GEN(dev_priv) >= 12)
> +
>  static inline bool intel_vtd_active(void)
>  {
>  #ifdef CONFIG_INTEL_IOMMU
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 578cfe11cbb9..d4611171f075 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1896,6 +1896,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define  PWR_DOWN_LN_3_1_0		(0xb << 4)
>  #define  PWR_DOWN_LN_MASK		(0xf << 4)
>  #define  PWR_DOWN_LN_SHIFT		4
> +#define  EDP4K2K_MODE_OVRD_EN		(1 << 3)
> +#define  EDP4K2K_MODE_OVRD_OPTIMIZED	(1 << 2)
>  
>  #define ICL_PORT_CL_DW12(phy)		_MMIO(_ICL_PORT_CL_DW(12, phy))
>  #define   ICL_LANE_ENABLE_AUX		(1 << 0)
> -- 
> 2.27.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/display: Implement HOBL

[Souza, Jose]

On Wed, 2020-06-03 at 23:33 +0300, Ville Syrjälä wrote:
> On Wed, Jun 03, 2020 at 12:43:07PM -0700, José Roberto de Souza wrote:
> > Hours Of Battery Life is a new GEN12+ power-saving feature that allows
> > supported motherboards to use a special voltage swing table for eDP
> > panels that uses less power.
> > 
> > So here if supported by HW, OEM will set it in VBT and i915 will try
> > to train link with HOBL vswing table if link training fails it fall
> > back to the original table.
> > 
> > Just not sure if DP compliance should also use this new voltage swing
> > table too, cced some folks that worked in DP compliance.
> > 
> > BSpec: 49291
> > BSpec: 49399
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 48 +++++++++++++++++--
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  .../drm/i915/display/intel_dp_link_training.c | 20 +++++++-
> >  drivers/gpu/drm/i915/i915_drv.h               |  2 +
> >  drivers/gpu/drm/i915/i915_reg.h               |  2 +
> >  5 files changed, 69 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 236f3762b6f9..57174a111976 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -692,6 +692,10 @@ static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_dp_hbr2[] =
> >  	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> >  };
> >  
> > +static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_edp_hbr2_hobl[] = {
> > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 }
> > +};
> > +
> >  static const struct ddi_buf_trans *
> >  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> >  {
> > @@ -2301,14 +2305,51 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> >  	intel_de_write(dev_priv, CNL_PORT_TX_DW5_GRP(port), val);
> >  }
> >  
> > +/*
> > + * If supported return HOBL vswing table and set registers to enable HOBL
> > + * otherwise returns NULL and unset registers to enable HOBL.
> > + */
> > +static const struct cnl_ddi_buf_trans *
> > +hobl_get_combo_buf_trans(struct drm_i915_private *dev_priv,
> > +			 struct intel_encoder *encoder, int type, int rate,
> > +			 u32 level, int *n_entries)
> > +{
> > +	const u32 hobl_en = EDP4K2K_MODE_OVRD_EN | EDP4K2K_MODE_OVRD_OPTIMIZED;
> > +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> > +	struct intel_dp *intel_dp;
> > +
> > +	if (!HAS_HOBL(dev_priv) || type != INTEL_OUTPUT_EDP)
> > +		return NULL;
> 
> Not a real fan of the "hobl" name. It just sounds like nonsense. Also
> bspec doesn't use that term at all. It only appears in the vbt spec.
> Not sure if there's a better one though.

Maybe power_optimized_edp?

> 
> > +
> > +	intel_dp = enc_to_intel_dp(encoder);
> > +	if (!intel_dp->try_hobl || rate > 540000) {
> > +		intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, 0);
> 
> I would vote for just doing this programming unconditionally in the normal
> sequence.

Thought about that but intel_combo_phy_power_up_lanes() that program this ICL_PORT_CL_DW10 is called right after tgl_ddi_vswing_sequence().

> 
> > +		return NULL;
> > +	}
> > +
> > +	drm_dbg_kms(&dev_priv->drm, "Enabling HOBL in PHY %c\n", phy_name(phy));
> > +	drm_WARN_ON_ONCE(&dev_priv->drm, level > 0);
> > +
> > +	intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, hobl_en);
> > +	/* Same table applies to TGL, RKL and DG1 */
> > +	*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > +	return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > +}
> > +
> >  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > -					u32 level, enum phy phy, int type,
> > -					int rate)
> > +					 struct intel_encoder *encoder,
> > +					 u32 level, enum phy phy, int type,
> > +					 int rate)
> 
> If we're passing in the encoder then a bunch of this other stuff is
> redundant.

Okay

> 
> >  {
> >  	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> >  	u32 n_entries, val;
> >  	int ln;
> >  
> > +	ddi_translations = hobl_get_combo_buf_trans(dev_priv, encoder, type,
> > +						    rate, level, &n_entries);
> > +	if (ddi_translations)
> > +		goto hobl_found;
> 
> Why not just put it into tgl_get_combo_buf_trans(). Hmm. I guess to not
> upset .voltage_max(). This feels a bit hackish, but I don't have better
> ideas for now.

Exactly.

> 
> > +
> >  	if (INTEL_GEN(dev_priv) >= 12)
> >  		ddi_translations = tgl_get_combo_buf_trans(dev_priv, type, rate,
> >  							   &n_entries);
> > @@ -2321,6 +2362,7 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> >  	if (!ddi_translations)
> >  		return;
> >  
> > +hobl_found:
> >  	if (level >= n_entries) {
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "DDI translation not found for level %d. Using %d instead.",
> > @@ -2428,7 +2470,7 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> >  	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
> >  
> >  	/* 5. Program swing and de-emphasis */
> > -	icl_ddi_combo_vswing_program(dev_priv, level, phy, type, rate);
> > +	icl_ddi_combo_vswing_program(dev_priv, encoder, level, phy, type, rate);
> >  
> >  	/* 6. Set training enable to trigger update */
> >  	val = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN0(phy));
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 4b0aaa3081c9..f8943b67819d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1375,6 +1375,8 @@ struct intel_dp {
> >  
> >  	/* Display stream compression testing */
> >  	bool force_dsc_en;
> > +
> > +	bool try_hobl;
> >  };
> >  
> >  enum lspcon_vendor {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index b9e4ee2dbddc..88f366bb28d7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -52,12 +52,24 @@ static u8 dp_voltage_max(u8 preemph)
> >  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> >  			       const u8 link_status[DP_LINK_STATUS_SIZE])
> >  {
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >  	u8 v = 0;
> >  	u8 p = 0;
> >  	int lane;
> >  	u8 voltage_max;
> >  	u8 preemph_max;
> >  
> > +	if (intel_dp->try_hobl) {
> > +		/*
> > +		 * Do not adjust, try now with the regular table using VSwing 0
> > +		 * and pre-emp 0
> > +		 */
> 
> What if the sink is still asking for vswing 0 + preemph 0? The spec is
> rather ambiguous when it comes to this stuff.

As it will fallback to regular table vswing 0 + preemph 0 that is not a issue.

> 
> The table also doesn't specify the vswing/preemph for which we should
> use this optimized value. Your interpretation of 0+0 seems like the most
> sensible thing, but given that the VBT can also specifiy the fast link
> training vswing/preemph as something else (and maybe there was also
> something like this for normal link training?) I'm not 100% sure.

Yeah don't make much sense it not be vswing 0 + preemph 0 but lets wait for BSpec clarification then.

> 
> Hmm. Actually noticed that all the eDP tables are missing the
> vswing/preemph levels (they do have the raw mV/dB values but not the
> DP spec levels). I filed a few issues in the hopes of clarification.
> 
> > +		intel_dp->try_hobl = false;
> > +		drm_dbg_kms(&dev_priv->drm, "HOBL vswing table failed link "
> > +			    "training, switching back to regular table\n");
> > +		return;
> > +	}
> > +
> >  	for (lane = 0; lane < intel_dp->lane_count; lane++) {
> >  		v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
> >  		p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
> > @@ -103,9 +115,13 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> >  }
> >  
> >  static bool
> > -intel_dp_reset_link_train(struct intel_dp *intel_dp,
> > -			u8 dp_train_pat)
> > +intel_dp_reset_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
> >  {
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > +	if (intel_dp_is_edp(intel_dp) && dev_priv->vbt.edp.hobl)
> > +		intel_dp->try_hobl = true;
> 
> If it failed once does it make sense to keep trying to use it?

It could pass in a different bit rate and would be to much complicated keep track of that.


Thanks for the review, lets wait for the BSpec clarifications that you asked.

> 
> > +
> >  	memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> >  	intel_dp_set_signal_levels(intel_dp);
> >  	return intel_dp_set_link_train(intel_dp, dp_train_pat);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2336c9231eef..c7e7df17eef2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1687,6 +1687,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >  #define INTEL_DISPLAY_ENABLED(dev_priv) \
> >  		(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
> >  
> > +#define HAS_HOBL(dev_priv) (INTEL_GEN(dev_priv) >= 12)
> > +
> >  static inline bool intel_vtd_active(void)
> >  {
> >  #ifdef CONFIG_INTEL_IOMMU
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 578cfe11cbb9..d4611171f075 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1896,6 +1896,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> >  #define  PWR_DOWN_LN_3_1_0		(0xb << 4)
> >  #define  PWR_DOWN_LN_MASK		(0xf << 4)
> >  #define  PWR_DOWN_LN_SHIFT		4
> > +#define  EDP4K2K_MODE_OVRD_EN		(1 << 3)
> > +#define  EDP4K2K_MODE_OVRD_OPTIMIZED	(1 << 2)
> >  
> >  #define ICL_PORT_CL_DW12(phy)		_MMIO(_ICL_PORT_CL_DW(12, phy))
> >  #define   ICL_LANE_ENABLE_AUX		(1 << 0)
> > -- 
> > 2.27.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bios: Parse HOBL parameter
URL   : https://patchwork.freedesktop.org/series/77966/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8579_full -> Patchwork_17857_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_17857_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_offset@basic-uaf:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([i915#95]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl3/igt@gem_mmap_offset@basic-uaf.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl1/igt@gem_mmap_offset@basic-uaf.html

  * igt@i915_module_load@reload:
    - shard-tglb:         [PASS][3] -> [DMESG-WARN][4] ([i915#402])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb3/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb7/igt@i915_module_load@reload.html

  * igt@kms_big_fb@x-tiled-32bpp-rotate-180:
    - shard-skl:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) +8 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl1/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl7/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-90:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl4/igt@kms_big_fb@y-tiled-32bpp-rotate-90.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl2/igt@kms_big_fb@y-tiled-32bpp-rotate-90.html

  * igt@kms_cursor_crc@pipe-c-cursor-size-change:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([i915#54])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-size-change.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-size-change.html

  * igt@kms_cursor_legacy@all-pipes-torture-bo:
    - shard-kbl:          [PASS][11] -> [DMESG-WARN][12] ([i915#128])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-kbl3/igt@kms_cursor_legacy@all-pipes-torture-bo.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-kbl1/igt@kms_cursor_legacy@all-pipes-torture-bo.html
    - shard-tglb:         [PASS][13] -> [DMESG-WARN][14] ([i915#128])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb8/igt@kms_cursor_legacy@all-pipes-torture-bo.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb2/igt@kms_cursor_legacy@all-pipes-torture-bo.html

  * igt@kms_cursor_legacy@cursorb-vs-flipa-toggle:
    - shard-glk:          [PASS][15] -> [DMESG-FAIL][16] ([i915#1925] / [i915#1926])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk9/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk8/igt@kms_cursor_legacy@cursorb-vs-flipa-toggle.html

  * igt@kms_flip_tiling@flip-yf-tiled:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl7/igt@kms_flip_tiling@flip-yf-tiled.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl6/igt@kms_flip_tiling@flip-yf-tiled.html

  * igt@kms_frontbuffer_tracking@fbc-badstride:
    - shard-glk:          [PASS][19] -> [DMESG-WARN][20] ([i915#1982])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk4/igt@kms_frontbuffer_tracking@fbc-badstride.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk9/igt@kms_frontbuffer_tracking@fbc-badstride.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move:
    - shard-tglb:         [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#49])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#1188])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl1/igt@kms_hdr@bpc-switch-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl5/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([i915#53])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl2/igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl1/igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([fdo#108145] / [i915#265]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-iclb7/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][33] -> [FAIL][34] ([i915#31])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl6/igt@kms_setmode@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl4/igt@kms_setmode@basic.html

  * igt@prime_mmap_coherency@write:
    - shard-glk:          [PASS][35] -> [DMESG-WARN][36] ([i915#118] / [i915#95])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk4/igt@prime_mmap_coherency@write.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk1/igt@prime_mmap_coherency@write.html

  * igt@syncobj_wait@single-wait-all-signaled:
    - shard-kbl:          [PASS][37] -> [DMESG-WARN][38] ([i915#93] / [i915#95]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-kbl7/igt@syncobj_wait@single-wait-all-signaled.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-kbl7/igt@syncobj_wait@single-wait-all-signaled.html

  
#### Possible fixes ####

  * {igt@gem_exec_reloc@basic-concurrent0}:
    - shard-glk:          [FAIL][39] ([i915#1930]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk4/igt@gem_exec_reloc@basic-concurrent0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk9/igt@gem_exec_reloc@basic-concurrent0.html

  * igt@gem_sync@basic-many-each:
    - shard-kbl:          [DMESG-WARN][41] ([i915#93] / [i915#95]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-kbl2/igt@gem_sync@basic-many-each.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-kbl3/igt@gem_sync@basic-many-each.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [DMESG-WARN][43] ([i915#180]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl8/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_big_fb@y-tiled-8bpp-rotate-0:
    - shard-apl:          [DMESG-WARN][45] ([i915#1982]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl2/igt@kms_big_fb@y-tiled-8bpp-rotate-0.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl4/igt@kms_big_fb@y-tiled-8bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen:
    - shard-tglb:         [DMESG-WARN][47] ([i915#402]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb2/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb1/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html

  * {igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1}:
    - shard-skl:          [FAIL][49] ([i915#46]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc:
    - shard-tglb:         [DMESG-WARN][51] ([i915#1982]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][53] ([i915#1188]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl1/igt@kms_hdr@bpc-switch.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl7/igt@kms_hdr@bpc-switch.html

  * igt@kms_plane@plane-panning-top-left-pipe-c-planes:
    - shard-skl:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56] +9 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-skl9/igt@kms_plane@plane-panning-top-left-pipe-c-planes.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-skl1/igt@kms_plane@plane-panning-top-left-pipe-c-planes.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-iclb:         [DMESG-WARN][57] ([i915#1982]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-iclb3/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-iclb1/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][59] ([fdo#109642] / [fdo#111068]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-iclb1/igt@kms_psr2_su@page_flip.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][61] ([fdo#109441]) -> [PASS][62] +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][63] ([i915#180]) -> [PASS][64] +4 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * {igt@perf@blocking-parameterized}:
    - shard-iclb:         [FAIL][65] ([i915#1542]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-iclb8/igt@perf@blocking-parameterized.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-iclb4/igt@perf@blocking-parameterized.html
    - shard-tglb:         [FAIL][67] ([i915#1542]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-tglb7/igt@perf@blocking-parameterized.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-tglb6/igt@perf@blocking-parameterized.html

  * igt@perf_pmu@rc6-runtime-pm:
    - shard-glk:          [TIMEOUT][69] ([i915#1958]) -> [PASS][70] +3 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk5/igt@perf_pmu@rc6-runtime-pm.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk9/igt@perf_pmu@rc6-runtime-pm.html

  * igt@vgem_basic@dmabuf-mmap:
    - shard-apl:          [DMESG-WARN][71] ([i915#95]) -> [PASS][72] +17 similar issues
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl2/igt@vgem_basic@dmabuf-mmap.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl4/igt@vgem_basic@dmabuf-mmap.html

  
#### Warnings ####

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          [TIMEOUT][73] ([i915#1319]) -> [TIMEOUT][74] ([i915#1319] / [i915#1635])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl1/igt@kms_content_protection@atomic-dpms.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl1/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_content_protection@lic:
    - shard-apl:          [TIMEOUT][75] ([i915#1319] / [i915#1635]) -> [FAIL][76] ([fdo#110321])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl6/igt@kms_content_protection@lic.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl7/igt@kms_content_protection@lic.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          [DMESG-FAIL][77] ([i915#49] / [i915#95]) -> [FAIL][78] ([i915#49])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-apl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-apl4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu.html

  * igt@kms_vblank@pipe-d-query-idle-hang:
    - shard-glk:          [TIMEOUT][79] ([i915#1640] / [i915#1958]) -> [SKIP][80] ([fdo#109271])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/shard-glk5/igt@kms_vblank@pipe-d-query-idle-hang.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/shard-glk9/igt@kms_vblank@pipe-d-query-idle-hang.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#128]: https://gitlab.freedesktop.org/drm/intel/issues/128
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1640]: https://gitlab.freedesktop.org/drm/intel/issues/1640
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1925]: https://gitlab.freedesktop.org/drm/intel/issues/1925
  [i915#1926]: https://gitlab.freedesktop.org/drm/intel/issues/1926
  [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#46]: https://gitlab.freedesktop.org/drm/intel/issues/46
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8579 -> Patchwork_17857

  CI-20190529: 20190529
  CI_DRM_8579: 289eb12c88c49a4ac8d325dc457d8878c7f5bdc0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5694: a9b6c4c74bfddf7d3d2da3be08804fe315945cea @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17857: e333b5ede9369eb28a775ce63e45def78b64d4a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17857/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/display: Implement HOBL

[Souza, Jose]

On Wed, 2020-06-03 at 20:55 +0000, Souza, Jose wrote:
> On Wed, 2020-06-03 at 23:33 +0300, Ville Syrjälä wrote:
> > On Wed, Jun 03, 2020 at 12:43:07PM -0700, José Roberto de Souza wrote:
> > > Hours Of Battery Life is a new GEN12+ power-saving feature that allows
> > > supported motherboards to use a special voltage swing table for eDP
> > > panels that uses less power.
> > > 
> > > So here if supported by HW, OEM will set it in VBT and i915 will try
> > > to train link with HOBL vswing table if link training fails it fall
> > > back to the original table.
> > > 
> > > Just not sure if DP compliance should also use this new voltage swing
> > > table too, cced some folks that worked in DP compliance.
> > > 
> > > BSpec: 49291
> > > BSpec: 49399
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c      | 48 +++++++++++++++++--
> > >  .../drm/i915/display/intel_display_types.h    |  2 +
> > >  .../drm/i915/display/intel_dp_link_training.c | 20 +++++++-
> > >  drivers/gpu/drm/i915/i915_drv.h               |  2 +
> > >  drivers/gpu/drm/i915/i915_reg.h               |  2 +
> > >  5 files changed, 69 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 236f3762b6f9..57174a111976 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -692,6 +692,10 @@ static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_dp_hbr2[] =
> > >  	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> > >  };
> > >  
> > > +static const struct cnl_ddi_buf_trans tgl_combo_phy_ddi_translations_edp_hbr2_hobl[] = {
> > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 }
> > > +};
> > > +
> > >  static const struct ddi_buf_trans *
> > >  bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> > >  {
> > > @@ -2301,14 +2305,51 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > >  	intel_de_write(dev_priv, CNL_PORT_TX_DW5_GRP(port), val);
> > >  }
> > >  
> > > +/*
> > > + * If supported return HOBL vswing table and set registers to enable HOBL
> > > + * otherwise returns NULL and unset registers to enable HOBL.
> > > + */
> > > +static const struct cnl_ddi_buf_trans *
> > > +hobl_get_combo_buf_trans(struct drm_i915_private *dev_priv,
> > > +			 struct intel_encoder *encoder, int type, int rate,
> > > +			 u32 level, int *n_entries)
> > > +{
> > > +	const u32 hobl_en = EDP4K2K_MODE_OVRD_EN | EDP4K2K_MODE_OVRD_OPTIMIZED;
> > > +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> > > +	struct intel_dp *intel_dp;
> > > +
> > > +	if (!HAS_HOBL(dev_priv) || type != INTEL_OUTPUT_EDP)
> > > +		return NULL;
> > 
> > Not a real fan of the "hobl" name. It just sounds like nonsense. Also
> > bspec doesn't use that term at all. It only appears in the vbt spec.
> > Not sure if there's a better one though.
> 
> Maybe power_optimized_edp?

In the lack of a better name will keep the current one, also it will allow for people to find some reference to it in BSpec.

> 
> > > +
> > > +	intel_dp = enc_to_intel_dp(encoder);
> > > +	if (!intel_dp->try_hobl || rate > 540000) {
> > > +		intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, 0);
> > 
> > I would vote for just doing this programming unconditionally in the normal
> > sequence.
> 
> Thought about that but intel_combo_phy_power_up_lanes() that program this ICL_PORT_CL_DW10 is called right after tgl_ddi_vswing_sequence().
> 
> > > +		return NULL;
> > > +	}
> > > +
> > > +	drm_dbg_kms(&dev_priv->drm, "Enabling HOBL in PHY %c\n", phy_name(phy));
> > > +	drm_WARN_ON_ONCE(&dev_priv->drm, level > 0);
> > > +
> > > +	intel_de_rmw(dev_priv, ICL_PORT_CL_DW10(phy), hobl_en, hobl_en);
> > > +	/* Same table applies to TGL, RKL and DG1 */
> > > +	*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > +	return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > +}
> > > +
> > >  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > > -					u32 level, enum phy phy, int type,
> > > -					int rate)
> > > +					 struct intel_encoder *encoder,
> > > +					 u32 level, enum phy phy, int type,
> > > +					 int rate)
> > 
> > If we're passing in the encoder then a bunch of this other stuff is
> > redundant.
> 
> Okay

Reduced to:

static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
                                        u32 level, enum intel_output_type type,
                                        int rate)
type can't be removed because of HDMI paths, rate could but it is used in the caller too so I left it.

> 
> > >  {
> > >  	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> > >  	u32 n_entries, val;
> > >  	int ln;
> > >  
> > > +	ddi_translations = hobl_get_combo_buf_trans(dev_priv, encoder, type,
> > > +						    rate, level, &n_entries);
> > > +	if (ddi_translations)
> > > +		goto hobl_found;
> > 
> > Why not just put it into tgl_get_combo_buf_trans(). Hmm. I guess to not
> > upset .voltage_max(). This feels a bit hackish, but I don't have better
> > ideas for now.
> 
> Exactly.
> 
> > > +
> > >  	if (INTEL_GEN(dev_priv) >= 12)
> > >  		ddi_translations = tgl_get_combo_buf_trans(dev_priv, type, rate,
> > >  							   &n_entries);
> > > @@ -2321,6 +2362,7 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > >  	if (!ddi_translations)
> > >  		return;
> > >  
> > > +hobl_found:
> > >  	if (level >= n_entries) {
> > >  		drm_dbg_kms(&dev_priv->drm,
> > >  			    "DDI translation not found for level %d. Using %d instead.",
> > > @@ -2428,7 +2470,7 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > >  	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
> > >  
> > >  	/* 5. Program swing and de-emphasis */
> > > -	icl_ddi_combo_vswing_program(dev_priv, level, phy, type, rate);
> > > +	icl_ddi_combo_vswing_program(dev_priv, encoder, level, phy, type, rate);
> > >  
> > >  	/* 6. Set training enable to trigger update */
> > >  	val = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN0(phy));
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 4b0aaa3081c9..f8943b67819d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1375,6 +1375,8 @@ struct intel_dp {
> > >  
> > >  	/* Display stream compression testing */
> > >  	bool force_dsc_en;
> > > +
> > > +	bool try_hobl;
> > >  };
> > >  
> > >  enum lspcon_vendor {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > index b9e4ee2dbddc..88f366bb28d7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > @@ -52,12 +52,24 @@ static u8 dp_voltage_max(u8 preemph)
> > >  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> > >  			       const u8 link_status[DP_LINK_STATUS_SIZE])
> > >  {
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >  	u8 v = 0;
> > >  	u8 p = 0;
> > >  	int lane;
> > >  	u8 voltage_max;
> > >  	u8 preemph_max;
> > >  
> > > +	if (intel_dp->try_hobl) {
> > > +		/*
> > > +		 * Do not adjust, try now with the regular table using VSwing 0
> > > +		 * and pre-emp 0
> > > +		 */
> > 
> > What if the sink is still asking for vswing 0 + preemph 0? The spec is
> > rather ambiguous when it comes to this stuff.
> 
> As it will fallback to regular table vswing 0 + preemph 0 that is not a issue.
> 
> > The table also doesn't specify the vswing/preemph for which we should
> > use this optimized value. Your interpretation of 0+0 seems like the most
> > sensible thing, but given that the VBT can also specifiy the fast link
> > training vswing/preemph as something else (and maybe there was also
> > something like this for normal link training?) I'm not 100% sure.
> 
> Yeah don't make much sense it not be vswing 0 + preemph 0 but lets wait for BSpec clarification then.
> 
> > Hmm. Actually noticed that all the eDP tables are missing the
> > vswing/preemph levels (they do have the raw mV/dB values but not the
> > DP spec levels). I filed a few issues in the hopes of clarification.
> > 
> > > +		intel_dp->try_hobl = false;
> > > +		drm_dbg_kms(&dev_priv->drm, "HOBL vswing table failed link "
> > > +			    "training, switching back to regular table\n");
> > > +		return;
> > > +	}
> > > +
> > >  	for (lane = 0; lane < intel_dp->lane_count; lane++) {
> > >  		v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
> > >  		p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
> > > @@ -103,9 +115,13 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> > >  }
> > >  
> > >  static bool
> > > -intel_dp_reset_link_train(struct intel_dp *intel_dp,
> > > -			u8 dp_train_pat)
> > > +intel_dp_reset_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
> > >  {
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +
> > > +	if (intel_dp_is_edp(intel_dp) && dev_priv->vbt.edp.hobl)
> > > +		intel_dp->try_hobl = true;
> > 
> > If it failed once does it make sense to keep trying to use it?
> 
> It could pass in a different bit rate and would be to much complicated keep track of that.
> 
> 
> Thanks for the review, lets wait for the BSpec clarifications that you asked.
> 
> > > +
> > >  	memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> > >  	intel_dp_set_signal_levels(intel_dp);
> > >  	return intel_dp_set_link_train(intel_dp, dp_train_pat);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 2336c9231eef..c7e7df17eef2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1687,6 +1687,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > >  #define INTEL_DISPLAY_ENABLED(dev_priv) \
> > >  		(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
> > >  
> > > +#define HAS_HOBL(dev_priv) (INTEL_GEN(dev_priv) >= 12)
> > > +
> > >  static inline bool intel_vtd_active(void)
> > >  {
> > >  #ifdef CONFIG_INTEL_IOMMU
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 578cfe11cbb9..d4611171f075 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -1896,6 +1896,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > >  #define  PWR_DOWN_LN_3_1_0		(0xb << 4)
> > >  #define  PWR_DOWN_LN_MASK		(0xf << 4)
> > >  #define  PWR_DOWN_LN_SHIFT		4
> > > +#define  EDP4K2K_MODE_OVRD_EN		(1 << 3)
> > > +#define  EDP4K2K_MODE_OVRD_OPTIMIZED	(1 << 2)
> > >  
> > >  #define ICL_PORT_CL_DW12(phy)		_MMIO(_ICL_PORT_CL_DW(12, phy))
> > >  #define   ICL_LANE_ENABLE_AUX		(1 << 0)
> > > -- 
> > > 2.27.0
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx