A series of eDP backlight fixes for i915 (rhbz 1811850)

1 files changed, 894 insertions, 0 deletions

diff --git a/drm-i915-backports.patch b/drm-i915-backports.patch
new file mode 100644
index 000000000..6fa8d2849
--- /dev/null
+++ b/drm-i915-backports.patch
@@ -0,0 +1,894 @@
+From 0fdb20f83f9962a3501e9cbdbfcc37ed5e721ab8 Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:31 -0400
+Subject: [PATCH 1/7] drm/i915: Fix eDP DPCD aux max backlight calculations
+
+Max backlight value for the panel was being calculated using byte
+count i.e. 0xffff if 2 bytes are supported for backlight brightness
+and 0xff if 1 byte is supported. However, EDP_PWMGEN_BIT_COUNT
+determines the number of active control bits used for the brightness
+setting. Thus, even if the panel uses 2 byte setting, it might not use
+all the control bits. Thus, max backlight should be set based on the
+value of EDP_PWMGEN_BIT_COUNT instead of assuming 65535 or 255.
+
+Additionally, EDP_PWMGEN_BIT_COUNT was being updated based on the VBT
+frequency which results in a different max backlight value. Thus,
+setting of EDP_PWMGEN_BIT_COUNT is moved to setup phase instead of
+enable so that max backlight can be calculated correctly. Only the
+frequency divider is set during the enable phase using the value of
+EDP_PWMGEN_BIT_COUNT.
+
+This is based off the original patch series from Furquan Shaikh
+<furquan@google.com>:
+
+https://patchwork.freedesktop.org/patch/317255/?series=62326&rev=3
+
+Changes since original patch:
+* Remove unused intel_dp variable in intel_dp_aux_setup_backlight()
+* Fix checkpatch issues
+* Make sure that we rewrite the pwmgen bit count whenever we bring the
+  panel out of D3 mode
+
+v2 by Jani:
+* rebase
+* fix readb return value check
+
+Cc: Furquan Shaikh <furquan@google.com>
+Tested-by: AceLan Kao <acelan.kao@canonical.com>
+Tested-by: Perry Yuan <pyuan@redhat.com>
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Signed-off-by: Jani Nikula <jani.nikula@intel.com>
+Link: https://patchwork.freedesktop.org/patch/msgid/20200116211623.53799-2-lyude@redhat.com
+---
+ .../drm/i915/display/intel_display_types.h    |   3 +
+ .../drm/i915/display/intel_dp_aux_backlight.c | 139 ++++++++++++------
+ 2 files changed, 95 insertions(+), 47 deletions(-)
+
+diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
+index 888ea8a170d1..778bd30743e5 100644
+--- a/drivers/gpu/drm/i915/display/intel_display_types.h
++++ b/drivers/gpu/drm/i915/display/intel_display_types.h
+@@ -214,6 +214,9 @@ struct intel_panel {
+ 		u8 controller;		/* bxt+ only */
+ 		struct pwm_device *pwm;
+ 
++		/* DPCD backlight */
++		u8 pwmgen_bit_count;
++
+ 		struct backlight_device *device;
+ 
+ 		/* Connector and platform specific backlight functions */
+diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+index 7c653f8c307f..345eed641455 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
++++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+@@ -111,61 +111,28 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
+ {
+ 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ 	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
+-	int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
+-	u8 pn, pn_min, pn_max;
++	const u8 pn = connector->panel.backlight.pwmgen_bit_count;
++	int freq, fxp, f, fxp_actual, fxp_min, fxp_max;
+ 
+-	/* Find desired value of (F x P)
+-	 * Note that, if F x P is out of supported range, the maximum value or
+-	 * minimum value will applied automatically. So no need to check that.
+-	 */
+ 	freq = dev_priv->vbt.backlight.pwm_freq_hz;
+-	DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq);
+ 	if (!freq) {
+ 		DRM_DEBUG_KMS("Use panel default backlight frequency\n");
+ 		return false;
+ 	}
+ 
+ 	fxp = DIV_ROUND_CLOSEST(KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ), freq);
++	f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255);
++	fxp_actual = f << pn;
+ 
+-	/* Use highest possible value of Pn for more granularity of brightness
+-	 * adjustment while satifying the conditions below.
+-	 * - Pn is in the range of Pn_min and Pn_max
+-	 * - F is in the range of 1 and 255
+-	 * - FxP is within 25% of desired value.
+-	 *   Note: 25% is arbitrary value and may need some tweak.
+-	 */
+-	if (drm_dp_dpcd_readb(&intel_dp->aux,
+-			       DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) {
+-		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n");
+-		return false;
+-	}
+-	if (drm_dp_dpcd_readb(&intel_dp->aux,
+-			       DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != 1) {
+-		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n");
+-		return false;
+-	}
+-	pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+-	pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+-
++	/* Ensure frequency is within 25% of desired value */
+ 	fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4);
+ 	fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4);
+-	if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) {
+-		DRM_DEBUG_KMS("VBT defined backlight frequency out of range\n");
+-		return false;
+-	}
+ 
+-	for (pn = pn_max; pn >= pn_min; pn--) {
+-		f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255);
+-		fxp_actual = f << pn;
+-		if (fxp_min <= fxp_actual && fxp_actual <= fxp_max)
+-			break;
+-	}
+-
+-	if (drm_dp_dpcd_writeb(&intel_dp->aux,
+-			       DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) {
+-		DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n");
++	if (fxp_min > fxp_actual || fxp_actual > fxp_max) {
++		DRM_DEBUG_KMS("Actual frequency out of range\n");
+ 		return false;
+ 	}
++
+ 	if (drm_dp_dpcd_writeb(&intel_dp->aux,
+ 			       DP_EDP_BACKLIGHT_FREQ_SET, (u8) f) < 0) {
+ 		DRM_DEBUG_KMS("Failed to write aux backlight freq\n");
+@@ -179,6 +146,7 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
+ {
+ 	struct intel_connector *connector = to_intel_connector(conn_state->connector);
+ 	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
++	struct intel_panel *panel = &connector->panel;
+ 	u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
+ 
+ 	if (drm_dp_dpcd_readb(&intel_dp->aux,
+@@ -197,6 +165,12 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
+ 	case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT:
+ 		new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
+ 		new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
++
++		if (drm_dp_dpcd_writeb(&intel_dp->aux,
++				       DP_EDP_PWMGEN_BIT_COUNT,
++				       panel->backlight.pwmgen_bit_count) < 0)
++			DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n");
++
+ 		break;
+ 
+ 	/* Do nothing when it is already DPCD mode */
+@@ -226,20 +200,91 @@ static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old
+ 				 false);
+ }
+ 
++static u32 intel_dp_aux_calc_max_backlight(struct intel_connector *connector)
++{
++	struct drm_i915_private *i915 = to_i915(connector->base.dev);
++	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
++	struct intel_panel *panel = &connector->panel;
++	u32 max_backlight = 0;
++	int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
++	u8 pn, pn_min, pn_max;
++
++	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT, &pn) == 1) {
++		pn &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
++		max_backlight = (1 << pn) - 1;
++	}
++
++	/* Find desired value of (F x P)
++	 * Note that, if F x P is out of supported range, the maximum value or
++	 * minimum value will applied automatically. So no need to check that.
++	 */
++	freq = i915->vbt.backlight.pwm_freq_hz;
++	DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq);
++	if (!freq) {
++		DRM_DEBUG_KMS("Use panel default backlight frequency\n");
++		return max_backlight;
++	}
++
++	fxp = DIV_ROUND_CLOSEST(KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ), freq);
++
++	/* Use highest possible value of Pn for more granularity of brightness
++	 * adjustment while satifying the conditions below.
++	 * - Pn is in the range of Pn_min and Pn_max
++	 * - F is in the range of 1 and 255
++	 * - FxP is within 25% of desired value.
++	 *   Note: 25% is arbitrary value and may need some tweak.
++	 */
++	if (drm_dp_dpcd_readb(&intel_dp->aux,
++			      DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) {
++		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n");
++		return max_backlight;
++	}
++	if (drm_dp_dpcd_readb(&intel_dp->aux,
++			      DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != 1) {
++		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n");
++		return max_backlight;
++	}
++	pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
++	pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
++
++	fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4);
++	fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4);
++	if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) {
++		DRM_DEBUG_KMS("VBT defined backlight frequency out of range\n");
++		return max_backlight;
++	}
++
++	for (pn = pn_max; pn >= pn_min; pn--) {
++		f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255);
++		fxp_actual = f << pn;
++		if (fxp_min <= fxp_actual && fxp_actual <= fxp_max)
++			break;
++	}
++
++	DRM_DEBUG_KMS("Using eDP pwmgen bit count of %d\n", pn);
++	if (drm_dp_dpcd_writeb(&intel_dp->aux,
++			       DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) {
++		DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n");
++		return max_backlight;
++	}
++	panel->backlight.pwmgen_bit_count = pn;
++
++	max_backlight = (1 << pn) - 1;
++
++	return max_backlight;
++}
++
+ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
+ 					enum pipe pipe)
+ {
+-	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
+ 	struct intel_panel *panel = &connector->panel;
+ 
+-	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
+-		panel->backlight.max = 0xFFFF;
+-	else
+-		panel->backlight.max = 0xFF;
++	panel->backlight.max = intel_dp_aux_calc_max_backlight(connector);
++	if (!panel->backlight.max)
++		return -ENODEV;
+ 
+ 	panel->backlight.min = 0;
+ 	panel->backlight.level = intel_dp_aux_get_backlight(connector);
+-
+ 	panel->backlight.enabled = panel->backlight.level != 0;
+ 
+ 	return 0;
+-- 
+2.25.1
+
+From 7dbe3f659d364de34b210baf0598913dc8c3cabd Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:32 -0400
+Subject: [PATCH 2/7] drm/i915: Assume 100% brightness when not in DPCD control
+ mode
+
+Currently we always determine the initial panel brightness level by
+simply reading the value from DP_EDP_BACKLIGHT_BRIGHTNESS_MSB/LSB. This
+seems wrong though, because if the panel is not currently in DPCD
+control mode there's not really any reason why there would be any
+brightness value programmed in the first place.
+
+This appears to be the case on the Lenovo ThinkPad X1 Extreme 2nd
+Generation, where the default value in these registers is always 0 on
+boot despite the fact the panel runs at max brightness by default.
+Getting the initial brightness value correct here is important as well,
+since the panel on this laptop doesn't behave well if it's ever put into
+DPCD control mode while the brightness level is programmed to 0.
+
+So, let's fix this by checking what the current backlight control mode
+is before reading the brightness level. If it's in DPCD control mode, we
+return the programmed brightness level. Otherwise we assume 100%
+brightness and return the highest possible brightness level. This also
+prevents us from accidentally programming a brightness level of 0.
+
+This is one of the many fixes that gets backlight controls working on
+the ThinkPad X1 Extreme 2nd Generation with optional 4K AMOLED screen.
+
+Changes since v1:
+* s/DP_EDP_DISPLAY_CONTROL_REGISTER/DP_EDP_BACKLIGHT_MODE_SET_REGISTER/
+  - Jani
+
+Tested-by: AceLan Kao <acelan.kao@canonical.com>
+Tested-by: Perry Yuan <pyuan@redhat.com>
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Signed-off-by: Jani Nikula <jani.nikula@intel.com>
+Link: https://patchwork.freedesktop.org/patch/msgid/20200116211623.53799-3-lyude@redhat.com
+---
+ .../drm/i915/display/intel_dp_aux_backlight.c   | 17 +++++++++++++++++
+ 1 file changed, 17 insertions(+)
+
+diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+index 345eed641455..5d4db5f8a165 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
++++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+@@ -59,8 +59,25 @@ static u32 intel_dp_aux_get_backlight(struct intel_connector *connector)
+ {
+ 	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
+ 	u8 read_val[2] = { 0x0 };
++	u8 mode_reg;
+ 	u16 level = 0;
+ 
++	if (drm_dp_dpcd_readb(&intel_dp->aux,
++			      DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
++			      &mode_reg) != 1) {
++		DRM_DEBUG_KMS("Failed to read the DPCD register 0x%x\n",
++			      DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
++		return 0;
++	}
++
++	/*
++	 * If we're not in DPCD control mode yet, the programmed brightness
++	 * value is meaningless and we should assume max brightness
++	 */
++	if ((mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) !=
++	    DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD)
++		return connector->panel.backlight.max;
++
+ 	if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
+ 			     &read_val, sizeof(read_val)) < 0) {
+ 		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
+-- 
+2.25.1
+
+From b2a29a70e386c2fbd92e1b7980091e7980495211 Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:33 -0400
+Subject: [PATCH 3/7] drm/i915: Fix DPCD register order in
+ intel_dp_aux_enable_backlight()
+
+For eDP panels, it appears it's expected that so long as the panel is in
+DPCD control mode that the brightness value is never set to 0. Instead,
+if the desired effect is to set the panel's backlight to 0 we're
+expected to simply turn off the backlight through the
+DP_EDP_DISPLAY_CONTROL_REGISTER.
+
+We already do the latter correctly in intel_dp_aux_disable_backlight().
+But, we make the mistake of writing the DPCD registers in the wrong
+order when enabling the backlight in intel_dp_aux_enable_backlight()
+since we currently enable the backlight through
+DP_EDP_DISPLAY_CONTROL_REGISTER before writing the brightness level. On
+the X1 Extreme 2nd Generation, this appears to have the potential of
+confusing the panel in such a way that further attempts to set the
+brightness don't actually change the backlight as expected and leave it
+off. Presumably, this happens because the incorrect register writing
+order briefly leaves the panel with DPCD mode enabled and a 0 brightness
+level set.
+
+So, reverse the order we write the DPCD registers when enabling the
+panel backlight so that we write the brightness value first, and enable
+the backlight second. This fix appears to be the final bit needed to get
+the backlight on the ThinkPad X1 Extreme 2nd Generation's AMOLED screen
+working.
+
+Tested-by: AceLan Kao <acelan.kao@canonical.com>
+Tested-by: Perry Yuan <pyuan@redhat.com>
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Signed-off-by: Jani Nikula <jani.nikula@intel.com>
+Link: https://patchwork.freedesktop.org/patch/msgid/20200116211623.53799-4-lyude@redhat.com
+---
+ drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+index 5d4db5f8a165..77a759361c5c 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
++++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+@@ -207,8 +207,9 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
+ 		}
+ 	}
+ 
++	intel_dp_aux_set_backlight(conn_state,
++				   connector->panel.backlight.level);
+ 	set_aux_backlight_enable(intel_dp, true);
+-	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
+ }
+ 
+ static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)
+-- 
+2.25.1
+
+From 8b2e6f450c1f8d34632d4789369030008e874a75 Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:34 -0400
+Subject: [PATCH 4/7] drm/i915: Auto detect DPCD backlight support by default
+
+Turns out we actually already have some companies, such as Lenovo,
+shipping machines with AMOLED screens that don't allow controlling the
+backlight through the usual PWM interface and only allow controlling it
+through the standard EDP DPCD interface. One example of one of these
+laptops is the X1 Extreme 2nd Generation.
+
+Since we've got systems that need this turned on by default now to have
+backlight controls working out of the box, let's start auto-detecting it
+for systems by default based on what the VBT tells us. We do this by
+changing the default value for the enable_dpcd_backlight module param
+from 0 to -1.
+
+Tested-by: AceLan Kao <acelan.kao@canonical.com>
+Tested-by: Perry Yuan <pyuan@redhat.com>
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Signed-off-by: Jani Nikula <jani.nikula@intel.com>
+Link: https://patchwork.freedesktop.org/patch/msgid/20200116211623.53799-6-lyude@redhat.com
+---
+ drivers/gpu/drm/i915/i915_params.c | 2 +-
+ drivers/gpu/drm/i915/i915_params.h | 2 +-
+ 2 files changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
+index 1dd1f3652795..31eed60c167e 100644
+--- a/drivers/gpu/drm/i915/i915_params.c
++++ b/drivers/gpu/drm/i915/i915_params.c
+@@ -172,7 +172,7 @@ i915_param_named_unsafe(inject_probe_failure, uint, 0400,
+ 
+ i915_param_named(enable_dpcd_backlight, int, 0600,
+ 	"Enable support for DPCD backlight control"
+-	"(-1=use per-VBT LFP backlight type setting, 0=disabled [default], 1=enabled)");
++	"(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enabled)");
+ 
+ #if IS_ENABLED(CONFIG_DRM_I915_GVT)
+ i915_param_named(enable_gvt, bool, 0400,
+diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
+index 31b88f297fbc..a79d0867f77a 100644
+--- a/drivers/gpu/drm/i915/i915_params.h
++++ b/drivers/gpu/drm/i915/i915_params.h
+@@ -64,7 +64,7 @@ struct drm_printer;
+ 	param(int, reset, 3) \
+ 	param(unsigned int, inject_probe_failure, 0) \
+ 	param(int, fastboot, -1) \
+-	param(int, enable_dpcd_backlight, 0) \
++	param(int, enable_dpcd_backlight, -1) \
+ 	param(char *, force_probe, CONFIG_DRM_I915_FORCE_PROBE) \
+ 	param(unsigned long, fake_lmem_start, 0) \
+ 	/* leave bools at the end to not create holes */ \
+-- 
+2.25.1
+
+From c10b0dfaac8385f9b712a552c9a5eed9976aacf2 Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:35 -0400
+Subject: [PATCH 5/7] drm/dp: Introduce EDID-based quirks
+
+The whole point of using OUIs is so that we can recognize certain
+devices and potentially apply quirks for them. Normally this should work
+quite well, but there appears to be quite a number of laptop panels out
+there that will fill the OUI but not the device ID. As such, for devices
+like this I can't imagine it's a very good idea to try relying on OUIs
+for applying quirks. As well, some laptop vendors have confirmed to us
+that their panels have this exact issue.
+
+So, let's introduce the ability to apply DP quirks based on EDID
+identification. We reuse the same quirk bits for OUI-based quirks, so
+that callers can simply check all possible quirks using
+drm_dp_has_quirk().
+
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Reviewed-by: Adam Jackson <ajax@redhat.com>
+Cc: Jani Nikula <jani.nikula@intel.com>
+---
+ drivers/gpu/drm/drm_dp_helper.c               | 61 +++++++++++++++++++
+ drivers/gpu/drm/drm_dp_mst_topology.c         |  3 +-
+ .../drm/i915/display/intel_display_types.h    |  1 +
+ drivers/gpu/drm/i915/display/intel_dp.c       | 11 ++--
+ drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 +-
+ drivers/gpu/drm/i915/display/intel_psr.c      |  2 +-
+ include/drm/drm_dp_helper.h                   | 11 +++-
+ 7 files changed, 81 insertions(+), 10 deletions(-)
+
+diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
+index a5364b5192b8..9b2ea2ae0204 100644
+--- a/drivers/gpu/drm/drm_dp_helper.c
++++ b/drivers/gpu/drm/drm_dp_helper.c
+@@ -1222,6 +1222,67 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+ #undef DEVICE_ID_ANY
+ #undef DEVICE_ID
+ 
++struct edid_quirk {
++	u8 mfg_id[2];
++	u8 prod_id[2];
++	u32 quirks;
++};
++
++#define MFG(first, second) { (first), (second) }
++#define PROD_ID(first, second) { (first), (second) }
++
++/*
++ * Some devices have unreliable OUIDs where they don't set the device ID
++ * correctly, and as a result we need to use the EDID for finding additional
++ * DP quirks in such cases.
++ */
++static const struct edid_quirk edid_quirk_list[] = {
++};
++
++#undef MFG
++#undef PROD_ID
++
++/**
++ * drm_dp_get_edid_quirks() - Check the EDID of a DP device to find additional
++ * DP-specific quirks
++ * @edid: The EDID to check
++ *
++ * While OUIDs are meant to be used to recognize a DisplayPort device, a lot
++ * of manufacturers don't seem to like following standards and neglect to fill
++ * the dev-ID in, making it impossible to only use OUIDs for determining
++ * quirks in some cases. This function can be used to check the EDID and look
++ * up any additional DP quirks. The bits returned by this function correspond
++ * to the quirk bits in &drm_dp_quirk.
++ *
++ * Returns: a bitmask of quirks, if any. The driver can check this using
++ * drm_dp_has_quirk().
++ */
++u32 drm_dp_get_edid_quirks(const struct edid *edid)
++{
++	const struct edid_quirk *quirk;
++	u32 quirks = 0;
++	int i;
++
++	if (!edid)
++		return 0;
++
++	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
++		quirk = &edid_quirk_list[i];
++		if (memcmp(quirk->mfg_id, edid->mfg_id,
++			   sizeof(edid->mfg_id)) == 0 &&
++		    memcmp(quirk->prod_id, edid->prod_code,
++			   sizeof(edid->prod_code)) == 0)
++			quirks |= quirk->quirks;
++	}
++
++	DRM_DEBUG_KMS("DP sink: EDID mfg %*phD prod-ID %*phD quirks: 0x%04x\n",
++		      (int)sizeof(edid->mfg_id), edid->mfg_id,
++		      (int)sizeof(edid->prod_code), edid->prod_code, quirks);
++
++	return quirks;
++}
++EXPORT_SYMBOL(drm_dp_get_edid_quirks);
++
+ /**
+  * drm_dp_read_desc - read sink/branch descriptor from DPCD
+  * @aux: DisplayPort AUX channel
+diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
+index cce0b1bba591..685c35e67144 100644
+--- a/drivers/gpu/drm/drm_dp_mst_topology.c
++++ b/drivers/gpu/drm/drm_dp_mst_topology.c
+@@ -5461,7 +5461,8 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
+ 	if (drm_dp_read_desc(port->mgr->aux, &desc, true))
+ 		return NULL;
+ 
+-	if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) &&
++	if (drm_dp_has_quirk(&desc, 0,
++			     DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) &&
+ 	    port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 &&
+ 	    port->parent == port->mgr->mst_primary) {
+ 		u8 downstreamport;
+diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
+index 778bd30743e5..8e3c5569603b 100644
+--- a/drivers/gpu/drm/i915/display/intel_display_types.h
++++ b/drivers/gpu/drm/i915/display/intel_display_types.h
+@@ -1253,6 +1253,7 @@ struct intel_dp {
+ 	int max_link_rate;
+ 	/* sink or branch descriptor */
+ 	struct drm_dp_desc desc;
++	u32 edid_quirks;
+ 	struct drm_dp_aux aux;
+ 	u32 aux_busy_last_status;
+ 	u8 train_set[4];
+diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
+index c7424e2a04a3..e20b85ff937d 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp.c
++++ b/drivers/gpu/drm/i915/display/intel_dp.c
+@@ -2373,7 +2373,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
+ 	struct intel_connector *intel_connector = intel_dp->attached_connector;
+ 	struct intel_digital_connector_state *intel_conn_state =
+ 		to_intel_digital_connector_state(conn_state);
+-	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
++	bool constant_n = drm_dp_has_quirk(&intel_dp->desc, 0,
+ 					   DP_DPCD_QUIRK_CONSTANT_N);
+ 	int ret = 0, output_bpp;
+ 
+@@ -4466,7 +4466,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
+ 	 * it don't care about read it here and in intel_edp_init_dpcd().
+ 	 */
+ 	if (!intel_dp_is_edp(intel_dp) &&
+-	    !drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_SINK_COUNT)) {
++	    !drm_dp_has_quirk(&intel_dp->desc, 0,
++			      DP_DPCD_QUIRK_NO_SINK_COUNT)) {
+ 		u8 count;
+ 		ssize_t r;
+ 
+@@ -5631,6 +5632,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
+ 
+ 	intel_dp->has_audio = drm_detect_monitor_audio(edid);
+ 	drm_dp_cec_set_edid(&intel_dp->aux, edid);
++	intel_dp->edid_quirks = drm_dp_get_edid_quirks(edid);
+ }
+ 
+ static void
+@@ -5643,6 +5645,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
+ 	intel_connector->detect_edid = NULL;
+ 
+ 	intel_dp->has_audio = false;
++	intel_dp->edid_quirks = 0;
+ }
+ 
+ static int
+@@ -7356,8 +7359,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
+ 	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
+ 	if (edid) {
+ 		if (drm_add_edid_modes(connector, edid)) {
+-			drm_connector_update_edid_property(connector,
+-								edid);
++			drm_connector_update_edid_property(connector, edid);
++			intel_dp->edid_quirks = drm_dp_get_edid_quirks(edid);
+ 		} else {
+ 			kfree(edid);
+ 			edid = ERR_PTR(-EINVAL);
+diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
+index cba68c5a80fa..4a1a2f868423 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
++++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
+@@ -50,7 +50,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
+ 	const struct drm_display_mode *adjusted_mode =
+ 		&crtc_state->hw.adjusted_mode;
+ 	void *port = connector->port;
+-	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
++	bool constant_n = drm_dp_has_quirk(&intel_dp->desc, 0,
+ 					   DP_DPCD_QUIRK_CONSTANT_N);
+ 	int bpp, slots = -EINVAL;
+ 
+diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
+index 83025052c965..82ba5624d14f 100644
+--- a/drivers/gpu/drm/i915/display/intel_psr.c
++++ b/drivers/gpu/drm/i915/display/intel_psr.c
+@@ -282,7 +282,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+ 	DRM_DEBUG_KMS("eDP panel supports PSR version %x\n",
+ 		      intel_dp->psr_dpcd[0]);
+ 
+-	if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_PSR)) {
++	if (drm_dp_has_quirk(&intel_dp->desc, 0, DP_DPCD_QUIRK_NO_PSR)) {
+ 		DRM_DEBUG_KMS("PSR support not currently available for this panel\n");
+ 		return;
+ 	}
+diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
+index bc04467f7c3a..1fe49e202dfb 100644
+--- a/include/drm/drm_dp_helper.h
++++ b/include/drm/drm_dp_helper.h
+@@ -1493,13 +1493,16 @@ struct drm_dp_desc {
+ 
+ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
+ 		     bool is_branch);
++u32 drm_dp_get_edid_quirks(const struct edid *edid);
+ 
+ /**
+  * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+  *
+  * Display Port sink and branch devices in the wild have a variety of bugs, try
+  * to collect them here. The quirks are shared, but it's up to the drivers to
+- * implement workarounds for them.
++ * implement workarounds for them. Note that because some devices have
++ * unreliable OUIDs, the EDID of sinks should also be checked for quirks using
++ * drm_dp_get_edid_quirks().
+  */
+ enum drm_dp_quirk {
+ 	/**
+@@ -1535,14 +1538,16 @@ enum drm_dp_quirk {
+ /**
+  * drm_dp_has_quirk() - does the DP device have a specific quirk
+  * @desc: Device decriptor filled by drm_dp_read_desc()
++ * @edid_quirks: Optional quirk bitmask filled by drm_dp_get_edid_quirks()
+  * @quirk: Quirk to query for
+  *
+  * Return true if DP device identified by @desc has @quirk.
+  */
+ static inline bool
+-drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
++drm_dp_has_quirk(const struct drm_dp_desc *desc, u32 edid_quirks,
++		 enum drm_dp_quirk quirk)
+ {
+-	return desc->quirks & BIT(quirk);
++	return (desc->quirks | edid_quirks) & BIT(quirk);
+ }
+ 
+ #ifdef CONFIG_DRM_DP_CEC
+-- 
+2.25.1
+
+From a21ec8aec8452de788d6b1fc175dc8281a57d5de Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:36 -0400
+Subject: [PATCH 6/7] drm/i915: Force DPCD backlight mode on X1 Extreme 2nd Gen
+ 4K AMOLED panel
+
+The X1 Extreme is one of the systems that lies about which backlight
+interface that it uses in its VBIOS as PWM backlight controls don't work
+at all on this machine. It's possible that this panel could be one of
+the infamous ones that can switch between PWM mode and DPCD backlight
+control mode, but we haven't gotten any more details on this from Lenovo
+just yet. For the time being though, making sure the backlight 'just
+works' is a bit more important.
+
+So, add a quirk to force DPCD backlight controls on for these systems
+based on EDID (since this panel doesn't appear to fill in the device ID).
+Hopefully in the future we'll figure out a better way of probing this.
+
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Reviewed-by: Adam Jackson <ajax@redhat.com>
+Cc: Jani Nikula <jani.nikula@intel.com>
+
+Changes since v2:
+* The bugzilla URL is deprecated, bug reporting happens on gitlab now.
+  Update the messages we print to reflect this
+* Also, take the opportunity to move FDO_BUG_URL out of i915_utils.c and
+  into i915_utils.h so that other places which print things that aren't
+  traditional errors but are worth filing bugs about, can actually use
+  it.
+
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+---
+ drivers/gpu/drm/drm_dp_helper.c               |  4 ++++
+ .../drm/i915/display/intel_dp_aux_backlight.c | 24 +++++++++++++++----
+ drivers/gpu/drm/i915/i915_utils.c             |  1 -
+ drivers/gpu/drm/i915/i915_utils.h             |  2 ++
+ include/drm/drm_dp_helper.h                   | 10 ++++++++
+ 5 files changed, 36 insertions(+), 5 deletions(-)
+
+diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
+index 9b2ea2ae0204..026f701eac69 100644
+--- a/drivers/gpu/drm/drm_dp_helper.c
++++ b/drivers/gpu/drm/drm_dp_helper.c
+@@ -1237,6 +1237,10 @@ struct edid_quirk {
+  * DP quirks in such cases.
+  */
+ static const struct edid_quirk edid_quirk_list[] = {
++	/* Optional 4K AMOLED panel in the ThinkPad X1 Extreme 2nd Generation
++	 * only supports DPCD backlight controls
++	 */
++	{ MFG(0x4c, 0x83), PROD_ID(0x41, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+ };
+ 
+ #undef MFG
+diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+index 77a759361c5c..a7c94c201b38 100644
+--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
++++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+@@ -328,15 +328,31 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
+ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
+ {
+ 	struct intel_panel *panel = &intel_connector->panel;
+-	struct drm_i915_private *dev_priv = to_i915(intel_connector->base.dev);
++	struct intel_dp *intel_dp = enc_to_intel_dp(intel_connector->encoder);
++	struct drm_device *dev = intel_connector->base.dev;
++	struct drm_i915_private *dev_priv = to_i915(dev);
+ 
+ 	if (i915_modparams.enable_dpcd_backlight == 0 ||
+-	    (i915_modparams.enable_dpcd_backlight == -1 &&
+-	    dev_priv->vbt.backlight.type != INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE))
++	    !intel_dp_aux_display_control_capable(intel_connector))
+ 		return -ENODEV;
+ 
+-	if (!intel_dp_aux_display_control_capable(intel_connector))
++	/*
++	 * There are a lot of machines that don't advertise the backlight
++	 * control interface to use properly in their VBIOS, :\
++	 */
++	if (dev_priv->vbt.backlight.type !=
++	    INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE &&
++	    !drm_dp_has_quirk(&intel_dp->desc, intel_dp->edid_quirks,
++			      DP_QUIRK_FORCE_DPCD_BACKLIGHT)) {
++		DRM_DEV_INFO(dev->dev,
++			     "Panel advertises DPCD backlight support, but "
++			     "VBT disagrees. If your backlight controls "
++			     "don't work try booting with "
++			     "i915.enable_dpcd_backlight=1. If your machine "
++			     "needs this, please file a _new_ bug report on "
++			     "drm/i915, see " FDO_BUG_URL " for details.\n");
+ 		return -ENODEV;
++	}
+ 
+ 	panel->backlight.setup = intel_dp_aux_setup_backlight;
+ 	panel->backlight.enable = intel_dp_aux_enable_backlight;
+diff --git a/drivers/gpu/drm/i915/i915_utils.c b/drivers/gpu/drm/i915/i915_utils.c
+index 632d6953c78d..029854ae65fc 100644
+--- a/drivers/gpu/drm/i915/i915_utils.c
++++ b/drivers/gpu/drm/i915/i915_utils.c
+@@ -8,7 +8,6 @@
+ #include "i915_drv.h"
+ #include "i915_utils.h"
+ 
+-#define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs"
+ #define FDO_BUG_MSG "Please file a bug on drm/i915; see " FDO_BUG_URL " for details."
+ 
+ void
+diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
+index b0ade76bec90..cae0ae520398 100644
+--- a/drivers/gpu/drm/i915/i915_utils.h
++++ b/drivers/gpu/drm/i915/i915_utils.h
+@@ -34,6 +34,8 @@
+ struct drm_i915_private;
+ struct timer_list;
+ 
++#define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs"
++
+ #undef WARN_ON
+ /* Many gcc seem to no see through this and fall over :( */
+ #if 0
+diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
+index 1fe49e202dfb..eff5a69051d6 100644
+--- a/include/drm/drm_dp_helper.h
++++ b/include/drm/drm_dp_helper.h
+@@ -1533,6 +1533,16 @@ enum drm_dp_quirk {
+ 	 * The DSC caps can be read from the physical aux instead.
+ 	 */
+ 	DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
++	/**
++	 * @DP_QUIRK_FORCE_DPCD_BACKLIGHT:
++	 *
++	 * The device is telling the truth when it says that it uses DPCD
++	 * backlight controls, even if the system's firmware disagrees. This
++	 * quirk should be checked against both the ident and panel EDID.
++	 * When present, the driver should honor the DPCD backlight
++	 * capabilities advertised.
++	 */
++	DP_QUIRK_FORCE_DPCD_BACKLIGHT,
+ };
+ 
+ /**
+-- 
+2.25.1
+
+From 057e7f8db05c2382b666270b1fbf986fdd172769 Mon Sep 17 00:00:00 2001
+From: Lyude Paul <lyude@redhat.com>
+Date: Tue, 10 Mar 2020 14:07:37 -0400
+Subject: [PATCH 7/7] drm/i915: Force DPCD backlight mode for some Dell CML
+ 2020 panels
+
+According to Dell, trying to match their panels via OUI is not reliable
+enough and we've been told that we should check against the EDID
+instead. As well, Dell seems to have some panels that are actually
+intended to switch between using PWM for backlight controls and DPCD for
+backlight controls depending on whether or not the panel is in HDR or
+SDR mode. Yikes.
+
+Regardless, we need to add quirks for these so that DPCD backlight
+controls get enabled by default, since without additional driver support
+that's the only form of brightness control that will work. Hopefully in
+the future we can remove these quirks once we have a better way of
+probing for this.
+
+Changes since v1:
+* Add one more EDID per Dell's request
+* Remove model number (which is possibly wrong) and replace with Dell
+  CML 2020 systems
+
+Signed-off-by: Lyude Paul <lyude@redhat.com>
+Reviewed-by: Adam Jackson <ajax@redhat.com>
+Cc: Jani Nikula <jani.nikula@intel.com>
+---
+ drivers/gpu/drm/drm_dp_helper.c | 14 ++++++++++++++
+ 1 file changed, 14 insertions(+)
+
+diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
+index 026f701eac69..d3a636a925d4 100644
+--- a/drivers/gpu/drm/drm_dp_helper.c
++++ b/drivers/gpu/drm/drm_dp_helper.c
+@@ -1241,6 +1241,20 @@ static const struct edid_quirk edid_quirk_list[] = {
+ 	 * only supports DPCD backlight controls
+ 	 */
+ 	{ MFG(0x4c, 0x83), PROD_ID(0x41, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
++	/*
++	 * Some Dell CML 2020 systems have panels support both AUX and PWM
++	 * backlight control, and some only support AUX backlight control. All
++	 * said panels start up in AUX mode by default, and we don't have any
++	 * support for disabling HDR mode on these panels which would be
++	 * required to switch to PWM backlight control mode (plus, I'm not
++	 * even sure we want PWM backlight controls over DPCD backlight
++	 * controls anyway...). Until we have a better way of detecting these,
++	 * force DPCD backlight mode on all of them.
++	 */
++	{ MFG(0x06, 0xaf), PROD_ID(0x9b, 0x32), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
++	{ MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
++	{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
++	{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+ };
+ 
+ #undef MFG
+-- 
+2.25.1
+