[PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[Imre Deak]

This is v3 of [1], with the following changes requested by Jani:

- Convert the internal quirk list to an enum list.
- Track both the internal and global quirks on a single list.
- Drop the change to support panel name specific quirks for now.

[1] https://lore.kernel.org/all/20250603121543.17842-1-imre.deak@intel.com

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>

Imre Deak (5):
  drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS
  drm/edid: Define the quirks in an enum list
  drm/edid: Add support for quirks visible to DRM core and drivers
  drm/dp: Add an EDID quirk for the DPCD register access probe
  drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

 drivers/gpu/drm/display/drm_dp_helper.c      |  44 ++--
 drivers/gpu/drm/drm_edid.c                   | 227 ++++++++++---------
 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 +
 include/drm/display/drm_dp_helper.h          |   6 +
 include/drm/drm_connector.h                  |   4 +-
 include/drm/drm_edid.h                       |   8 +
 8 files changed, 189 insertions(+), 123 deletions(-)

-- 
2.44.2

[PATCH v3 1/5] drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS

[Imre Deak]

Reading DPCD registers has side-effects in general. In particular
accessing registers outside of the link training register range
(0x102-0x106, 0x202-0x207, 0x200c-0x200f, 0x2216) is explicitly
forbidden by the DP v2.1 Standard, see

3.6.5.1 DPTX AUX Transaction Handling Mandates
3.6.7.4 128b/132b DP Link Layer LTTPR Link Training Mandates

Based on my tests, accessing the DPCD_REV register during the link
training of an UHBR TBT DP tunnel sink leads to link training failures.

Solve the above by using the DP_LANE0_1_STATUS (0x202) register for the
DPCD register access quirk.

Cc: <stable@vger.kernel.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index f2a6559a27100..dc622c78db9d4 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -725,7 +725,7 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
 	 * monitor doesn't power down exactly after the throw away read.
 	 */
 	if (!aux->is_remote) {
-		ret = drm_dp_dpcd_probe(aux, DP_DPCD_REV);
+		ret = drm_dp_dpcd_probe(aux, DP_LANE0_1_STATUS);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.44.2

[PATCH v3 2/5] drm/edid: Define the quirks in an enum list

[Imre Deak]

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 <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 218 +++++++++++++++++++------------------
 1 file changed, 112 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 74e77742b2bd4..857ae0c47a1c3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -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;
-- 
2.44.2

[PATCH v3 3/5] drm/edid: Add support for quirks visible to DRM core and drivers

[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ä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 857ae0c47a1c3..9cca1e6e4736c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -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);
 }
 
+bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk)
+{
+	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)))
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 73903c3c842f3..137773dd138ea 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -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;
 
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b38409670868d..77fd42608e706 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -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__ */
-- 
2.44.2

[PATCH v3 4/5] drm/dp: Add an EDID quirk for the DPCD register access probe

[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ä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index dc622c78db9d4..d0b9f672d743c 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -691,6 +691,34 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
 }
 EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
 
+/**
+ * drm_dp_dpcd_set_dpcd_probe_quirk() - Set whether a probing before DPCD access is done
+ * @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;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9cca1e6e4736c..5f45820ad62d5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -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)),
 };
 
 /*
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index e4ca35143ff96..75fa9548aefa0 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -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,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 77fd42608e706..3d1aecfec9b2a 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -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,
 };
 
-- 
2.44.2

[PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[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ä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 208a953b04a2f..d65a18fc1aeb9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -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);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index bf8e8e0cc19c9..410252ba6fd16 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -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)
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 74fe398663d63..1093c6c3714c0 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -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);
-- 
2.44.2

Re: [PATCH v3 2/5] drm/edid: Define the quirks in an enum list

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> 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 <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/drm_edid.c | 218 +++++++++++++++++++------------------
>  1 file changed, 112 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 74e77742b2bd4..857ae0c47a1c3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -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;

-- 
Jani Nikula, Intel

Re: [PATCH v3 2/5] drm/edid: Define the quirks in an enum list

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> +static bool drm_edid_has_internal_quirk(struct drm_connector *connector,

Nitpick, this could've been const. No big deal.

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

-- 
Jani Nikula, Intel

Re: [PATCH v3 3/5] drm/edid: Add support for quirks visible to DRM core and drivers

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  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(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 857ae0c47a1c3..9cca1e6e4736c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -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);
>  }
>  
> +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 <jani.nikula@intel.com>



> +{
> +	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)))
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 73903c3c842f3..137773dd138ea 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -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;
>  
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b38409670868d..77fd42608e706 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -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__ */

