[Intel-gfx] [PATCH 0/6] drm/i915: Accept more fixed modes with VRR/DMRRS panels

[Intel-gfx] [PATCH 0/6] drm/i915: Accept more fixed modes with VRR/DMRRS panels

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VRR panels generally declare modes with different timings instead
of just modes with different clocks that we get with DRRS panels.
Let's accept more fixed modes for VRR panels.

There are also funny machines that declare DMRRS support but not
DRRS support. Treating those as static DRRS seems like the easiest
short term solution.

Ville Syrjälä (6):
  drm/i915: Parse VRR capability from VBT
  drm/i915: Print out rejected fixed modes
  drm/i915: Accept more fixed modes with VRR panels
  drm/i915/bios: Fix aggressiveness typos
  drm/i915/bios: Define more BDB contents
  drm/i915: Treat DMRRS as static DRRS

 drivers/gpu/drm/i915/display/intel_bios.c     | 29 ++++++++--
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
 drivers/gpu/drm/i915/display/intel_lvds.c     |  3 +-
 drivers/gpu/drm/i915/display/intel_panel.c    | 52 +++++++++++++----
 drivers/gpu/drm/i915/display/intel_panel.h    |  3 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c     |  2 +-
 drivers/gpu/drm/i915/display/intel_vbt_defs.h | 58 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_vrr.c      | 22 +++++--
 9 files changed, 137 insertions(+), 37 deletions(-)

-- 
2.35.1

[Intel-gfx] [PATCH 1/6] drm/i915: Parse VRR capability from VBT

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VBT seems to have an extra flag for VRR vs. not. Let's consult
that for eDP panels.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c     |  5 +++++
 .../drm/i915/display/intel_display_types.h    |  2 ++
 drivers/gpu/drm/i915/display/intel_vrr.c      | 22 ++++++++++++++-----
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 337277ae3dae..0774238e429b 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1293,6 +1293,8 @@ parse_power_conservation_features(struct drm_i915_private *i915,
 	const struct bdb_lfp_power *power;
 	u8 panel_type = panel->vbt.panel_type;
 
+	panel->vbt.vrr = true; /* matches Windows behaviour */
+
 	if (i915->vbt.version < 228)
 		return;
 
@@ -1313,6 +1315,9 @@ parse_power_conservation_features(struct drm_i915_private *i915,
 
 	if (i915->vbt.version >= 232)
 		panel->vbt.edp.hobl = power->hobl & BIT(panel_type);
+
+	if (i915->vbt.version >= 233)
+		panel->vbt.vrr = power->vrr_feature_enabled & BIT(panel_type);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index a27d66fd4383..7a76ba1a3b47 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -294,6 +294,8 @@ struct intel_vbt_panel_data {
 	unsigned int lvds_dither:1;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
 
+	bool vrr;
+
 	u8 seamless_drrs_min_refresh_rate;
 	enum drrs_type drrs_type;
 
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 081e52dd6c4e..04250a0fec3c 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -15,19 +15,29 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct intel_dp *intel_dp;
 
-	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
-	    connector->base.connector_type != DRM_MODE_CONNECTOR_DisplayPort)
-		return false;
-
-	intel_dp = intel_attached_dp(connector);
 	/*
 	 * DP Sink is capable of VRR video timings if
 	 * Ignore MSA bit is set in DPCD.
 	 * EDID monitor range also should be atleast 10 for reasonable
 	 * Adaptive Sync or Variable Refresh Rate end user experience.
 	 */
+	switch (connector->base.connector_type) {
+	case DRM_MODE_CONNECTOR_eDP:
+		if (!connector->panel.vbt.vrr)
+			return false;
+		fallthrough;
+	case DRM_MODE_CONNECTOR_DisplayPort:
+		intel_dp = intel_attached_dp(connector);
+
+		if (!drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd))
+			return false;
+
+		break;
+	default:
+		return false;
+	}
+
 	return HAS_VRR(i915) &&
-		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
 		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
 }
 
-- 
2.35.1

[Intel-gfx] [PATCH 2/6] drm/i915: Print out rejected fixed modes

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To help with debugging DRRS/VRR panel init let's dump out all
the fixed modes we rejected for whatever reason.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_panel.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index d055e4118558..f55e4eafd74e 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -217,6 +217,10 @@ static void intel_panel_destroy_probed_modes(struct intel_connector *connector)
 	struct drm_display_mode *mode, *next;
 
 	list_for_each_entry_safe(mode, next, &connector->base.probed_modes, head) {
+		drm_dbg_kms(&i915->drm,
+			    "[CONNECTOR:%d:%s] not using EDID mode: " DRM_MODE_FMT "\n",
+			    connector->base.base.id, connector->base.name,
+			    DRM_MODE_ARG(mode));
 		list_del(&mode->head);
 		drm_mode_destroy(&i915->drm, mode);
 	}
-- 
2.35.1

[Intel-gfx] [PATCH 3/6] drm/i915: Accept more fixed modes with VRR panels

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It seem that when dealing with VRR capable eDP panels we need
to accept fixed modes with variable vblank length (which is what
VRR varies dynamically). Typically the preferred mode seems to be
a non-VRR more (lowish dotclock/refresh rate + short vblank).

We also have examples where it looks like even the hblank length
is a bit different between the preferred mode vs. VRR mode(s).
So let's just accept anything that has matching hdisp+vdisp+flags.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/125
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c    |  3 +-
 drivers/gpu/drm/i915/display/intel_lvds.c  |  3 +-
 drivers/gpu/drm/i915/display/intel_panel.c | 48 ++++++++++++++++------
 drivers/gpu/drm/i915/display/intel_panel.h |  3 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c  |  2 +-
 5 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1bc1f6458e81..b8e2d3cd4d68 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5217,7 +5217,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 			      IS_ERR(edid) ? NULL : edid);
 
 	intel_panel_add_edid_fixed_modes(intel_connector,
-					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE);
+					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,
+					 intel_vrr_is_capable(intel_connector));
 
 	/* MSO requires information from the EDID */
 	intel_edp_mso_init(intel_dp);
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index 595f03343939..e55802b45461 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -972,7 +972,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 
 	/* Try EDID first */
 	intel_panel_add_edid_fixed_modes(intel_connector,
-					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE);
+					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,
+					 false);
 
 	/* Failed to get EDID, what about VBT? */
 	if (!intel_panel_preferred_fixed_mode(intel_connector))
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index f55e4eafd74e..963b24293b50 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -71,6 +71,27 @@ intel_panel_fixed_mode(struct intel_connector *connector,
 	return best_mode;
 }
 
