[PATCH v2 10/10] drm/nouveau: honor HF-EEODB EDIDs by converting to struct drm_edid

[Mohamed Ahmed]

HDMI 2.1 sinks ship HF-EEODB EDIDs. Byte 126 deliberately claims one
extension block for legacy sources, and the true count lives in the CTA
block's first data block. Their high-refresh timings sit in DisplayID
extension blocks 2+.

The kernel already reads these EDIDs whole, and on GSP boards RM returns
the full EDID with its true size. However, nouveau then hands the buffer
to the legacy API which sizes the EDID from byte 126 so the DisplayID
blocks are not parsed/exposed and the sysfs blob truncates to 256 bytes.
The mode list then tops out at the CTA VICs.

Convert to the struct drm_edid API, which carries the real size.
Read via drm_edid_read_ddc()/drm_edid_read_switcheroo() where an
adapter exists, and serve RM's buffer through drm_edid_read_custom()
with a block-reading callback where not, which also routes the GSP
path through drm's block validation and the debugfs EDID override.

A failed RM read now takes the same "no EDID" path as an empty DDC read
which stops it from leaking the runtime-PM reference detect() holds. The
property, display_info, and mode list go through
drm_edid_connector_update()/_add_modes() and every EDID source is
funneled through nouveau_connector_set_edid() so all of them reach the
property that drm_edid_connector_add_modes() reads from.

nouveau_acpi_edid() now returns a drm_edid sized from the EDID's
extension count but never past the length _DDC actually returned, so
padded buffers pass and truncated ones are rejected, and every
firmware-provided EDID (OF, ACPI, VBIOS-embedded) is validated with
drm_edid_valid(). An invalid one is treated as no EDID. get_modes()
re-syncs the property from the connector's copy when the probe helper
has cleared it for a forced-off connector, before adding modes.

The few raw EDID readers are converted along with it, so no raw struct
edid pointer is kept. Ownership and freeing move to the drm_edid. MST
connectors keep their separate legacy path.

