drm/dp: Limit the DPCD probe quirk to the affected monitor

An enum list is better suited to define a quirk list, do that. This makes looking up a quirk more robust and also allows for separating quirks internal to the EDID parser and global quirks which can be queried outside of the EDID parser (added as a follow-up).

Suggested-by Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/drm_edid.c | 218 +++++++++++++++++++------------------
 1 file changed, 112 insertions(+), 106 deletions(-)

drivers/gpu/drm/drm_edid.c +112 -106

index 74e77742b2bd4..857ae0c47a1c3 100644
@@ -66,34 +66,36 @@ static int oui(u8 first, u8 second, u8 third)
  * on as many displays as possible).
  */
 
-/* First detailed mode wrong, use largest 60Hz mode */
-#define EDID_QUIRK_PREFER_LARGE_60		(1 << 0)
-/* Reported 135MHz pixel clock is too high, needs adjustment */
-#define EDID_QUIRK_135_CLOCK_TOO_HIGH		(1 << 1)
-/* Prefer the largest mode at 75 Hz */
-#define EDID_QUIRK_PREFER_LARGE_75		(1 << 2)
-/* Detail timing is in cm not mm */
-#define EDID_QUIRK_DETAILED_IN_CM		(1 << 3)
-/* Detailed timing descriptors have bogus size values, so just take the
- * maximum size and use that.
- */
-#define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE	(1 << 4)
-/* use +hsync +vsync for detailed mode */
-#define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
-/* Force reduced-blanking timings for detailed modes */
-#define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
-/* Force 8bpc */
-#define EDID_QUIRK_FORCE_8BPC			(1 << 8)
-/* Force 12bpc */
-#define EDID_QUIRK_FORCE_12BPC			(1 << 9)
-/* Force 6bpc */
-#define EDID_QUIRK_FORCE_6BPC			(1 << 10)
-/* Force 10bpc */
-#define EDID_QUIRK_FORCE_10BPC			(1 << 11)
-/* Non desktop display (i.e. HMD) */
-#define EDID_QUIRK_NON_DESKTOP			(1 << 12)
-/* Cap the DSC target bitrate to 15bpp */
-#define EDID_QUIRK_CAP_DSC_15BPP		(1 << 13)
+enum drm_edid_internal_quirk {
+	/* First detailed mode wrong, use largest 60Hz mode */
+	EDID_QUIRK_PREFER_LARGE_60,
+	/* Reported 135MHz pixel clock is too high, needs adjustment */
+	EDID_QUIRK_135_CLOCK_TOO_HIGH,
+	/* Prefer the largest mode at 75 Hz */
+	EDID_QUIRK_PREFER_LARGE_75,
+	/* Detail timing is in cm not mm */
+	EDID_QUIRK_DETAILED_IN_CM,
+	/* Detailed timing descriptors have bogus size values, so just take the
+	 * maximum size and use that.
+	 */
+	EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE,
+	/* use +hsync +vsync for detailed mode */
+	EDID_QUIRK_DETAILED_SYNC_PP,
+	/* Force reduced-blanking timings for detailed modes */
+	EDID_QUIRK_FORCE_REDUCED_BLANKING,
+	/* Force 8bpc */
+	EDID_QUIRK_FORCE_8BPC,
+	/* Force 12bpc */
+	EDID_QUIRK_FORCE_12BPC,
+	/* Force 6bpc */
+	EDID_QUIRK_FORCE_6BPC,
+	/* Force 10bpc */
+	EDID_QUIRK_FORCE_10BPC,
+	/* Non desktop display (i.e. HMD) */
+	EDID_QUIRK_NON_DESKTOP,
+	/* Cap the DSC target bitrate to 15bpp */
+	EDID_QUIRK_CAP_DSC_15BPP,
+};
 
 #define MICROSOFT_IEEE_OUI	0xca125c
 