+static bool is_alt_drrs_mode(const struct drm_display_mode *mode,
+			     const struct drm_display_mode *preferred_mode)
+{
+	return drm_mode_match(mode, preferred_mode,
+			      DRM_MODE_MATCH_TIMINGS |
+			      DRM_MODE_MATCH_FLAGS |
+			      DRM_MODE_MATCH_3D_FLAGS) &&
+		mode->clock != preferred_mode->clock;
+}
+
+static bool is_alt_vrr_mode(const struct drm_display_mode *mode,
+			    const struct drm_display_mode *preferred_mode)
+{
+	return drm_mode_match(mode, preferred_mode,
+			      DRM_MODE_MATCH_FLAGS |
+			      DRM_MODE_MATCH_3D_FLAGS) &&
+		mode->hdisplay == preferred_mode->hdisplay &&
+		mode->vdisplay == preferred_mode->vdisplay &&
+		mode->clock != preferred_mode->clock;
+}
+
 const struct drm_display_mode *
 intel_panel_downclock_mode(struct intel_connector *connector,
 			   const struct drm_display_mode *adjusted_mode)
@@ -83,7 +104,8 @@ intel_panel_downclock_mode(struct intel_connector *connector,
 	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
 		int vrefresh = drm_mode_vrefresh(fixed_mode);
 
-		if (vrefresh >= min_vrefresh && vrefresh < max_vrefresh) {
+		if (is_alt_drrs_mode(fixed_mode, adjusted_mode) &&
+		    vrefresh >= min_vrefresh && vrefresh < max_vrefresh) {
 			max_vrefresh = vrefresh;
 			best_mode = fixed_mode;
 		}
@@ -151,16 +173,17 @@ int intel_panel_compute_config(struct intel_connector *connector,
 }
 
 static bool is_alt_fixed_mode(const struct drm_display_mode *mode,
-			      const struct drm_display_mode *preferred_mode)
+			      const struct drm_display_mode *preferred_mode,
+			      bool has_vrr)
 {
-	return drm_mode_match(mode, preferred_mode,
-			      DRM_MODE_MATCH_TIMINGS |
-			      DRM_MODE_MATCH_FLAGS |
-			      DRM_MODE_MATCH_3D_FLAGS) &&
-		mode->clock != preferred_mode->clock;
+	if (has_vrr)
+		return is_alt_vrr_mode(mode, preferred_mode);
+	else
+		return is_alt_drrs_mode(mode, preferred_mode);
 }
 
-static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connector)
+static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connector,
+						 bool has_vrr)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	const struct drm_display_mode *preferred_mode =
@@ -168,7 +191,7 @@ static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connect
 	struct drm_display_mode *mode, *next;
 
 	list_for_each_entry_safe(mode, next, &connector->base.probed_modes, head) {
-		if (!is_alt_fixed_mode(mode, preferred_mode))
+		if (!is_alt_fixed_mode(mode, preferred_mode, has_vrr))
 			continue;
 
 		drm_dbg_kms(&dev_priv->drm,
@@ -226,11 +249,12 @@ static void intel_panel_destroy_probed_modes(struct intel_connector *connector)
 	}
 }
 
-void intel_panel_add_edid_fixed_modes(struct intel_connector *connector, bool has_drrs)
+void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
+				      bool has_drrs, bool has_vrr)
 {
 	intel_panel_add_edid_preferred_mode(connector);
-	if (intel_panel_preferred_fixed_mode(connector) && has_drrs)
-		intel_panel_add_edid_alt_fixed_modes(connector);
+	if (intel_panel_preferred_fixed_mode(connector) && (has_drrs || has_vrr))
+		intel_panel_add_edid_alt_fixed_modes(connector, has_vrr);
 	intel_panel_destroy_probed_modes(connector);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index 2e32bb728beb..b087c0c3cc6d 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -40,7 +40,8 @@ int intel_panel_fitting(struct intel_crtc_state *crtc_state,
 			const struct drm_connector_state *conn_state);
 int intel_panel_compute_config(struct intel_connector *connector,
 			       struct drm_display_mode *adjusted_mode);
-void intel_panel_add_edid_fixed_modes(struct intel_connector *connector, bool has_drrs);
+void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
+				      bool has_drrs, bool has_vrr);
 void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector);
 void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector);
 void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index d9de2c4d67a7..2b78a790e1b6 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2911,7 +2911,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 
 	if (!intel_panel_preferred_fixed_mode(intel_connector)) {
 		intel_ddc_get_modes(connector, &intel_sdvo->ddc);
-		intel_panel_add_edid_fixed_modes(intel_connector, false);
+		intel_panel_add_edid_fixed_modes(intel_connector, false, false);
 	}
 
 	intel_panel_init(intel_connector);
-- 
2.35.1

[Intel-gfx] [PATCH 4/6] drm/i915/bios: Fix aggressiveness typos

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fix various typos around "aggressiveness". Note that
the VBT spec also sometimes missspells it as
"agressiveness" so I guess that's where some of the typos
came from.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_vbt_defs.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index 4b98bab3b890..39109f204c6d 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -856,9 +856,9 @@ struct als_data_entry {
 	u16 lux;
 } __packed;
 