Signed-off-by: Mohamed Ahmed <mohamedahmedegypt2001@xxxxxxxxx>
---
 drivers/gpu/drm/nouveau/dispnv04/dfp.c      |   5 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c     |   4 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c      |  21 ++-
 drivers/gpu/drm/nouveau/nouveau_acpi.h      |  10 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c | 147 +++++++++++++++-----
 drivers/gpu/drm/nouveau/nouveau_connector.h |  12 +-
 6 files changed, 151 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index c9f96ec8455d..00eb2aacbe93 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -344,9 +344,8 @@ static void nv04_dfp_mode_set(struct drm_encoder *encoder,
 		regp->fp_control |= (2 << 24);
 	if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS) {
 		bool duallink = false, dummy;
-		if (nv_connector->edid &&
-		    nv_connector->type == DCB_CONNECTOR_LVDS_SPWG) {
-			duallink = (((u8 *)nv_connector->edid)[121] == 2);
+		if (nv_connector->spwg_links) {
+			duallink = nv_connector->spwg_links == 2;
 		} else {
 			nouveau_bios_parse_lvds_table(dev, output_mode->clock,
 						      &duallink, &dummy);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2c66e480b511..add19d479ebe 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1803,8 +1803,8 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_commit *st
 			lvds_dual = bios->fp.dual_link;
 			lvds_8bpc = bios->fp.if_is_24bit;
 		} else {
-			if (nv_connector->type == DCB_CONNECTOR_LVDS_SPWG) {
-				if (((u8 *)nv_connector->edid)[121] == 2)
+			if (nv_connector->spwg_links) {
+				if (nv_connector->spwg_links == 2)
 					lvds_dual = true;
 			} else
 			if (mode->clock >= bios->fp.duallink_transition_clk) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index 21b56cc7605c..ab2868edd42b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -360,9 +360,10 @@ void nouveau_unregister_dsm_handler(void) {}
 void nouveau_switcheroo_optimus_dsm(void) {}
 #endif
 
-void *
+const struct drm_edid *
 nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
 {
+	const struct drm_edid *drm_edid;
 	struct acpi_device *acpidev;
 	int type, ret;
 	void *edid;
@@ -384,7 +385,23 @@ nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
 	if (ret < 0)
 		return NULL;
 
-	return edid;
+	/* Never let the EDID's own extension count reach past what _DDC
+	 * actually returned. Drop the padding some firmware appends so the
+	 * container is exactly the EDID, then validate it like the DDC
+	 * readers would.
+	 */
+	if (ret >= EDID_LENGTH)
+		ret = min_t(int, ret, EDID_LENGTH *
+				      (1 + ((const struct edid *)edid)->extensions));
+	drm_edid = drm_edid_alloc(edid, ret);
+	kfree(edid);
+
+	if (drm_edid && !drm_edid_valid(drm_edid)) {
+		drm_dbg_kms(dev, "Invalid EDID from ACPI _DDC\n");
+		drm_edid_free(drm_edid);
+		drm_edid = NULL;
+	}
+	return drm_edid;
 }
 
 bool nouveau_acpi_video_backlight_use_native(void)
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.h b/drivers/gpu/drm/nouveau/nouveau_acpi.h
index e39dd8b94b8b..bc3ccca7b338 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.h
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.h
@@ -4,13 +4,15 @@
 
 #define ROM_BIOS_PAGE 4096
 
+struct drm_edid;
+
 #if defined(CONFIG_ACPI) && defined(CONFIG_X86)
 bool nouveau_is_optimus(void);
 bool nouveau_is_v1_dsm(void);
 void nouveau_register_dsm_handler(void);
 void nouveau_unregister_dsm_handler(void);
 void nouveau_switcheroo_optimus_dsm(void);
-void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
+const struct drm_edid *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
 bool nouveau_acpi_video_backlight_use_native(void);
 void nouveau_acpi_video_register_backlight(void);
 #else
@@ -19,7 +21,11 @@ static inline bool nouveau_is_v1_dsm(void) { return false; };
 static inline void nouveau_register_dsm_handler(void) {}
 static inline void nouveau_unregister_dsm_handler(void) {}
 static inline void nouveau_switcheroo_optimus_dsm(void) {}
-static inline void *nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector) { return NULL; }
+static inline const struct drm_edid *
+nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
+{
+	return NULL;
+}
 static inline bool nouveau_acpi_video_backlight_use_native(void) { return true; }
 static inline void nouveau_acpi_video_register_backlight(void) {}
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index b0b0ad9a0c24..55912da83cd0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -397,7 +397,7 @@ nouveau_connector_destroy(struct drm_connector *connector)
 	struct nouveau_connector *nv_connector = nouveau_connector(connector);
 	nvif_event_dtor(&nv_connector->irq);
 	nvif_event_dtor(&nv_connector->hpd);
-	kfree(nv_connector->edid);
+	drm_edid_free(nv_connector->drm_edid);
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	if (nv_connector->aux.transfer)
@@ -469,6 +469,36 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 	return found;
 }
 
+static void
+nouveau_connector_set_edid(struct nouveau_connector *nv_connector,
+			   const struct drm_edid *drm_edid)
+{
+	if (nv_connector->drm_edid == drm_edid)
+		return;
+
+	/* Updates the EDID property and display_info with HF-EEODB-aware
+	 * sizing. The legacy helpers truncate both to what EDID byte 126
+	 * admits, hiding the DisplayID extension blocks that carry the
+	 * high-refresh timings.
+	 */
+	drm_edid_connector_update(&nv_connector->base, drm_edid);
+
+	drm_edid_free(nv_connector->drm_edid);
+	nv_connector->drm_edid = drm_edid;
+
+	/* The SPWG link-count byte lives in a vendor descriptor drm has no
+	 * accessor for. Peek at it once here so nothing else needs the raw
+	 * EDID.
+	 */
+	nv_connector->spwg_links = 0;
+	if (nv_connector->type == DCB_CONNECTOR_LVDS_SPWG) {
+		const u8 *raw = (const u8 *)drm_edid_raw(drm_edid);
+
+		if (raw)
+			nv_connector->spwg_links = raw[121] == 2 ? 2 : 1;
+	}
+}
+
 static struct nouveau_encoder *
 nouveau_connector_of_detect(struct drm_connector *connector)
 {
@@ -490,8 +520,17 @@ nouveau_connector_of_detect(struct drm_connector *connector)
 		int idx = name ? name[strlen(name) - 1] - 'A' : 0;
 
 		if (nv_encoder->dcb->i2c_index == idx && edid) {
-			nv_connector->edid =
-				kmemdup(edid, EDID_LENGTH, GFP_KERNEL);
+			const struct drm_edid *drm_edid =
+				drm_edid_alloc(edid, EDID_LENGTH);
+
+			/* Firmware-provided, so validate it like the DDC
+			 * readers would.
+			 */
+			if (drm_edid && !drm_edid_valid(drm_edid)) {
+				drm_edid_free(drm_edid);
+				drm_edid = NULL;
+			}
+			nouveau_connector_set_edid(nv_connector, drm_edid);
 			return nv_encoder;
 		}
 	}
@@ -546,17 +585,23 @@ nouveau_connector_set_encoder(struct drm_connector *connector,
 	}
 }
 
-static void
-nouveau_connector_set_edid(struct nouveau_connector *nv_connector,
-			   struct edid *edid)
+struct nouveau_rm_edid {
+	u8 *data;
+	size_t size;
+};
+
+static int
+nouveau_connector_rm_edid_block(void *context, u8 *buf, unsigned int block,
+				size_t len)
 {
-	if (nv_connector->edid != edid) {
-		struct edid *old_edid = nv_connector->edid;
+	struct nouveau_rm_edid *rm = context;
+	size_t offset = (size_t)block * EDID_LENGTH;
 
-		drm_connector_update_edid_property(&nv_connector->base, edid);
-		kfree(old_edid);
-		nv_connector->edid = edid;
-	}
+	if (offset + len > rm->size)
+		return -EINVAL;
+
+	memcpy(buf, rm->data + offset, len);
+	return 0;
 }
 
 static enum drm_connector_status
@@ -590,22 +635,37 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
 
 	nv_encoder = nouveau_connector_ddc_detect(connector);
 	if (nv_encoder) {
-		struct edid *new_edid = NULL;
+		const struct drm_edid *new_edid = NULL;
 
 		if (nv_encoder->i2c) {
 			if ((vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
 			    nv_connector->type == DCB_CONNECTOR_LVDS)
-				new_edid = drm_get_edid_switcheroo(connector, nv_encoder->i2c);
+				new_edid = drm_edid_read_switcheroo(connector, nv_encoder->i2c);
 			else
-				new_edid = drm_get_edid(connector, nv_encoder->i2c);
+				new_edid = drm_edid_read_ddc(connector, nv_encoder->i2c);
 		} else {
-			ret = nvif_outp_edid_get(&nv_encoder->outp, (u8 **)&new_edid);
-			if (ret < 0)
-				return connector_status_disconnected;
+			struct nouveau_rm_edid rm = {};
+
+			/* RM (which owns the DDC pads on GSP boards) reads the
+			 * EDID whole and returns its true size, which for an
+			 * HF-EEODB EDID exceeds what byte 126 admits. Serve it
+			 * through drm's block reader so EEODB sizing, block
+			 * validation, and the debugfs EDID override all apply.
+			 * A failed read is treated like an empty DDC read,
+			 * which releases the runtime-PM reference.
+			 */
+			ret = nvif_outp_edid_get(&nv_encoder->outp, &rm.data);
+			if (ret >= 0) {
+				rm.size = ret;
+				new_edid = drm_edid_read_custom(connector,
+								nouveau_connector_rm_edid_block,
+								&rm);
+				kfree(rm.data);
+			}
 		}
 
 		nouveau_connector_set_edid(nv_connector, new_edid);
-		if (!nv_connector->edid) {
+		if (!nv_connector->drm_edid) {
 			NV_ERROR(drm, "DDC responded, but no EDID for %s\n",
 				 connector->name);
 			goto detect_analog;
@@ -626,7 +686,7 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
 				    nv_partner->dcb->type == DCB_OUTPUT_TMDS) ||
 				   (nv_encoder->dcb->type == DCB_OUTPUT_TMDS &&
 				    nv_partner->dcb->type == DCB_OUTPUT_ANALOG))) {
-			if (nv_connector->edid->input & DRM_EDID_INPUT_DIGITAL)
+			if (drm_edid_is_digital(nv_connector->drm_edid))
 				type = DCB_OUTPUT_TMDS;
 			else
 				type = DCB_OUTPUT_ANALOG;
@@ -638,7 +698,8 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
 		conn_status = connector_status_connected;
 
 		if (nv_encoder->dcb->type == DCB_OUTPUT_DP)
-			drm_dp_cec_set_edid(&nv_connector->aux, nv_connector->edid);
+			drm_dp_cec_attach(&nv_connector->aux,
+					  connector->display_info.source_physical_address);
 
 		goto out;
 	} else {
@@ -670,7 +731,7 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
 	}
 
  out:
-	if (!nv_connector->edid)
+	if (!nv_connector->drm_edid)
 		drm_dp_cec_unset_edid(&nv_connector->aux);
 
 	pm_runtime_mark_last_busy(dev->dev);
@@ -686,7 +747,7 @@ nouveau_connector_detect_lvds(struct drm_connector *connector, bool force)
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_connector *nv_connector = nouveau_connector(connector);
 	struct nouveau_encoder *nv_encoder = NULL;
-	struct edid *edid = NULL;
+	const struct drm_edid *edid = NULL;
 	enum drm_connector_status status = connector_status_disconnected;
 
 	nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
@@ -697,7 +758,7 @@ nouveau_connector_detect_lvds(struct drm_connector *connector, bool force)
 	if (!drm->vbios.fp_no_ddc) {
 		status = nouveau_connector_detect(connector, force);
 		if (status == connector_status_connected) {
-			edid = nv_connector->edid;
+			edid = nv_connector->drm_edid;
 			goto out;
 		}
 	}
@@ -733,9 +794,17 @@ nouveau_connector_detect_lvds(struct drm_connector *connector, bool force)
 	 * stored for the panel stored in them.
 	 */
 	if (!drm->vbios.fp_no_ddc) {
-		edid = (struct edid *)nouveau_bios_embedded_edid(dev);
-		if (edid) {
-			edid = kmemdup(edid, EDID_LENGTH, GFP_KERNEL);
+		const void *embedded = nouveau_bios_embedded_edid(dev);
+
+		if (embedded) {
+			edid = drm_edid_alloc(embedded, EDID_LENGTH);
+			/* Firmware-provided, so validate it like the DDC
+			 * readers would.
+			 */
+			if (edid && !drm_edid_valid(edid)) {
+				drm_edid_free(edid);
+				edid = NULL;
+			}
 			if (edid)
 				status = connector_status_connected;
 		}
@@ -886,7 +955,7 @@ nouveau_connector_detect_depth(struct drm_connector *connector)
 	bool duallink;
 
 	/* if the edid is feeling nice enough to provide this info, use it */
-	if (nv_connector->edid && connector->display_info.bpc)
+	if (nv_connector->drm_edid && connector->display_info.bpc)
 		return;
 
 	/* EDID 1.4 is *supposed* to be supported on eDP, but, Apple... */
@@ -913,9 +982,8 @@ nouveau_connector_detect_depth(struct drm_connector *connector)
 	/* LVDS: DDC panel, need to first determine the number of links to
 	 * know which if_is_24bit flag to check...
 	 */
-	if (nv_connector->edid &&
-	    nv_connector->type == DCB_CONNECTOR_LVDS_SPWG)
-		duallink = ((u8 *)nv_connector->edid)[121] == 2;
+	if (nv_connector->spwg_links)
+		duallink = nv_connector->spwg_links == 2;
 	else
 		duallink = mode->clock >= bios->fp.duallink_transition_clk;
 
@@ -973,12 +1041,17 @@ nouveau_connector_get_modes(struct drm_connector *connector)
 		nv_connector->native_mode = NULL;
 	}
 
-	if (nv_connector->edid)
-		ret = drm_add_edid_modes(connector, nv_connector->edid);
-	else
-	if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
-	    (nv_encoder->dcb->lvdsconf.use_straps_for_mode ||
-	     drm->vbios.fp_no_ddc) && nouveau_bios_fp_mode(dev, NULL)) {
+	if (nv_connector->drm_edid) {
+		/* The probe helper clears the property and display_info for
+		 * a forced-off connector without calling detect(). Re-sync
+		 * from our copy then, since add_modes() reads the property.
+		 */
+		if (!connector->edid_blob_ptr)
+			drm_edid_connector_update(connector, nv_connector->drm_edid);
+		ret = drm_edid_connector_add_modes(connector);
+	} else if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
+		   (nv_encoder->dcb->lvdsconf.use_straps_for_mode ||
+		    drm->vbios.fp_no_ddc) && nouveau_bios_fp_mode(dev, NULL)) {
 		struct drm_display_mode mode;
 
 		nouveau_bios_fp_mode(dev, &mode);
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h
index 0608cabed058..eb292d2ba4bc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -43,7 +43,7 @@
 
 struct nvkm_i2c_port;
 struct dcb_output;
-struct edid;
+struct drm_edid;
 
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
 struct nouveau_backlight {
@@ -121,6 +121,13 @@ struct nouveau_connector {
 	struct drm_connector base;
 	enum dcb_connector_type type;
 	u8 index;
+	/* LVDS_SPWG panels state their link count in EDID descriptor 4 (SPWG
+	 * byte 0x79), cached by nouveau_connector_set_edid() so nothing else
+	 * needs the raw EDID. 0 = unknown (not an SPWG panel, or no EDID) and
+	 * callers fall back to their transition-clock/VBIOS heuristics;
+	 * 1 = single link; 2 = dual link.
+	 */
+	u8 spwg_links;
 
 	struct nvif_conn conn;
 	u64 hpd_pending;
@@ -137,7 +144,8 @@ struct nouveau_connector {
 	int scaling_mode;
 
 	struct nouveau_encoder *detected_encoder;
-	struct edid *edid;
+	/* Owner of the sink's EDID, HF-EEODB-complete. */
+	const struct drm_edid *drm_edid;
 	struct drm_display_mode *native_mode;
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
 	struct nouveau_backlight *backlight;
-- 
2.55.0