-- 
Jani Nikula, Intel

Re: [PATCH v3 4/5] drm/dp: Add an EDID quirk for the DPCD register access probe

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  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(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index dc622c78db9d4..d0b9f672d743c 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -691,6 +691,34 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
>  
> +/**
> + * 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 <jani.nikula@intel.com>


> + * @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;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9cca1e6e4736c..5f45820ad62d5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -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)),
>  };
>  
>  /*
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index e4ca35143ff96..75fa9548aefa0 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -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,
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 77fd42608e706..3d1aecfec9b2a 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -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,
>  };

-- 
Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

I'll get back to this tomorrow, don't have time now to check all the
paths now. At a glance looks okay.

BR,
Jani.

> ---
>  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(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 208a953b04a2f..d65a18fc1aeb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -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);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index bf8e8e0cc19c9..410252ba6fd16 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -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)
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 74fe398663d63..1093c6c3714c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -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);

-- 
Jani Nikula, Intel

Re: [PATCH v3 3/5] drm/edid: Add support for quirks visible to DRM core and drivers

[Imre Deak]

On Thu, Jun 05, 2025 at 04:07:15PM +0300, Jani Nikula wrote:
> On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > 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ä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  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(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 857ae0c47a1c3..9cca1e6e4736c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -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);
> >  }
> >  
> > +bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk)
> 
> Nitpick, this could've been const.

I'm unsure what rule to follow for atomic object pointers. Ville said
once that unlike atomic state pointers (i.e. drm_connector_state for
instance) these shouldn't be constified. Maybe that applies only to
cases where the object pointer is passed on to another function (so not
in this simple case clearly)?

> Regardless,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> 
> > +{
> > +	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)))
> >  
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 73903c3c842f3..137773dd138ea 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -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;
> >  
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index b38409670868d..77fd42608e706 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -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__ */
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v3 4/5] drm/dp: Add an EDID quirk for the DPCD register access probe

[Imre Deak]