-struct agressiveness_profile_entry {
-	u8 dpst_agressiveness : 4;
-	u8 lace_agressiveness : 4;
+struct aggressiveness_profile_entry {
+	u8 dpst_aggressiveness : 4;
+	u8 lace_aggressiveness : 4;
 } __packed;
 
 struct bdb_lfp_power {
@@ -873,7 +873,7 @@ struct bdb_lfp_power {
 	u16 dmrrs;
 	u16 adb;
 	u16 lace_enabled_status;
-	struct agressiveness_profile_entry aggressivenes[16];
+	struct aggressiveness_profile_entry aggressiveness[16];
 	u16 hobl; /* 232+ */
 	u16 vrr_feature_enabled; /* 233+ */
 } __packed;
-- 
2.35.1

[Intel-gfx] [PATCH 5/6] drm/i915/bios: Define more BDB contents

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a bunch of new struff we're missing in various BDB blocks.

TODO: Bunch of these might actually need to be taken
into use...

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_vbt_defs.h | 50 ++++++++++++++++---
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index 39109f204c6d..be99f585b1d0 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -564,7 +564,9 @@ struct bdb_driver_features {
 	u16 tbt_enabled:1;
 	u16 psr_enabled:1;
 	u16 ips_enabled:1;
-	u16 reserved3:4;
+	u16 reserved3:1;
+	u16 dmrrs_enabled:1;
+	u16 reserved4:2;
 	u16 pc_feature_valid:1;
 } __packed;
 
@@ -666,6 +668,16 @@ struct edp_full_link_params {
 	u8 vswing:4;
 } __packed;
 
+struct edp_apical_params {
+	u32 panel_oui;
+	u32 dpcd_base_address;
+	u32 dpcd_idridix_control_0;
+	u32 dpcd_option_select;
+	u32 dpcd_backlight;
+	u32 ambient_light;
+	u32 backlight_scale;
+} __packed;
+
 struct bdb_edp {
 	struct edp_power_seq power_seqs[16];
 	u32 color_depth;
@@ -681,6 +693,9 @@ struct bdb_edp {
 	struct edp_pwm_delays pwm_delays[16];			/* 186 */
 	u16 full_link_params_provided;				/* 199 */
 	struct edp_full_link_params full_link_params[16];	/* 199 */
+	u16 apical_enable;					/* 203 */
+	struct edp_apical_params apical_params[16];		/* 203 */
+	u16 edp_fast_link_training_rate[16];			/* 224 */
 } __packed;
 
 /*
@@ -717,6 +732,7 @@ struct bdb_lvds_options {
 
 	u16 lcdvcc_s0_enable;					/* 200 */
 	u32 rotation;						/* 228 */
+	u32 position;						/* 240 */
 } __packed;
 
 /*
@@ -843,13 +859,22 @@ struct bdb_lfp_backlight_data {
 	u8 level[16]; /* Obsolete from 234+ */
 	struct lfp_backlight_control_method backlight_control[16];
 	struct lfp_brightness_level brightness_level[16];		/* 234+ */
-	struct lfp_brightness_level brightness_min_level[16];	/* 234+ */
-	u8 brightness_precision_bits[16];						/* 236+ */
+	struct lfp_brightness_level brightness_min_level[16];		/* 234+ */
+	u8 brightness_precision_bits[16];				/* 236+ */
+	u16 hdr_dpcd_refresh_timeout[16];				/* 239+ */
 } __packed;
 
 /*
  * Block 44 - LFP Power Conservation Features Block
  */
+struct lfp_features {
+	u8 reserved1:1;
+	u8 power_conservation_pref:3;
+	u8 reserved2:1;
+	u8 lace_enabled_status:1;
+	u8 lace_support:1;
+	u8 als_enable:1;
+} __packed;
 
 struct als_data_entry {
 	u16 backlight_adjust;
@@ -861,10 +886,16 @@ struct aggressiveness_profile_entry {
 	u8 lace_aggressiveness : 4;
 } __packed;
 
+struct aggressiveness_profile2_entry {
+	u8 opst_aggressiveness : 4;
+	u8 elp_aggressiveness : 4;
+} __packed;
+
 struct bdb_lfp_power {
-	u8 lfp_feature_bits;
+	struct lfp_features features;
 	struct als_data_entry als[5];
-	u8 lace_aggressiveness_profile;
+	u8 lace_aggressiveness_profile:3;
+	u8 reserved1:5;
 	u16 dpst;
 	u16 psr;
 	u16 drrs;
@@ -876,6 +907,9 @@ struct bdb_lfp_power {
 	struct aggressiveness_profile_entry aggressiveness[16];
 	u16 hobl; /* 232+ */
 	u16 vrr_feature_enabled; /* 233+ */
+	u16 elp; /* 247+ */
+	u16 opst; /* 247+ */
+	struct aggressiveness_profile2_entry aggressiveness2[16]; /* 247+ */
 } __packed;
 
 /*
@@ -885,8 +919,10 @@ struct bdb_lfp_power {
 #define MAX_MIPI_CONFIGURATIONS	6
 
 struct bdb_mipi_config {
-	struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
-	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
+	struct mipi_config config[MAX_MIPI_CONFIGURATIONS]; /* 175 */
+	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS]; /* 177 */
+	struct edp_pwm_delays pwm_delays[MAX_MIPI_CONFIGURATIONS]; /* 186 */
+	u8 pmic_i2c_bus_number[MAX_MIPI_CONFIGURATIONS]; /* 190 */
 } __packed;
 
 /*
-- 
2.35.1

[Intel-gfx] [PATCH 6/6] drm/i915: Treat DMRRS as static DRRS

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some machines declare DRRS type = seamless, DRRS = no, DMRRS = yes.
I *think* DMRRS stands for "dynamcic media refresh rate", and
I suspect the way it's meant to work is that it lets the driver
switch refresh rates to match the frame rate for media playback.
Obviously for us all that kind of policy stuff is entirely up to
userspace, so the only thing we may do is make the extra refresh
rate(s) available.

So let's treat this case as just static DRRS for now. In the
future We might want to differentiate the "seamless w/ downclocking"
vs. "seamless w/o downclocking" cases so that we could do seamless
refresh rate changes for systems that only claim to support DMRRS.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/125
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 24 +++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 0774238e429b..c42b9e7d0dce 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1279,8 +1279,16 @@ parse_panel_driver_features(struct drm_i915_private *i915,
 		 * static DRRS is 0 and DRRS not supported is represented by
 		 * driver->drrs_enabled=false
 		 */
-		if (!driver->drrs_enabled)
-			panel->vbt.drrs_type = DRRS_TYPE_NONE;
+		if (!driver->drrs_enabled && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
+			/*
+			 * FIXME Should DMRRS perhaps be treated as seamless
+			 * but without the automatic downclocking?
+			 */
+			if (driver->dmrrs_enabled)
+				panel->vbt.drrs_type = DRRS_TYPE_STATIC;
+			else
+				panel->vbt.drrs_type = DRRS_TYPE_NONE;
+		}
 
 		panel->vbt.psr.enable = driver->psr_enabled;
 	}
