Reviewed-by: Lyude Paul <[email protected]>
On Thu, 2026-08-20 at 20:49 +0400, Mohamed Ahmed wrote:
> 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 <[email protected]>
> ---
> 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,
> + nouv
> eau_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;
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.