On Thu, Jun 05, 2025 at 04:11:55PM +0300, Jani Nikula wrote:
> On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > 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ä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  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(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> > index dc622c78db9d4..d0b9f672d743c 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -691,6 +691,34 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
> >  }
> >  EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
> >  
> > +/**
> > + * 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.

Ok, makes sense, I can rename it also removing the repetition in the
name to drm_dp_dcpd_set_probe().

> Not a big deal.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> > + * @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;
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 9cca1e6e4736c..5f45820ad62d5 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -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)),
> >  };
> >  
> >  /*
> > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> > index e4ca35143ff96..75fa9548aefa0 100644
> > --- a/include/drm/display/drm_dp_helper.h
> > +++ b/include/drm/display/drm_dp_helper.h
> > @@ -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,
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 77fd42608e706..3d1aecfec9b2a 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -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,
> >  };
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Jani Nikula]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  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(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 208a953b04a2f..d65a18fc1aeb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -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.

> +		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);

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

>  		/*
>  		 * 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);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index bf8e8e0cc19c9..410252ba6fd16 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -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)
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 74fe398663d63..1093c6c3714c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -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);

-- 
Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Imre Deak]

On Fri, Jun 06, 2025 at 04:44:37PM +0300, Jani Nikula wrote:
> On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > 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ä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  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(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 208a953b04a2f..d65a18fc1aeb9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -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.

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.

> > +		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);
> 
> 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).

> 
> >  		/*
> >  		 * 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);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > index bf8e8e0cc19c9..410252ba6fd16 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > @@ -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)
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > index 74fe398663d63..1093c6c3714c0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -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);
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Imre Deak]

On Fri, Jun 06, 2025 at 04:50:03PM +0300, Imre Deak wrote:
> On Fri, Jun 06, 2025 at 04:44:37PM +0300, Jani Nikula wrote:
> > On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > > 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ä <ville.syrjala@linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  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(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 208a953b04a2f..d65a18fc1aeb9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -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.
> 
> 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.

> > > +		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);
> > 
> > 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).
> 
> > 
> > >  		/*
> > >  		 * 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);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > index bf8e8e0cc19c9..410252ba6fd16 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > @@ -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)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > index 74fe398663d63..1093c6c3714c0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > @@ -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);
> > 
> > -- 
> > Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Jani Nikula]

On Fri, 06 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, Jun 06, 2025 at 04:50:03PM +0300, Imre Deak wrote:
>> On Fri, Jun 06, 2025 at 04:44:37PM +0300, Jani Nikula wrote:
>> > On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
>> > > 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ä <ville.syrjala@linux.intel.com>
>> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> > > ---
>> > >  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(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > index 208a953b04a2f..d65a18fc1aeb9 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > @@ -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.
>> 
>> 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.

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.

>
>> > > +		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);
>> > 
>> > 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).
>> 
>> > 
>> > >  		/*
>> > >  		 * 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);
>> > >  
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > > index bf8e8e0cc19c9..410252ba6fd16 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > > @@ -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)
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > > index 74fe398663d63..1093c6c3714c0 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> > > @@ -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);
>> > 
>> > -- 
>> > Jani Nikula, Intel

-- 
Jani Nikula, Intel

Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Imre Deak]

On Fri, Jun 06, 2025 at 05:04:36PM +0300, Jani Nikula wrote:
> On Fri, 06 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, Jun 06, 2025 at 04:50:03PM +0300, Imre Deak wrote:
> >> On Fri, Jun 06, 2025 at 04:44:37PM +0300, Jani Nikula wrote:
> >> > On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> >> > > 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ä <ville.syrjala@linux.intel.com>
> >> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> >> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >> > > ---
> >> > >  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(-)
> >> > >
> >> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > > index 208a953b04a2f..d65a18fc1aeb9 100644
> >> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > > @@ -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.
> >> 
> >> 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.
> 
> 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.

That wouldn't work at all places, since after a hotplug/resuming the
connector can be still in MST mode, but probing needs to be enabled
in that case for the subsequent EDID read.

> BR,
> Jani.
> 
> >
> >> > > +		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);
> >> > 
> >> > 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).
> >> 
> >> > 
> >> > >  		/*
> >> > >  		 * 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);
> >> > >  
> >> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> >> > > index bf8e8e0cc19c9..410252ba6fd16 100644
> >> > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> >> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> >> > > @@ -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)
> >> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> >> > > index 74fe398663d63..1093c6c3714c0 100644
> >> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> >> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> >> > > @@ -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);
> >> > 
> >> > -- 
> >> > Jani Nikula, Intel
> 
> -- 
> Jani Nikula, Intel

[PATCH v4 4/5] drm/dp: Add an EDID quirk for the DPCD register access probe

[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)
v4:
- s/drm_dp_dpcd_set_probe_quirk/drm_dp_dpcd_set_probe (Jani)
- Fix documentation of drm_dp_dpcd_set_probe().
- Add comment at the end of internal quirk entries.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 42 +++++++++++++++++--------
 drivers/gpu/drm/drm_edid.c              |  8 +++++
 include/drm/display/drm_dp_helper.h     |  6 ++++
 include/drm/drm_edid.h                  |  3 ++
 4 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index dc622c78db9d4..632358b23b333 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -691,6 +691,34 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
 }
 EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
 
+/**
+ * drm_dp_dpcd_set_probe() - Set whether a probing before DPCD access is done
+ * @aux: DisplayPort AUX channel
+ * @enable: Enable the probing if required
+ */
+void drm_dp_dpcd_set_probe(struct drm_dp_aux *aux, bool enable)
+{
+	WRITE_ONCE(aux->dpcd_probe_disabled, !enable);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_set_probe);
+
+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;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9cca1e6e4736c..9c8822b337e2e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -248,6 +248,14 @@ 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)),
+
+	/*
+	 * @drm_edid_internal_quirk entries end here, following with the
+	 * @drm_edid_quirk entries.
+	 */
+
+	/* HP ZR24w DP AUX DPCD access requires probing to prevent corruption. */
+	EDID_QUIRK('H', 'W', 'P', 0x2869, BIT(DRM_EDID_QUIRK_DP_DPCD_PROBE)),
 };
 
 /*
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index e4ca35143ff96..3e35a68b2b412 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -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(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,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 77fd42608e706..3d1aecfec9b2a 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -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,
 };
 
-- 
2.44.2

[PATCH v4 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[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.

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

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 208a953b04a2f..c089036a745fd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -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)
 	 * 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);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index eff3414c05dbf..0657f56811966 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -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__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index bf8e8e0cc19c9..7bec964c0496f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -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
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -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

RE: [PATCH v4 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Kahola, Mika]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre Deak
> Sent: Monday, 9 June 2025 15.56
> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>

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 <mika.kahola@intel.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 208a953b04a2f..c089036a745fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -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)
>  	 * 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);
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index eff3414c05dbf..0657f56811966 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -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__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index bf8e8e0cc19c9..7bec964c0496f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -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
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -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

Re: [PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[Imre Deak]

Hi Maxim, Thomas, Maarten,

could you please ack merging this patchset via drm-intel?

Thanks,
Imre

On Thu, Jun 05, 2025 at 11:28:45AM +0300, Imre Deak wrote:
> This is v3 of [1], with the following changes requested by Jani:
> 
> - Convert the internal quirk list to an enum list.
> - Track both the internal and global quirks on a single list.
> - Drop the change to support panel name specific quirks for now.
> 
> [1] https://lore.kernel.org/all/20250603121543.17842-1-imre.deak@intel.com
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> 
> Imre Deak (5):
>   drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS
>   drm/edid: Define the quirks in an enum list
>   drm/edid: Add support for quirks visible to DRM core and drivers
>   drm/dp: Add an EDID quirk for the DPCD register access probe
>   drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required
> 
>  drivers/gpu/drm/display/drm_dp_helper.c      |  44 ++--
>  drivers/gpu/drm/drm_edid.c                   | 227 ++++++++++---------
>  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 +
>  include/drm/display/drm_dp_helper.h          |   6 +
>  include/drm/drm_connector.h                  |   4 +-
>  include/drm/drm_edid.h                       |   8 +
>  8 files changed, 189 insertions(+), 123 deletions(-)
> 
> -- 
> 2.44.2
> 

RE: [PATCH v4 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Jani Nikula]

On Tue, 10 Jun 2025, "Kahola, Mika" <mika.kahola@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre Deak
>> Sent: Monday, 9 June 2025 15.56
>> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>
>> 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ä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>
> 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 <mika.kahola@intel.com>

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

>
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> ---
>>  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(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 208a953b04a2f..c089036a745fd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -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)
>>  	 * 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);
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index eff3414c05dbf..0657f56811966 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -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__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> index bf8e8e0cc19c9..7bec964c0496f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> @@ -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
>> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
>> @@ -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
>

-- 
Jani Nikula, Intel

Re: [PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[Imre Deak]

Hi,

On Tue, Jun 10, 2025 at 06:42:04PM +0300, Imre Deak wrote:
> Hi Maxim, Thomas, Maarten,
> 
> could you please ack merging this patchset via drm-intel?

any objection to merge the patchset via drm-intel? If not, could
someone ack it?

Patches 1-4 could be also merged to drm-misc-next instead, but then
would need to wait with patch 5 until drm-misc-next is merged to
drm-intel.

Thanks,
Imre

> On Thu, Jun 05, 2025 at 11:28:45AM +0300, Imre Deak wrote:
> > This is v3 of [1], with the following changes requested by Jani:
> > 
> > - Convert the internal quirk list to an enum list.
> > - Track both the internal and global quirks on a single list.
> > - Drop the change to support panel name specific quirks for now.
> > 
> > [1] https://lore.kernel.org/all/20250603121543.17842-1-imre.deak@intel.com
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > 
> > Imre Deak (5):
> >   drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS
> >   drm/edid: Define the quirks in an enum list
> >   drm/edid: Add support for quirks visible to DRM core and drivers
> >   drm/dp: Add an EDID quirk for the DPCD register access probe
> >   drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required
> > 
> >  drivers/gpu/drm/display/drm_dp_helper.c      |  44 ++--
> >  drivers/gpu/drm/drm_edid.c                   | 227 ++++++++++---------
> >  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 +
> >  include/drm/display/drm_dp_helper.h          |   6 +
> >  include/drm/drm_connector.h                  |   4 +-
> >  include/drm/drm_edid.h                       |   8 +
> >  8 files changed, 189 insertions(+), 123 deletions(-)
> > 
> > -- 
> > 2.44.2
> > 

Re: [PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[Thomas Zimmermann]

Hi

Am 12.06.25 um 15:29 schrieb Imre Deak:
> Hi,
>
> On Tue, Jun 10, 2025 at 06:42:04PM +0300, Imre Deak wrote:
>> Hi Maxim, Thomas, Maarten,
>>
>> could you please ack merging this patchset via drm-intel?
> any objection to merge the patchset via drm-intel? If not, could
> someone ack it?

Sorry for missing that. I'm OK with merging it through Intel trees. Go 
ahead.

Best regards
Thomas

>
> Patches 1-4 could be also merged to drm-misc-next instead, but then
> would need to wait with patch 5 until drm-misc-next is merged to
> drm-intel.
>
> Thanks,
> Imre
>
>> On Thu, Jun 05, 2025 at 11:28:45AM +0300, Imre Deak wrote:
>>> This is v3 of [1], with the following changes requested by Jani:
>>>
>>> - Convert the internal quirk list to an enum list.
>>> - Track both the internal and global quirks on a single list.
>>> - Drop the change to support panel name specific quirks for now.
>>>
>>> [1] https://lore.kernel.org/all/20250603121543.17842-1-imre.deak@intel.com
>>>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>>
>>> Imre Deak (5):
>>>    drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS
>>>    drm/edid: Define the quirks in an enum list
>>>    drm/edid: Add support for quirks visible to DRM core and drivers
>>>    drm/dp: Add an EDID quirk for the DPCD register access probe
>>>    drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required
>>>
>>>   drivers/gpu/drm/display/drm_dp_helper.c      |  44 ++--
>>>   drivers/gpu/drm/drm_edid.c                   | 227 ++++++++++---------
>>>   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 +
>>>   include/drm/display/drm_dp_helper.h          |   6 +
>>>   include/drm/drm_connector.h                  |   4 +-
>>>   include/drm/drm_edid.h                       |   8 +
>>>   8 files changed, 189 insertions(+), 123 deletions(-)
>>>
>>> -- 
>>> 2.44.2
>>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v3 0/5] drm/dp: Limit the DPCD probe quirk to the affected monitor

[Imre Deak]

On Thu, Jun 12, 2025 at 03:54:51PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.06.25 um 15:29 schrieb Imre Deak:
> > Hi,
> > 
> > On Tue, Jun 10, 2025 at 06:42:04PM +0300, Imre Deak wrote:
> > > Hi Maxim, Thomas, Maarten,
> > > 
> > > could you please ack merging this patchset via drm-intel?
> > any objection to merge the patchset via drm-intel? If not, could
> > someone ack it?
> 
> Sorry for missing that. I'm OK with merging it through Intel trees. Go
> ahead.

Ok, thanks for the follow-up, acks and reviews, patchset is pushed to
drm-intel-next.

> Best regards
> Thomas
> 
> > 
> > Patches 1-4 could be also merged to drm-misc-next instead, but then
> > would need to wait with patch 5 until drm-misc-next is merged to
> > drm-intel.
> > 
> > Thanks,
> > Imre
> > 
> > > On Thu, Jun 05, 2025 at 11:28:45AM +0300, Imre Deak wrote:
> > > > This is v3 of [1], with the following changes requested by Jani:
> > > > 
> > > > - Convert the internal quirk list to an enum list.
> > > > - Track both the internal and global quirks on a single list.
> > > > - Drop the change to support panel name specific quirks for now.
> > > > 
> > > > [1] https://lore.kernel.org/all/20250603121543.17842-1-imre.deak@intel.com
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > 
> > > > Imre Deak (5):
> > > >    drm/dp: Change AUX DPCD probe address from DPCD_REV to LANE0_1_STATUS
> > > >    drm/edid: Define the quirks in an enum list
> > > >    drm/edid: Add support for quirks visible to DRM core and drivers
> > > >    drm/dp: Add an EDID quirk for the DPCD register access probe
> > > >    drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required
> > > > 
> > > >   drivers/gpu/drm/display/drm_dp_helper.c      |  44 ++--
> > > >   drivers/gpu/drm/drm_edid.c                   | 227 ++++++++++---------
> > > >   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 +
> > > >   include/drm/display/drm_dp_helper.h          |   6 +
> > > >   include/drm/drm_connector.h                  |   4 +-
> > > >   include/drm/drm_edid.h                       |   8 +
> > > >   8 files changed, 189 insertions(+), 123 deletions(-)
> > > > 
> > > > -- 
> > > > 2.44.2
> > > > 
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>