@@ -1310,8 +1318,16 @@ parse_power_conservation_features(struct drm_i915_private *i915,
 	 * static DRRS is 0 and DRRS not supported is represented by
 	 * power->drrs & BIT(panel_type)=false
 	 */
-	if (!(power->drrs & BIT(panel_type)))
-		panel->vbt.drrs_type = DRRS_TYPE_NONE;
+	if (!(power->drrs & BIT(panel_type)) && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
+		/*
+		 * FIXME Should DMRRS perhaps be treated as seamless
+		 * but without the automatic downclocking?
+		 */
+		if (power->dmrrs & BIT(panel_type))
+			panel->vbt.drrs_type = DRRS_TYPE_STATIC;
+		else
+			panel->vbt.drrs_type = DRRS_TYPE_NONE;
+	}
 
 	if (i915->vbt.version >= 232)
 		panel->vbt.edp.hobl = power->hobl & BIT(panel_type);
-- 
2.35.1

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Accept more fixed modes with VRR/DMRRS panels

[Patchwork]

== Series Details ==

Series: drm/i915: Accept more fixed modes with VRR/DMRRS panels
URL   : https://patchwork.freedesktop.org/series/104467/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/104467/revisions/1/mbox/ not applied
Applying: drm/i915: Parse VRR capability from VBT
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_bios.c
M	drivers/gpu/drm/i915/display/intel_display_types.h
M	drivers/gpu/drm/i915/display/intel_vrr.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_vrr.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_vrr.c
Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display_types.h
Auto-merging drivers/gpu/drm/i915/display/intel_bios.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_bios.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: Parse VRR capability from VBT
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [Intel-gfx] [PATCH 1/6] drm/i915: Parse VRR capability from VBT

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VBT seems to have an extra flag for VRR vs. not. Let's consult
> that for eDP panels.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     |  5 +++++
>  .../drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_vrr.c      | 22 ++++++++++++++-----
>  3 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 337277ae3dae..0774238e429b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1293,6 +1293,8 @@ parse_power_conservation_features(struct drm_i915_private *i915,
>  	const struct bdb_lfp_power *power;
>  	u8 panel_type = panel->vbt.panel_type;
>  
> +	panel->vbt.vrr = true; /* matches Windows behaviour */
> +
>  	if (i915->vbt.version < 228)
>  		return;
>  
> @@ -1313,6 +1315,9 @@ parse_power_conservation_features(struct drm_i915_private *i915,
>  
>  	if (i915->vbt.version >= 232)
>  		panel->vbt.edp.hobl = power->hobl & BIT(panel_type);
> +
> +	if (i915->vbt.version >= 233)
> +		panel->vbt.vrr = power->vrr_feature_enabled & BIT(panel_type);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index a27d66fd4383..7a76ba1a3b47 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -294,6 +294,8 @@ struct intel_vbt_panel_data {
>  	unsigned int lvds_dither:1;
>  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
>  
> +	bool vrr;
> +
>  	u8 seamless_drrs_min_refresh_rate;
>  	enum drrs_type drrs_type;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 081e52dd6c4e..04250a0fec3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -15,19 +15,29 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct intel_dp *intel_dp;
>  
> -	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
> -	    connector->base.connector_type != DRM_MODE_CONNECTOR_DisplayPort)
> -		return false;
> -
> -	intel_dp = intel_attached_dp(connector);
>  	/*
>  	 * DP Sink is capable of VRR video timings if
>  	 * Ignore MSA bit is set in DPCD.
>  	 * EDID monitor range also should be atleast 10 for reasonable
>  	 * Adaptive Sync or Variable Refresh Rate end user experience.
>  	 */
> +	switch (connector->base.connector_type) {
> +	case DRM_MODE_CONNECTOR_eDP:
> +		if (!connector->panel.vbt.vrr)
> +			return false;
> +		fallthrough;
> +	case DRM_MODE_CONNECTOR_DisplayPort:
> +		intel_dp = intel_attached_dp(connector);
> +
> +		if (!drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd))
> +			return false;
> +
> +		break;
> +	default:
> +		return false;
> +	}
> +
>  	return HAS_VRR(i915) &&

Feels like !HAS_VRR() should be an early return at the top. But that's
not part of this patch.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> -		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
>  		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/6] drm/i915: Print out rejected fixed modes

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> To help with debugging DRRS/VRR panel init let's dump out all
> the fixed modes we rejected for whatever reason.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index d055e4118558..f55e4eafd74e 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -217,6 +217,10 @@ static void intel_panel_destroy_probed_modes(struct intel_connector *connector)
>  	struct drm_display_mode *mode, *next;
>  
>  	list_for_each_entry_safe(mode, next, &connector->base.probed_modes, head) {
> +		drm_dbg_kms(&i915->drm,
> +			    "[CONNECTOR:%d:%s] not using EDID mode: " DRM_MODE_FMT "\n",
> +			    connector->base.base.id, connector->base.name,
> +			    DRM_MODE_ARG(mode));
>  		list_del(&mode->head);
>  		drm_mode_destroy(&i915->drm, mode);
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 3/6] drm/i915: Accept more fixed modes with VRR panels

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> It seem that when dealing with VRR capable eDP panels we need
> to accept fixed modes with variable vblank length (which is what
> VRR varies dynamically). Typically the preferred mode seems to be
> a non-VRR more (lowish dotclock/refresh rate + short vblank).
>
> We also have examples where it looks like even the hblank length
> is a bit different between the preferred mode vs. VRR mode(s).
> So let's just accept anything that has matching hdisp+vdisp+flags.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/125
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c    |  3 +-
>  drivers/gpu/drm/i915/display/intel_lvds.c  |  3 +-
>  drivers/gpu/drm/i915/display/intel_panel.c | 48 ++++++++++++++++------
>  drivers/gpu/drm/i915/display/intel_panel.h |  3 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c  |  2 +-
>  5 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1bc1f6458e81..b8e2d3cd4d68 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5217,7 +5217,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  			      IS_ERR(edid) ? NULL : edid);
>  
>  	intel_panel_add_edid_fixed_modes(intel_connector,
> -					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE);
> +					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,
> +					 intel_vrr_is_capable(intel_connector));
>  
>  	/* MSO requires information from the EDID */
>  	intel_edp_mso_init(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index 595f03343939..e55802b45461 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -972,7 +972,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  
>  	/* Try EDID first */
>  	intel_panel_add_edid_fixed_modes(intel_connector,
> -					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE);
> +					 intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,
> +					 false);
>  
>  	/* Failed to get EDID, what about VBT? */
>  	if (!intel_panel_preferred_fixed_mode(intel_connector))
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index f55e4eafd74e..963b24293b50 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -71,6 +71,27 @@ intel_panel_fixed_mode(struct intel_connector *connector,
>  	return best_mode;
>  }
>  
> +static bool is_alt_drrs_mode(const struct drm_display_mode *mode,
> +			     const struct drm_display_mode *preferred_mode)
> +{
> +	return drm_mode_match(mode, preferred_mode,
> +			      DRM_MODE_MATCH_TIMINGS |
> +			      DRM_MODE_MATCH_FLAGS |
> +			      DRM_MODE_MATCH_3D_FLAGS) &&
> +		mode->clock != preferred_mode->clock;
> +}
> +
> +static bool is_alt_vrr_mode(const struct drm_display_mode *mode,
> +			    const struct drm_display_mode *preferred_mode)
> +{
> +	return drm_mode_match(mode, preferred_mode,
> +			      DRM_MODE_MATCH_FLAGS |
> +			      DRM_MODE_MATCH_3D_FLAGS) &&
> +		mode->hdisplay == preferred_mode->hdisplay &&
> +		mode->vdisplay == preferred_mode->vdisplay &&
> +		mode->clock != preferred_mode->clock;
> +}
> +
>  const struct drm_display_mode *
>  intel_panel_downclock_mode(struct intel_connector *connector,
>  			   const struct drm_display_mode *adjusted_mode)
> @@ -83,7 +104,8 @@ intel_panel_downclock_mode(struct intel_connector *connector,
>  	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
>  		int vrefresh = drm_mode_vrefresh(fixed_mode);
>  
> -		if (vrefresh >= min_vrefresh && vrefresh < max_vrefresh) {
> +		if (is_alt_drrs_mode(fixed_mode, adjusted_mode) &&
> +		    vrefresh >= min_vrefresh && vrefresh < max_vrefresh) {
>  			max_vrefresh = vrefresh;
>  			best_mode = fixed_mode;
>  		}
> @@ -151,16 +173,17 @@ int intel_panel_compute_config(struct intel_connector *connector,
>  }
>  
>  static bool is_alt_fixed_mode(const struct drm_display_mode *mode,
> -			      const struct drm_display_mode *preferred_mode)
> +			      const struct drm_display_mode *preferred_mode,
> +			      bool has_vrr)
>  {
> -	return drm_mode_match(mode, preferred_mode,
> -			      DRM_MODE_MATCH_TIMINGS |
> -			      DRM_MODE_MATCH_FLAGS |
> -			      DRM_MODE_MATCH_3D_FLAGS) &&
> -		mode->clock != preferred_mode->clock;
> +	if (has_vrr)
> +		return is_alt_vrr_mode(mode, preferred_mode);
> +	else
> +		return is_alt_drrs_mode(mode, preferred_mode);

This assumes is_alt_drrs_mode() accepts a subset of
is_alt_vrr_mode(). Which is true and fine. But maybe deserves a comment,
as otherwise I believe we should try both if the vrr one fails.

A really defensive solution would be to WARN_ON(!is_alt_vrr_mode()) in
is_alt_drrs_mode() if everything else is true. Maybe overkill.

>  }
>  
> -static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connector)
> +static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connector,
> +						 bool has_vrr)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	const struct drm_display_mode *preferred_mode =
> @@ -168,7 +191,7 @@ static void intel_panel_add_edid_alt_fixed_modes(struct intel_connector *connect
>  	struct drm_display_mode *mode, *next;
>  
>  	list_for_each_entry_safe(mode, next, &connector->base.probed_modes, head) {
> -		if (!is_alt_fixed_mode(mode, preferred_mode))
> +		if (!is_alt_fixed_mode(mode, preferred_mode, has_vrr))
>  			continue;

Should we perhaps debug log if it was vrr or drrs?

Anyway, with the comment added,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

The rest is up to you.

>  
>  		drm_dbg_kms(&dev_priv->drm,
> @@ -226,11 +249,12 @@ static void intel_panel_destroy_probed_modes(struct intel_connector *connector)
>  	}
>  }
>  
> -void intel_panel_add_edid_fixed_modes(struct intel_connector *connector, bool has_drrs)
> +void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
> +				      bool has_drrs, bool has_vrr)
>  {
>  	intel_panel_add_edid_preferred_mode(connector);
> -	if (intel_panel_preferred_fixed_mode(connector) && has_drrs)
> -		intel_panel_add_edid_alt_fixed_modes(connector);
> +	if (intel_panel_preferred_fixed_mode(connector) && (has_drrs || has_vrr))
> +		intel_panel_add_edid_alt_fixed_modes(connector, has_vrr);
>  	intel_panel_destroy_probed_modes(connector);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
> index 2e32bb728beb..b087c0c3cc6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -40,7 +40,8 @@ int intel_panel_fitting(struct intel_crtc_state *crtc_state,
>  			const struct drm_connector_state *conn_state);
>  int intel_panel_compute_config(struct intel_connector *connector,
>  			       struct drm_display_mode *adjusted_mode);
> -void intel_panel_add_edid_fixed_modes(struct intel_connector *connector, bool has_drrs);
> +void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
> +				      bool has_drrs, bool has_vrr);
>  void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector);
>  void intel_panel_add_vbt_sdvo_fixed_mode(struct intel_connector *connector);
>  void intel_panel_add_encoder_fixed_mode(struct intel_connector *connector,
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index d9de2c4d67a7..2b78a790e1b6 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2911,7 +2911,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  
>  	if (!intel_panel_preferred_fixed_mode(intel_connector)) {
>  		intel_ddc_get_modes(connector, &intel_sdvo->ddc);
> -		intel_panel_add_edid_fixed_modes(intel_connector, false);
> +		intel_panel_add_edid_fixed_modes(intel_connector, false, false);
>  	}
>  
>  	intel_panel_init(intel_connector);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 4/6] drm/i915/bios: Fix aggressiveness typos

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Fix various typos around "aggressiveness". Note that
> the VBT spec also sometimes missspells it as
> "agressiveness" so I guess that's where some of the typos
> came from.