@@ -128,124 +130,124 @@ static const struct edid_quirk {
 	u32 quirks;
 } edid_quirk_list[] = {
 	/* Acer AL1706 */
-	EDID_QUIRK('A', 'C', 'R', 44358, EDID_QUIRK_PREFER_LARGE_60),
+	EDID_QUIRK('A', 'C', 'R', 44358, BIT(EDID_QUIRK_PREFER_LARGE_60)),
 	/* Acer F51 */
-	EDID_QUIRK('A', 'P', 'I', 0x7602, EDID_QUIRK_PREFER_LARGE_60),
+	EDID_QUIRK('A', 'P', 'I', 0x7602, BIT(EDID_QUIRK_PREFER_LARGE_60)),
 
 	/* AEO model 0 reports 8 bpc, but is a 6 bpc panel */
-	EDID_QUIRK('A', 'E', 'O', 0, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('A', 'E', 'O', 0, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* BenQ GW2765 */
-	EDID_QUIRK('B', 'N', 'Q', 0x78d6, EDID_QUIRK_FORCE_8BPC),
+	EDID_QUIRK('B', 'N', 'Q', 0x78d6, BIT(EDID_QUIRK_FORCE_8BPC)),
 
 	/* BOE model on HP Pavilion 15-n233sl reports 8 bpc, but is a 6 bpc panel */
-	EDID_QUIRK('B', 'O', 'E', 0x78b, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('B', 'O', 'E', 0x78b, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* CPT panel of Asus UX303LA reports 8 bpc, but is a 6 bpc panel */
-	EDID_QUIRK('C', 'P', 'T', 0x17df, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('C', 'P', 'T', 0x17df, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* SDC panel of Lenovo B50-80 reports 8 bpc, but is a 6 bpc panel */
-	EDID_QUIRK('S', 'D', 'C', 0x3652, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('S', 'D', 'C', 0x3652, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* BOE model 0x0771 reports 8 bpc, but is a 6 bpc panel */
-	EDID_QUIRK('B', 'O', 'E', 0x0771, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('B', 'O', 'E', 0x0771, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* Belinea 10 15 55 */
-	EDID_QUIRK('M', 'A', 'X', 1516, EDID_QUIRK_PREFER_LARGE_60),
-	EDID_QUIRK('M', 'A', 'X', 0x77e, EDID_QUIRK_PREFER_LARGE_60),
+	EDID_QUIRK('M', 'A', 'X', 1516, BIT(EDID_QUIRK_PREFER_LARGE_60)),
+	EDID_QUIRK('M', 'A', 'X', 0x77e, BIT(EDID_QUIRK_PREFER_LARGE_60)),
 
 	/* Envision Peripherals, Inc. EN-7100e */
-	EDID_QUIRK('E', 'P', 'I', 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH),
+	EDID_QUIRK('E', 'P', 'I', 59264, BIT(EDID_QUIRK_135_CLOCK_TOO_HIGH)),
 	/* Envision EN2028 */
-	EDID_QUIRK('E', 'P', 'I', 8232, EDID_QUIRK_PREFER_LARGE_60),
+	EDID_QUIRK('E', 'P', 'I', 8232, BIT(EDID_QUIRK_PREFER_LARGE_60)),
 
 	/* Funai Electronics PM36B */
-	EDID_QUIRK('F', 'C', 'M', 13600, EDID_QUIRK_PREFER_LARGE_75 |
-				       EDID_QUIRK_DETAILED_IN_CM),
+	EDID_QUIRK('F', 'C', 'M', 13600, BIT(EDID_QUIRK_PREFER_LARGE_75) |
+					 BIT(EDID_QUIRK_DETAILED_IN_CM)),
 
 	/* LG 27GP950 */
-	EDID_QUIRK('G', 'S', 'M', 0x5bbf, EDID_QUIRK_CAP_DSC_15BPP),
+	EDID_QUIRK('G', 'S', 'M', 0x5bbf, BIT(EDID_QUIRK_CAP_DSC_15BPP)),
 
 	/* LG 27GN950 */
-	EDID_QUIRK('G', 'S', 'M', 0x5b9a, EDID_QUIRK_CAP_DSC_15BPP),
+	EDID_QUIRK('G', 'S', 'M', 0x5b9a, BIT(EDID_QUIRK_CAP_DSC_15BPP)),
 
 	/* LGD panel of HP zBook 17 G2, eDP 10 bpc, but reports unknown bpc */
-	EDID_QUIRK('L', 'G', 'D', 764, EDID_QUIRK_FORCE_10BPC),
+	EDID_QUIRK('L', 'G', 'D', 764, BIT(EDID_QUIRK_FORCE_10BPC)),
 
 	/* LG Philips LCD LP154W01-A5 */
-	EDID_QUIRK('L', 'P', 'L', 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE),
-	EDID_QUIRK('L', 'P', 'L', 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE),
+	EDID_QUIRK('L', 'P', 'L', 0, BIT(EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE)),
+	EDID_QUIRK('L', 'P', 'L', 0x2a00, BIT(EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE)),
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
-	EDID_QUIRK('S', 'A', 'M', 541, EDID_QUIRK_DETAILED_SYNC_PP),
+	EDID_QUIRK('S', 'A', 'M', 541, BIT(EDID_QUIRK_DETAILED_SYNC_PP)),
 	/* Samsung SyncMaster 22[5-6]BW */
-	EDID_QUIRK('S', 'A', 'M', 596, EDID_QUIRK_PREFER_LARGE_60),
-	EDID_QUIRK('S', 'A', 'M', 638, EDID_QUIRK_PREFER_LARGE_60),
+	EDID_QUIRK('S', 'A', 'M', 596, BIT(EDID_QUIRK_PREFER_LARGE_60)),
+	EDID_QUIRK('S', 'A', 'M', 638, BIT(EDID_QUIRK_PREFER_LARGE_60)),
 
 	/* Sony PVM-2541A does up to 12 bpc, but only reports max 8 bpc */
-	EDID_QUIRK('S', 'N', 'Y', 0x2541, EDID_QUIRK_FORCE_12BPC),
+	EDID_QUIRK('S', 'N', 'Y', 0x2541, BIT(EDID_QUIRK_FORCE_12BPC)),
 
 	/* ViewSonic VA2026w */
-	EDID_QUIRK('V', 'S', 'C', 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING),
+	EDID_QUIRK('V', 'S', 'C', 5020, BIT(EDID_QUIRK_FORCE_REDUCED_BLANKING)),
 
 	/* Medion MD 30217 PG */
-	EDID_QUIRK('M', 'E', 'D', 0x7b8, EDID_QUIRK_PREFER_LARGE_75),
+	EDID_QUIRK('M', 'E', 'D', 0x7b8, BIT(EDID_QUIRK_PREFER_LARGE_75)),
 
 	/* Lenovo G50 */
-	EDID_QUIRK('S', 'D', 'C', 18514, EDID_QUIRK_FORCE_6BPC),
+	EDID_QUIRK('S', 'D', 'C', 18514, BIT(EDID_QUIRK_FORCE_6BPC)),
 
 	/* Panel in Samsung NP700G7A-S01PL notebook reports 6bpc */
-	EDID_QUIRK('S', 'E', 'C', 0xd033, EDID_QUIRK_FORCE_8BPC),
+	EDID_QUIRK('S', 'E', 'C', 0xd033, BIT(EDID_QUIRK_FORCE_8BPC)),
 
 	/* Rotel RSX-1058 forwards sink's EDID but only does HDMI 1.1*/
-	EDID_QUIRK('E', 'T', 'R', 13896, EDID_QUIRK_FORCE_8BPC),
+	EDID_QUIRK('E', 'T', 'R', 13896, BIT(EDID_QUIRK_FORCE_8BPC)),
 
 	/* Valve Index Headset */
-	EDID_QUIRK('V', 'L', 'V', 0x91a8, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b0, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b1, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b2, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b3, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b4, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b5, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b6, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b7, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b8, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91b9, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91ba, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91bb, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91bc, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91bd, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91be, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('V', 'L', 'V', 0x91bf, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('V', 'L', 'V', 0x91a8, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b0, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b1, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b2, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b3, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b4, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b5, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b6, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b7, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b8, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91b9, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91ba, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91bb, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91bc, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91bd, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91be, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('V', 'L', 'V', 0x91bf, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* HTC Vive and Vive Pro VR Headsets */
-	EDID_QUIRK('H', 'V', 'R', 0xaa01, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('H', 'V', 'R', 0xaa02, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('H', 'V', 'R', 0xaa01, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('H', 'V', 'R', 0xaa02, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* Oculus Rift DK1, DK2, CV1 and Rift S VR Headsets */
-	EDID_QUIRK('O', 'V', 'R', 0x0001, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('O', 'V', 'R', 0x0003, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('O', 'V', 'R', 0x0004, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('O', 'V', 'R', 0x0012, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('O', 'V', 'R', 0x0001, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('O', 'V', 'R', 0x0003, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('O', 'V', 'R', 0x0004, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('O', 'V', 'R', 0x0012, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* Windows Mixed Reality Headsets */
-	EDID_QUIRK('A', 'C', 'R', 0x7fce, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('L', 'E', 'N', 0x0408, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('F', 'U', 'J', 0x1970, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('D', 'E', 'L', 0x7fce, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('S', 'E', 'C', 0x144a, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('A', 'U', 'S', 0xc102, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('A', 'C', 'R', 0x7fce, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('L', 'E', 'N', 0x0408, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('F', 'U', 'J', 0x1970, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('D', 'E', 'L', 0x7fce, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('S', 'E', 'C', 0x144a, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('A', 'U', 'S', 0xc102, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* Sony PlayStation VR Headset */
-	EDID_QUIRK('S', 'N', 'Y', 0x0704, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('S', 'N', 'Y', 0x0704, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* Sensics VR Headsets */
-	EDID_QUIRK('S', 'E', 'N', 0x1019, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('S', 'E', 'N', 0x1019, BIT(EDID_QUIRK_NON_DESKTOP)),
 
 	/* OSVR HDK and HDK2 VR Headsets */
-	EDID_QUIRK('S', 'V', 'R', 0x1019, EDID_QUIRK_NON_DESKTOP),
-	EDID_QUIRK('A', 'U', 'O', 0x1111, EDID_QUIRK_NON_DESKTOP),
+	EDID_QUIRK('S', 'V', 'R', 0x1019, BIT(EDID_QUIRK_NON_DESKTOP)),
+	EDID_QUIRK('A', 'U', 'O', 0x1111, BIT(EDID_QUIRK_NON_DESKTOP)),
 };
 
 /*

@@ -2951,6 +2953,12 @@ static u32 edid_get_quirks(const struct drm_edid *drm_edid)
 	return 0;
 }
 
+static bool drm_edid_has_internal_quirk(struct drm_connector *connector,
+					enum drm_edid_internal_quirk quirk)
+{
+	return connector->display_info.quirks & BIT(quirk);
+}
+
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
 #define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
 

@@ -2960,7 +2968,6 @@ static u32 edid_get_quirks(const struct drm_edid *drm_edid)
  */
 static void edid_fixup_preferred(struct drm_connector *connector)
 {
-	const struct drm_display_info *info = &connector->display_info;
 	struct drm_display_mode *t, *cur_mode, *preferred_mode;
 	int target_refresh = 0;
 	int cur_vrefresh, preferred_vrefresh;

@@ -2968,9 +2975,9 @@ static void edid_fixup_preferred(struct drm_connector *connector)
 	if (list_empty(&connector->probed_modes))
 		return;
 
-	if (info->quirks & EDID_QUIRK_PREFER_LARGE_60)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_PREFER_LARGE_60))
 		target_refresh = 60;
-	if (info->quirks & EDID_QUIRK_PREFER_LARGE_75)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_PREFER_LARGE_75))
 		target_refresh = 75;
 
 	preferred_mode = list_first_entry(&connector->probed_modes,

@@ -3474,7 +3481,6 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_connector *connecto
 						  const struct drm_edid *drm_edid,
 						  const struct detailed_timing *timing)
 {
-	const struct drm_display_info *info = &connector->display_info;
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *mode;
 	const struct detailed_pixel_timing *pt = &timing->data.pixel_data;

@@ -3508,7 +3514,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_connector *connecto
 		return NULL;
 	}
 
-	if (info->quirks & EDID_QUIRK_FORCE_REDUCED_BLANKING) {
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_FORCE_REDUCED_BLANKING)) {
 		mode = drm_cvt_mode(dev, hactive, vactive, 60, true, false, false);
 		if (!mode)
 			return NULL;

@@ -3520,7 +3526,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_connector *connecto
 	if (!mode)
 		return NULL;
 
-	if (info->quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_135_CLOCK_TOO_HIGH))
 		mode->clock = 1088 * 10;
 	else
 		mode->clock = le16_to_cpu(timing->pixel_clock) * 10;

@@ -3551,7 +3557,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_connector *connecto
 
 	drm_mode_do_interlace_quirk(mode, pt);
 
-	if (info->quirks & EDID_QUIRK_DETAILED_SYNC_PP) {
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_DETAILED_SYNC_PP)) {
 		mode->flags |= DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC;
 	} else {
 		mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?

@@ -3564,12 +3570,12 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_connector *connecto
 	mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0) << 4;
 	mode->height_mm = pt->height_mm_lo | (pt->width_height_mm_hi & 0xf) << 8;
 
-	if (info->quirks & EDID_QUIRK_DETAILED_IN_CM) {
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_DETAILED_IN_CM)) {
 		mode->width_mm *= 10;
 		mode->height_mm *= 10;
 	}
 
-	if (info->quirks & EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE) {
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE)) {
 		mode->width_mm = drm_edid->edid->width_cm * 10;
 		mode->height_mm = drm_edid->edid->height_cm * 10;
 	}

@@ -6734,26 +6740,26 @@ static void update_display_info(struct drm_connector *connector,
 	drm_update_mso(connector, drm_edid);
 
 out:
-	if (info->quirks & EDID_QUIRK_NON_DESKTOP) {
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_NON_DESKTOP)) {
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Non-desktop display%s\n",
 			    connector->base.id, connector->name,
 			    info->non_desktop ? " (redundant quirk)" : "");
 		info->non_desktop = true;
 	}
 
-	if (info->quirks & EDID_QUIRK_CAP_DSC_15BPP)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_CAP_DSC_15BPP))
 		info->max_dsc_bpp = 15;
 
-	if (info->quirks & EDID_QUIRK_FORCE_6BPC)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_FORCE_6BPC))
 		info->bpc = 6;
 
-	if (info->quirks & EDID_QUIRK_FORCE_8BPC)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_FORCE_8BPC))
 		info->bpc = 8;
 
-	if (info->quirks & EDID_QUIRK_FORCE_10BPC)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_FORCE_10BPC))
 		info->bpc = 10;
 
-	if (info->quirks & EDID_QUIRK_FORCE_12BPC)
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_FORCE_12BPC))
 		info->bpc = 12;
 
 	/* Depends on info->cea_rev set by drm_parse_cea_ext() above */

@@ -6918,7 +6924,6 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
 static int _drm_edid_connector_add_modes(struct drm_connector *connector,
 					 const struct drm_edid *drm_edid)
 {
-	const struct drm_display_info *info = &connector->display_info;
 	int num_modes = 0;
 
 	if (!drm_edid)

@@ -6948,7 +6953,8 @@ static int _drm_edid_connector_add_modes(struct drm_connector *connector,
 	if (drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ)
 		num_modes += add_inferred_modes(connector, drm_edid);
 
-	if (info->quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
+	if (drm_edid_has_internal_quirk(connector, EDID_QUIRK_PREFER_LARGE_60) ||
+	    drm_edid_has_internal_quirk(connector, EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector);
 
 	return num_modes;

+static bool drm_edid_has_internal_quirk(struct drm_connector *connector,

drivers/gpu/drm/drm_edid.c +5 -0

+					enum drm_edid_internal_quirk quirk)
+{
+	return connector->display_info.quirks & BIT(quirk);
+}
+

Add support for EDID based quirks which can be queried outside of the EDID parser iteself by DRM core and drivers. There are at least two such quirks applicable to all drivers: the DPCD register access probe quirk and the 128b/132b DPRX Lane Count Conversion quirk (see 3.5.2.16.3 in the v2.1a DP Standard). The latter quirk applies to panels with specific EDID panel names, support for defining a quirk this way will be added as a follow-up.

v2: Reset global_quirks in drm_reset_display_info().
v3: (Jani)
- Use one list for both the global and internal quirks.
- Drop change for panel name specific quirks.
- Add comment about the way quirks should be queried.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/drm_edid.c  | 8 +++++++-
 include/drm/drm_connector.h | 4 +++-
 include/drm/drm_edid.h      | 5 +++++
 3 files changed, 15 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_edid.c +1 -1

index 857ae0c47a1c3..9cca1e6e4736c 100644
@@ -68,7 +68,7 @@ static int oui(u8 first, u8 second, u8 third)
 
 enum drm_edid_internal_quirk {
 	/* First detailed mode wrong, use largest 60Hz mode */
-	EDID_QUIRK_PREFER_LARGE_60,
+	EDID_QUIRK_PREFER_LARGE_60 = DRM_EDID_QUIRK_NUM,
 	/* Reported 135MHz pixel clock is too high, needs adjustment */
 	EDID_QUIRK_135_CLOCK_TOO_HIGH,
 	/* Prefer the largest mode at 75 Hz */

drivers/gpu/drm/drm_edid.c +5 -0

+{
+	return connector->display_info.quirks & BIT(quirk);
+}
+EXPORT_SYMBOL(drm_edid_has_quirk);
+
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
 #define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
 

include/drm/drm_connector.h +3 -1

index 73903c3c842f3..137773dd138ea 100644
@@ -843,7 +843,9 @@ struct drm_display_info {
 	int vics_len;
 
 	/**
-	 * @quirks: EDID based quirks. Internal to EDID parsing.
+	 * @quirks: EDID based quirks. DRM core and drivers can query the
+	 * @drm_edid_quirk quirks using drm_edid_has_quirk(), the rest of
+	 * the quirks also tracked here are internal to EDID parsing.
 	 */
 	u32 quirks;
 

include/drm/drm_edid.h +5 -0

index b38409670868d..77fd42608e706 100644
@@ -109,6 +109,10 @@ struct detailed_data_string {
 #define DRM_EDID_CVT_FLAGS_STANDARD_BLANKING (1 << 3)
 #define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING  (1 << 4)
 
+enum drm_edid_quirk {
+	DRM_EDID_QUIRK_NUM,
+};
+
 struct detailed_data_monitor_range {
 	u8 min_vfreq;
 	u8 max_vfreq;

@@ -476,5 +480,6 @@ void drm_edid_print_product_id(struct drm_printer *p,
 u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
 bool drm_edid_match(const struct drm_edid *drm_edid,
 		    const struct drm_edid_ident *ident);
+bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk);
 
 #endif /* __DRM_EDID_H__ */

Quoted 40 lines · Imre Deak

Add support for EDID based quirks which can be queried outside of the EDID parser iteself by DRM core and drivers. There are at least two such quirks applicable to all drivers: the DPCD register access probe quirk and the 128b/132b DPRX Lane Count Conversion quirk (see 3.5.2.16.3 in the v2.1a DP Standard). The latter quirk applies to panels with specific EDID panel names, support for defining a quirk this way will be added as a follow-up.

v2: Reset global_quirks in drm_reset_display_info().
v3: (Jani)
- Use one list for both the global and internal quirks.
- Drop change for panel name specific quirks.
- Add comment about the way quirks should be queried.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/drm_edid.c  | 8 +++++++-
 include/drm/drm_connector.h | 4 +++-
 include/drm/drm_edid.h      | 5 +++++
 3 files changed, 15 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_edid.c +1 -1

index 857ae0c47a1c3..9cca1e6e4736c 100644
@@ -68,7 +68,7 @@ static int oui(u8 first, u8 second, u8 third)
 
 enum drm_edid_internal_quirk {
 	/* First detailed mode wrong, use largest 60Hz mode */
-	EDID_QUIRK_PREFER_LARGE_60,
+	EDID_QUIRK_PREFER_LARGE_60 = DRM_EDID_QUIRK_NUM,
 	/* Reported 135MHz pixel clock is too high, needs adjustment */
 	EDID_QUIRK_135_CLOCK_TOO_HIGH,
 	/* Prefer the largest mode at 75 Hz */

@@ -2959,6 +2959,12 @@ static bool drm_edid_has_internal_quirk(struct drm_connector *connector,
 	return connector->display_info.quirks & BIT(quirk);
 }
 

drivers/gpu/drm/drm_edid.c static bool drm_edid_has_internal_quirk(struct drm_connector *connector, · line 2962

Quoted 1 line

drivers/gpu/drm/drm_edid.c +1 -0

+bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk)

Nitpick, this could've been const.

Regardless,

Reviewed-by Jani Nikula <[email protected]>

Quoted 45 lines

drivers/gpu/drm/drm_edid.c +5 -0

+{
+	return connector->display_info.quirks & BIT(quirk);
+}
+EXPORT_SYMBOL(drm_edid_has_quirk);
+
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
 #define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
 

include/drm/drm_connector.h +3 -1

index 73903c3c842f3..137773dd138ea 100644
@@ -843,7 +843,9 @@ struct drm_display_info {
 	int vics_len;
 
 	/**
-	 * @quirks: EDID based quirks. Internal to EDID parsing.
+	 * @quirks: EDID based quirks. DRM core and drivers can query the
+	 * @drm_edid_quirk quirks using drm_edid_has_quirk(), the rest of
+	 * the quirks also tracked here are internal to EDID parsing.
 	 */
 	u32 quirks;
 

include/drm/drm_edid.h +5 -0

index b38409670868d..77fd42608e706 100644
@@ -109,6 +109,10 @@ struct detailed_data_string {
 #define DRM_EDID_CVT_FLAGS_STANDARD_BLANKING (1 << 3)
 #define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING  (1 << 4)
 
+enum drm_edid_quirk {
+	DRM_EDID_QUIRK_NUM,
+};
+
 struct detailed_data_monitor_range {
 	u8 min_vfreq;
 	u8 max_vfreq;

@@ -476,5 +480,6 @@ void drm_edid_print_product_id(struct drm_printer *p,
 u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
 bool drm_edid_match(const struct drm_edid *drm_edid,
 		    const struct drm_edid_ident *ident);
+bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk);
 
 #endif /* __DRM_EDID_H__ */

Signature · 1 line

Jani Nikula, Intel

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. Add an EDID quirk for this. Leave the quirk enabled by default, allowing it to be disabled after the monitor is detected.

v2: Fix lockdep wrt. drm_dp_aux::hw_mutex when calling
drm_dp_dpcd_set_probe_quirk() with a dependent lock already held.
v3: Add a helper for determining if DPCD probing is needed. (Jani)

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/display/drm_dp_helper.c | 42 +++++++++++++++++--------
 drivers/gpu/drm/drm_edid.c              |  3 ++
 include/drm/display/drm_dp_helper.h     |  6 ++++
 include/drm/drm_edid.h                  |  3 ++
 4 files changed, 41 insertions(+), 13 deletions(-)

drivers/gpu/drm/display/drm_dp_helper.c +0 -0

index dc622c78db9d4..d0b9f672d743c 100644

drivers/gpu/drm/display/drm_dp_helper.c +27 -13

+ * @aux: DisplayPort AUX channel
+ * @enable: Enable the probing if required
+ */
+void drm_dp_dpcd_set_probe_quirk(struct drm_dp_aux *aux, bool enable)
+{
+	WRITE_ONCE(aux->dpcd_probe_disabled, !enable);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_set_probe_quirk);
+
+static bool dpcd_access_needs_probe(struct drm_dp_aux *aux)
+{
+	/*
+	 * HP ZR24w corrupts the first DPCD access after entering power save
+	 * mode. Eg. on a read, the entire buffer will be filled with the same
+	 * byte. Do a throw away read to avoid corrupting anything we care
+	 * about. Afterwards things will work correctly until the monitor
+	 * gets woken up and subsequently re-enters power save mode.
+	 *
+	 * The user pressing any button on the monitor is enough to wake it
+	 * up, so there is no particularly good place to do the workaround.
+	 * We just have to do it before any DPCD access and hope that the
+	 * monitor doesn't power down exactly after the throw away read.
+	 */
+	return !aux->is_remote && !READ_ONCE(aux->dpcd_probe_disabled);
+}
+
 /**
  * drm_dp_dpcd_read() - read a series of bytes from the DPCD
  * @aux: DisplayPort AUX channel (SST or MST)

@@ -712,19 +740,7 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 {
 	int ret;
 
-	/*
-	 * HP ZR24w corrupts the first DPCD access after entering power save
-	 * mode. Eg. on a read, the entire buffer will be filled with the same
-	 * byte. Do a throw away read to avoid corrupting anything we care
-	 * about. Afterwards things will work correctly until the monitor
-	 * gets woken up and subsequently re-enters power save mode.
-	 *
-	 * The user pressing any button on the monitor is enough to wake it
-	 * up, so there is no particularly good place to do the workaround.
-	 * We just have to do it before any DPCD access and hope that the
-	 * monitor doesn't power down exactly after the throw away read.
-	 */
-	if (!aux->is_remote) {
+	if (dpcd_access_needs_probe(aux)) {
 		ret = drm_dp_dpcd_probe(aux, DP_LANE0_1_STATUS);
 		if (ret < 0)
 			return ret;

drivers/gpu/drm/drm_edid.c +3 -0

index 9cca1e6e4736c..5f45820ad62d5 100644
@@ -248,6 +248,9 @@ static const struct edid_quirk {
 	/* OSVR HDK and HDK2 VR Headsets */
 	EDID_QUIRK('S', 'V', 'R', 0x1019, BIT(EDID_QUIRK_NON_DESKTOP)),
 	EDID_QUIRK('A', 'U', 'O', 0x1111, BIT(EDID_QUIRK_NON_DESKTOP)),
+
+	/* HP ZR24w DP AUX DPCD access requires probing to prevent corruption. */
+	EDID_QUIRK('H', 'W', 'P', 0x2869, BIT(DRM_EDID_QUIRK_DP_DPCD_PROBE)),
 };
 
 /*

include/drm/display/drm_dp_helper.h +6 -0

index e4ca35143ff96..75fa9548aefa0 100644
@@ -523,10 +523,16 @@ struct drm_dp_aux {
 	 * @no_zero_sized: If the hw can't use zero sized transfers (NVIDIA)
 	 */
 	bool no_zero_sized;
+
+	/**
+	 * @dpcd_probe_disabled: If probing before a DPCD access is disabled.
+	 */
+	bool dpcd_probe_disabled;
 };
 
 int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset);
 void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered);
+void drm_dp_dpcd_set_probe_quirk(struct drm_dp_aux *aux, bool enable);
 ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 			 void *buffer, size_t size);
 ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,

include/drm/drm_edid.h +3 -0

index 77fd42608e706..3d1aecfec9b2a 100644
@@ -110,6 +110,9 @@ struct detailed_data_string {
 #define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING  (1 << 4)
 
 enum drm_edid_quirk {
+	/* Do a dummy read before DPCD accesses, to prevent corruption. */
+	DRM_EDID_QUIRK_DP_DPCD_PROBE,
+
 	DRM_EDID_QUIRK_NUM,
 };

Quoted 29 lines · Imre Deak

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. Add an EDID quirk for this. Leave the quirk enabled by default, allowing it to be disabled after the monitor is detected.

v2: Fix lockdep wrt. drm_dp_aux::hw_mutex when calling
drm_dp_dpcd_set_probe_quirk() with a dependent lock already held.
v3: Add a helper for determining if DPCD probing is needed. (Jani)

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/display/drm_dp_helper.c | 42 +++++++++++++++++--------
 drivers/gpu/drm/drm_edid.c              |  3 ++
 include/drm/display/drm_dp_helper.h     |  6 ++++
 include/drm/drm_edid.h                  |  3 ++
 4 files changed, 41 insertions(+), 13 deletions(-)

drivers/gpu/drm/display/drm_dp_helper.c +0 -0

index dc622c78db9d4..d0b9f672d743c 100644
@@ -691,6 +691,34 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
 }
 EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
 

drivers/gpu/drm/display/drm_dp_helper.c void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered) · line 695

Quoted 2 lines

drivers/gpu/drm/display/drm_dp_helper.c +2 -0

+/**
+ * drm_dp_dpcd_set_dpcd_probe_quirk() - Set whether a probing before DPCD access is done

Musing, not sure if this needs to be called "quirk". This is just used to enable/disable the extra probe. aux->dpcd_probe_disabled doesn't mention the quirk either, and shouldn't.

Not a big deal.

Reviewed-by Jani Nikula <[email protected]>

Quoted 98 lines

drivers/gpu/drm/display/drm_dp_helper.c +27 -13

+ * @aux: DisplayPort AUX channel
+ * @enable: Enable the probing if required
+ */
+void drm_dp_dpcd_set_probe_quirk(struct drm_dp_aux *aux, bool enable)
+{
+	WRITE_ONCE(aux->dpcd_probe_disabled, !enable);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_set_probe_quirk);
+
+static bool dpcd_access_needs_probe(struct drm_dp_aux *aux)
+{
+	/*
+	 * HP ZR24w corrupts the first DPCD access after entering power save
+	 * mode. Eg. on a read, the entire buffer will be filled with the same
+	 * byte. Do a throw away read to avoid corrupting anything we care
+	 * about. Afterwards things will work correctly until the monitor
+	 * gets woken up and subsequently re-enters power save mode.
+	 *
+	 * The user pressing any button on the monitor is enough to wake it
+	 * up, so there is no particularly good place to do the workaround.
+	 * We just have to do it before any DPCD access and hope that the
+	 * monitor doesn't power down exactly after the throw away read.
+	 */
+	return !aux->is_remote && !READ_ONCE(aux->dpcd_probe_disabled);
+}
+
 /**
  * drm_dp_dpcd_read() - read a series of bytes from the DPCD
  * @aux: DisplayPort AUX channel (SST or MST)

@@ -712,19 +740,7 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 {
 	int ret;
 
-	/*
-	 * HP ZR24w corrupts the first DPCD access after entering power save
-	 * mode. Eg. on a read, the entire buffer will be filled with the same
-	 * byte. Do a throw away read to avoid corrupting anything we care
-	 * about. Afterwards things will work correctly until the monitor
-	 * gets woken up and subsequently re-enters power save mode.
-	 *
-	 * The user pressing any button on the monitor is enough to wake it
-	 * up, so there is no particularly good place to do the workaround.
-	 * We just have to do it before any DPCD access and hope that the
-	 * monitor doesn't power down exactly after the throw away read.
-	 */
-	if (!aux->is_remote) {
+	if (dpcd_access_needs_probe(aux)) {
 		ret = drm_dp_dpcd_probe(aux, DP_LANE0_1_STATUS);
 		if (ret < 0)
 			return ret;

drivers/gpu/drm/drm_edid.c +3 -0

index 9cca1e6e4736c..5f45820ad62d5 100644
@@ -248,6 +248,9 @@ static const struct edid_quirk {
 	/* OSVR HDK and HDK2 VR Headsets */
 	EDID_QUIRK('S', 'V', 'R', 0x1019, BIT(EDID_QUIRK_NON_DESKTOP)),
 	EDID_QUIRK('A', 'U', 'O', 0x1111, BIT(EDID_QUIRK_NON_DESKTOP)),
+
+	/* HP ZR24w DP AUX DPCD access requires probing to prevent corruption. */
+	EDID_QUIRK('H', 'W', 'P', 0x2869, BIT(DRM_EDID_QUIRK_DP_DPCD_PROBE)),
 };
 
 /*

include/drm/display/drm_dp_helper.h +6 -0

index e4ca35143ff96..75fa9548aefa0 100644
@@ -523,10 +523,16 @@ struct drm_dp_aux {
 	 * @no_zero_sized: If the hw can't use zero sized transfers (NVIDIA)
 	 */
 	bool no_zero_sized;
+
+	/**
+	 * @dpcd_probe_disabled: If probing before a DPCD access is disabled.
+	 */
+	bool dpcd_probe_disabled;
 };
 
 int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset);
 void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered);
+void drm_dp_dpcd_set_probe_quirk(struct drm_dp_aux *aux, bool enable);
 ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 			 void *buffer, size_t size);
 ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,

include/drm/drm_edid.h +3 -0

index 77fd42608e706..3d1aecfec9b2a 100644
@@ -110,6 +110,9 @@ struct detailed_data_string {
 #define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING  (1 << 4)
 
 enum drm_edid_quirk {
+	/* Do a dummy read before DPCD accesses, to prevent corruption. */
+	DRM_EDID_QUIRK_DP_DPCD_PROBE,
+
 	DRM_EDID_QUIRK_NUM,
 };

Signature · 1 line

Jani Nikula, Intel

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +9 -2

index 208a953b04a2f..d65a18fc1aeb9 100644
@@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	/* Below we depend on display info having been updated */
 	drm_edid_connector_update(&connector->base, drm_edid);
 
+	if (!intel_dp_is_edp(intel_dp))
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux,
+					    drm_edid_has_quirk(&connector->base,
+							       DRM_EDID_QUIRK_DP_DPCD_PROBE));
+
 	vrr_capable = intel_vrr_is_capable(connector);
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
 		    connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));

@@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector,
 	intel_dp_print_rates(intel_dp);
 
 	if (intel_dp->is_mst) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false);
 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index 208a953b04a2f..d65a18fc1aeb9 100644

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Quoted 26 lines · Imre Deak

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index 208a953b04a2f..d65a18fc1aeb9 100644
@@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	/* Below we depend on display info having been updated */
 	drm_edid_connector_update(&connector->base, drm_edid);
 

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_set_edid(struct intel_dp *intel_dp) · line 5750

Quoted 1 line

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

+	if (!intel_dp_is_edp(intel_dp))

Why does this depend on !edp?

Feels like unnecessary optimization based on your knowledge of that one specific display.

Quoted 10 lines

drivers/gpu/drm/i915/display/intel_dp.c +4 -0

+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux,
+					    drm_edid_has_quirk(&connector->base,
+							       DRM_EDID_QUIRK_DP_DPCD_PROBE));
+
 	vrr_capable = intel_vrr_is_capable(connector);
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
 		    connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));

@@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector,
 	intel_dp_print_rates(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_detect(struct drm_connector *_connector, · line 5889

Quoted 2 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

 	if (intel_dp->is_mst) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false);

Isn't this excessive? Haven't we already set the quirk state?

Quoted 62 lines

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Signature · 1 line

Jani Nikula, Intel

Quoted 31 lines · Jani Nikula

Quoted 23 lines · Imre Deak

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index 208a953b04a2f..d65a18fc1aeb9 100644

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_set_edid(struct intel_dp *intel_dp) · line 5750

Quoted 4 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

@@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	/* Below we depend on display info having been updated */
 	drm_edid_connector_update(&connector->base, drm_edid);
 
+	if (!intel_dp_is_edp(intel_dp))

Why does this depend on !edp?

Quoted 2 lines

Feels like unnecessary optimization based on your knowledge of that one specific display.

The detection itself requires probing before each DPCD access. I want to avoid it whenever possible and since the quirk is relevant only the particular HP external display, we can avoid the probing on eDP completely.

Quoted 10 lines

Quoted 10 lines

drivers/gpu/drm/i915/display/intel_dp.c +4 -0

+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux,
+					    drm_edid_has_quirk(&connector->base,
+							       DRM_EDID_QUIRK_DP_DPCD_PROBE));
+
 	vrr_capable = intel_vrr_is_capable(connector);
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
 		    connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));

@@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector,
 	intel_dp_print_rates(intel_dp);
 

Quoted 4 lines

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_detect(struct drm_connector *_connector, · line 5889

Quoted 2 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

 	if (intel_dp->is_mst) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false);

Isn't this excessive? Haven't we already set the quirk state?

No, this is the MST root connector's detection and we don't read the EDID for it (we read it for MST non-root connectors, but those are not relavant in any case). So this should be set here explicitly, with the same justification as above for eDP (on MST the probing is never needed, so we can avoid it on such links completely).

Quoted 65 lines

Quoted 62 lines

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Signature · 1 line

Jani Nikula, Intel

Quoted 35 lines · Imre Deak

Quoted 33 lines · Jani Nikula

Quoted 23 lines · Imre Deak

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index 208a953b04a2f..d65a18fc1aeb9 100644

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_set_edid(struct intel_dp *intel_dp) · line 5750

Quoted 4 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

@@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	/* Below we depend on display info having been updated */
 	drm_edid_connector_update(&connector->base, drm_edid);
 
+	if (!intel_dp_is_edp(intel_dp))

Why does this depend on !edp?

Feels like unnecessary optimization based on your knowledge of that one specific display.

Quoted 4 lines

The detection itself requires probing before each DPCD access. I want to avoid it whenever possible and since the quirk is relevant only the particular HP external display, we can avoid the probing on eDP completely.

Ok, the eDP check here can be removed, as the panel's EDID panel ID should not match the quirk's panel ID. Will remove it.

Quoted 87 lines

Quoted 11 lines

Quoted 11 lines

drivers/gpu/drm/i915/display/intel_dp.c +4 -0

+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux,
+					    drm_edid_has_quirk(&connector->base,
+							       DRM_EDID_QUIRK_DP_DPCD_PROBE));
+
 	vrr_capable = intel_vrr_is_capable(connector);
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
 		    connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));

@@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector,
 	intel_dp_print_rates(intel_dp);
 

Quoted 4 lines

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_detect(struct drm_connector *_connector, · line 5889

Quoted 2 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

 	if (intel_dp->is_mst) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false);

Isn't this excessive? Haven't we already set the quirk state?

No, this is the MST root connector's detection and we don't read the EDID for it (we read it for MST non-root connectors, but those are not relavant in any case). So this should be set here explicitly, with the same justification as above for eDP (on MST the probing is never needed, so we can avoid it on such links completely).

Quoted 65 lines

Quoted 62 lines

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Signature · 1 line

Jani Nikula, Intel

Quoted 41 lines · Imre Deak

Quoted 39 lines · Imre Deak

Quoted 27 lines · Jani Nikula

Quoted 26 lines · Imre Deak

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 11 +++++++++--
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index 208a953b04a2f..d65a18fc1aeb9 100644
@@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	/* Below we depend on display info having been updated */
 	drm_edid_connector_update(&connector->base, drm_edid);
 

Quoted 6 lines

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_set_edid(struct intel_dp *intel_dp) · line 5750

Quoted 1 line

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

+	if (!intel_dp_is_edp(intel_dp))

Why does this depend on !edp?

Feels like unnecessary optimization based on your knowledge of that one specific display.

The detection itself requires probing before each DPCD access. I want to avoid it whenever possible and since the quirk is relevant only the particular HP external display, we can avoid the probing on eDP completely.

Quoted 2 lines

Ok, the eDP check here can be removed, as the panel's EDID panel ID should not match the quirk's panel ID. Will remove it.

I'm wondering if we should add a local "do the quirk" helper that checks for eDP and mst and the actual quirk. Not sure if it would make this more readable.

BR,
Jani.

Quoted 87 lines

Quoted 87 lines

Quoted 11 lines

Quoted 11 lines

drivers/gpu/drm/i915/display/intel_dp.c +4 -0

+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux,
+					    drm_edid_has_quirk(&connector->base,
+							       DRM_EDID_QUIRK_DP_DPCD_PROBE));
+
 	vrr_capable = intel_vrr_is_capable(connector);
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
 		    connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));

@@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector,
 	intel_dp_print_rates(intel_dp);
 

Quoted 4 lines

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_detect(struct drm_connector *_connector, · line 5889

Quoted 2 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0

 	if (intel_dp->is_mst) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false);

Isn't this excessive? Haven't we already set the quirk state?

No, this is the MST root connector's detection and we don't read the EDID for it (we read it for MST non-root connectors, but those are not relavant in any case). So this should be set here explicitly, with the same justification as above for eDP (on MST the probing is never needed, so we can avoid it on such links completely).

Quoted 65 lines

Quoted 62 lines

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 		/*
 		 * If we are in MST mode then this connector
 		 * won't appear connected or have anything

@@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);
 

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..410252ba6fd16 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp));
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder)

drivers/gpu/drm/i915/display/intel_hotplug.c +10 -0

index 74fe398663d63..1093c6c3714c0 100644
@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)
 {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+		if (!intel_dp_is_edp(intel_dp))
+			drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true);
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);

Signature · 1 line

Jani Nikula, Intel

Signature · 1 line

Jani Nikula, Intel

-----Original Message-----
From: Intel-gfx <[email protected]> On Behalf Of Imre Deak
Sent: Monday, 9 June 2025 15.56
To: [email protected]; [email protected]; [email protected]
Cc: Ville Syrjälä <[email protected]>; Jani Nikula <[email protected]>
Subject: [PATCH v4 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

v2: Add a helper for determining the need/setting the probing. (Jani)

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 31 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp.h      |  1 +
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++  drivers/gpu/drm/i915/display/intel_hotplug.c |  6 ++++
 4 files changed, 38 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +25 -0

index 208a953b04a2f..c089036a745fd 100644
@@ -5791,6 +5791,28 @@ intel_dp_detect_sdp_caps(struct intel_dp *intel_dp)
 		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);  }
 
+static bool intel_dp_needs_dpcd_probe(struct intel_dp *intel_dp, bool
+force_on_external) {
+	struct intel_connector *connector = intel_dp->attached_connector;
+
+	if (intel_dp_is_edp(intel_dp))
+		return false;
+
+	if (force_on_external)
+		return true;
+
+	if (intel_dp->is_mst)
+		return false;
+
+	return drm_edid_has_quirk(&connector->base,
+DRM_EDID_QUIRK_DP_DPCD_PROBE); }
+
+void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
+force_on_external) {
+	drm_dp_dpcd_set_probe(&intel_dp->aux,
+			      intel_dp_needs_dpcd_probe(intel_dp, force_on_external)); }
+
 static int
 intel_dp_detect(struct drm_connector *_connector,
 		struct drm_modeset_acquire_ctx *ctx,

@@ -5919,6 +5941,8 @@ intel_dp_detect(struct drm_connector *_connector,
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
+	intel_dp_dpcd_set_probe(intel_dp, false);
+
 	if (!intel_dp_is_edp(intel_dp))
 		drm_dp_set_subconnector_property(&connector->base,
 						 status,

@@ -5949,6 +5973,8 @@ intel_dp_force(struct drm_connector *_connector)
 		return;
 
 	intel_dp_set_edid(intel_dp);
+
+	intel_dp_dpcd_set_probe(intel_dp, false);
 }
 
 static int intel_dp_get_modes(struct drm_connector *_connector) @@ -6321,10 +6347,11 @@ intel_dp_hpd_pulse(struct

intel_digital_port *dig_port, bool long_hpd)

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		intel_dp_dpcd_set_probe(intel_dp, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);

drivers/gpu/drm/i915/display/intel_dp.h +2 -0

index eff3414c05dbf..0657f56811966 100644
@@ -213,5 +213,6 @@ int intel_dp_compute_min_hblank(struct intel_crtc_state *crtc_state,
 				const struct drm_connector_state *conn_state);
 
 int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
+void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
+force_on_external);
 
 #endif /* __INTEL_DP_H__ */

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..7bec964c0496f 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	intel_dp_dpcd_set_probe(intel_dp, true);
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder) diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c

b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 74fe398663d63..901fda434af12 100644

drivers/gpu/drm/i915/display/intel_hotplug.c +6 -1

@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,14 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)  {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder)
+		intel_dp_dpcd_set_probe(enc_to_intel_dp(encoder), true);
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);
--

2.44.2

Quoted 12 lines

-----Original Message-----
From: Intel-gfx <[email protected]> On Behalf Of Imre Deak
Sent: Monday, 9 June 2025 15.56
To: [email protected]; [email protected]; [email protected]
Cc: Ville Syrjälä <[email protected]>; Jani Nikula <[email protected]>
Subject: [PATCH v4 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

Reading DPCD registers has side-effects and some of these can cause a problem for instance during link training. Based on this it's better to avoid the probing quirk done before each DPCD register read, limiting this to the monitor which requires it. The only known problematic monitor is an external SST sink, so keep the quirk disabled always for eDP and MST sinks. Reenable the quirk after a hotplug event and after resuming from a power state without hotplug support, until the subsequent EDID based detection.

Quoted 4 lines

v2: Add a helper for determining the need/setting the probing. (Jani)

Cc Ville Syrjälä <[email protected]>

Cc Jani Nikula <[email protected]>

Jani has already reviewed most of the patch and now that this check for dpcd probe requirement is in place, the patch looks ok to me.

Reviewed-by Mika Kahola <[email protected]>

Quoted 124 lines

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/display/intel_dp.c      | 31 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp.h      |  1 +
 drivers/gpu/drm/i915/display/intel_dp_aux.c  |  2 ++  drivers/gpu/drm/i915/display/intel_hotplug.c |  6 ++++
 4 files changed, 38 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/display/intel_dp.c +25 -0

index 208a953b04a2f..c089036a745fd 100644
@@ -5791,6 +5791,28 @@ intel_dp_detect_sdp_caps(struct intel_dp *intel_dp)
 		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);  }
 
+static bool intel_dp_needs_dpcd_probe(struct intel_dp *intel_dp, bool
+force_on_external) {
+	struct intel_connector *connector = intel_dp->attached_connector;
+
+	if (intel_dp_is_edp(intel_dp))
+		return false;
+
+	if (force_on_external)
+		return true;
+
+	if (intel_dp->is_mst)
+		return false;
+
+	return drm_edid_has_quirk(&connector->base,
+DRM_EDID_QUIRK_DP_DPCD_PROBE); }
+
+void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
+force_on_external) {
+	drm_dp_dpcd_set_probe(&intel_dp->aux,
+			      intel_dp_needs_dpcd_probe(intel_dp, force_on_external)); }
+
 static int
 intel_dp_detect(struct drm_connector *_connector,
 		struct drm_modeset_acquire_ctx *ctx,

@@ -5919,6 +5941,8 @@ intel_dp_detect(struct drm_connector *_connector,
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
+	intel_dp_dpcd_set_probe(intel_dp, false);
+
 	if (!intel_dp_is_edp(intel_dp))
 		drm_dp_set_subconnector_property(&connector->base,
 						 status,

@@ -5949,6 +5973,8 @@ intel_dp_force(struct drm_connector *_connector)
 		return;
 
 	intel_dp_set_edid(intel_dp);
+
+	intel_dp_dpcd_set_probe(intel_dp, false);
 }
 
 static int intel_dp_get_modes(struct drm_connector *_connector) @@ -6321,10 +6347,11 @@ intel_dp_hpd_pulse(struct

intel_digital_port *dig_port, bool long_hpd)

drivers/gpu/drm/i915/display/intel_dp.c +3 -2

 	 * complete the DP tunnel BW request for the latter connector/encoder
 	 * waiting for this encoder's DPRX read, perform a dummy read here.
 	 */
-	if (long_hpd)
+	if (long_hpd) {
+		intel_dp_dpcd_set_probe(intel_dp, true);
+
 		intel_dp_read_dprx_caps(intel_dp, dpcd);
 
-	if (long_hpd) {
 		intel_dp->reset_link_params = true;
 		intel_dp_invalidate_source_oui(intel_dp);

drivers/gpu/drm/i915/display/intel_dp.h +2 -0

index eff3414c05dbf..0657f56811966 100644
@@ -213,5 +213,6 @@ int intel_dp_compute_min_hblank(struct intel_crtc_state *crtc_state,
 				const struct drm_connector_state *conn_state);
 
 int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
+void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
+force_on_external);
 
 #endif /* __INTEL_DP_H__ */

drivers/gpu/drm/i915/display/intel_dp_aux.c +2 -0

index bf8e8e0cc19c9..7bec964c0496f 100644
@@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp)
 
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 	cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	intel_dp_dpcd_set_probe(intel_dp, true);
 }
 
 static enum aux_ch default_aux_ch(struct intel_encoder *encoder) diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c

b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 74fe398663d63..901fda434af12 100644

drivers/gpu/drm/i915/display/intel_hotplug.c +6 -1

@@ -33,6 +33,7 @@
 #include "intel_display_core.h"
 #include "intel_display_rpm.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"

@@ -906,9 +907,14 @@ void intel_hpd_poll_enable(struct intel_display *display)
  */
 void intel_hpd_poll_disable(struct intel_display *display)  {
+	struct intel_encoder *encoder;
+
 	if (!HAS_DISPLAY(display))
 		return;
 
+	for_each_intel_dp(display->drm, encoder)
+		intel_dp_dpcd_set_probe(enc_to_intel_dp(encoder), true);
+
 	WRITE_ONCE(display->hotplug.poll_enabled, false);
 
 	spin_lock_irq(&display->irq.lock);
--

2.44.2