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) },