Could nitpick unrelated things like space before ":" in bitfields or the
struct naming, but this patch does what it says on the box.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 4b98bab3b890..39109f204c6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -856,9 +856,9 @@ struct als_data_entry {
>  	u16 lux;
>  } __packed;
>  
> -struct agressiveness_profile_entry {
> -	u8 dpst_agressiveness : 4;
> -	u8 lace_agressiveness : 4;
> +struct aggressiveness_profile_entry {
> +	u8 dpst_aggressiveness : 4;
> +	u8 lace_aggressiveness : 4;
>  } __packed;
>  
>  struct bdb_lfp_power {
> @@ -873,7 +873,7 @@ struct bdb_lfp_power {
>  	u16 dmrrs;
>  	u16 adb;
>  	u16 lace_enabled_status;
> -	struct agressiveness_profile_entry aggressivenes[16];
> +	struct aggressiveness_profile_entry aggressiveness[16];
>  	u16 hobl; /* 232+ */
>  	u16 vrr_feature_enabled; /* 233+ */
>  } __packed;

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 5/6] drm/i915/bios: Define more BDB contents

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a bunch of new struff we're missing in various BDB blocks.
>
> TODO: Bunch of these might actually need to be taken
> into use...

Cc: Jouni, Lyude for some HDR backlight stuff below.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 50 ++++++++++++++++---
>  1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 39109f204c6d..be99f585b1d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -564,7 +564,9 @@ struct bdb_driver_features {
>  	u16 tbt_enabled:1;
>  	u16 psr_enabled:1;
>  	u16 ips_enabled:1;
> -	u16 reserved3:4;
> +	u16 reserved3:1;
> +	u16 dmrrs_enabled:1;

Should we start logging the version ranges here too, since it's obsolete
from 228. Kinda duplicating the info though. *shrug*.

> +	u16 reserved4:2;
>  	u16 pc_feature_valid:1;
>  } __packed;
>  
> @@ -666,6 +668,16 @@ struct edp_full_link_params {
>  	u8 vswing:4;
>  } __packed;
>  
> +struct edp_apical_params {
> +	u32 panel_oui;
> +	u32 dpcd_base_address;
> +	u32 dpcd_idridix_control_0;
> +	u32 dpcd_option_select;
> +	u32 dpcd_backlight;
> +	u32 ambient_light;
> +	u32 backlight_scale;
> +} __packed;
> +
>  struct bdb_edp {
>  	struct edp_power_seq power_seqs[16];
>  	u32 color_depth;
> @@ -681,6 +693,9 @@ struct bdb_edp {
>  	struct edp_pwm_delays pwm_delays[16];			/* 186 */
>  	u16 full_link_params_provided;				/* 199 */
>  	struct edp_full_link_params full_link_params[16];	/* 199 */
> +	u16 apical_enable;					/* 203 */
> +	struct edp_apical_params apical_params[16];		/* 203 */
> +	u16 edp_fast_link_training_rate[16];			/* 224 */

Another eDP port link rate param would go here? Could be added in
another patch.

>  } __packed;
>  
>  /*
> @@ -717,6 +732,7 @@ struct bdb_lvds_options {
>  
>  	u16 lcdvcc_s0_enable;					/* 200 */
>  	u32 rotation;						/* 228 */
> +	u32 position;						/* 240 */
>  } __packed;
>  
>  /*
> @@ -843,13 +859,22 @@ struct bdb_lfp_backlight_data {
>  	u8 level[16]; /* Obsolete from 234+ */
>  	struct lfp_backlight_control_method backlight_control[16];
>  	struct lfp_brightness_level brightness_level[16];		/* 234+ */
> -	struct lfp_brightness_level brightness_min_level[16];	/* 234+ */
> -	u8 brightness_precision_bits[16];						/* 236+ */
> +	struct lfp_brightness_level brightness_min_level[16];		/* 234+ */
> +	u8 brightness_precision_bits[16];				/* 236+ */
> +	u16 hdr_dpcd_refresh_timeout[16];				/* 239+ */

Jouni, Lyude, this is probably interesting to you:

"""
This table of values (for 16 panels, 1 value per panel) is used to
specify the time required by the TCON (with Intel HDR Aux Interface
Support) to refresh the DPCD set with Intel HDR CAPS (DPCD offset:
340h-344h).

The value is in units of 10 us(microseconds).
"""

>  } __packed;
>  
>  /*
>   * Block 44 - LFP Power Conservation Features Block
>   */
> +struct lfp_features {

Nit, maybe lfp_power_features.

Anyway,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +	u8 reserved1:1;
> +	u8 power_conservation_pref:3;
> +	u8 reserved2:1;
> +	u8 lace_enabled_status:1;
> +	u8 lace_support:1;
> +	u8 als_enable:1;
> +} __packed;
>  
>  struct als_data_entry {
>  	u16 backlight_adjust;
> @@ -861,10 +886,16 @@ struct aggressiveness_profile_entry {
>  	u8 lace_aggressiveness : 4;
>  } __packed;
>  
> +struct aggressiveness_profile2_entry {
> +	u8 opst_aggressiveness : 4;
> +	u8 elp_aggressiveness : 4;
> +} __packed;
> +
>  struct bdb_lfp_power {
> -	u8 lfp_feature_bits;
> +	struct lfp_features features;
>  	struct als_data_entry als[5];
> -	u8 lace_aggressiveness_profile;
> +	u8 lace_aggressiveness_profile:3;
> +	u8 reserved1:5;
>  	u16 dpst;
>  	u16 psr;
>  	u16 drrs;
> @@ -876,6 +907,9 @@ struct bdb_lfp_power {
>  	struct aggressiveness_profile_entry aggressiveness[16];
>  	u16 hobl; /* 232+ */
>  	u16 vrr_feature_enabled; /* 233+ */
> +	u16 elp; /* 247+ */
> +	u16 opst; /* 247+ */
> +	struct aggressiveness_profile2_entry aggressiveness2[16]; /* 247+ */
>  } __packed;
>  
>  /*
> @@ -885,8 +919,10 @@ struct bdb_lfp_power {
>  #define MAX_MIPI_CONFIGURATIONS	6
>  
>  struct bdb_mipi_config {
> -	struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
> -	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
> +	struct mipi_config config[MAX_MIPI_CONFIGURATIONS]; /* 175 */
> +	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS]; /* 177 */
> +	struct edp_pwm_delays pwm_delays[MAX_MIPI_CONFIGURATIONS]; /* 186 */
> +	u8 pmic_i2c_bus_number[MAX_MIPI_CONFIGURATIONS]; /* 190 */
>  } __packed;
>  
>  /*

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Treat DMRRS as static DRRS

[Jani Nikula]

On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Some machines declare DRRS type = seamless, DRRS = no, DMRRS = yes.
> I *think* DMRRS stands for "dynamcic media refresh rate", and
> I suspect the way it's meant to work is that it lets the driver
> switch refresh rates to match the frame rate for media playback.
> Obviously for us all that kind of policy stuff is entirely up to
> userspace, so the only thing we may do is make the extra refresh
> rate(s) available.
>
> So let's treat this case as just static DRRS for now. In the
> future We might want to differentiate the "seamless w/ downclocking"
> vs. "seamless w/o downclocking" cases so that we could do seamless
> refresh rate changes for systems that only claim to support DMRRS.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/125
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 24 +++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 0774238e429b..c42b9e7d0dce 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1279,8 +1279,16 @@ parse_panel_driver_features(struct drm_i915_private *i915,
>  		 * static DRRS is 0 and DRRS not supported is represented by
>  		 * driver->drrs_enabled=false
>  		 */
> -		if (!driver->drrs_enabled)
> -			panel->vbt.drrs_type = DRRS_TYPE_NONE;
> +		if (!driver->drrs_enabled && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
> +			/*
> +			 * FIXME Should DMRRS perhaps be treated as seamless
> +			 * but without the automatic downclocking?
> +			 */
> +			if (driver->dmrrs_enabled)
> +				panel->vbt.drrs_type = DRRS_TYPE_STATIC;
> +			else
> +				panel->vbt.drrs_type = DRRS_TYPE_NONE;
> +		}
>  
>  		panel->vbt.psr.enable = driver->psr_enabled;
>  	}
> @@ -1310,8 +1318,16 @@ parse_power_conservation_features(struct drm_i915_private *i915,
>  	 * static DRRS is 0 and DRRS not supported is represented by
>  	 * power->drrs & BIT(panel_type)=false
>  	 */
> -	if (!(power->drrs & BIT(panel_type)))
> -		panel->vbt.drrs_type = DRRS_TYPE_NONE;
> +	if (!(power->drrs & BIT(panel_type)) && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
> +		/*
> +		 * FIXME Should DMRRS perhaps be treated as seamless
> +		 * but without the automatic downclocking?
> +		 */
> +		if (power->dmrrs & BIT(panel_type))
> +			panel->vbt.drrs_type = DRRS_TYPE_STATIC;
> +		else
> +			panel->vbt.drrs_type = DRRS_TYPE_NONE;
> +	}
>  
>  	if (i915->vbt.version >= 232)
>  		panel->vbt.edp.hobl = power->hobl & BIT(panel_type);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Treat DMRRS as static DRRS

[Jani Nikula]

On Mon, 30 May 2022, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Some machines declare DRRS type = seamless, DRRS = no, DMRRS = yes.
>> I *think* DMRRS stands for "dynamcic media refresh rate", and
>> I suspect the way it's meant to work is that it lets the driver
>> switch refresh rates to match the frame rate for media playback.
>> Obviously for us all that kind of policy stuff is entirely up to
>> userspace, so the only thing we may do is make the extra refresh
>> rate(s) available.
>>
>> So let's treat this case as just static DRRS for now. In the
>> future We might want to differentiate the "seamless w/ downclocking"
>> vs. "seamless w/o downclocking" cases so that we could do seamless
>> refresh rate changes for systems that only claim to support DMRRS.
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/125
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Oh, the reasoning for some Acked-bys instead of Reviewed-bys today in
this and another series:

They could all be r-b in the sense that they do what they say on the
box. But I don't really have the information to confirm they are the
right thing to do. I'm acking "let's go with this, and see where it gets
us".

Make sense?


BR,
Jani.



>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 24 +++++++++++++++++++----
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index 0774238e429b..c42b9e7d0dce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1279,8 +1279,16 @@ parse_panel_driver_features(struct drm_i915_private *i915,
>>  		 * static DRRS is 0 and DRRS not supported is represented by
>>  		 * driver->drrs_enabled=false
>>  		 */
>> -		if (!driver->drrs_enabled)
>> -			panel->vbt.drrs_type = DRRS_TYPE_NONE;
>> +		if (!driver->drrs_enabled && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
>> +			/*
>> +			 * FIXME Should DMRRS perhaps be treated as seamless
>> +			 * but without the automatic downclocking?
>> +			 */
>> +			if (driver->dmrrs_enabled)
>> +				panel->vbt.drrs_type = DRRS_TYPE_STATIC;
>> +			else
>> +				panel->vbt.drrs_type = DRRS_TYPE_NONE;
>> +		}
>>  
>>  		panel->vbt.psr.enable = driver->psr_enabled;
>>  	}
>> @@ -1310,8 +1318,16 @@ parse_power_conservation_features(struct drm_i915_private *i915,
>>  	 * static DRRS is 0 and DRRS not supported is represented by
>>  	 * power->drrs & BIT(panel_type)=false
>>  	 */
>> -	if (!(power->drrs & BIT(panel_type)))
>> -		panel->vbt.drrs_type = DRRS_TYPE_NONE;
>> +	if (!(power->drrs & BIT(panel_type)) && panel->vbt.drrs_type != DRRS_TYPE_NONE) {
>> +		/*
>> +		 * FIXME Should DMRRS perhaps be treated as seamless
>> +		 * but without the automatic downclocking?
>> +		 */
>> +		if (power->dmrrs & BIT(panel_type))
>> +			panel->vbt.drrs_type = DRRS_TYPE_STATIC;
>> +		else
>> +			panel->vbt.drrs_type = DRRS_TYPE_NONE;
>> +	}
>>  
>>  	if (i915->vbt.version >= 232)
>>  		panel->vbt.edp.hobl = power->hobl & BIT(panel_type);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 5/6] drm/i915/bios: Define more BDB contents

[Lyude Paul]

On Mon, 2022-05-30 at 15:55 +0300, Jani Nikula wrote:
> On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add a bunch of new struff we're missing in various BDB blocks.
> > 
> > TODO: Bunch of these might actually need to be taken
> > into use...
> 
> Cc: Jouni, Lyude for some HDR backlight stuff below.
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 50 ++++++++++++++++---
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > index 39109f204c6d..be99f585b1d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > @@ -564,7 +564,9 @@ struct bdb_driver_features {
> >         u16 tbt_enabled:1;
> >         u16 psr_enabled:1;
> >         u16 ips_enabled:1;
> > -       u16 reserved3:4;
> > +       u16 reserved3:1;
> > +       u16 dmrrs_enabled:1;
> 
> Should we start logging the version ranges here too, since it's obsolete
> from 228. Kinda duplicating the info though. *shrug*.
> 
> > +       u16 reserved4:2;
> >         u16 pc_feature_valid:1;
> >  } __packed;
> >  
> > @@ -666,6 +668,16 @@ struct edp_full_link_params {
> >         u8 vswing:4;
> >  } __packed;
> >  
> > +struct edp_apical_params {
> > +       u32 panel_oui;
> > +       u32 dpcd_base_address;
> > +       u32 dpcd_idridix_control_0;
> > +       u32 dpcd_option_select;
> > +       u32 dpcd_backlight;
> > +       u32 ambient_light;
> > +       u32 backlight_scale;
> > +} __packed;

If we could get some more info on what all of this stuff is that'd be
appreciated! I had thought that knowing which 

> > +
> >  struct bdb_edp {
> >         struct edp_power_seq power_seqs[16];
> >         u32 color_depth;
> > @@ -681,6 +693,9 @@ struct bdb_edp {
> >         struct edp_pwm_delays pwm_delays[16];                   /* 186 */
> >         u16 full_link_params_provided;                          /* 199 */
> >         struct edp_full_link_params full_link_params[16];       /* 199 */
> > +       u16 apical_enable;                                      /* 203 */
> > +       struct edp_apical_params apical_params[16];             /* 203 */
> > +       u16 edp_fast_link_training_rate[16];                    /* 224 */
> 
> Another eDP port link rate param would go here? Could be added in
> another patch.
> 
> >  } __packed;
> >  
> >  /*
> > @@ -717,6 +732,7 @@ struct bdb_lvds_options {
> >  
> >         u16 lcdvcc_s0_enable;                                   /* 200 */
> >         u32 rotation;                                           /* 228 */
> > +       u32 position;                                           /* 240 */
> >  } __packed;
> >  
> >  /*
> > @@ -843,13 +859,22 @@ struct bdb_lfp_backlight_data {
> >         u8 level[16]; /* Obsolete from 234+ */
> >         struct lfp_backlight_control_method backlight_control[16];
> >         struct lfp_brightness_level brightness_level[16];               /*
> > 234+ */
> > -       struct lfp_brightness_level brightness_min_level[16];   /* 234+ */
> > -       u8
> > brightness_precision_bits[16];                                            
> >    /* 236+ */
> > +       struct lfp_brightness_level brightness_min_level[16];           /*
> > 234+ */
> > +       u8 brightness_precision_bits[16];                               /*
> > 236+ */
> > +       u16 hdr_dpcd_refresh_timeout[16];                               /*
> > 239+ */
> 
> Jouni, Lyude, this is probably interesting to you:
> 
> """
> This table of values (for 16 panels, 1 value per panel) is used to
> specify the time required by the TCON (with Intel HDR Aux Interface
> Support) to refresh the DPCD set with Intel HDR CAPS (DPCD offset:
> 340h-344h).
> 
> The value is in units of 10 us(microseconds).
> """

Ville had actually mentioned this one before! I hadn't added support for it
quite yet though since I don't think any of the machines I've run into thus
far actually had this present in their VBTs. Since we're adding this here
though, I may as well dig up the patch I wrote for that and post it in a bit

> 
> >  } __packed;
> >  
> >  /*
> >   * Block 44 - LFP Power Conservation Features Block
> >   */
> > +struct lfp_features {
> 
> Nit, maybe lfp_power_features.
> 
> Anyway,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> > +       u8 reserved1:1;
> > +       u8 power_conservation_pref:3;
> > +       u8 reserved2:1;
> > +       u8 lace_enabled_status:1;
> > +       u8 lace_support:1;
> > +       u8 als_enable:1;
> > +} __packed;
> >  
> >  struct als_data_entry {
> >         u16 backlight_adjust;
> > @@ -861,10 +886,16 @@ struct aggressiveness_profile_entry {
> >         u8 lace_aggressiveness : 4;
> >  } __packed;
> >  
> > +struct aggressiveness_profile2_entry {
> > +       u8 opst_aggressiveness : 4;
> > +       u8 elp_aggressiveness : 4;
> > +} __packed;
> > +
> >  struct bdb_lfp_power {
> > -       u8 lfp_feature_bits;
> > +       struct lfp_features features;
> >         struct als_data_entry als[5];
> > -       u8 lace_aggressiveness_profile;
> > +       u8 lace_aggressiveness_profile:3;
> > +       u8 reserved1:5;
> >         u16 dpst;
> >         u16 psr;
> >         u16 drrs;
> > @@ -876,6 +907,9 @@ struct bdb_lfp_power {
> >         struct aggressiveness_profile_entry aggressiveness[16];
> >         u16 hobl; /* 232+ */
> >         u16 vrr_feature_enabled; /* 233+ */
> > +       u16 elp; /* 247+ */
> > +       u16 opst; /* 247+ */
> > +       struct aggressiveness_profile2_entry aggressiveness2[16]; /* 247+
> > */
> >  } __packed;
> >  
> >  /*
> > @@ -885,8 +919,10 @@ struct bdb_lfp_power {
> >  #define MAX_MIPI_CONFIGURATIONS        6
> >  
> >  struct bdb_mipi_config {
> > -       struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
> > -       struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
> > +       struct mipi_config config[MAX_MIPI_CONFIGURATIONS]; /* 175 */
> > +       struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS]; /* 177 */
> > +       struct edp_pwm_delays pwm_delays[MAX_MIPI_CONFIGURATIONS]; /* 186
> > */
> > +       u8 pmic_i2c_bus_number[MAX_MIPI_CONFIGURATIONS]; /* 190 */
> >  } __packed;
> >  
> >  /*
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [Intel-gfx] [PATCH 5/6] drm/i915/bios: Define more BDB contents

[Ville Syrjälä]

On Mon, May 30, 2022 at 03:55:52PM +0300, Jani Nikula wrote:
> On Fri, 27 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add a bunch of new struff we're missing in various BDB blocks.
> >
> > TODO: Bunch of these might actually need to be taken
> > into use...
> 
> Cc: Jouni, Lyude for some HDR backlight stuff below.
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 50 ++++++++++++++++---
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > index 39109f204c6d..be99f585b1d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > @@ -564,7 +564,9 @@ struct bdb_driver_features {
> >  	u16 tbt_enabled:1;
> >  	u16 psr_enabled:1;
> >  	u16 ips_enabled:1;
> > -	u16 reserved3:4;
> > +	u16 reserved3:1;
> > +	u16 dmrrs_enabled:1;
> 
> Should we start logging the version ranges here too, since it's obsolete
> from 228. Kinda duplicating the info though. *shrug*.
> 
> > +	u16 reserved4:2;
> >  	u16 pc_feature_valid:1;
> >  } __packed;
> >  
> > @@ -666,6 +668,16 @@ struct edp_full_link_params {
> >  	u8 vswing:4;
> >  } __packed;
> >  
> > +struct edp_apical_params {
> > +	u32 panel_oui;
> > +	u32 dpcd_base_address;
> > +	u32 dpcd_idridix_control_0;
> > +	u32 dpcd_option_select;
> > +	u32 dpcd_backlight;
> > +	u32 ambient_light;
> > +	u32 backlight_scale;
> > +} __packed;
> > +
> >  struct bdb_edp {
> >  	struct edp_power_seq power_seqs[16];
> >  	u32 color_depth;
> > @@ -681,6 +693,9 @@ struct bdb_edp {
> >  	struct edp_pwm_delays pwm_delays[16];			/* 186 */
> >  	u16 full_link_params_provided;				/* 199 */
> >  	struct edp_full_link_params full_link_params[16];	/* 199 */
> > +	u16 apical_enable;					/* 203 */
> > +	struct edp_apical_params apical_params[16];		/* 203 */
> > +	u16 edp_fast_link_training_rate[16];			/* 224 */
> 
> Another eDP port link rate param would go here? Could be added in
> another patch.

LOL+sigh. There is already a max link rate in the child dev but
naturally it's only specified using three bits so it's not in any
way future proof, or even capable of handling intermediate link rates.
I suppose that is the reason they added this second thing.

Oh and there's also two ways to specify the fast link training
rate as well. Man, I love VBT.

I was going to type up a quick patch but looks like there's
another chicken vs. egg situation due to panel_type again.
That is, we currently do the set_*_rates() stuff before the
per-panel VBT parsing. So I'll need to figure out if there's
anything preventing us from initializing the source rates
after the per-panel VBT parsing has been done...

-- 
Ville Syrjälä
Intel