From cbf5b2678361fdf917904d45adf7c63ba67d079a Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 10:38:33 -0700 Subject: [PATCH 1/8] feat(linux): enumerate monitors with EDID identity (RandR joined to DRM sysfs) os_display_edid_enumerate was a stub off Windows, so the per-monitor DP registry (#69 / ADR-015) stayed empty on Linux. The new desktop-Linux enumerator ties each RandR monitor (placement rect, primary) to one DRM connector from /sys/class/drm (EDID, connector name, modes): by output name, else a unique physical-size match, else a unique pixel-mode match. It never guesses: an ambiguous monitor is listed without EDID identity. With no X server every connected, enabled connector becomes a DRM-only record flagged origin_unknown. The EDID parser reads the manufacturer/product ids (bytes 8-11), the serial (12-15), the cm size (21/22) and the first detailed timing's pixels, mm and refresh. os_display_edid_monitor gains runtime-private fields (serial, mm, device mode, connector, RandR name, join method, origin_unknown); the plug-in-facing xrt_display_descriptor is unchanged. aux_os stays free of logging and new link dependencies. Multi-screen plan M0, part of #1850. Co-Authored-By: Claude Opus 5.5 --- src/xrt/auxiliary/os/CMakeLists.txt | 5 + src/xrt/auxiliary/os/os_display_edid.h | 58 +++ src/xrt/auxiliary/os/os_display_edid_linux.c | 462 +++++++++++++++++++ src/xrt/auxiliary/os/os_display_edid_linux.h | 133 ++++++ 4 files changed, 658 insertions(+) create mode 100644 src/xrt/auxiliary/os/os_display_edid_linux.c create mode 100644 src/xrt/auxiliary/os/os_display_edid_linux.h diff --git a/src/xrt/auxiliary/os/CMakeLists.txt b/src/xrt/auxiliary/os/CMakeLists.txt index 3fdfe5610..b004e3b10 100644 --- a/src/xrt/auxiliary/os/CMakeLists.txt +++ b/src/xrt/auxiliary/os/CMakeLists.txt @@ -29,6 +29,11 @@ target_sources(aux_os PRIVATE os_display_edid.h) if(WIN32) target_sources(aux_os PRIVATE os_display_edid_win32.c) target_link_libraries(aux_os PRIVATE setupapi) +elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT ANDROID) + # Desktop Linux (multi-screen M0): RandR monitors joined to DRM sysfs + # connectors. Needs os_display_desktop_x11.c + os_display_connector_linux.c, + # added below under the same condition. + target_sources(aux_os PRIVATE os_display_edid_linux.h os_display_edid_linux.c) else() target_sources(aux_os PRIVATE os_display_edid_stubs.c) endif() diff --git a/src/xrt/auxiliary/os/os_display_edid.h b/src/xrt/auxiliary/os/os_display_edid.h index 7c97d4e98..6fea78390 100644 --- a/src/xrt/auxiliary/os/os_display_edid.h +++ b/src/xrt/auxiliary/os/os_display_edid.h @@ -9,6 +9,13 @@ * Windows: reads EDID from the registry via SetupAPI, correlates with * EnumDisplayMonitors for HMONITOR handles and screen coordinates. * + * Desktop Linux: joins the RandR monitors (`os_display_desktop_enumerate`: + * placement rect, primary flag) to the DRM connectors in sysfs + * (`/sys/class/drm/card*-*`: EDID blob, status), see + * `os_display_edid_linux.h` for the join rules. With no reachable X server + * (pure Wayland) the records come from DRM alone and carry + * @ref os_display_edid_monitor::origin_unknown. + * * Other platforms: stubs that return zero results. */ @@ -36,6 +43,19 @@ enum os_edid_diag_error OS_EDID_DIAG_NO_CORRELATION = 4, //!< Got EDID data but couldn't match to GDI monitors }; +/*! + * How a monitor's placement record was tied to its EDID (desktop Linux; the + * other platforms leave @ref OS_EDID_JOIN_NONE). + */ +enum os_display_edid_join +{ + OS_EDID_JOIN_NONE = 0, //!< No EDID source tied to this monitor. + OS_EDID_JOIN_NAME = 1, //!< RandR output name == normalised DRM connector name. + OS_EDID_JOIN_MM = 2, //!< Unique match on physical size (mm, with tolerance). + OS_EDID_JOIN_MODE = 3, //!< Unique match on pixel mode. + OS_EDID_JOIN_DRM_ONLY = 4, //!< No placement source (no X server): DRM record alone. +}; + /*! * EDID-derived identity for a connected monitor. */ @@ -50,6 +70,26 @@ struct os_display_edid_monitor uint32_t refresh_hz; //!< Current refresh rate in Hz bool is_primary; //!< True if this is the primary monitor void *hmonitor; //!< HMONITOR on Windows, NULL elsewhere + + /*! + * @name Runtime-private extras (desktop Linux today; zero elsewhere) + * + * Not plug-in ABI: @ref xrt_display_descriptor is built from the fields + * above and is unchanged. These feed `displayxr-cli displays`, the + * monitor id, and the runtime's own panel matching. + * @{ + */ + uint32_t serial_number; //!< EDID bytes 12-15 (little-endian); 0 = none. + uint32_t physical_width_mm; //!< Detailed-timing mm, else bytes 21 x 10, else RandR; 0 = unknown. + uint32_t physical_height_mm; //!< As above, bytes 22 x 10. + uint32_t native_width; //!< The connector's device mode (may differ from pixel_width + uint32_t native_height; //!< under a scaled X screen); 0 = unknown. + char connector[32]; //!< DRM connector, e.g. "HDMI-A-1"; "" = unknown. + char output_name[32]; //!< RandR output name, e.g. "HDMI-1"; "" = none. + bool origin_unknown; //!< screen_left/top are NOT a desktop position (DRM-only record). + enum os_display_edid_join join; //!< How the EDID was tied to the placement record. + + /*! @} */ }; /*! @@ -75,6 +115,7 @@ struct os_display_edid_list * * On Windows, uses SetupAPI to read EDID from the registry and * correlates with EnumDisplayMonitors for HMONITOR handles. + * On desktop Linux, joins RandR monitors to DRM sysfs connectors. * On other platforms, sets count to 0. * * @param[out] out_list Receives the enumerated monitors. @@ -98,6 +139,23 @@ os_display_edid_find_in_table(const struct os_display_edid_list *list, const uint16_t table[][2], uint32_t table_len); +/*! + * Short name of a join method ("name", "mm", "mode", "drm-only", "none"), for + * logs and `displayxr-cli displays`. Never NULL. + */ +static inline const char * +os_display_edid_join_str(enum os_display_edid_join join) +{ + switch (join) { + case OS_EDID_JOIN_NAME: return "name"; + case OS_EDID_JOIN_MM: return "mm"; + case OS_EDID_JOIN_MODE: return "mode"; + case OS_EDID_JOIN_DRM_ONLY: return "drm-only"; + case OS_EDID_JOIN_NONE: + default: return "none"; + } +} + #ifdef __cplusplus } #endif diff --git a/src/xrt/auxiliary/os/os_display_edid_linux.c b/src/xrt/auxiliary/os/os_display_edid_linux.c new file mode 100644 index 000000000..d31b89d76 --- /dev/null +++ b/src/xrt/auxiliary/os/os_display_edid_linux.c @@ -0,0 +1,462 @@ +// Copyright 2026, The DisplayXR Project +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Desktop Linux EDID enumeration: RandR monitors joined to DRM sysfs + * connectors (multi-screen M0, #69). See os_display_edid_linux.h. + * @ingroup aux_os + * + * No libdrm and no new link dependency: sysfs is plain files, and RandR comes + * from `os_display_desktop_enumerate`, which dlopens Xlib (aux_os no-DSO rule). + * No logging here either (aux_os sits below u_logging); the runtime logs the + * join per monitor where it turns these records into descriptors + * (`target_plugin_build_descriptors`). + */ + +#include "os_display_edid_linux.h" +#include "os_display_connector_linux.h" + +#include +#include +#include +#include + +#define DRM_SYSFS_ROOT "/sys/class/drm" +#define EDID_MAX_READ 512 + + +/* + * + * EDID. + * + */ + +static uint16_t +le16(const uint8_t *p) +{ + return (uint16_t)(p[0] | (p[1] << 8)); +} + +bool +os_display_edid_parse(const uint8_t *edid, size_t len, struct os_display_edid_parsed *out) +{ + static const uint8_t header[8] = {0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00}; + + if (out == NULL) { + return false; + } + memset(out, 0, sizeof(*out)); + if (edid == NULL || len < 128 || memcmp(edid, header, sizeof(header)) != 0) { + return false; + } + + out->manufacturer_id = le16(&edid[8]); + out->product_id = le16(&edid[10]); + out->serial_number = + (uint32_t)edid[12] | ((uint32_t)edid[13] << 8) | ((uint32_t)edid[14] << 16) | ((uint32_t)edid[15] << 24); + + // Bytes 21/22: max image size in cm. One of them 0 means the other is an + // aspect ratio (EDID 1.4), not a size — report nothing then. + if (edid[21] != 0 && edid[22] != 0) { + out->cm_width_mm = (uint32_t)edid[21] * 10u; + out->cm_height_mm = (uint32_t)edid[22] * 10u; + } + + // First detailed timing descriptor (bytes 54-71). Pixel clock 0 means it + // is a display descriptor (name, range limits...) instead — eDP panels + // often carry their timing in a DisplayID extension and leave this empty. + const uint8_t *d = &edid[54]; + const uint32_t clock_10khz = le16(&d[0]); + if (clock_10khz != 0) { + const uint32_t h_active = (uint32_t)d[2] | ((uint32_t)(d[4] & 0xF0) << 4); + const uint32_t h_blank = (uint32_t)d[3] | ((uint32_t)(d[4] & 0x0F) << 8); + const uint32_t v_active = (uint32_t)d[5] | ((uint32_t)(d[7] & 0xF0) << 4); + const uint32_t v_blank = (uint32_t)d[6] | ((uint32_t)(d[7] & 0x0F) << 8); + out->dtd_width_px = h_active; + out->dtd_height_px = v_active; + out->dtd_width_mm = (uint32_t)d[12] | ((uint32_t)(d[14] & 0xF0) << 4); + out->dtd_height_mm = (uint32_t)d[13] | ((uint32_t)(d[14] & 0x0F) << 8); + const uint64_t total = (uint64_t)(h_active + h_blank) * (uint64_t)(v_active + v_blank); + if (total > 0) { + out->dtd_refresh_mhz = + (uint32_t)(((uint64_t)clock_10khz * 10000ull * 1000ull + total / 2) / total); + } + } + + return true; +} + +void +os_display_edid_parsed_mm(const struct os_display_edid_parsed *p, uint32_t *out_w_mm, uint32_t *out_h_mm) +{ + uint32_t w = 0; + uint32_t h = 0; + if (p != NULL) { + if (p->dtd_width_mm > 0 && p->dtd_height_mm > 0) { + w = p->dtd_width_mm; + h = p->dtd_height_mm; + } else { + w = p->cm_width_mm; + h = p->cm_height_mm; + } + } + *out_w_mm = w; + *out_h_mm = h; +} + + +/* + * + * DRM sysfs. + * + */ + +static bool +read_first_line(const char *path, char *buf, size_t size) +{ + FILE *f = fopen(path, "r"); + if (f == NULL) { + return false; + } + const bool ok = fgets(buf, (int)size, f) != NULL; + fclose(f); + return ok; +} + +uint32_t +os_display_drm_read_connectors(const char *root, struct os_display_drm_connector *out, uint32_t max) +{ + if (root == NULL || out == NULL || max == 0) { + return 0; + } + DIR *dir = opendir(root); + if (dir == NULL) { + return 0; + } + + uint32_t count = 0; + struct dirent *e; + while ((e = readdir(dir)) != NULL && count < max) { + // "card-"; skip "card", "renderD", "version". + if (strncmp(e->d_name, "card", 4) != 0) { + continue; + } + const char *dash = strchr(e->d_name, '-'); + if (dash == NULL || dash[1] == '\0') { + continue; + } + + char path[512]; + char line[64]; + (void)snprintf(path, sizeof(path), "%s/%s/status", root, e->d_name); + if (!read_first_line(path, line, sizeof(line)) || strncmp(line, "connected", 9) != 0) { + continue; + } + + struct os_display_drm_connector *c = &out[count]; + memset(c, 0, sizeof(*c)); + (void)snprintf(c->name, sizeof(c->name), "%s", dash + 1); + + // Absent on old kernels: treat as enabled, the status said connected. + (void)snprintf(path, sizeof(path), "%s/%s/enabled", root, e->d_name); + c->enabled = !read_first_line(path, line, sizeof(line)) || strncmp(line, "disabled", 8) != 0; + + (void)snprintf(path, sizeof(path), "%s/%s/edid", root, e->d_name); + FILE *f = fopen(path, "rb"); + if (f != NULL) { + uint8_t buf[EDID_MAX_READ]; + const size_t n = fread(buf, 1, sizeof(buf), f); + fclose(f); + c->has_edid = os_display_edid_parse(buf, n, &c->edid); + } + + (void)snprintf(path, sizeof(path), "%s/%s/modes", root, e->d_name); + f = fopen(path, "r"); + if (f != NULL) { + while (c->mode_count < OS_DISPLAY_DRM_MAX_MODES && fgets(line, sizeof(line), f) != NULL) { + unsigned w = 0, h = 0; + if (sscanf(line, "%ux%u", &w, &h) == 2 && w > 0 && h > 0) { + c->mode_w[c->mode_count] = w; + c->mode_h[c->mode_count] = h; + c->mode_count++; + } + } + fclose(f); + } + + count++; + } + closedir(dir); + return count; +} + + +/* + * + * The join. + * + */ + +static uint32_t +abs_diff(uint32_t a, uint32_t b) +{ + return a > b ? a - b : b - a; +} + +static bool +mm_close(uint32_t aw, uint32_t ah, uint32_t bw, uint32_t bh) +{ + return aw > 0 && ah > 0 && bw > 0 && bh > 0 && abs_diff(aw, bw) <= OS_DISPLAY_EDID_JOIN_MM_TOLERANCE && + abs_diff(ah, bh) <= OS_DISPLAY_EDID_JOIN_MM_TOLERANCE; +} + +//! Does connector @p c match RandR's mm? Either EDID size source counts. +static bool +drm_matches_mm(const struct os_display_drm_connector *c, uint32_t w_mm, uint32_t h_mm) +{ + if (!c->has_edid) { + return false; + } + return mm_close(c->edid.dtd_width_mm, c->edid.dtd_height_mm, w_mm, h_mm) || + mm_close(c->edid.cm_width_mm, c->edid.cm_height_mm, w_mm, h_mm); +} + +static bool +drm_has_mode(const struct os_display_drm_connector *c, uint32_t w, uint32_t h) +{ + for (uint32_t i = 0; i < c->mode_count; i++) { + if (c->mode_w[i] == w && c->mode_h[i] == h) { + return true; + } + } + return false; +} + +//! Copy a connector's identity (EDID, mm, connector name) onto a record. +static void +apply_drm(struct os_display_edid_monitor *m, const struct os_display_drm_connector *c) +{ + (void)snprintf(m->connector, sizeof(m->connector), "%s", c->name); + if (c->has_edid) { + m->manufacturer_id = c->edid.manufacturer_id; + m->product_id = c->edid.product_id; + m->serial_number = c->edid.serial_number; + uint32_t w_mm = 0, h_mm = 0; + os_display_edid_parsed_mm(&c->edid, &w_mm, &h_mm); + if (w_mm > 0 && h_mm > 0) { + m->physical_width_mm = w_mm; + m->physical_height_mm = h_mm; + } + } + if (m->native_width == 0 && c->mode_count > 0) { + // The kernel lists the preferred mode first; the RandR rect when it + // is itself a mode (the device mode on an unscaled X screen). + m->native_width = c->mode_w[0]; + m->native_height = c->mode_h[0]; + if (drm_has_mode(c, m->pixel_width, m->pixel_height)) { + m->native_width = m->pixel_width; + m->native_height = m->pixel_height; + } + } + // The preferred timing's refresh, only when it is the mode in use. The + // current refresh is not in sysfs; this is the best a file read can do. + if (c->has_edid && c->edid.dtd_refresh_mhz > 0 && c->edid.dtd_width_px == m->native_width && + c->edid.dtd_height_px == m->native_height) { + m->refresh_hz = (c->edid.dtd_refresh_mhz + 500u) / 1000u; + } +} + +uint32_t +os_display_edid_linux_join(const struct os_display_desktop_info *randr, + uint32_t randr_count, + const struct os_display_drm_connector *drm, + uint32_t drm_count, + struct os_display_edid_monitor *out, + uint32_t max) +{ + if (out == NULL || max == 0) { + return 0; + } + if (randr == NULL) { + randr_count = 0; + } + if (drm == NULL) { + drm_count = 0; + } + if (drm_count > OS_DISPLAY_DRM_MAX_CONNECTORS) { + drm_count = OS_DISPLAY_DRM_MAX_CONNECTORS; + } + + // No placement source: one record per enabled connector, origin unknown. + if (randr_count == 0) { + uint32_t n = 0; + for (uint32_t j = 0; j < drm_count && n < max; j++) { + if (!drm[j].enabled) { + continue; + } + struct os_display_edid_monitor *m = &out[n++]; + memset(m, 0, sizeof(*m)); + m->origin_unknown = true; + m->join = OS_EDID_JOIN_DRM_ONLY; + if (drm[j].mode_count > 0) { + m->pixel_width = drm[j].mode_w[0]; + m->pixel_height = drm[j].mode_h[0]; + } + apply_drm(m, &drm[j]); + } + return n; + } + + const uint32_t n = randr_count < max ? randr_count : max; + bool used[OS_DISPLAY_DRM_MAX_CONNECTORS] = {0}; + int32_t pick[OS_DISPLAY_EDID_MAX_MONITORS > OS_DISPLAY_DESKTOP_MAX_MONITORS ? OS_DISPLAY_EDID_MAX_MONITORS + : OS_DISPLAY_DESKTOP_MAX_MONITORS]; + enum os_display_edid_join how[sizeof(pick) / sizeof(pick[0])]; + const uint32_t cap = (uint32_t)(sizeof(pick) / sizeof(pick[0])); + const uint32_t rn = n < cap ? n : cap; + for (uint32_t i = 0; i < rn; i++) { + pick[i] = -1; + how[i] = OS_EDID_JOIN_NONE; + } + + // Pass 1: names. + for (uint32_t i = 0; i < rn; i++) { + char rname[64]; + os_display_connector_normalise(randr[i].device_name, rname, sizeof(rname)); + if (rname[0] == '\0') { + continue; + } + for (uint32_t j = 0; j < drm_count; j++) { + char dname[64]; + os_display_connector_normalise(drm[j].name, dname, sizeof(dname)); + if (!used[j] && strcmp(rname, dname) == 0) { + pick[i] = (int32_t)j; + how[i] = OS_EDID_JOIN_NAME; + used[j] = true; + break; + } + } + } + + // Pass 2: physical size, only when exactly one unused connector fits. + for (uint32_t i = 0; i < rn; i++) { + if (pick[i] >= 0 || randr[i].physical_width_mm == 0 || randr[i].physical_height_mm == 0) { + continue; + } + int32_t found = -1; + uint32_t hits = 0; + for (uint32_t j = 0; j < drm_count; j++) { + if (!used[j] && + drm_matches_mm(&drm[j], randr[i].physical_width_mm, randr[i].physical_height_mm)) { + found = (int32_t)j; + hits++; + } + } + if (hits == 1) { + pick[i] = found; + how[i] = OS_EDID_JOIN_MM; + used[found] = true; + } + } + + // Pass 3: pixel mode (device mode when known, else the RandR rect). + for (uint32_t i = 0; i < rn; i++) { + if (pick[i] >= 0) { + continue; + } + const uint32_t w = randr[i].native_width > 0 ? randr[i].native_width : randr[i].width; + const uint32_t h = randr[i].native_height > 0 ? randr[i].native_height : randr[i].height; + int32_t found = -1; + uint32_t hits = 0; + for (uint32_t j = 0; j < drm_count; j++) { + if (!used[j] && drm_has_mode(&drm[j], w, h)) { + found = (int32_t)j; + hits++; + } + } + if (hits == 1) { + pick[i] = found; + how[i] = OS_EDID_JOIN_MODE; + used[found] = true; + } + } + + for (uint32_t i = 0; i < rn; i++) { + const struct os_display_desktop_info *r = &randr[i]; + struct os_display_edid_monitor *m = &out[i]; + memset(m, 0, sizeof(*m)); + m->screen_left = r->left; + m->screen_top = r->top; + m->pixel_width = r->width; + m->pixel_height = r->height; + m->is_primary = r->is_primary; + m->native_width = r->native_width; + m->native_height = r->native_height; + m->physical_width_mm = r->physical_width_mm; + m->physical_height_mm = r->physical_height_mm; + (void)snprintf(m->output_name, sizeof(m->output_name), "%.31s", r->device_name); + m->join = how[i]; + if (pick[i] >= 0) { + apply_drm(m, &drm[pick[i]]); + } + } + return rn; +} + + +/* + * + * Public entry points. + * + */ + +bool +os_display_edid_enumerate(struct os_display_edid_list *out_list) +{ + if (out_list == NULL) { + return false; + } + memset(out_list, 0, sizeof(*out_list)); + + struct os_display_desktop_info randr[OS_DISPLAY_DESKTOP_MAX_MONITORS]; + const uint32_t rn = os_display_desktop_enumerate(randr, OS_DISPLAY_DESKTOP_MAX_MONITORS); + + struct os_display_drm_connector drm[OS_DISPLAY_DRM_MAX_CONNECTORS]; + const uint32_t dn = os_display_drm_read_connectors(DRM_SYSFS_ROOT, drm, OS_DISPLAY_DRM_MAX_CONNECTORS); + + for (uint32_t j = 0; j < dn; j++) { + if (drm[j].has_edid) { + out_list->diag_edid_read_count++; + } + } + + out_list->count = os_display_edid_linux_join(rn > 0 ? randr : NULL, rn, drm, dn, out_list->monitors, + OS_DISPLAY_EDID_MAX_MONITORS); + + // diag_gdi_count is "monitors the placement source saw" — RandR here. + out_list->diag_gdi_count = rn; + if (out_list->count == 0) { + out_list->diag_error = OS_EDID_DIAG_NO_GDI_MONITORS; + } else if (out_list->diag_edid_read_count == 0) { + out_list->diag_error = OS_EDID_DIAG_NO_EDID_DATA; + } + + return out_list->count > 0; +} + +const struct os_display_edid_monitor * +os_display_edid_find_in_table(const struct os_display_edid_list *list, const uint16_t table[][2], uint32_t table_len) +{ + if (list == NULL || table == NULL || table_len == 0) { + return NULL; + } + for (uint32_t m = 0; m < list->count; m++) { + for (uint32_t t = 0; t < table_len; t++) { + if (list->monitors[m].manufacturer_id == table[t][0] && + list->monitors[m].product_id == table[t][1]) { + return &list->monitors[m]; + } + } + } + return NULL; +} diff --git a/src/xrt/auxiliary/os/os_display_edid_linux.h b/src/xrt/auxiliary/os/os_display_edid_linux.h new file mode 100644 index 000000000..76351feb9 --- /dev/null +++ b/src/xrt/auxiliary/os/os_display_edid_linux.h @@ -0,0 +1,133 @@ +// Copyright 2026, The DisplayXR Project +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Desktop Linux, aux_os-internal: the pieces behind + * @ref os_display_edid_enumerate — an EDID parser, the DRM sysfs + * reader and the RandR <-> DRM join. Exposed so host tests can pin + * them without a display. + * @ingroup aux_os + * + * ## Why a join at all + * + * Neither source alone is a monitor record. RandR (through + * `os_display_desktop_enumerate`) knows where a monitor is on the desktop and + * which one is primary, but under XWayland it publishes no EDID property. DRM + * sysfs has the EDID blob and the connector name, but no desktop position. So + * each RandR monitor is tied to one DRM connector, in this order: + * + * 1. **name** — the RandR output name equals the DRM connector name once the + * kernel's subtype letter is dropped ("HDMI-A-1" -> "HDMI-1"). This is the + * normal case on X11 and on Mutter's XWayland. + * 2. **mm** — exactly one unused connector whose EDID physical size is within + * @ref OS_DISPLAY_EDID_JOIN_MM_TOLERANCE of RandR's mm. + * 3. **mode** — exactly one unused connector that has the monitor's pixel size + * (device mode when known, else the RandR rect) among its modes. + * + * A RandR monitor nothing joins is still listed, with no EDID identity. With + * no RandR monitors at all (no X server: pure Wayland, or headless) every + * connected, enabled connector becomes a DRM-only record whose desktop origin + * is unknown. + */ + +#pragma once + +#include "os_display_desktop.h" +#include "os_display_edid.h" + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +//! Physical-size slack for the mm join. EDID stores cm in the base block and +//! mm in the detailed timing, and RandR may report either: 344 vs 340 is the +//! same DS1. +#define OS_DISPLAY_EDID_JOIN_MM_TOLERANCE 10 + +//! Modes kept per DRM connector. +#define OS_DISPLAY_DRM_MAX_MODES 16 + +//! Connectors kept from sysfs. +#define OS_DISPLAY_DRM_MAX_CONNECTORS 32 + +/*! + * What the runtime reads out of one EDID base block. + */ +struct os_display_edid_parsed +{ + uint16_t manufacturer_id; //!< Bytes 8-9, raw (little-endian load), as on Windows. + uint16_t product_id; //!< Bytes 10-11, little-endian. + uint32_t serial_number; //!< Bytes 12-15, little-endian; 0 = none. + uint32_t cm_width_mm; //!< Byte 21 x 10; 0 when absent or an aspect-ratio encoding. + uint32_t cm_height_mm; //!< Byte 22 x 10; as above. + uint32_t dtd_width_mm; //!< First detailed timing's image size; 0 = none. + uint32_t dtd_height_mm; + uint32_t dtd_width_px; //!< First detailed timing's active pixels; 0 = none. + uint32_t dtd_height_px; + uint32_t dtd_refresh_mhz; //!< First detailed timing's refresh, milli-Hz; 0 = none. +}; + +/*! + * Parse an EDID base block. Checks the 8-byte header and the length (>= 128); + * the checksum is not enforced (sysfs hands over what the sink sent, and a bad + * checksum still carries a usable vendor id). + * + * @return false on a short or headerless blob; @p out is zeroed either way. + */ +bool +os_display_edid_parse(const uint8_t *edid, size_t len, struct os_display_edid_parsed *out); + +/*! + * Best physical size from a parsed EDID: the detailed timing's mm when it is + * present, else the base block's cm. 0 when neither is. + */ +void +os_display_edid_parsed_mm(const struct os_display_edid_parsed *p, uint32_t *out_w_mm, uint32_t *out_h_mm); + +/*! + * One DRM connector from sysfs. + */ +struct os_display_drm_connector +{ + char name[32]; //!< Kernel name without the card prefix, e.g. "HDMI-A-1". + bool enabled; //!< `enabled` reads "enabled" (or the file is absent). + bool has_edid; //!< `edid` parsed. + struct os_display_edid_parsed edid; + uint32_t mode_w[OS_DISPLAY_DRM_MAX_MODES]; //!< `modes`, in kernel order + uint32_t mode_h[OS_DISPLAY_DRM_MAX_MODES]; //!< (the first is the preferred one). + uint32_t mode_count; +}; + +/*! + * Read every CONNECTED connector under @p root (normally `/sys/class/drm`; + * a directory of `card-/{status,enabled,edid,modes}` in tests). + * + * @return the number written to @p out. + */ +uint32_t +os_display_drm_read_connectors(const char *root, struct os_display_drm_connector *out, uint32_t max); + +/*! + * The join described in the file comment. Pure: no I/O, no logging. + * + * @param randr RandR monitors (may be NULL when @p randr_count is 0). + * @param drm Connected DRM connectors. + * @param[out] out One record per RandR monitor, or per enabled connector + * when there is no RandR monitor. + * @return the number of records written. + */ +uint32_t +os_display_edid_linux_join(const struct os_display_desktop_info *randr, + uint32_t randr_count, + const struct os_display_drm_connector *drm, + uint32_t drm_count, + struct os_display_edid_monitor *out, + uint32_t max); + +#ifdef __cplusplus +} +#endif From bc0b43b2ae043ef9fe1dbdc5d615a434eb5a8f47 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 10:39:05 -0700 Subject: [PATCH 2/8] feat(linux): collect display claims from every plug-in; place the back-compat claim on the panel POSIX ensure_display_sources consulted only the active plug-in. It now loads every manifest plug-in as a claim source through a POSIX collect_display_sources_platform, the twin of the Windows one: same roots and order as discovery, DXR_PLUGIN_EXCLUSIVE honoured, the active plug-in reused. try_load_one is split into load_and_probe_one + the active-plug-in wrapper, as on Windows. Non-active plug-ins are claim sources only; active selection is unchanged. Android keeps the single active source. A plug-in without probe_displays gets a synthesized EDID claim on the primary monitor. With the registry now populated on Linux, that put the vendor's claim on the laptop screen, and because the active plug-in wins every monitor it claims (#1521) the laptop would route to the vendor DP and the 3D panel to sim-display. Off-Windows the ACTIVE plug-in's synthesized claim now lands on the monitor its panel matches (ADR-033 rules: origin, connector mode, pixel size). The match uses the get_display_info noted by the builder and the display-info apply path (target_plugin_note_active_panel). Windows is unchanged, and a real probe_displays always decides. monitor_id also hashes the connector name when there is one (unchanged on Windows). The loader logs each monitor's join method once at INFO. `displayxr-cli displays` prints serial, mm, device mode, output, connector and join. `displays --claims` brings the system up headlessly first, so its placement matches the runtime's. Multi-screen plan M0, part of #1850. Co-Authored-By: Claude Opus 5.5 --- src/xrt/targets/cli/cli_cmd_displays.c | 99 ++++- src/xrt/targets/cli/cli_query.c | 6 +- .../common/target_builder_sim_display.c | 4 + src/xrt/targets/common/target_instance.c | 10 +- src/xrt/targets/common/target_plugin_loader.c | 347 +++++++++++++++++- src/xrt/targets/common/target_plugin_loader.h | 33 +- 6 files changed, 469 insertions(+), 30 deletions(-) diff --git a/src/xrt/targets/cli/cli_cmd_displays.c b/src/xrt/targets/cli/cli_cmd_displays.c index f81133f19..7685e5505 100644 --- a/src/xrt/targets/cli/cli_cmd_displays.c +++ b/src/xrt/targets/cli/cli_cmd_displays.c @@ -2,7 +2,8 @@ // SPDX-License-Identifier: BSL-1.0 /*! * @file - * @brief `displays` subcommand — enumerate connected displays via EDID. + * @brief `displays` subcommand — enumerate connected displays via EDID + * (Windows: SetupAPI; desktop Linux: RandR joined to DRM sysfs). * * Vendor-neutral list of every connected monitor (manufacturer/product, * resolution, position, primary), independent of which display processor is @@ -14,9 +15,12 @@ */ #include "cli_common.h" +#include "cli_query.h" #include "os/os_display_edid.h" #include "xrt/xrt_compositor.h" +#include "xrt/xrt_config_os.h" +#include "xrt/xrt_instance.h" #include "xrt/xrt_plugin.h" #include "target_plugin_loader.h" @@ -85,15 +89,52 @@ apis_to_str(const struct xrt_dp_registry_entry *e, char *out, size_t cap) } } +/*! + * Off-Windows, bring the system up headlessly (no compositor) and tear it down + * again, so the active plug-in's `get_display_info` has been read — the + * builder notes it with `target_plugin_note_active_panel`, which places a + * plug-in-without-`probe_displays` back-compat claim on its panel's monitor + * exactly as the runtime does at instance create. Windows keeps the plain + * loader path (its back-compat claim is the primary monitor either way). + */ +static void +note_active_panel_headless(void) +{ +#ifndef XRT_OS_WINDOWS + struct cli_query_handles h = {0}; + if (xrt_instance_create(NULL, &h.xi) == XRT_SUCCESS) { + (void)xrt_instance_create_system(h.xi, &h.xsys, &h.xsysd, &h.xso, NULL); + } + cli_query_teardown(&h); +#endif +} + +//! The enumerated record behind a registry entry, by monitor id. +static const struct os_display_edid_monitor * +monitor_for_id(const struct os_display_edid_list *list, + const struct xrt_display_descriptor *descs, + uint32_t dn, + uint64_t monitor_id) +{ + for (uint32_t i = 0; i < dn && i < list->count; i++) { + if (descs[i].monitor_id == monitor_id) { + return &list->monitors[i]; + } + } + return NULL; +} + /*! * `displays --claims`: enumerate EDID, ask the registered plug-ins which * monitors they claim, and print the resolved monitor→plug-in registry - * (#69 / ADR-015). Loads the active plug-in(s) — same exposure as + * (#69 / ADR-015). Loads the registered plug-ins — same exposure as * `selftest`/`info`. Plain `displays` stays vendor-blind (no plug-in load). */ static int cli_cmd_displays_claims(const struct os_display_edid_list *list, bool json) { + note_active_panel_headless(); + struct xrt_display_descriptor descs[XRT_DP_REGISTRY_MAX_ENTRIES]; uint32_t dn = target_plugin_build_descriptors(list, descs, XRT_DP_REGISTRY_MAX_ENTRIES); @@ -122,6 +163,21 @@ cli_cmd_displays_claims(const struct os_display_edid_list *list, bool json) cJSON_AddNumberToObject(c, "pixel_height", (double)e->pixel_height); cJSON_AddNumberToObject(c, "screen_left", (double)e->screen_left); cJSON_AddNumberToObject(c, "screen_top", (double)e->screen_top); + const struct os_display_edid_monitor *m = monitor_for_id(list, descs, dn, e->monitor_id); + if (m != NULL) { + char pnp[4]; + pnp_code(m->manufacturer_id, pnp); + char prod[8]; + snprintf(prod, sizeof(prod), "%04X", m->product_id); + cJSON_AddStringToObject(c, "manufacturer", pnp); + cJSON_AddStringToObject(c, "product", prod); + cJSON_AddNumberToObject(c, "edid_serial", (double)m->serial_number); + cJSON_AddNumberToObject(c, "physical_width_mm", (double)m->physical_width_mm); + cJSON_AddNumberToObject(c, "physical_height_mm", (double)m->physical_height_mm); + cJSON_AddStringToObject(c, "connector", m->connector); + cJSON_AddStringToObject(c, "output_name", m->output_name); + cJSON_AddBoolToObject(c, "primary", m->is_primary); + } cJSON_AddItemToArray(arr, c); } char *out = cJSON_Print(root); @@ -142,11 +198,24 @@ cli_cmd_displays_claims(const struct os_display_edid_list *list, bool json) const struct xrt_dp_registry_entry *e = ®.entries[i]; char apis[64]; apis_to_str(e, apis, sizeof(apis)); - PT("monitor 0x%016llx %ux%u @ (%d,%d)\n", (unsigned long long)e->monitor_id, e->pixel_width, - e->pixel_height, e->screen_left, e->screen_top); + const struct os_display_edid_monitor *m = monitor_for_id(list, descs, dn, e->monitor_id); + PT("monitor 0x%016llx %ux%u @ (%d,%d)%s\n", (unsigned long long)e->monitor_id, e->pixel_width, + e->pixel_height, e->screen_left, e->screen_top, + (m != NULL && m->origin_unknown) ? " (origin unknown)" : ""); + if (m != NULL) { + char pnp[4]; + pnp_code(m->manufacturer_id, pnp); + PT(" %s %04X serial=0x%08X %ux%u mm%s%s%s%s%s\n", pnp, m->product_id, m->serial_number, + m->physical_width_mm, m->physical_height_mm, m->output_name[0] != '\0' ? " output=" : "", + m->output_name, m->connector[0] != '\0' ? " connector=" : "", m->connector, + m->is_primary ? " [primary]" : ""); + } PT(" plug-in='%s' confidence=%s apis=%s%s%s\n", e->plugin_id, confidence_label(e->confidence), apis, e->serial[0] != '\0' ? " serial=" : "", e->serial); } + if (reg.entry_count < dn) { + PT("(%u of %u monitor(s) claimed by no plug-in)\n", dn - reg.entry_count, dn); + } return 0; } @@ -182,6 +251,15 @@ cli_cmd_displays(int argc, const char **argv) cJSON_AddNumberToObject(d, "screen_left", (double)m->screen_left); cJSON_AddNumberToObject(d, "screen_top", (double)m->screen_top); cJSON_AddBoolToObject(d, "primary", m->is_primary); + cJSON_AddNumberToObject(d, "edid_serial", (double)m->serial_number); + cJSON_AddNumberToObject(d, "physical_width_mm", (double)m->physical_width_mm); + cJSON_AddNumberToObject(d, "physical_height_mm", (double)m->physical_height_mm); + cJSON_AddNumberToObject(d, "native_width", (double)m->native_width); + cJSON_AddNumberToObject(d, "native_height", (double)m->native_height); + cJSON_AddStringToObject(d, "connector", m->connector); + cJSON_AddStringToObject(d, "output_name", m->output_name); + cJSON_AddStringToObject(d, "join", os_display_edid_join_str(m->join)); + cJSON_AddBoolToObject(d, "origin_unknown", m->origin_unknown); cJSON_AddItemToArray(arr, d); } cJSON *diag = cJSON_AddObjectToObject(root, "diag"); @@ -206,15 +284,22 @@ cli_cmd_displays(int argc, const char **argv) PT("(none enumerated; diag_error=%d gdi=%u setupdi=%u edid_reads=%u win32err=%u)\n", (int)list.diag_error, list.diag_gdi_count, list.diag_setupdi_count, list.diag_edid_read_count, list.diag_win32_error); - PT("Note: EDID enumeration is Windows-only; other platforms report none.\n"); + PT("Note: EDID enumeration is implemented on Windows and desktop Linux; other platforms report " + "none.\n"); return 0; } for (uint32_t i = 0; i < list.count; i++) { const struct os_display_edid_monitor *m = &list.monitors[i]; char pnp[4]; pnp_code(m->manufacturer_id, pnp); - PT("[%u] %s %04X %ux%u @ %uHz pos (%d,%d)%s\n", i, pnp, m->product_id, m->pixel_width, - m->pixel_height, m->refresh_hz, m->screen_left, m->screen_top, m->is_primary ? " [primary]" : ""); + PT("[%u] %s %04X %ux%u @ %uHz pos (%d,%d)%s%s\n", i, pnp, m->product_id, m->pixel_width, + m->pixel_height, m->refresh_hz, m->screen_left, m->screen_top, m->is_primary ? " [primary]" : "", + m->origin_unknown ? " (origin unknown)" : ""); + if (m->connector[0] != '\0' || m->output_name[0] != '\0') { + PT(" serial=0x%08X %ux%u mm native %ux%u output='%s' connector='%s' join=%s\n", + m->serial_number, m->physical_width_mm, m->physical_height_mm, m->native_width, + m->native_height, m->output_name, m->connector, os_display_edid_join_str(m->join)); + } } return 0; } diff --git a/src/xrt/targets/cli/cli_query.c b/src/xrt/targets/cli/cli_query.c index 4cdd6d24c..503c8eb62 100644 --- a/src/xrt/targets/cli/cli_query.c +++ b/src/xrt/targets/cli/cli_query.c @@ -778,9 +778,9 @@ dp_confidence_label(uint32_t c) * picking the entry exactly as `comp_dp_factory_for_window(COMP_DP_PRIMARY_ * MONITOR)` does (`xrt_dp_registry_primary_entry`: the active plug-in's * monitor, else entries[0]). Runs after the active plug-in is known; safe headless (no - * service, no GPU). Off-Windows the EDID enumerator yields no monitors, so the - * registry is empty and the service path falls back to the scalar — reported as - * agreement, never a false mismatch. + * service, no GPU). On macOS/Android the EDID enumerator yields no monitors, so + * the registry is empty and the service path falls back to the scalar — reported + * as agreement, never a false mismatch. */ static void probe_dp_selection(struct cli_query_result *r, const struct xrt_plugin_iface *active) diff --git a/src/xrt/targets/common/target_builder_sim_display.c b/src/xrt/targets/common/target_builder_sim_display.c index 89d5cb262..3c83b7da1 100644 --- a/src/xrt/targets/common/target_builder_sim_display.c +++ b/src/xrt/targets/common/target_builder_sim_display.c @@ -138,6 +138,10 @@ sim_display_open_system_impl(struct xrt_builder *xb, struct xrt_plugin_display_info pdi = {0}; pdi.struct_size = (uint32_t)sizeof(pdi); if (plugin->get_display_info(target_plugin_get_active_instance(), head, &pdi)) { + // The per-monitor registry places this plug-in's back-compat + // display claim on its panel's monitor (multi-screen M0). + target_plugin_note_active_panel(&pdi); + screen_height_m = pdi.display_height_m; nominal_z_m = pdi.nominal_viewer_z_m; diff --git a/src/xrt/targets/common/target_instance.c b/src/xrt/targets/common/target_instance.c index d8f1dd3c0..8919f265f 100644 --- a/src/xrt/targets/common/target_instance.c +++ b/src/xrt/targets/common/target_instance.c @@ -160,8 +160,10 @@ fill_dp_factories_from_plugin(struct xrt_system_compositor_info *info, const str * service compositor) via `comp_dp_factory_for_window`. They agree because the * loader now ENFORCES it (#1521): the active plug-in wins any monitor it claims, * so the primary-monitor winner is the active plug-in whenever it claims that - * monitor — it is no longer merely assumed from EDID confidence. No-op (empty - * registry) off-Windows, where the EDID enumerator returns no monitors. + * monitor — it is no longer merely assumed from EDID confidence. Populated on + * Windows and desktop Linux (RandR joined to DRM sysfs, multi-screen M0); a + * no-op (empty registry) on macOS and Android, where the EDID enumerator + * returns no monitors. */ static void build_dp_registry(struct xrt_system_compositor_info *info) @@ -271,6 +273,10 @@ apply_plugin_display_info(struct xrt_system_compositor_info *info, const struct xrt_plugin_iface *plugin, const struct xrt_plugin_display_info *pdi) { + // Steers the active plug-in's back-compat display claim onto its panel's + // monitor when build_dp_registry runs next (multi-screen M0). + target_plugin_note_active_panel(pdi); + info->display_width_m = pdi->display_width_m; info->display_height_m = pdi->display_height_m; info->nominal_viewer_x_m = pdi->nominal_viewer_x_m; diff --git a/src/xrt/targets/common/target_plugin_loader.c b/src/xrt/targets/common/target_plugin_loader.c index a23d9821e..f8facf4e6 100644 --- a/src/xrt/targets/common/target_plugin_loader.c +++ b/src/xrt/targets/common/target_plugin_loader.c @@ -38,6 +38,7 @@ #include "os/os_threading.h" #include "os/os_display_edid.h" +#include "os/os_display_desktop.h" #include "util/u_logging.h" #include @@ -2598,10 +2599,21 @@ user_manifest_dir(char *out, size_t cap) #endif } +/*! + * Load + negotiate + ABI-check + probe one manifest plug-in. The POSIX twin of + * the Windows @ref load_and_probe_one: no "active plug-in" line and no + * status_set_active, so it serves both the single-winner discovery + * (@ref try_load_one) and the load-all display-claim collection (#69, + * multi-screen M0). Successful loads leak the dlopen handle so the iface's + * function pointers stay callable for the process lifetime. + */ static const struct xrt_plugin_iface * -try_load_one(const struct plugin_entry *e, struct xrt_plugin_instance **out_inst) +load_and_probe_one(const struct plugin_entry *e, struct xrt_plugin_instance **out_inst, uint32_t *out_version) { *out_inst = NULL; + if (out_version != NULL) { + *out_version = 0; + } g_last_reject_reason[0] = '\0'; g_last_reject_declined = false; (void)status_begin(e->id, e->display_name, e->version, e->probe_order); @@ -2708,6 +2720,30 @@ try_load_one(const struct plugin_entry *e, struct xrt_plugin_instance **out_inst } } + if (out_version != NULL) { + *out_version = plugin_version; + } + status_finish(e->id, TARGET_PLUGIN_RESULT_CLAIMED, 0, NULL); + + /* dlopen handle intentionally leaked: the iface's function pointers + * remain reachable into the dylib for the process's lifetime. */ + return iface; +} + +/*! + * Try one manifest plug-in as the single active winner: @ref + * load_and_probe_one plus the canonical "active plug-in:" line (parsed by + * diagnostics) and the ACTIVE status. + */ +static const struct xrt_plugin_iface * +try_load_one(const struct plugin_entry *e, struct xrt_plugin_instance **out_inst) +{ + uint32_t plugin_version = 0; + const struct xrt_plugin_iface *iface = load_and_probe_one(e, out_inst, &plugin_version); + if (iface == NULL) { + return NULL; + } + U_LOG_W( "plugin loader: active plug-in: id=%s name='%s' vendor='%s' version='%s' " "plugin_api=%u probe_order=%u path=%s", @@ -2715,9 +2751,6 @@ try_load_one(const struct plugin_entry *e, struct xrt_plugin_instance **out_inst iface->vendor ? iface->vendor : e->vendor, e->version, plugin_version, e->probe_order, e->binary_path); status_set_active(e->id); - - /* dlopen handle intentionally leaked: the iface's function pointers - * remain reachable into the dylib for the process's lifetime. */ return iface; } @@ -2817,6 +2850,72 @@ discover_active_plugin(struct xrt_plugin_instance **out_inst, uint32_t max_probe return NULL; } +/*! + * Load EVERY manifest plug-in and return them as display-claim sources for the + * per-monitor registry — the POSIX twin of the Windows function of the same + * name (#69 / ADR-015, multi-screen M0). Same manifest roots and order as + * @ref discover_active_plugin; `DXR_PLUGIN_EXCLUSIVE` keeps every other + * plug-in out of the process exactly as it does for discovery. The active + * plug-in is reused, never loaded twice. The others are claim sources only: + * nothing creates a device from them, and they never become active here. + * Returns the source count in ascending ProbeOrder. + * + * Caller holds @ref g_refresh_mutex and has loaded the active plug-in. + */ +static int +collect_display_sources_platform(struct plugin_display_source *out, int max) +{ + char roots[8][PATH_MAX]; + int n_roots = target_plugin_build_discovery_roots(roots, (int)(sizeof(roots) / sizeof(roots[0]))); + struct plugin_entry entries[MAX_PLUGIN_ENTRIES]; + int n = 0; + for (int r = 0; r < n_roots; r++) { + n = enumerate_dir(roots[r], entries, n, MAX_PLUGIN_ENTRIES); + } + qsort(entries, (size_t)n, sizeof(entries[0]), compare_by_filename); + + const char *active_id = (g_active_iface != NULL && g_active_iface->id != NULL) ? g_active_iface->id : NULL; + bool active_seen = false; + + int count = 0; + for (int i = 0; i < n && count < max; i++) { + if (plugin_id_excluded(entries[i].id)) { + continue; + } + if (active_id != NULL && strcmp(entries[i].id, active_id) == 0) { + out[count].iface = g_active_iface; + out[count].inst = g_active_instance; + out[count].probe_order = g_active_probe_order; + count++; + active_seen = true; + continue; + } + + uint32_t ver = 0; + struct xrt_plugin_instance *inst = NULL; + const struct xrt_plugin_iface *iface = load_and_probe_one(&entries[i], &inst, &ver); + if (iface == NULL) { + continue; // declined / failed -> contributes no claims + } + U_LOG_I("plugin loader: display-claim source id=%s (ProbeOrder=%u)", entries[i].id, + entries[i].probe_order); + out[count].iface = iface; + out[count].inst = inst; + out[count].probe_order = entries[i].probe_order; + count++; + } + + // An active plug-in that no manifest names any more (removed after it + // loaded) is still the one devices come from: keep it as a source. + if (!active_seen && g_active_iface != NULL && count < max) { + out[count].iface = g_active_iface; + out[count].inst = g_active_instance; + out[count].probe_order = g_active_probe_order; + count++; + } + return count; +} + /* * * Public enumeration + PreferredPlugin override (POSIX: macOS / Linux). @@ -3061,7 +3160,7 @@ target_plugin_refresh_active(void) * Phase 1; EDID device-instance-path keying is the Phase 2/3 hardening. */ static uint64_t -monitor_id_from_edid(uint16_t mfr, uint16_t product, int32_t left, int32_t top) +monitor_id_from_edid(uint16_t mfr, uint16_t product, int32_t left, int32_t top, const char *platform_key) { uint64_t h = 1469598103934665603ULL; /* FNV-1a-64 offset basis */ const uint64_t prime = 1099511628211ULL; @@ -3074,9 +3173,105 @@ monitor_id_from_edid(uint16_t mfr, uint16_t product, int32_t left, int32_t top) h ^= (uint64_t)bytes[i]; h *= prime; } + // The platform's connector key (desktop Linux: "HDMI-A-1"), when there is + // one. Empty on Windows, so ids there are unchanged. It separates two + // identical panels that share a position — every DRM-only record (no X + // server) sits at (0, 0). + for (const char *c = platform_key; c != NULL && *c != '\0'; c++) { + h ^= (uint64_t)(uint8_t)*c; + h *= prime; + } return h; } +/*! + * Runtime-private side table: the full @ref os_display_edid_monitor behind + * each descriptor the last @ref target_plugin_build_descriptors produced, keyed + * by monitor id. The descriptor is plug-in ABI and stays as it is; this is + * where the connector name, mm and device mode live for the runtime's own + * use (the back-compat claim below). Guarded by @ref g_refresh_mutex. + */ +static struct +{ + uint64_t monitor_id; + struct os_display_edid_monitor mon; +} g_monitor_side[XRT_DP_REGISTRY_MAX_ENTRIES]; +static uint32_t g_monitor_side_count = 0; + +/*! + * What the ACTIVE plug-in said about its panel (`get_display_info`), noted by + * @ref target_plugin_note_active_panel. Steers the active plug-in's + * back-compat claim off-Windows. Guarded by @ref g_refresh_mutex. + */ +static struct os_display_panel_hint g_active_panel_hint; +static bool g_active_panel_valid = false; + +void +target_plugin_note_active_panel(const struct xrt_plugin_display_info *pdi) +{ + if (pdi == NULL) { + return; + } + if (g_refresh_mutex_initialized) { + os_mutex_lock(&g_refresh_mutex); + } + g_active_panel_hint.screen_left = pdi->display_screen_left; + g_active_panel_hint.screen_top = pdi->display_screen_top; + g_active_panel_hint.pixel_width = pdi->display_pixel_width; + g_active_panel_hint.pixel_height = pdi->display_pixel_height; + g_active_panel_hint.width_m = pdi->display_width_m; + g_active_panel_hint.height_m = pdi->display_height_m; + g_active_panel_valid = pdi->display_pixel_width > 0 && pdi->display_pixel_height > 0; + if (g_refresh_mutex_initialized) { + os_mutex_unlock(&g_refresh_mutex); + } +} + +/*! + * Log how each monitor was identified (desktop Linux: which RandR <-> DRM + * join rule fired), at INFO and only when the set changes, so a service that + * re-enumerates on every client connect logs it once. Silent when no record + * carries platform identity (Windows), whose enumerator has its own logs. + */ +static void +log_monitor_join_once(const struct os_display_edid_list *list) +{ + static uint64_t s_last = 0; + uint64_t h = 1469598103934665603ULL; + bool any = false; + for (uint32_t i = 0; i < list->count; i++) { + const struct os_display_edid_monitor *m = &list->monitors[i]; + any = any || m->join != OS_EDID_JOIN_NONE || m->output_name[0] != '\0'; + const uint64_t parts[] = {m->manufacturer_id, + m->product_id, + m->serial_number, + (uint64_t)(uint32_t)m->screen_left, + (uint64_t)(uint32_t)m->screen_top, + m->pixel_width, + (uint64_t)m->join}; + for (size_t k = 0; k < sizeof(parts) / sizeof(parts[0]); k++) { + h ^= parts[k]; + h *= 1099511628211ULL; + } + } + h ^= list->count; + if (!any || h == s_last) { + return; + } + s_last = h; + + for (uint32_t i = 0; i < list->count; i++) { + const struct os_display_edid_monitor *m = &list->monitors[i]; + U_LOG_I( + "plugin loader: monitor %u output='%s' connector='%s' join=%s mfr=0x%04x product=0x%04x " + "serial=0x%08x %ux%u%s at (%d,%d)%s %ux%u mm", + i, m->output_name, m->connector, os_display_edid_join_str(m->join), m->manufacturer_id, + m->product_id, m->serial_number, m->pixel_width, m->pixel_height, m->is_primary ? " primary" : "", + (int)m->screen_left, (int)m->screen_top, m->origin_unknown ? " (origin unknown)" : "", + m->physical_width_mm, m->physical_height_mm); + } +} + uint32_t target_plugin_build_descriptors(const struct os_display_edid_list *list, struct xrt_display_descriptor *out, @@ -3085,6 +3280,7 @@ target_plugin_build_descriptors(const struct os_display_edid_list *list, if (list == NULL || out == NULL || max == 0) { return 0; } + log_monitor_join_once(list); uint32_t n = list->count; if (n > max) { n = max; @@ -3094,7 +3290,8 @@ target_plugin_build_descriptors(const struct os_display_edid_list *list, struct xrt_display_descriptor *d = &out[i]; memset(d, 0, sizeof(*d)); d->struct_size = (uint32_t)sizeof(*d); - d->monitor_id = monitor_id_from_edid(m->manufacturer_id, m->product_id, m->screen_left, m->screen_top); + d->monitor_id = monitor_id_from_edid(m->manufacturer_id, m->product_id, m->screen_left, m->screen_top, + m->connector[0] != '\0' ? m->connector : m->output_name); d->edid_manufacturer = m->manufacturer_id; d->edid_product = m->product_id; d->pixel_width = m->pixel_width; @@ -3104,14 +3301,120 @@ target_plugin_build_descriptors(const struct os_display_edid_list *list, d->screen_top = m->screen_top; d->flags = m->is_primary ? 1u : 0u; } + + if (g_refresh_mutex_initialized) { + os_mutex_lock(&g_refresh_mutex); + } + g_monitor_side_count = n < XRT_DP_REGISTRY_MAX_ENTRIES ? n : XRT_DP_REGISTRY_MAX_ENTRIES; + for (uint32_t i = 0; i < g_monitor_side_count; i++) { + g_monitor_side[i].monitor_id = out[i].monitor_id; + g_monitor_side[i].mon = list->monitors[i]; + } + if (g_refresh_mutex_initialized) { + os_mutex_unlock(&g_refresh_mutex); + } return n; } +#ifndef XRT_OS_WINDOWS +static const struct os_display_edid_monitor * +monitor_side_find(uint64_t monitor_id) +{ + for (uint32_t i = 0; i < g_monitor_side_count; i++) { + if (g_monitor_side[i].monitor_id == monitor_id) { + return &g_monitor_side[i].mon; + } + } + return NULL; +} +#endif + +/*! + * Off-Windows: which descriptor is the ACTIVE plug-in's panel, by the same + * ADR-033 rules the desktop-rect resolver uses (origin, then connector mode, + * then pixel size; ties on physical size). Windows keeps the primary-monitor + * back-compat claim exactly as before. + * + * Why it exists: a 3D panel is a SECOND monitor on essentially every real + * deployment, so "the primary" lands the vendor's back-compat claim on the + * laptop screen — and the active plug-in wins every monitor it claims (#1521), + * so the registry would route the laptop screen to the vendor DP and the + * panel to the fallback. Caller holds @ref g_refresh_mutex. + */ +static bool +pick_active_panel_monitor(const struct xrt_display_descriptor *descs, uint32_t n, uint32_t *out_pick) +{ +#ifdef XRT_OS_WINDOWS + (void)descs; + (void)n; + (void)out_pick; + return false; +#else + if (!g_active_panel_valid || n == 0) { + return false; + } + if (n > XRT_DP_REGISTRY_MAX_ENTRIES) { + n = XRT_DP_REGISTRY_MAX_ENTRIES; + } + const struct os_display_panel_hint *hint = &g_active_panel_hint; + + struct os_display_desktop_info mons[XRT_DP_REGISTRY_MAX_ENTRIES]; + memset(mons, 0, sizeof(mons)); + bool origin_known[XRT_DP_REGISTRY_MAX_ENTRIES]; + for (uint32_t i = 0; i < n; i++) { + struct os_display_desktop_info *m = &mons[i]; + m->left = descs[i].screen_left; + m->top = descs[i].screen_top; + m->width = descs[i].pixel_width; + m->height = descs[i].pixel_height; + m->width_in_caller_dpi = m->width; + m->height_in_caller_dpi = m->height; + m->is_primary = (descs[i].flags & 1u) != 0; + origin_known[i] = true; + const struct os_display_edid_monitor *side = monitor_side_find(descs[i].monitor_id); + if (side != NULL) { + m->physical_width_mm = side->physical_width_mm; + m->physical_height_mm = side->physical_height_mm; + m->native_width = side->native_width; + m->native_height = side->native_height; + (void)snprintf(m->device_name, sizeof(m->device_name), "%s", + side->output_name[0] != '\0' ? side->output_name : side->connector); + origin_known[i] = !side->origin_unknown; + } + } + + // Rule 1: the plug-in placed its panel. Trust a non-(0,0) origin that + // falls inside a monitor whose own origin is a real desktop position. + if (hint->screen_left != 0 || hint->screen_top != 0) { + for (uint32_t i = 0; i < n; i++) { + const struct os_display_desktop_info *m = &mons[i]; + if (origin_known[i] && hint->screen_left >= m->left && + hint->screen_left < m->left + (int64_t)m->width && hint->screen_top >= m->top && + hint->screen_top < m->top + (int64_t)m->height) { + *out_pick = i; + return true; + } + } + } + + // Rules 2/3: connector mode, then pixel size. + struct os_display_panel_match match = {0}; + const int32_t idx = os_display_desktop_select_by_size(mons, n, hint, &match); + if (idx >= 0) { + *out_pick = (uint32_t)idx; + return true; + } + return false; +#endif +} + /*! * Synthesize the back-compat claim for a loaded plug-in that has no * `probe_displays` but whose binary `probe()` succeeded: one * @ref XRT_DISPLAY_CLAIM_EDID claim on the primary monitor (or the first - * descriptor if none is flagged primary). `supported_apis` is set to all + * descriptor if none is flagged primary) — except off-Windows for the ACTIVE + * plug-in once its panel is known, where it lands on that panel's monitor + * (@ref pick_active_panel_monitor). `supported_apis` is set to all * bits — the actual factory set is masked against the plug-in's non-NULL * factory pointers at fill time. */ @@ -3119,7 +3422,8 @@ static uint32_t synth_primary_edid_claim(const struct xrt_display_descriptor *descs, uint32_t n, struct xrt_display_claim *out, - uint32_t max) + uint32_t max, + const struct xrt_plugin_iface *iface) { if (n == 0 || max == 0) { return 0; @@ -3131,6 +3435,21 @@ synth_primary_edid_claim(const struct xrt_display_descriptor *descs, break; } } + // Off-Windows, the ACTIVE plug-in's back-compat claim follows its panel + // rather than the primary monitor (multi-screen M0). A plug-in that ships + // `probe_displays` never reaches here, so its own claims always win. + uint32_t panel = 0; + if (iface != NULL && iface == g_active_iface && pick_active_panel_monitor(descs, n, &panel)) { + if (panel != pick) { + U_LOG_I( + "plugin loader: '%s' has no probe_displays — back-compat claim placed on its panel " + "(monitor 0x%016llx, %ux%u at (%d,%d)) instead of the primary monitor", + iface->id != NULL ? iface->id : "?", (unsigned long long)descs[panel].monitor_id, + descs[panel].pixel_width, descs[panel].pixel_height, (int)descs[panel].screen_left, + (int)descs[panel].screen_top); + } + pick = panel; + } out[0].monitor_id = descs[pick].monitor_id; out[0].confidence = (uint32_t)XRT_DISPLAY_CLAIM_EDID; out[0].supported_apis = 0xFFFFFFFFu; @@ -3158,7 +3477,7 @@ query_source_claims(const struct plugin_display_source *src, iface->probe_displays != NULL) { return iface->probe_displays(src->inst, descs, n, out, max); } - return synth_primary_edid_claim(descs, n, out, max); + return synth_primary_edid_claim(descs, n, out, max, iface); } /*! @@ -3293,11 +3612,15 @@ ensure_display_sources(void) return; } g_display_source_count = 0; -#ifdef XRT_OS_WINDOWS +#if !defined(XRT_OS_ANDROID) + // Windows (registry) and macOS/Linux (manifests): every registered + // plug-in is a claim source, so the fallback and a vendor plug-in both + // contribute. macOS has no EDID enumerator yet, so resolution is a no-op + // there before this is ever reached. g_display_source_count = collect_display_sources_platform(g_display_sources, TARGET_PLUGIN_MAX_SOURCES); #else - // Off-Windows the EDID enumerator yields no monitors, so resolution is - // a no-op regardless; the single active plug-in suffices as the lone + // Android: one plug-in ships inside the runtime APK (ADR-038) and the + // EDID enumerator yields no monitors; the active plug-in is the lone // source for any caller that supplies its own descriptors. if (g_active_iface != NULL) { g_display_sources[0].iface = g_active_iface; diff --git a/src/xrt/targets/common/target_plugin_loader.h b/src/xrt/targets/common/target_plugin_loader.h index c1f225d6e..ca878c7c4 100644 --- a/src/xrt/targets/common/target_plugin_loader.h +++ b/src/xrt/targets/common/target_plugin_loader.h @@ -345,9 +345,12 @@ struct os_display_edid_list; * Map an enumerated EDID monitor list (`os_display_edid_enumerate`) into the * vendor-neutral @ref xrt_display_descriptor array handed to * `probe_displays()`. Assigns each monitor a stable-for-this-boot - * `monitor_id` (hashed from EDID manufacturer/product + screen position), + * `monitor_id` (hashed from EDID manufacturer/product + screen position, plus + * the DRM connector / RandR output name where the platform has one), * converts `refresh_hz`→`refresh_mhz`, and maps `is_primary`→`flags` bit 0. - * Writes up to @p max descriptors and returns the count. + * Writes up to @p max descriptors and returns the count. Also keeps the full + * monitor records (connector, mm, device mode) in a runtime-private side + * table keyed by monitor id; the descriptor struct (plug-in ABI) is unchanged. * * Issue #69 / ADR-015. */ @@ -362,7 +365,8 @@ target_plugin_build_descriptors(const struct os_display_edid_list *list, * its `probe_displays()` claims — or, for a plug-in without `probe_displays` * whose binary `probe()` succeeded, synthesizes a single * @ref XRT_DISPLAY_CLAIM_EDID claim on the primary monitor (single-display - * back-compat) — then resolves per monitor (highest confidence wins; ties by + * back-compat; off-Windows the ACTIVE plug-in's lands on its panel's monitor + * once @ref target_plugin_note_active_panel has run) — then resolves per monitor (highest confidence wins; ties by * ascending ProbeOrder) into @p out_registry. A monitor no plug-in claims * gets no entry. * @@ -372,15 +376,32 @@ target_plugin_build_descriptors(const struct os_display_edid_list *list, * for the process lifetime (rebuilt when @ref target_plugin_refresh_active * swaps in a better plug-in). Mutex-guarded like the refresh path. * - * Off-Windows the EDID enumerator yields no monitors, so a 0-length - * descriptor list resolves to an empty registry (`entry_count == 0`), - * signalling callers to use the scalar `dp_factory_*` fields. + * Windows and desktop Linux enumerate monitors (Linux: RandR joined to DRM + * sysfs, multi-screen M0) and load every registered plug-in as a claim + * source; macOS and Android yield no monitors, so a 0-length descriptor list + * resolves to an empty registry (`entry_count == 0`), signalling callers to + * use the scalar `dp_factory_*` fields. */ void target_plugin_resolve_displays(const struct xrt_display_descriptor *descriptors, uint32_t descriptor_count, struct xrt_dp_factory_registry *out_registry); +struct xrt_plugin_display_info; + +/*! + * Record what the ACTIVE plug-in reported about its panel + * (`get_display_info`: pixel size, physical size, desktop origin). Called + * wherever the runtime pulls that info (the builder at system create, and the + * display-info apply path on refresh). Off-Windows it steers the active + * plug-in's back-compat display claim — the one synthesized when it has no + * `probe_displays` — onto the monitor that matches the panel by the ADR-033 + * rules, instead of the primary monitor. No effect on Windows, and none on a + * plug-in that implements `probe_displays`. + */ +void +target_plugin_note_active_panel(const struct xrt_plugin_display_info *pdi); + #ifdef __cplusplus } #endif From 58d665cb376db61fcfcd571cfcc8b7be569aca4c Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 10:39:06 -0700 Subject: [PATCH 3/8] test(linux): pin the EDID parser, DRM sysfs reader and RandR<->DRM join Hardware-free tests for multi-screen M0. The fixtures are the DS1's real EDID base block (ACR 0001, serial 0x322EF05E, DTD 3840x2160@60, 344x193 mm) and an eDP block with no serial and no detailed timing (cm fallback). Cases: name, mm and mode joins; identical panels left unjoined; one connector per monitor; DRM-only records with an unknown origin when no X server is reachable; the sysfs reader against a temp-dir fixture (connected/disconnected, enabled, edid, modes). Co-Authored-By: Claude Opus 5.5 --- tests/CMakeLists.txt | 6 + tests/tests_aux_display_edid_linux.cpp | 389 +++++++++++++++++++++++++ 2 files changed, 395 insertions(+) create mode 100644 tests/tests_aux_display_edid_linux.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c6cd94665..998c1ba0d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -132,6 +132,12 @@ if(XRT_HAVE_VULKAN # format modifiers (a CI runner's lavapipe has no DRM device). list(APPEND tests tests_vk_dmabuf) endif() +if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT ANDROID) + # Multi-screen M0 (#69) -- the desktop-Linux EDID enumeration: EDID + # parser, DRM sysfs reader (against a temp-dir fixture) and the + # RandR <-> DRM join. Hardware-free. + list(APPEND tests tests_aux_display_edid_linux) +endif() if(TARGET comp_vk_native AND CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT ANDROID diff --git a/tests/tests_aux_display_edid_linux.cpp b/tests/tests_aux_display_edid_linux.cpp new file mode 100644 index 000000000..876a298c4 --- /dev/null +++ b/tests/tests_aux_display_edid_linux.cpp @@ -0,0 +1,389 @@ +// Copyright 2026, The DisplayXR Project +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Pins the desktop-Linux EDID enumeration (multi-screen M0, #69): the + * EDID parser, the DRM sysfs reader and the RandR <-> DRM join, on + * fixtures modelled on the DS1 box (laptop eDP-1 + Acer DS1 on HDMI). + */ + +#include "catch_amalgamated.hpp" + +#include "os/os_display_edid_linux.h" + +#include +#include +#include +#include +#include + +#include +#include + +namespace { + +/* + * The Acer SpatialLabs DS1's EDID base block as the kernel exposes it on + * HDMI-A-1 (`/sys/class/drm/card1-HDMI-A-1/edid`, first 128 bytes): vendor + * ACR, product 0x0001, serial 0x322EF05E, 34x19 cm, first detailed timing + * 3840x2160@60 at 344x193 mm. + */ +const uint8_t DS1_EDID[128] = { + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x04, 0x72, 0x01, 0x00, 0x5e, 0xf0, 0x2e, 0x32, 0x16, 0x21, 0x01, + 0x03, 0x80, 0x22, 0x13, 0x78, 0x2a, 0xce, 0xb5, 0xa6, 0x54, 0x34, 0xb6, 0x25, 0x0e, 0x50, 0x54, 0xbd, 0xef, 0x00, + 0x71, 0x4f, 0x81, 0x40, 0x81, 0x80, 0x81, 0xc0, 0x95, 0x00, 0xb3, 0x00, 0xd1, 0xc0, 0x01, 0x01, 0x08, 0xe8, 0x00, + 0x30, 0xf2, 0x70, 0x5a, 0x80, 0xb0, 0x58, 0x8a, 0x00, 0x58, 0xc1, 0x10, 0x00, 0x00, 0x1e, 0x00, 0x00, 0x00, 0xfe, + 0x00, 0x46, 0x46, 0x52, 0x32, 0x41, 0x41, 0x41, 0x30, 0x30, 0x31, 0x0a, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc, 0x00, + 0x44, 0x53, 0x31, 0x5f, 0x31, 0x35, 0x36, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x17, + 0x4c, 0x1e, 0x87, 0x3c, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x01, 0x71, +}; + +/*! + * A laptop eDP panel in the shape this box's has: vendor SDC, no serial in + * bytes 12-15, 30x19 cm, and NO detailed timing in the base block (the timing + * lives in a DisplayID extension), so the cm values are all there is. + */ +std::vector +edp_no_serial_edid() +{ + std::vector e(128, 0); + const uint8_t header[8] = {0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00}; + std::memcpy(e.data(), header, 8); + e[8] = 0x4c; // "SDC" + e[9] = 0x83; + e[10] = 0x3f; + e[11] = 0x42; + // bytes 12-15: serial 0 + e[18] = 1; + e[19] = 4; + e[21] = 30; // cm + e[22] = 19; + // byte 54..71: first descriptor left as a display descriptor (clock 0) + e[54 + 3] = 0xfd; + return e; +} + +constexpr uint16_t ACR_RAW = 0x7204; // bytes 08 72 loaded little-endian ("ACR") +constexpr uint16_t SDC_RAW = 0x834c; + +os_display_desktop_info +randr_mon(const char *name, int32_t left, uint32_t w, uint32_t h, uint32_t mm_w, uint32_t mm_h, bool primary) +{ + os_display_desktop_info m = {}; + m.left = left; + m.width = w; + m.height = h; + m.width_in_caller_dpi = w; + m.height_in_caller_dpi = h; + m.is_primary = primary; + m.physical_width_mm = mm_w; + m.physical_height_mm = mm_h; + std::snprintf(m.device_name, sizeof(m.device_name), "%s", name); + return m; +} + +os_display_drm_connector +drm_conn(const char *name, const uint8_t *edid, size_t len, uint32_t mode_w, uint32_t mode_h, bool enabled = true) +{ + os_display_drm_connector c = {}; + std::snprintf(c.name, sizeof(c.name), "%s", name); + c.enabled = enabled; + if (edid != nullptr) { + c.has_edid = os_display_edid_parse(edid, len, &c.edid); + } + c.mode_w[0] = mode_w; + c.mode_h[0] = mode_h; + c.mode_count = 1; + return c; +} + +//! The DS1 box's two monitors as RandR reports them under XWayland. +void +box_randr(os_display_desktop_info out[2], const char *n0 = "eDP-1", const char *n1 = "HDMI-1") +{ + out[0] = randr_mon(n0, 0, 3456, 2160, 300, 190, true); + out[0].native_width = 2880; + out[0].native_height = 1800; + out[1] = randr_mon(n1, 3456, 3840, 2160, 340, 190, false); + out[1].native_width = 3840; + out[1].native_height = 2160; +} + +void +box_drm(os_display_drm_connector out[2]) +{ + const std::vector edp = edp_no_serial_edid(); + out[0] = drm_conn("eDP-1", edp.data(), edp.size(), 2880, 1800); + out[1] = drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160); +} + +} // namespace + +TEST_CASE("edid: parses the DS1 base block") +{ + os_display_edid_parsed p = {}; + REQUIRE(os_display_edid_parse(DS1_EDID, sizeof(DS1_EDID), &p)); + CHECK(p.manufacturer_id == ACR_RAW); + CHECK(p.product_id == 0x0001); + CHECK(p.serial_number == 0x322EF05Eu); + CHECK(p.cm_width_mm == 340); + CHECK(p.cm_height_mm == 190); + CHECK(p.dtd_width_px == 3840); + CHECK(p.dtd_height_px == 2160); + CHECK(p.dtd_width_mm == 344); + CHECK(p.dtd_height_mm == 193); + CHECK(p.dtd_refresh_mhz == 60000); + + uint32_t w = 0, h = 0; + os_display_edid_parsed_mm(&p, &w, &h); + CHECK(w == 344); // the detailed timing's mm beat the base block's cm + CHECK(h == 193); + + // The PNP letters, decoded the way the CLI does. + const uint16_t v = (uint16_t)((p.manufacturer_id >> 8) | (p.manufacturer_id << 8)); + CHECK((char)(((v >> 10) & 0x1F) + 'A' - 1) == 'A'); + CHECK((char)(((v >> 5) & 0x1F) + 'A' - 1) == 'C'); + CHECK((char)((v & 0x1F) + 'A' - 1) == 'R'); +} + +TEST_CASE("edid: an eDP with no serial and no detailed timing falls back to cm") +{ + const std::vector e = edp_no_serial_edid(); + os_display_edid_parsed p = {}; + REQUIRE(os_display_edid_parse(e.data(), e.size(), &p)); + CHECK(p.manufacturer_id == SDC_RAW); + CHECK(p.product_id == 0x423f); + CHECK(p.serial_number == 0); + CHECK(p.dtd_width_px == 0); + CHECK(p.dtd_refresh_mhz == 0); + uint32_t w = 0, h = 0; + os_display_edid_parsed_mm(&p, &w, &h); + CHECK(w == 300); + CHECK(h == 190); +} + +TEST_CASE("edid: aspect-ratio encoding in bytes 21/22 is not a size") +{ + std::vector e = edp_no_serial_edid(); + e[21] = 0x4f; // landscape aspect ratio, EDID 1.4 + e[22] = 0; + os_display_edid_parsed p = {}; + REQUIRE(os_display_edid_parse(e.data(), e.size(), &p)); + CHECK(p.cm_width_mm == 0); + CHECK(p.cm_height_mm == 0); +} + +TEST_CASE("edid: rejects short and headerless blobs") +{ + os_display_edid_parsed p = {}; + CHECK_FALSE(os_display_edid_parse(DS1_EDID, 127, &p)); + CHECK_FALSE(os_display_edid_parse(nullptr, 128, &p)); + std::vector bad(DS1_EDID, DS1_EDID + 128); + bad[0] = 0x01; + CHECK_FALSE(os_display_edid_parse(bad.data(), bad.size(), &p)); + CHECK(p.manufacturer_id == 0); +} + +TEST_CASE("join: RandR output names tie to DRM connectors by name") +{ + os_display_desktop_info randr[2]; + box_randr(randr); + os_display_drm_connector drm[2]; + box_drm(drm); + + os_display_edid_monitor out[4] = {}; + REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 4) == 2); + + CHECK(out[0].join == OS_EDID_JOIN_NAME); + CHECK(out[0].manufacturer_id == SDC_RAW); + CHECK(out[0].serial_number == 0); + CHECK(out[0].is_primary); + CHECK(std::string(out[0].connector) == "eDP-1"); + CHECK(out[0].physical_width_mm == 300); + CHECK(out[0].native_width == 2880); + CHECK(out[0].refresh_hz == 0); // no detailed timing to read it from + + CHECK(out[1].join == OS_EDID_JOIN_NAME); + CHECK(out[1].manufacturer_id == ACR_RAW); + CHECK(out[1].product_id == 0x0001); + CHECK(out[1].serial_number == 0x322EF05Eu); + CHECK(std::string(out[1].connector) == "HDMI-A-1"); + CHECK(std::string(out[1].output_name) == "HDMI-1"); + CHECK(out[1].screen_left == 3456); + CHECK(out[1].pixel_width == 3840); + CHECK(out[1].physical_width_mm == 344); + CHECK(out[1].physical_height_mm == 193); + CHECK(out[1].refresh_hz == 60); + CHECK_FALSE(out[1].is_primary); + CHECK_FALSE(out[1].origin_unknown); +} + +TEST_CASE("join: unrelated output names fall back to physical size") +{ + os_display_desktop_info randr[2]; + box_randr(randr, "XWAYLAND0", "XWAYLAND1"); + os_display_drm_connector drm[2]; + box_drm(drm); + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + CHECK(out[0].join == OS_EDID_JOIN_MM); + CHECK(out[0].manufacturer_id == SDC_RAW); + CHECK(out[1].join == OS_EDID_JOIN_MM); // RandR 340x190 vs DTD 344x193: within tolerance + CHECK(out[1].manufacturer_id == ACR_RAW); +} + +TEST_CASE("join: no names and no mm fall back to the pixel mode") +{ + os_display_desktop_info randr[2]; + box_randr(randr, "XWAYLAND0", "XWAYLAND1"); + for (auto &m : randr) { + m.physical_width_mm = 0; + m.physical_height_mm = 0; + } + os_display_drm_connector drm[2]; + box_drm(drm); + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + CHECK(out[0].join == OS_EDID_JOIN_MODE); // device mode 2880x1800 + CHECK(out[0].manufacturer_id == SDC_RAW); + CHECK(out[1].join == OS_EDID_JOIN_MODE); + CHECK(out[1].manufacturer_id == ACR_RAW); +} + +TEST_CASE("join: two identical panels with no name match stay unjoined, never guessed") +{ + os_display_desktop_info randr[2] = { + randr_mon("XWAYLAND0", 0, 3840, 2160, 340, 190, true), + randr_mon("XWAYLAND1", 3840, 3840, 2160, 340, 190, false), + }; + os_display_drm_connector drm[2] = { + drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + drm_conn("HDMI-A-2", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + }; + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + CHECK(out[0].join == OS_EDID_JOIN_NONE); + CHECK(out[0].manufacturer_id == 0); + CHECK(out[1].join == OS_EDID_JOIN_NONE); + CHECK(out[0].pixel_width == 3840); // still listed, with its placement +} + +TEST_CASE("join: a connector is used at most once") +{ + // Both RandR monitors would match HDMI-A-1 by mm; the name pass takes it + // first, so the second must not reuse it. + os_display_desktop_info randr[2] = { + randr_mon("HDMI-1", 0, 3840, 2160, 340, 190, true), + randr_mon("XWAYLAND1", 3840, 3840, 2160, 340, 190, false), + }; + os_display_drm_connector drm[1] = {drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160)}; + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, 2, drm, 1, out, 2) == 2); + CHECK(out[0].join == OS_EDID_JOIN_NAME); + CHECK(out[1].join == OS_EDID_JOIN_NONE); +} + +TEST_CASE("join: no X server gives DRM-only records with an unknown origin") +{ + os_display_drm_connector drm[3]; + box_drm(drm); + drm[2] = drm_conn("DP-1", DS1_EDID, sizeof(DS1_EDID), 1920, 1080, /*enabled=*/false); + + os_display_edid_monitor out[4] = {}; + REQUIRE(os_display_edid_linux_join(nullptr, 0, drm, 3, out, 4) == 2); // disabled DP-1 skipped + for (int i = 0; i < 2; i++) { + CHECK(out[i].join == OS_EDID_JOIN_DRM_ONLY); + CHECK(out[i].origin_unknown); + CHECK_FALSE(out[i].is_primary); + CHECK(out[i].screen_left == 0); + CHECK(out[i].output_name[0] == '\0'); + } + CHECK(out[1].manufacturer_id == ACR_RAW); + CHECK(out[1].pixel_width == 3840); + CHECK(out[1].native_width == 3840); + CHECK(std::string(out[1].connector) == "HDMI-A-1"); +} + +namespace { + +void +write_file(const std::string &path, const void *data, size_t len) +{ + FILE *f = std::fopen(path.c_str(), "wb"); + REQUIRE(f != nullptr); + if (len > 0) { + REQUIRE(std::fwrite(data, 1, len, f) == len); + } + std::fclose(f); +} + +void +write_text(const std::string &path, const char *text) +{ + write_file(path, text, std::strlen(text)); +} + +} // namespace + +TEST_CASE("sysfs: reads connected connectors, their EDID, enabled state and modes") +{ + char tmpl[] = "/tmp/dxr_drm_fixture_XXXXXX"; + REQUIRE(mkdtemp(tmpl) != nullptr); + const std::string root = tmpl; + + const std::string hdmi = root + "/card1-HDMI-A-1"; + const std::string dp = root + "/card1-DP-1"; + const std::string edp = root + "/card1-eDP-1"; + REQUIRE(mkdir(hdmi.c_str(), 0700) == 0); + REQUIRE(mkdir(dp.c_str(), 0700) == 0); + REQUIRE(mkdir(edp.c_str(), 0700) == 0); + REQUIRE(mkdir((root + "/card1").c_str(), 0700) == 0); + write_text(root + "/version", "drm 1.1.0\n"); + + write_text(hdmi + "/status", "connected\n"); + write_text(hdmi + "/enabled", "enabled\n"); + write_file(hdmi + "/edid", DS1_EDID, sizeof(DS1_EDID)); + write_text(hdmi + "/modes", "3840x2160\n3840x2160\n1920x1080\n"); + + write_text(dp + "/status", "disconnected\n"); + write_text(dp + "/enabled", "disabled\n"); + write_file(dp + "/edid", nullptr, 0); + + const std::vector e = edp_no_serial_edid(); + write_text(edp + "/status", "connected\n"); + write_text(edp + "/enabled", "disabled\n"); + write_file(edp + "/edid", e.data(), e.size()); + write_text(edp + "/modes", "2880x1800\n"); + + os_display_drm_connector c[8] = {}; + const uint32_t n = os_display_drm_read_connectors(root.c_str(), c, 8); + REQUIRE(n == 2); + + const os_display_drm_connector *h = std::strcmp(c[0].name, "HDMI-A-1") == 0 ? &c[0] : &c[1]; + const os_display_drm_connector *l = h == &c[0] ? &c[1] : &c[0]; + CHECK(std::string(h->name) == "HDMI-A-1"); + CHECK(h->enabled); + CHECK(h->has_edid); + CHECK(h->edid.serial_number == 0x322EF05Eu); + REQUIRE(h->mode_count == 3); + CHECK(h->mode_w[0] == 3840); + CHECK(h->mode_h[2] == 1080); + CHECK(std::string(l->name) == "eDP-1"); + CHECK_FALSE(l->enabled); + CHECK(l->has_edid); + + // Clean up the fixture tree. + for (const std::string &d : {hdmi, dp, edp}) { + for (const char *f : {"/status", "/enabled", "/edid", "/modes"}) { + (void)unlink((d + f).c_str()); + } + (void)rmdir(d.c_str()); + } + (void)rmdir((root + "/card1").c_str()); + (void)unlink((root + "/version").c_str()); + (void)rmdir(root.c_str()); +} From 3a60d8c046fa2eacac01620e7d207bae2bc93b89 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 10:39:08 -0700 Subject: [PATCH 4/8] docs(plugin-discovery): desktop-Linux monitor enumeration and multi-plug-in claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New §3.4 covers the RandR<->DRM join rules, the DRM-only fallback, the runtime-private extras and monitor_id, POSIX claim collection from every plug-in (and the XRT_PREFERRED_PLUGIN_ID consequence), and the active plug-in's back-compat claim following its panel off-Windows. Co-Authored-By: Claude Opus 5.5 --- docs/specs/runtime/plugin-discovery.md | 76 ++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/docs/specs/runtime/plugin-discovery.md b/docs/specs/runtime/plugin-discovery.md index 5ffa9c78b..d0be49001 100644 --- a/docs/specs/runtime/plugin-discovery.md +++ b/docs/specs/runtime/plugin-discovery.md @@ -420,6 +420,82 @@ fall through on miss/failure, sticky for the process" semantics: Android exposes only the env-var read (the writable override and `dp list` are not shipped there in v1; see Non-goals above). +### 3.4 Desktop Linux: monitor enumeration and display claims (multi-screen M0) + +The per-monitor registry (§1.1, #69 / ADR-015) is populated on desktop Linux +as on Windows. Two pieces make that possible. + +**Monitor enumeration** (`os_display_edid_enumerate`, +`src/xrt/auxiliary/os/os_display_edid_linux.c`). No single source is a monitor +record: RandR knows where each monitor sits on the desktop and which one is +primary, but XWayland publishes no EDID property; DRM sysfs +(`/sys/class/drm/card*-*/{status,enabled,edid,modes}`) has the EDID and the +connector name but no desktop position. The enumerator takes the RandR +monitors from `os_display_desktop_enumerate` and ties each one to a connected +DRM connector, first rule that fires: + +1. **name** — RandR output name equals the connector name with the kernel's + subtype letter dropped (`HDMI-A-1` → `HDMI-1`). The normal case on X11 and + on Mutter's XWayland. +2. **mm** — exactly one unused connector whose EDID physical size is within + 10 mm of RandR's (EDID stores cm in the base block and mm in the detailed + timing; 340 vs 344 mm is the same panel). +3. **mode** — exactly one unused connector with the monitor's pixel size + (device mode when known, else the RandR rect) among its modes. + +Ambiguity is never guessed: a monitor no rule ties uniquely is listed with +its placement and no EDID identity. A connector is used at most once. From +the EDID the enumerator reads the manufacturer and product id (bytes 8–11), +the serial (12–15), the size in cm (21/22) and the first detailed timing's +pixels, mm and refresh. With **no reachable X server** (pure Wayland, +headless) every connected, enabled connector becomes a DRM-only record flagged +`origin_unknown` at (0, 0). The join method per monitor is logged once at INFO +(`plugin loader: monitor N … join=name|mm|mode|drm-only|none`), and +`displayxr-cli displays` prints it. + +The plug-in-facing `xrt_display_descriptor` is unchanged (no ABI change). The +connector name, mm and device mode stay runtime-side (`os_display_edid_monitor` +plus a loader side table keyed by `monitor_id`). The `monitor_id` hash adds the +connector name where the platform has one, so DRM-only records at (0, 0) stay +distinct. Windows ids are unchanged. + +**Claims from every plug-in.** `target_plugin_resolve_displays` now loads every +manifest plug-in on POSIX as a claim source (`collect_display_sources_platform`, +the twin of the Windows one): same roots and order as discovery, +`DXR_PLUGIN_EXCLUSIVE` honoured, the active plug-in reused rather than loaded +twice. The other plug-ins are claim sources only. No device is created from +them, and active-plug-in selection is unchanged. One consequence carries over +from Windows: with `XRT_PREFERRED_PLUGIN_ID=sim-display`, an installed vendor +plug-in is now loaded and probed for its claims. Use `DXR_PLUGIN_EXCLUSIVE` to +keep it out of the process (§2.2). + +**Back-compat claim for a plug-in without `probe_displays`.** Such a plug-in +gets one synthesized `EDID`-confidence claim. On Windows that claim stays on +the primary monitor. Off-Windows, the **active** plug-in's claim goes to the +monitor its panel matches once the runtime has read `get_display_info` +(`target_plugin_note_active_panel`, called by the builder at system create and +on every display-info apply). The matching uses the ADR-033 resolver's rules: +the plug-in's origin when non-zero, then the connector's device mode, then the +pixel size, with ties broken on physical size. Without this, a laptop with the +3D panel on an external connector would route the laptop screen (the primary) +to the vendor DP and the panel to the fallback, because the active plug-in +wins every monitor it claims (#1521). A plug-in that implements +`probe_displays` never reaches the synthesized path, so its own claims always +decide. `displayxr-cli displays --claims` brings the system up headlessly +first, so it reports the same placement the runtime uses. + +Example from a laptop (eDP-1, primary) with an Acer DS1 on HDMI, with +`leia-sr` active and no `probe_displays` in the plug-in yet: + +``` +monitor … 3456x2160 @ (0,0) SDC 423F 300x190 mm output=eDP-1 → sim-display FALLBACK +monitor … 3840x2160 @ (3456,0) ACR 0001 344x193 mm output=HDMI-1 → leia-sr EDID +``` + +On Linux nothing in the compositors reads the registry yet. The Vulkan +compositor uses the scalar `dp_factory_*`, so this milestone changes no +weaving. macOS and Android still enumerate no monitors. + --- ## 4. Plug-in DLL contract From e10532f6f10338230d1931db6ee8f7b6c38656dd Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 13:54:03 -0700 Subject: [PATCH 5/8] fix(linux): harden the RandR<->DRM join (X server EDID first, confirmed names, card prefix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #1851, finding 1. A bare name match could tie a monitor to the wrong connector. The NVIDIA X driver numbers outputs from 0 (DP-0, DP-1) while nvidia-drm connectors start at 1, and two GPUs can each have an HDMI-A-1. - New first rule (randr-edid): when the X server publishes the output's own EDID property (native X; XWayland does not), that is the identity. The connector is the one enabled connector with the same vendor/product/serial, or, among identical ones, the one whose name agrees. os_display_x11_read_monitor_edids reads it, with Xrandr still dlopen'd and the new entry points optional. - A name match now counts only when the connector agrees physically: its modes hold the monitor's device mode, or its EDID mm is within tolerance. The device mode is the compositor's (Mutter) mode, else the RandR rect, never the DRM-derived mode, which was itself found by name. Same-named connectors on two cards are both considered. - The connector name keeps its card prefix ("card1-HDMI-A-1"), and so does the monitor_id hash. - DRM connectors are sorted by name, so DRM-only order is stable across boots. - Connected-but-disabled connectors are excluded from the mm and mode joins. Tests: NVIDIA off-by-one naming, the same name on two cards, a disabled twin of the panel, X server EDID with and without identical connectors, and the sorted sysfs read. plugin-discovery.md §3.4 updated. Co-Authored-By: Claude Opus 5.5 --- docs/specs/runtime/plugin-discovery.md | 40 ++-- src/xrt/auxiliary/os/os_display_desktop_x11.c | 84 ++++++++ src/xrt/auxiliary/os/os_display_edid.h | 14 +- src/xrt/auxiliary/os/os_display_edid_linux.c | 192 +++++++++++++++--- src/xrt/auxiliary/os/os_display_edid_linux.h | 68 ++++++- src/xrt/targets/common/target_plugin_loader.c | 2 +- tests/tests_aux_display_edid_linux.cpp | 150 ++++++++++++-- 7 files changed, 470 insertions(+), 80 deletions(-) diff --git a/docs/specs/runtime/plugin-discovery.md b/docs/specs/runtime/plugin-discovery.md index d0be49001..f9093c508 100644 --- a/docs/specs/runtime/plugin-discovery.md +++ b/docs/specs/runtime/plugin-discovery.md @@ -434,30 +434,46 @@ connector name but no desktop position. The enumerator takes the RandR monitors from `os_display_desktop_enumerate` and ties each one to a connected DRM connector, first rule that fires: -1. **name** — RandR output name equals the connector name with the kernel's - subtype letter dropped (`HDMI-A-1` → `HDMI-1`). The normal case on X11 and - on Mutter's XWayland. -2. **mm** — exactly one unused connector whose EDID physical size is within - 10 mm of RandR's (EDID stores cm in the base block and mm in the detailed - timing; 340 vs 344 mm is the same panel). -3. **mode** — exactly one unused connector with the monitor's pixel size - (device mode when known, else the RandR rect) among its modes. +0. **randr-edid** — the X server publishes the output's own `EDID` property + (native X does; XWayland does not). That is the monitor's identity. The + connector is the one enabled connector carrying the same EDID (vendor, + product, serial), or, among several identical ones, the one whose name + agrees. +1. **name** — RandR output name equals the connector name with the card prefix + and the kernel's subtype letter dropped (`card1-HDMI-A-1` → `HDMI-1`), and + the connector agrees physically: its modes hold the monitor's device mode, + or its EDID size is within 10 mm of RandR's. A bare name is not enough: the + NVIDIA X driver numbers outputs from 0 (`DP-0`) while nvidia-drm numbers + connectors from 1, and two GPUs can each have an `HDMI-A-1`. This is the + normal case on Mutter's XWayland. +2. **mm** — exactly one unused, enabled connector whose EDID physical size is + within 10 mm of RandR's (EDID stores cm in the base block and mm in the + detailed timing; 340 vs 344 mm is the same panel). +3. **mode** — exactly one unused, enabled connector with the monitor's device + mode among its modes. + +"Device mode" is the compositor's current mode when Mutter reports it, else +the RandR rect. It is never the DRM-derived mode, which was itself found by +connector name. Ambiguity is never guessed: a monitor no rule ties uniquely is listed with -its placement and no EDID identity. A connector is used at most once. From +its placement and no EDID identity. A connector is used at most once, and a +connected-but-disabled connector never joins by mm or mode. The record keeps +the card prefix (`card1-HDMI-A-1`). Connectors are read in name order, so +DRM-only records come out in the same order on every boot. From the EDID the enumerator reads the manufacturer and product id (bytes 8–11), the serial (12–15), the size in cm (21/22) and the first detailed timing's pixels, mm and refresh. With **no reachable X server** (pure Wayland, headless) every connected, enabled connector becomes a DRM-only record flagged `origin_unknown` at (0, 0). The join method per monitor is logged once at INFO -(`plugin loader: monitor N … join=name|mm|mode|drm-only|none`), and +(`plugin loader: monitor N … join=randr-edid|name|mm|mode|drm-only|none`), and `displayxr-cli displays` prints it. The plug-in-facing `xrt_display_descriptor` is unchanged (no ABI change). The connector name, mm and device mode stay runtime-side (`os_display_edid_monitor` plus a loader side table keyed by `monitor_id`). The `monitor_id` hash adds the -connector name where the platform has one, so DRM-only records at (0, 0) stay -distinct. Windows ids are unchanged. +connector name (with its card prefix) where the platform has one, so DRM-only +records at (0, 0) stay distinct. Windows ids are unchanged. **Claims from every plug-in.** `target_plugin_resolve_displays` now loads every manifest plug-in on POSIX as a claim source (`collect_display_sources_platform`, diff --git a/src/xrt/auxiliary/os/os_display_desktop_x11.c b/src/xrt/auxiliary/os/os_display_desktop_x11.c index bfb935232..fea00b4e3 100644 --- a/src/xrt/auxiliary/os/os_display_desktop_x11.c +++ b/src/xrt/auxiliary/os/os_display_desktop_x11.c @@ -47,6 +47,7 @@ #include "os_display_desktop.h" #include "os_display_connector_linux.h" +#include "os_display_edid_linux.h" #include #include @@ -88,6 +89,23 @@ struct x11_fns struct os_xrr_monitor_info *(*XRRGetMonitors)(os_x_display *, os_x_window, int, int *); void (*XRRFreeMonitors)(struct os_xrr_monitor_info *); + + // Optional: only the EDID reader uses these, and a missing one just means + // "no EDID from the X server". + os_x_atom (*XInternAtom)(os_x_display *, const char *, int); + int (*XRRGetOutputProperty)(os_x_display *, + unsigned long, + os_x_atom, + long, + long, + int, + int, + os_x_atom, + os_x_atom *, + int *, + unsigned long *, + unsigned long *, + unsigned char **); }; static bool @@ -126,6 +144,9 @@ x11_fns_load(struct x11_fns *f) #undef LOAD + *(void **)(&f->XInternAtom) = dlsym(f->lib_x11, "XInternAtom"); + *(void **)(&f->XRRGetOutputProperty) = dlsym(f->lib_xrandr, "XRRGetOutputProperty"); + return true; fail: @@ -255,6 +276,69 @@ query_monitors(struct os_display_desktop_info *out_infos, uint32_t max_infos) return written; } +uint32_t +os_display_x11_read_monitor_edids(struct os_display_randr_edid *out, uint32_t max) +{ + if (out == NULL || max == 0) { + return 0; + } + memset(out, 0, sizeof(*out) * max); + + struct x11_fns f; + if (!x11_fns_load(&f)) { + return 0; + } + uint32_t written = 0; + os_x_display *dpy = (f.XInternAtom != NULL && f.XRRGetOutputProperty != NULL) ? f.XOpenDisplay(NULL) : NULL; + if (dpy != NULL) { + // only_if_exists: a server that never published an EDID property + // has no such atom, and creating one would be a side effect. + const os_x_atom edid_atom = f.XInternAtom(dpy, "EDID", 1); + int count = 0; + struct os_xrr_monitor_info *mons = + edid_atom != 0 ? f.XRRGetMonitors(dpy, f.XDefaultRootWindow(dpy), 1, &count) : NULL; + for (int i = 0; mons != NULL && i < count && written < max; i++) { + const struct os_xrr_monitor_info *m = &mons[i]; + if (m->width <= 0 || m->height <= 0) { + continue; // same filter as query_monitors, so names line up + } + struct os_display_randr_edid *o = &out[written++]; + char *name = f.XGetAtomName(dpy, m->name); + if (name != NULL) { + (void)snprintf(o->name, sizeof(o->name), "%s", name); + f.XFree(name); + } + if (m->noutput < 1 || m->outputs == NULL) { + continue; + } + const unsigned long output = ((const unsigned long *)m->outputs)[0]; + os_x_atom actual_type = 0; + int actual_format = 0; + unsigned long nitems = 0, bytes_after = 0; + unsigned char *prop = NULL; + // Length is in 32-bit units: 64 = 256 bytes. + if (f.XRRGetOutputProperty(dpy, output, edid_atom, 0, OS_DISPLAY_RANDR_EDID_MAX / 4, 0, 0, + 0 /* AnyPropertyType */, &actual_type, &actual_format, &nitems, + &bytes_after, &prop) == 0 /* Success */ + && prop != NULL && actual_format == 8 && nitems >= 128) { + const uint32_t n = + nitems > OS_DISPLAY_RANDR_EDID_MAX ? OS_DISPLAY_RANDR_EDID_MAX : (uint32_t)nitems; + memcpy(o->edid, prop, n); + o->len = n; + } + if (prop != NULL) { + f.XFree(prop); + } + } + if (mons != NULL) { + f.XRRFreeMonitors(mons); + } + f.XCloseDisplay(dpy); + } + x11_fns_unload(&f); + return written; +} + uint32_t os_display_desktop_enumerate(struct os_display_desktop_info *out_infos, uint32_t max_infos) { diff --git a/src/xrt/auxiliary/os/os_display_edid.h b/src/xrt/auxiliary/os/os_display_edid.h index 6fea78390..979a542ab 100644 --- a/src/xrt/auxiliary/os/os_display_edid.h +++ b/src/xrt/auxiliary/os/os_display_edid.h @@ -49,11 +49,12 @@ enum os_edid_diag_error */ enum os_display_edid_join { - OS_EDID_JOIN_NONE = 0, //!< No EDID source tied to this monitor. - OS_EDID_JOIN_NAME = 1, //!< RandR output name == normalised DRM connector name. - OS_EDID_JOIN_MM = 2, //!< Unique match on physical size (mm, with tolerance). - OS_EDID_JOIN_MODE = 3, //!< Unique match on pixel mode. - OS_EDID_JOIN_DRM_ONLY = 4, //!< No placement source (no X server): DRM record alone. + OS_EDID_JOIN_NONE = 0, //!< No EDID source tied to this monitor. + OS_EDID_JOIN_NAME = 1, //!< RandR output name == normalised DRM connector name. + OS_EDID_JOIN_MM = 2, //!< Unique match on physical size (mm, with tolerance). + OS_EDID_JOIN_MODE = 3, //!< Unique match on pixel mode. + OS_EDID_JOIN_DRM_ONLY = 4, //!< No placement source (no X server): DRM record alone. + OS_EDID_JOIN_RANDR_EDID = 5, //!< The X server's own EDID output property. }; /*! @@ -84,7 +85,7 @@ struct os_display_edid_monitor uint32_t physical_height_mm; //!< As above, bytes 22 x 10. uint32_t native_width; //!< The connector's device mode (may differ from pixel_width uint32_t native_height; //!< under a scaled X screen); 0 = unknown. - char connector[32]; //!< DRM connector, e.g. "HDMI-A-1"; "" = unknown. + char connector[32]; //!< DRM connector, e.g. "card1-HDMI-A-1"; "" = unknown. char output_name[32]; //!< RandR output name, e.g. "HDMI-1"; "" = none. bool origin_unknown; //!< screen_left/top are NOT a desktop position (DRM-only record). enum os_display_edid_join join; //!< How the EDID was tied to the placement record. @@ -151,6 +152,7 @@ os_display_edid_join_str(enum os_display_edid_join join) case OS_EDID_JOIN_MM: return "mm"; case OS_EDID_JOIN_MODE: return "mode"; case OS_EDID_JOIN_DRM_ONLY: return "drm-only"; + case OS_EDID_JOIN_RANDR_EDID: return "randr-edid"; case OS_EDID_JOIN_NONE: default: return "none"; } diff --git a/src/xrt/auxiliary/os/os_display_edid_linux.c b/src/xrt/auxiliary/os/os_display_edid_linux.c index d31b89d76..f0276bbc8 100644 --- a/src/xrt/auxiliary/os/os_display_edid_linux.c +++ b/src/xrt/auxiliary/os/os_display_edid_linux.c @@ -17,6 +17,7 @@ #include "os_display_connector_linux.h" #include +#include #include #include #include @@ -123,6 +124,13 @@ read_first_line(const char *path, char *buf, size_t size) return ok; } +static int +compare_connector_name(const void *a, const void *b) +{ + return strcmp(((const struct os_display_drm_connector *)a)->name, + ((const struct os_display_drm_connector *)b)->name); +} + uint32_t os_display_drm_read_connectors(const char *root, struct os_display_drm_connector *out, uint32_t max) { @@ -155,7 +163,8 @@ os_display_drm_read_connectors(const char *root, struct os_display_drm_connector struct os_display_drm_connector *c = &out[count]; memset(c, 0, sizeof(*c)); - (void)snprintf(c->name, sizeof(c->name), "%s", dash + 1); + // Keep the card prefix: two GPUs can each have an HDMI-A-1. + (void)snprintf(c->name, sizeof(c->name), "%.31s", e->d_name); // Absent on old kernels: treat as enabled, the status said connected. (void)snprintf(path, sizeof(path), "%s/%s/enabled", root, e->d_name); @@ -187,6 +196,10 @@ os_display_drm_read_connectors(const char *root, struct os_display_drm_connector count++; } closedir(dir); + + // readdir order is filesystem order; DRM-only records and the + // first-candidate tie-breaks must not depend on it. + qsort(out, count, sizeof(out[0]), compare_connector_name); return count; } @@ -232,21 +245,68 @@ drm_has_mode(const struct os_display_drm_connector *c, uint32_t w, uint32_t h) return false; } +//! The connector name the way RandR spells it: no card prefix, no subtype +//! letter ("card1-HDMI-A-1" -> "HDMI-1"). +static void +drm_name_for_randr(const struct os_display_drm_connector *c, char *out, size_t size) +{ + const char *n = c->name; + if (strncmp(n, "card", 4) == 0) { + const char *dash = strchr(n, '-'); + if (dash != NULL) { + n = dash + 1; + } + } + os_display_connector_normalise(n, out, size); +} + +/*! + * The monitor's device mode, from a source that is independent of the DRM + * name match: the compositor's current mode, else the RandR rect. The + * DRM-derived native mode is NOT used — it was itself found by connector + * name, so checking a name match against it would be circular. + */ +static void +randr_device_mode(const struct os_display_desktop_info *r, uint32_t *w, uint32_t *h) +{ + if (r->native_source == OS_DISPLAY_NATIVE_SOURCE_COMPOSITOR && r->native_width > 0 && r->native_height > 0) { + *w = r->native_width; + *h = r->native_height; + } else { + *w = r->width; + *h = r->height; + } +} + +static bool +edid_same_identity(const struct os_display_edid_parsed *a, const struct os_display_edid_parsed *b) +{ + return a->manufacturer_id == b->manufacturer_id && a->product_id == b->product_id && + a->serial_number == b->serial_number; +} + +//! Copy an EDID's identity (ids, serial, mm) onto a record. +static void +apply_edid(struct os_display_edid_monitor *m, const struct os_display_edid_parsed *e) +{ + m->manufacturer_id = e->manufacturer_id; + m->product_id = e->product_id; + m->serial_number = e->serial_number; + uint32_t w_mm = 0, h_mm = 0; + os_display_edid_parsed_mm(e, &w_mm, &h_mm); + if (w_mm > 0 && h_mm > 0) { + m->physical_width_mm = w_mm; + m->physical_height_mm = h_mm; + } +} + //! Copy a connector's identity (EDID, mm, connector name) onto a record. static void apply_drm(struct os_display_edid_monitor *m, const struct os_display_drm_connector *c) { (void)snprintf(m->connector, sizeof(m->connector), "%s", c->name); if (c->has_edid) { - m->manufacturer_id = c->edid.manufacturer_id; - m->product_id = c->edid.product_id; - m->serial_number = c->edid.serial_number; - uint32_t w_mm = 0, h_mm = 0; - os_display_edid_parsed_mm(&c->edid, &w_mm, &h_mm); - if (w_mm > 0 && h_mm > 0) { - m->physical_width_mm = w_mm; - m->physical_height_mm = h_mm; - } + apply_edid(m, &c->edid); } if (m->native_width == 0 && c->mode_count > 0) { // The kernel lists the preferred mode first; the RandR rect when it @@ -268,6 +328,7 @@ apply_drm(struct os_display_edid_monitor *m, const struct os_display_drm_connect uint32_t os_display_edid_linux_join(const struct os_display_desktop_info *randr, + const struct os_display_randr_identity *randr_id, uint32_t randr_count, const struct os_display_drm_connector *drm, uint32_t drm_count, @@ -307,29 +368,69 @@ os_display_edid_linux_join(const struct os_display_desktop_info *randr, return n; } - const uint32_t n = randr_count < max ? randr_count : max; + uint32_t rn = randr_count < max ? randr_count : max; + if (rn > OS_DISPLAY_DESKTOP_MAX_MONITORS) { + rn = OS_DISPLAY_DESKTOP_MAX_MONITORS; + } bool used[OS_DISPLAY_DRM_MAX_CONNECTORS] = {0}; - int32_t pick[OS_DISPLAY_EDID_MAX_MONITORS > OS_DISPLAY_DESKTOP_MAX_MONITORS ? OS_DISPLAY_EDID_MAX_MONITORS - : OS_DISPLAY_DESKTOP_MAX_MONITORS]; - enum os_display_edid_join how[sizeof(pick) / sizeof(pick[0])]; - const uint32_t cap = (uint32_t)(sizeof(pick) / sizeof(pick[0])); - const uint32_t rn = n < cap ? n : cap; + int32_t pick[OS_DISPLAY_DESKTOP_MAX_MONITORS]; + enum os_display_edid_join how[OS_DISPLAY_DESKTOP_MAX_MONITORS]; for (uint32_t i = 0; i < rn; i++) { pick[i] = -1; how[i] = OS_EDID_JOIN_NONE; } - // Pass 1: names. + char rname[OS_DISPLAY_DESKTOP_MAX_MONITORS][64]; for (uint32_t i = 0; i < rn; i++) { - char rname[64]; - os_display_connector_normalise(randr[i].device_name, rname, sizeof(rname)); - if (rname[0] == '\0') { + os_display_connector_normalise(randr[i].device_name, rname[i], sizeof(rname[i])); + } + + // Pass 0: the X server's own EDID. The identity is settled; find the + // connector carrying the same EDID, by name among several identical ones. + for (uint32_t i = 0; i < rn; i++) { + if (randr_id == NULL || !randr_id[i].valid) { continue; } + how[i] = OS_EDID_JOIN_RANDR_EDID; + int32_t found = -1; + uint32_t hits = 0; + int32_t by_name = -1; for (uint32_t j = 0; j < drm_count; j++) { + if (used[j] || !drm[j].enabled || !drm[j].has_edid || + !edid_same_identity(&drm[j].edid, &randr_id[i].edid)) { + continue; + } + found = (int32_t)j; + hits++; char dname[64]; - os_display_connector_normalise(drm[j].name, dname, sizeof(dname)); - if (!used[j] && strcmp(rname, dname) == 0) { + drm_name_for_randr(&drm[j], dname, sizeof(dname)); + if (rname[i][0] != '\0' && strcmp(rname[i], dname) == 0) { + by_name = (int32_t)j; + } + } + const int32_t take = hits == 1 ? found : by_name; + if (take >= 0) { + pick[i] = take; + used[take] = true; + } + } + + // Pass 1: names, confirmed by the device mode or the physical size. + for (uint32_t i = 0; i < rn; i++) { + if (how[i] != OS_EDID_JOIN_NONE || rname[i][0] == '\0') { + continue; + } + uint32_t w = 0, h = 0; + randr_device_mode(&randr[i], &w, &h); + for (uint32_t j = 0; j < drm_count; j++) { + char dname[64]; + drm_name_for_randr(&drm[j], dname, sizeof(dname)); + if (used[j] || strcmp(rname[i], dname) != 0) { + continue; + } + if (drm_has_mode(&drm[j], w, h) || + drm_matches_mm(&drm[j], randr[i].physical_width_mm, randr[i].physical_height_mm)) { + // Same name on two cards: the one that agrees physically. pick[i] = (int32_t)j; how[i] = OS_EDID_JOIN_NAME; used[j] = true; @@ -338,15 +439,16 @@ os_display_edid_linux_join(const struct os_display_desktop_info *randr, } } - // Pass 2: physical size, only when exactly one unused connector fits. + // Pass 2: physical size, only when exactly one unused enabled connector fits. for (uint32_t i = 0; i < rn; i++) { - if (pick[i] >= 0 || randr[i].physical_width_mm == 0 || randr[i].physical_height_mm == 0) { + if (how[i] != OS_EDID_JOIN_NONE || randr[i].physical_width_mm == 0 || + randr[i].physical_height_mm == 0) { continue; } int32_t found = -1; uint32_t hits = 0; for (uint32_t j = 0; j < drm_count; j++) { - if (!used[j] && + if (!used[j] && drm[j].enabled && drm_matches_mm(&drm[j], randr[i].physical_width_mm, randr[i].physical_height_mm)) { found = (int32_t)j; hits++; @@ -359,17 +461,17 @@ os_display_edid_linux_join(const struct os_display_desktop_info *randr, } } - // Pass 3: pixel mode (device mode when known, else the RandR rect). + // Pass 3: device mode, only when exactly one unused enabled connector has it. for (uint32_t i = 0; i < rn; i++) { - if (pick[i] >= 0) { + if (how[i] != OS_EDID_JOIN_NONE) { continue; } - const uint32_t w = randr[i].native_width > 0 ? randr[i].native_width : randr[i].width; - const uint32_t h = randr[i].native_height > 0 ? randr[i].native_height : randr[i].height; + uint32_t w = 0, h = 0; + randr_device_mode(&randr[i], &w, &h); int32_t found = -1; uint32_t hits = 0; for (uint32_t j = 0; j < drm_count; j++) { - if (!used[j] && drm_has_mode(&drm[j], w, h)) { + if (!used[j] && drm[j].enabled && drm_has_mode(&drm[j], w, h)) { found = (int32_t)j; hits++; } @@ -399,6 +501,11 @@ os_display_edid_linux_join(const struct os_display_desktop_info *randr, if (pick[i] >= 0) { apply_drm(m, &drm[pick[i]]); } + // The X server's EDID wins over the connector's: it is this + // output's by construction, the connector's only by the join. + if (randr_id != NULL && randr_id[i].valid) { + apply_edid(m, &randr_id[i].edid); + } } return rn; } @@ -421,6 +528,25 @@ os_display_edid_enumerate(struct os_display_edid_list *out_list) struct os_display_desktop_info randr[OS_DISPLAY_DESKTOP_MAX_MONITORS]; const uint32_t rn = os_display_desktop_enumerate(randr, OS_DISPLAY_DESKTOP_MAX_MONITORS); + // The X server's own EDID per monitor, when it publishes one (native X). + struct os_display_randr_identity randr_id[OS_DISPLAY_DESKTOP_MAX_MONITORS]; + memset(randr_id, 0, sizeof(randr_id)); + bool have_randr_id = false; + if (rn > 0) { + struct os_display_randr_edid blobs[OS_DISPLAY_DESKTOP_MAX_MONITORS]; + const uint32_t bn = os_display_x11_read_monitor_edids(blobs, OS_DISPLAY_DESKTOP_MAX_MONITORS); + for (uint32_t i = 0; i < rn; i++) { + for (uint32_t k = 0; k < bn; k++) { + if (blobs[k].len > 0 && strcmp(blobs[k].name, randr[i].device_name) == 0) { + randr_id[i].valid = + os_display_edid_parse(blobs[k].edid, blobs[k].len, &randr_id[i].edid); + have_randr_id = have_randr_id || randr_id[i].valid; + break; + } + } + } + } + struct os_display_drm_connector drm[OS_DISPLAY_DRM_MAX_CONNECTORS]; const uint32_t dn = os_display_drm_read_connectors(DRM_SYSFS_ROOT, drm, OS_DISPLAY_DRM_MAX_CONNECTORS); @@ -430,14 +556,14 @@ os_display_edid_enumerate(struct os_display_edid_list *out_list) } } - out_list->count = os_display_edid_linux_join(rn > 0 ? randr : NULL, rn, drm, dn, out_list->monitors, - OS_DISPLAY_EDID_MAX_MONITORS); + out_list->count = os_display_edid_linux_join(rn > 0 ? randr : NULL, have_randr_id ? randr_id : NULL, rn, drm, + dn, out_list->monitors, OS_DISPLAY_EDID_MAX_MONITORS); // diag_gdi_count is "monitors the placement source saw" — RandR here. out_list->diag_gdi_count = rn; if (out_list->count == 0) { out_list->diag_error = OS_EDID_DIAG_NO_GDI_MONITORS; - } else if (out_list->diag_edid_read_count == 0) { + } else if (out_list->diag_edid_read_count == 0 && !have_randr_id) { out_list->diag_error = OS_EDID_DIAG_NO_EDID_DATA; } diff --git a/src/xrt/auxiliary/os/os_display_edid_linux.h b/src/xrt/auxiliary/os/os_display_edid_linux.h index 76351feb9..65363ba7e 100644 --- a/src/xrt/auxiliary/os/os_display_edid_linux.h +++ b/src/xrt/auxiliary/os/os_display_edid_linux.h @@ -16,18 +16,31 @@ * sysfs has the EDID blob and the connector name, but no desktop position. So * each RandR monitor is tied to one DRM connector, in this order: * + * 0. **randr-edid** — the X server publishes the output's own `EDID` property + * (native X does; XWayland does not). That IS the monitor's identity; the + * DRM connector is then the one unused, enabled connector carrying the same + * EDID (vendor, product, serial), when exactly one does — or, among several + * identical ones, the one whose name agrees. * 1. **name** — the RandR output name equals the DRM connector name once the - * kernel's subtype letter is dropped ("HDMI-A-1" -> "HDMI-1"). This is the - * normal case on X11 and on Mutter's XWayland. - * 2. **mm** — exactly one unused connector whose EDID physical size is within - * @ref OS_DISPLAY_EDID_JOIN_MM_TOLERANCE of RandR's mm. - * 3. **mode** — exactly one unused connector that has the monitor's pixel size - * (device mode when known, else the RandR rect) among its modes. + * card prefix and the kernel's subtype letter are dropped + * ("card1-HDMI-A-1" -> "HDMI-1"), AND the connector agrees on something + * physical: its mode list holds the monitor's device mode, or its EDID size + * is within tolerance of RandR's mm. A bare name is not enough: the NVIDIA + * X driver numbers outputs from 0 (DP-0, DP-1) while nvidia-drm numbers + * connectors from 1, and two GPUs can each have an HDMI-A-1. + * 2. **mm** — exactly one unused, enabled connector whose EDID physical size + * is within @ref OS_DISPLAY_EDID_JOIN_MM_TOLERANCE of RandR's mm. + * 3. **mode** — exactly one unused, enabled connector that has the monitor's + * device mode among its modes. + * + * "Device mode" is the compositor's current mode when Mutter reported it, + * else the RandR rect — never the DRM-derived mode, which itself came from a + * name match and would make rule 1 circular. * * A RandR monitor nothing joins is still listed, with no EDID identity. With * no RandR monitors at all (no X server: pure Wayland, or headless) every * connected, enabled connector becomes a DRM-only record whose desktop origin - * is unknown. + * is unknown, in connector-name order so the order is stable across boots. */ #pragma once @@ -93,7 +106,7 @@ os_display_edid_parsed_mm(const struct os_display_edid_parsed *p, uint32_t *out_ */ struct os_display_drm_connector { - char name[32]; //!< Kernel name without the card prefix, e.g. "HDMI-A-1". + char name[32]; //!< Kernel name WITH the card prefix, e.g. "card1-HDMI-A-1". bool enabled; //!< `enabled` reads "enabled" (or the file is absent). bool has_edid; //!< `edid` parsed. struct os_display_edid_parsed edid; @@ -104,17 +117,30 @@ struct os_display_drm_connector /*! * Read every CONNECTED connector under @p root (normally `/sys/class/drm`; - * a directory of `card-/{status,enabled,edid,modes}` in tests). + * a directory of `card-/{status,enabled,edid,modes}` in tests), + * sorted by name so the order does not depend on readdir. * * @return the number written to @p out. */ uint32_t os_display_drm_read_connectors(const char *root, struct os_display_drm_connector *out, uint32_t max); +/*! + * What the X server itself says a RandR monitor is: the parsed `EDID` output + * property, when it publishes one. + */ +struct os_display_randr_identity +{ + bool valid; + struct os_display_edid_parsed edid; +}; + /*! * The join described in the file comment. Pure: no I/O, no logging. * * @param randr RandR monitors (may be NULL when @p randr_count is 0). + * @param randr_id Per-RandR-monitor EDID from the X server, aligned with + * @p randr; NULL when none was read. * @param drm Connected DRM connectors. * @param[out] out One record per RandR monitor, or per enabled connector * when there is no RandR monitor. @@ -122,12 +148,36 @@ os_display_drm_read_connectors(const char *root, struct os_display_drm_connector */ uint32_t os_display_edid_linux_join(const struct os_display_desktop_info *randr, + const struct os_display_randr_identity *randr_id, uint32_t randr_count, const struct os_display_drm_connector *drm, uint32_t drm_count, struct os_display_edid_monitor *out, uint32_t max); +//! Largest EDID blob kept from the X server (base block + one extension). +#define OS_DISPLAY_RANDR_EDID_MAX 256 + +/*! + * One monitor's `EDID` output property as the X server publishes it. + */ +struct os_display_randr_edid +{ + char name[64]; //!< RandR monitor name, as in os_display_desktop_info::device_name. + uint8_t edid[OS_DISPLAY_RANDR_EDID_MAX]; + uint32_t len; //!< 0 = the server publishes no EDID for this monitor. +}; + +/*! + * Read the `EDID` property of each RandR monitor's first output + * (implemented in os_display_desktop_x11.c, Xrandr dlopen'd). Native X + * servers publish it; XWayland does not, and then every entry has len 0. + * + * @return the number of monitors written, 0 without an X server. + */ +uint32_t +os_display_x11_read_monitor_edids(struct os_display_randr_edid *out, uint32_t max); + #ifdef __cplusplus } #endif diff --git a/src/xrt/targets/common/target_plugin_loader.c b/src/xrt/targets/common/target_plugin_loader.c index f8facf4e6..607718cfc 100644 --- a/src/xrt/targets/common/target_plugin_loader.c +++ b/src/xrt/targets/common/target_plugin_loader.c @@ -3173,7 +3173,7 @@ monitor_id_from_edid(uint16_t mfr, uint16_t product, int32_t left, int32_t top, h ^= (uint64_t)bytes[i]; h *= prime; } - // The platform's connector key (desktop Linux: "HDMI-A-1"), when there is + // The platform's connector key (desktop Linux: "card1-HDMI-A-1"), when there is // one. Empty on Windows, so ids there are unchanged. It separates two // identical panels that share a position — every DRM-only record (no X // server) sits at (0, 0). diff --git a/tests/tests_aux_display_edid_linux.cpp b/tests/tests_aux_display_edid_linux.cpp index 876a298c4..0d497c806 100644 --- a/tests/tests_aux_display_edid_linux.cpp +++ b/tests/tests_aux_display_edid_linux.cpp @@ -104,17 +104,19 @@ box_randr(os_display_desktop_info out[2], const char *n0 = "eDP-1", const char * out[0] = randr_mon(n0, 0, 3456, 2160, 300, 190, true); out[0].native_width = 2880; out[0].native_height = 1800; + out[0].native_source = OS_DISPLAY_NATIVE_SOURCE_COMPOSITOR; out[1] = randr_mon(n1, 3456, 3840, 2160, 340, 190, false); out[1].native_width = 3840; out[1].native_height = 2160; + out[1].native_source = OS_DISPLAY_NATIVE_SOURCE_COMPOSITOR; } void box_drm(os_display_drm_connector out[2]) { const std::vector edp = edp_no_serial_edid(); - out[0] = drm_conn("eDP-1", edp.data(), edp.size(), 2880, 1800); - out[1] = drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160); + out[0] = drm_conn("card1-eDP-1", edp.data(), edp.size(), 2880, 1800); + out[1] = drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160); } } // namespace @@ -192,13 +194,13 @@ TEST_CASE("join: RandR output names tie to DRM connectors by name") box_drm(drm); os_display_edid_monitor out[4] = {}; - REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 4) == 2); + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 4) == 2); CHECK(out[0].join == OS_EDID_JOIN_NAME); CHECK(out[0].manufacturer_id == SDC_RAW); CHECK(out[0].serial_number == 0); CHECK(out[0].is_primary); - CHECK(std::string(out[0].connector) == "eDP-1"); + CHECK(std::string(out[0].connector) == "card1-eDP-1"); CHECK(out[0].physical_width_mm == 300); CHECK(out[0].native_width == 2880); CHECK(out[0].refresh_hz == 0); // no detailed timing to read it from @@ -207,7 +209,7 @@ TEST_CASE("join: RandR output names tie to DRM connectors by name") CHECK(out[1].manufacturer_id == ACR_RAW); CHECK(out[1].product_id == 0x0001); CHECK(out[1].serial_number == 0x322EF05Eu); - CHECK(std::string(out[1].connector) == "HDMI-A-1"); + CHECK(std::string(out[1].connector) == "card1-HDMI-A-1"); CHECK(std::string(out[1].output_name) == "HDMI-1"); CHECK(out[1].screen_left == 3456); CHECK(out[1].pixel_width == 3840); @@ -226,7 +228,7 @@ TEST_CASE("join: unrelated output names fall back to physical size") box_drm(drm); os_display_edid_monitor out[2] = {}; - REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 2) == 2); CHECK(out[0].join == OS_EDID_JOIN_MM); CHECK(out[0].manufacturer_id == SDC_RAW); CHECK(out[1].join == OS_EDID_JOIN_MM); // RandR 340x190 vs DTD 344x193: within tolerance @@ -245,7 +247,7 @@ TEST_CASE("join: no names and no mm fall back to the pixel mode") box_drm(drm); os_display_edid_monitor out[2] = {}; - REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 2) == 2); CHECK(out[0].join == OS_EDID_JOIN_MODE); // device mode 2880x1800 CHECK(out[0].manufacturer_id == SDC_RAW); CHECK(out[1].join == OS_EDID_JOIN_MODE); @@ -259,12 +261,12 @@ TEST_CASE("join: two identical panels with no name match stay unjoined, never gu randr_mon("XWAYLAND1", 3840, 3840, 2160, 340, 190, false), }; os_display_drm_connector drm[2] = { - drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), - drm_conn("HDMI-A-2", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + drm_conn("card1-HDMI-A-2", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), }; os_display_edid_monitor out[2] = {}; - REQUIRE(os_display_edid_linux_join(randr, 2, drm, 2, out, 2) == 2); + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 2) == 2); CHECK(out[0].join == OS_EDID_JOIN_NONE); CHECK(out[0].manufacturer_id == 0); CHECK(out[1].join == OS_EDID_JOIN_NONE); @@ -279,10 +281,10 @@ TEST_CASE("join: a connector is used at most once") randr_mon("HDMI-1", 0, 3840, 2160, 340, 190, true), randr_mon("XWAYLAND1", 3840, 3840, 2160, 340, 190, false), }; - os_display_drm_connector drm[1] = {drm_conn("HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160)}; + os_display_drm_connector drm[1] = {drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160)}; os_display_edid_monitor out[2] = {}; - REQUIRE(os_display_edid_linux_join(randr, 2, drm, 1, out, 2) == 2); + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 1, out, 2) == 2); CHECK(out[0].join == OS_EDID_JOIN_NAME); CHECK(out[1].join == OS_EDID_JOIN_NONE); } @@ -291,10 +293,10 @@ TEST_CASE("join: no X server gives DRM-only records with an unknown origin") { os_display_drm_connector drm[3]; box_drm(drm); - drm[2] = drm_conn("DP-1", DS1_EDID, sizeof(DS1_EDID), 1920, 1080, /*enabled=*/false); + drm[2] = drm_conn("card1-DP-1", DS1_EDID, sizeof(DS1_EDID), 1920, 1080, /*enabled=*/false); os_display_edid_monitor out[4] = {}; - REQUIRE(os_display_edid_linux_join(nullptr, 0, drm, 3, out, 4) == 2); // disabled DP-1 skipped + REQUIRE(os_display_edid_linux_join(nullptr, nullptr, 0, drm, 3, out, 4) == 2); // disabled DP-1 skipped for (int i = 0; i < 2; i++) { CHECK(out[i].join == OS_EDID_JOIN_DRM_ONLY); CHECK(out[i].origin_unknown); @@ -305,7 +307,116 @@ TEST_CASE("join: no X server gives DRM-only records with an unknown origin") CHECK(out[1].manufacturer_id == ACR_RAW); CHECK(out[1].pixel_width == 3840); CHECK(out[1].native_width == 3840); - CHECK(std::string(out[1].connector) == "HDMI-A-1"); + CHECK(std::string(out[1].connector) == "card1-HDMI-A-1"); +} + +TEST_CASE("join: NVIDIA's off-by-one output names do not tie to the wrong connector") +{ + // The NVIDIA X driver names outputs from 0, nvidia-drm from 1. RandR DP-1 + // is really the panel on card0-DP-2; card0-DP-1 is a different monitor. + const std::vector edp = edp_no_serial_edid(); + os_display_desktop_info randr[2] = { + randr_mon("DP-0", 0, 2880, 1800, 300, 190, true), + randr_mon("DP-1", 2880, 3840, 2160, 340, 190, false), + }; + os_display_drm_connector drm[2] = { + drm_conn("card0-DP-1", edp.data(), edp.size(), 2880, 1800), + drm_conn("card0-DP-2", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + }; + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 2) == 2); + // RandR DP-1 must NOT take card0-DP-1 (wrong mode, wrong mm): it falls + // through to the mm rule and finds the DS1 on card0-DP-2. + CHECK(out[1].manufacturer_id == ACR_RAW); + CHECK(std::string(out[1].connector) == "card0-DP-2"); + CHECK(out[1].join == OS_EDID_JOIN_MM); + CHECK(out[0].manufacturer_id == SDC_RAW); + CHECK(std::string(out[0].connector) == "card0-DP-1"); +} + +TEST_CASE("join: the same connector name on two cards picks the one that agrees physically") +{ + const std::vector edp = edp_no_serial_edid(); + os_display_desktop_info randr[1] = {randr_mon("HDMI-1", 0, 3840, 2160, 340, 190, true)}; + os_display_drm_connector drm[2] = { + drm_conn("card0-HDMI-A-1", edp.data(), edp.size(), 2880, 1800), + drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + }; + + os_display_edid_monitor out[1] = {}; + REQUIRE(os_display_edid_linux_join(randr, nullptr, 1, drm, 2, out, 1) == 1); + CHECK(out[0].join == OS_EDID_JOIN_NAME); + CHECK(std::string(out[0].connector) == "card1-HDMI-A-1"); + CHECK(out[0].manufacturer_id == ACR_RAW); +} + +TEST_CASE("join: a connected but disabled connector never joins by mm or mode") +{ + os_display_desktop_info randr[1] = {randr_mon("XWAYLAND0", 0, 3840, 2160, 340, 190, true)}; + os_display_drm_connector drm[2] = { + drm_conn("card1-DP-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160, /*enabled=*/false), + drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + }; + + os_display_edid_monitor out[1] = {}; + REQUIRE(os_display_edid_linux_join(randr, nullptr, 1, drm, 2, out, 1) == 1); + // Two identical EDIDs, but only one is lit: unambiguous. + CHECK(out[0].join == OS_EDID_JOIN_MM); + CHECK(std::string(out[0].connector) == "card1-HDMI-A-1"); + + // With the enabled one gone, the disabled one is still not used. + REQUIRE(os_display_edid_linux_join(randr, nullptr, 1, drm, 1, out, 1) == 1); + CHECK(out[0].join == OS_EDID_JOIN_NONE); + CHECK(out[0].connector[0] == '\0'); +} + +TEST_CASE("join: the X server's own EDID property is the identity when published") +{ + // Native X: names that tie nothing ("default") and no mm, but the server + // publishes each output's EDID. + const std::vector edp = edp_no_serial_edid(); + os_display_desktop_info randr[2] = { + randr_mon("default0", 0, 2880, 1800, 0, 0, true), + randr_mon("default1", 2880, 3840, 2160, 0, 0, false), + }; + os_display_randr_identity ids[2] = {}; + REQUIRE(os_display_edid_parse(edp.data(), edp.size(), &ids[0].edid)); + ids[0].valid = true; + REQUIRE(os_display_edid_parse(DS1_EDID, sizeof(DS1_EDID), &ids[1].edid)); + ids[1].valid = true; + os_display_drm_connector drm[2] = { + drm_conn("card0-DP-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + drm_conn("card0-eDP-1", edp.data(), edp.size(), 2880, 1800), + }; + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, ids, 2, drm, 2, out, 2) == 2); + CHECK(out[0].join == OS_EDID_JOIN_RANDR_EDID); + CHECK(out[0].manufacturer_id == SDC_RAW); + CHECK(std::string(out[0].connector) == "card0-eDP-1"); + CHECK(out[1].join == OS_EDID_JOIN_RANDR_EDID); + CHECK(out[1].serial_number == 0x322EF05Eu); + CHECK(std::string(out[1].connector) == "card0-DP-1"); + CHECK(out[1].physical_width_mm == 344); +} + +TEST_CASE("join: X server EDID with two identical connectors uses the name to choose") +{ + os_display_desktop_info randr[1] = {randr_mon("HDMI-2", 0, 3840, 2160, 340, 190, true)}; + os_display_randr_identity ids[1] = {}; + REQUIRE(os_display_edid_parse(DS1_EDID, sizeof(DS1_EDID), &ids[0].edid)); + ids[0].valid = true; + os_display_drm_connector drm[2] = { + drm_conn("card1-HDMI-A-1", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + drm_conn("card1-HDMI-A-2", DS1_EDID, sizeof(DS1_EDID), 3840, 2160), + }; + + os_display_edid_monitor out[1] = {}; + REQUIRE(os_display_edid_linux_join(randr, ids, 1, drm, 2, out, 1) == 1); + CHECK(out[0].join == OS_EDID_JOIN_RANDR_EDID); + CHECK(std::string(out[0].connector) == "card1-HDMI-A-2"); + CHECK(out[0].manufacturer_id == ACR_RAW); } namespace { @@ -363,16 +474,17 @@ TEST_CASE("sysfs: reads connected connectors, their EDID, enabled state and mode const uint32_t n = os_display_drm_read_connectors(root.c_str(), c, 8); REQUIRE(n == 2); - const os_display_drm_connector *h = std::strcmp(c[0].name, "HDMI-A-1") == 0 ? &c[0] : &c[1]; - const os_display_drm_connector *l = h == &c[0] ? &c[1] : &c[0]; - CHECK(std::string(h->name) == "HDMI-A-1"); + // Sorted by name, card prefix kept: "card1-HDMI-A-1" < "card1-eDP-1". + const os_display_drm_connector *h = &c[0]; + const os_display_drm_connector *l = &c[1]; + CHECK(std::string(h->name) == "card1-HDMI-A-1"); CHECK(h->enabled); CHECK(h->has_edid); CHECK(h->edid.serial_number == 0x322EF05Eu); REQUIRE(h->mode_count == 3); CHECK(h->mode_w[0] == 3840); CHECK(h->mode_h[2] == 1080); - CHECK(std::string(l->name) == "eDP-1"); + CHECK(std::string(l->name) == "card1-eDP-1"); CHECK_FALSE(l->enabled); CHECK(l->has_edid); From a52111a4ee00d9b9e54fd40664ef9bb4cb3b5258 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 13:55:32 -0700 Subject: [PATCH 6/8] fix(linux): load other plug-ins for claims only when they can matter; release them with the instance Review of #1851, finding 2. On POSIX, every installed plug-in was loaded and probed as a claim source even when the outcome was already settled. With XRT_PREFERRED_PLUGIN_ID=sim-display, every in-process app dlopened the Leia plug-in and ran its SR probe against the shared DS1, and that claim-only instance was never released. - The active plug-in wins every monitor it claims (#1521), and only a different pinned plug-in outranks it. When the active plug-in claims every monitor and no other plug-in is preferred, the POSIX source set is now the active plug-in alone, so nothing else is dlopened or probed. The check is repeated on each resolve (a later descriptor set may hold a monitor it does not claim). This covers XRT_PREFERRED_PLUGIN_ID=sim-display and a sim-display-only box. - New target_plugin_release_claim_sources(), called from t_instance_destroy. It calls destroy() on every claim-only instance (never the active one), pairing with the probe() that made it, and drops the cache so the next resolve re-collects. dlopen handles stay loaded, as for every plug-in. Windows keeps its existing behaviour (all registered plug-ins are claim sources for the process lifetime); Android has no claim-only sources. Verified on ds1-linux: XRT_PREFERRED_PLUGIN_ID=sim-display `displays --claims` never loads leia-sr; the default run still resolves eDP-1 -> sim-display and HDMI-1 -> leia-sr, and selftest logs the release of sim-display at teardown. Co-Authored-By: Claude Opus 5.5 --- docs/specs/runtime/plugin-discovery.md | 22 +++- .../targets/common/target_instance_parts.h | 5 + src/xrt/targets/common/target_plugin_loader.c | 115 +++++++++++++++++- src/xrt/targets/common/target_plugin_loader.h | 12 ++ 4 files changed, 147 insertions(+), 7 deletions(-) diff --git a/docs/specs/runtime/plugin-discovery.md b/docs/specs/runtime/plugin-discovery.md index f9093c508..8155f0df2 100644 --- a/docs/specs/runtime/plugin-discovery.md +++ b/docs/specs/runtime/plugin-discovery.md @@ -480,10 +480,24 @@ manifest plug-in on POSIX as a claim source (`collect_display_sources_platform`, the twin of the Windows one): same roots and order as discovery, `DXR_PLUGIN_EXCLUSIVE` honoured, the active plug-in reused rather than loaded twice. The other plug-ins are claim sources only. No device is created from -them, and active-plug-in selection is unchanged. One consequence carries over -from Windows: with `XRT_PREFERRED_PLUGIN_ID=sim-display`, an installed vendor -plug-in is now loaded and probed for its claims. Use `DXR_PLUGIN_EXCLUSIVE` to -keep it out of the process (§2.2). +them, and active-plug-in selection is unchanged. + +Unlike Windows, POSIX loads the others **only when they could matter**. The +active plug-in wins every monitor it claims (#1521), and only a different +pinned plug-in outranks it. So when the active plug-in claims every monitor +and no other plug-in is preferred, the source set is the active plug-in alone, +and nothing else is `dlopen`ed or probed. That covers +`XRT_PREFERRED_PLUGIN_ID=sim-display` (sim-display claims every monitor), so +pinning sim-display does not load the Leia plug-in or touch the SR service. +The check is repeated on every resolve. + +Plug-in instances loaded only as claim sources are released with the +`xrt_instance` (`target_plugin_release_claim_sources`, from +`t_instance_destroy`): `destroy()` is called on each, the source cache is +dropped, and the next resolve re-collects. The `dlopen` handles stay loaded, +as for every plug-in. Windows is unchanged and keeps its claim sources for +the process lifetime. `DXR_PLUGIN_EXCLUSIVE` (§2.2) still keeps every other +plug-in out of the process entirely. **Back-compat claim for a plug-in without `probe_displays`.** Such a plug-in gets one synthesized `EDID`-confidence claim. On Windows that claim stays on diff --git a/src/xrt/targets/common/target_instance_parts.h b/src/xrt/targets/common/target_instance_parts.h index 46ceac230..c361c1580 100644 --- a/src/xrt/targets/common/target_instance_parts.h +++ b/src/xrt/targets/common/target_instance_parts.h @@ -10,6 +10,7 @@ #pragma once #include "target_lists.h" +#include "target_plugin_loader.h" #include "xrt/xrt_prober.h" #include "xrt/xrt_instance.h" @@ -85,6 +86,10 @@ t_instance_destroy(struct xrt_instance *xinst) xrt_prober_destroy(&tinst->xp); + // Plug-ins loaded only to collect per-monitor display claims (multi-screen + // M0) are released with the instance; the active plug-in stays. + target_plugin_release_claim_sources(); + #ifdef XRT_OS_ANDROID android_instance_base_cleanup(&tinst->android, xinst); #endif // XRT_OS_ANDROID diff --git a/src/xrt/targets/common/target_plugin_loader.c b/src/xrt/targets/common/target_plugin_loader.c index 607718cfc..1571bd5c8 100644 --- a/src/xrt/targets/common/target_plugin_loader.c +++ b/src/xrt/targets/common/target_plugin_loader.c @@ -589,6 +589,15 @@ struct plugin_display_source static struct plugin_display_source g_display_sources[TARGET_PLUGIN_MAX_SOURCES]; static int g_display_source_count = -1; +/*! + * POSIX: the cached source set is ONLY the active plug-in, because it claimed + * every monitor of the last resolve, so no other plug-in could have changed + * the outcome (see @ref ensure_display_sources). Re-checked on every resolve, + * since a later descriptor set may hold a monitor it does not claim. Guarded + * by @ref g_refresh_mutex. + */ +static bool g_display_sources_active_only = false; + /* * @@ -3134,6 +3143,7 @@ target_plugin_refresh_active(void) // Invalidate the display-claim source cache so the next // target_plugin_resolve_displays rebuilds it against the new winner. g_display_source_count = -1; + g_display_sources_active_only = false; } const struct xrt_plugin_iface *result = g_active_iface; @@ -3601,16 +3611,81 @@ fill_registry_entry(struct xrt_dp_registry_entry *e, e->owning_instance = src->inst; } +#if !defined(XRT_OS_WINDOWS) && !defined(XRT_OS_ANDROID) +/*! + * POSIX: does the active plug-in, on its own, already decide every monitor? + * True when it claims every descriptor and no OTHER plug-in is pinned as + * preferred. The active plug-in wins every monitor it claims (#1521), and a + * preferred plug-in is the only thing that outranks it — so when both hold, + * loading and probing every other installed plug-in cannot change a single + * registry entry. It would only dlopen them, run their vendor probes (Leia's + * reaches the SR service and the shared panel), and keep their instances. + * Typical cases: `XRT_PREFERRED_PLUGIN_ID=sim-display`, or sim-display active + * because no vendor plug-in is installed. + */ +static bool +active_plugin_decides_every_monitor(const struct xrt_display_descriptor *descs, uint32_t n) +{ + if (g_active_iface == NULL || descs == NULL || n == 0) { + return false; + } + char preferred[64] = {0}; + if (target_plugin_get_preferred(preferred, sizeof(preferred)) && + (g_active_iface->id == NULL || strcmp(preferred, g_active_iface->id) != 0)) { + return false; // a different plug-in is pinned: it may outrank the active one + } + const struct plugin_display_source active = { + .iface = g_active_iface, + .inst = g_active_instance, + .probe_order = g_active_probe_order, + }; + struct xrt_display_claim claims[XRT_DP_REGISTRY_MAX_ENTRIES]; + const uint32_t cn = query_source_claims(&active, descs, n, claims, XRT_DP_REGISTRY_MAX_ENTRIES); + for (uint32_t d = 0; d < n; d++) { + bool claimed = false; + for (uint32_t c = 0; c < cn && !claimed; c++) { + claimed = claims[c].monitor_id == descs[d].monitor_id; + } + if (!claimed) { + return false; + } + } + return true; +} +#endif + /*! * Build the cached display-claim source set if not already collected. Caller * holds @ref g_refresh_mutex and has loaded the active plug-in. + * + * POSIX only loads the other plug-ins when they could matter: if the active + * plug-in decides every monitor in @p descs on its own + * (@ref active_plugin_decides_every_monitor) the set is just the active + * plug-in, and that answer is re-checked on the next resolve. */ static void -ensure_display_sources(void) +ensure_display_sources(const struct xrt_display_descriptor *descs, uint32_t n) { +#if !defined(XRT_OS_WINDOWS) && !defined(XRT_OS_ANDROID) + if (g_display_source_count >= 0 && !g_display_sources_active_only) { + return; + } + if (active_plugin_decides_every_monitor(descs, n)) { + g_display_sources[0].iface = g_active_iface; + g_display_sources[0].inst = g_active_instance; + g_display_sources[0].probe_order = g_active_probe_order; + g_display_source_count = 1; + g_display_sources_active_only = true; + return; + } + g_display_sources_active_only = false; +#else + (void)descs; + (void)n; if (g_display_source_count >= 0) { return; } +#endif g_display_source_count = 0; #if !defined(XRT_OS_ANDROID) // Windows (registry) and macOS/Linux (manifests): every registered @@ -3652,13 +3727,13 @@ target_plugin_resolve_displays(const struct xrt_display_descriptor *descriptors, os_mutex_lock(&g_refresh_mutex); } - ensure_display_sources(); - uint32_t dn = descriptor_count; if (dn > XRT_DP_REGISTRY_MAX_ENTRIES) { dn = XRT_DP_REGISTRY_MAX_ENTRIES; } + ensure_display_sources(descriptors, dn); + // Query every source once for the full descriptor set. static struct xrt_display_claim src_claims[TARGET_PLUGIN_MAX_SOURCES][XRT_DP_REGISTRY_MAX_ENTRIES]; uint32_t src_claim_count[TARGET_PLUGIN_MAX_SOURCES] = {0}; @@ -3801,3 +3876,37 @@ target_plugin_resolve_displays(const struct xrt_display_descriptor *descriptors, os_mutex_unlock(&g_refresh_mutex); } } + +void +target_plugin_release_claim_sources(void) +{ +#if !defined(XRT_OS_WINDOWS) && !defined(XRT_OS_ANDROID) + if (g_refresh_mutex_initialized) { + os_mutex_lock(&g_refresh_mutex); + } + for (int s = 0; s < g_display_source_count; s++) { + struct plugin_display_source *src = &g_display_sources[s]; + // The active plug-in's instance belongs to discovery and outlives + // any one xrt_instance; only the claim-only instances are ours. + // destroy() pairs with the probe() that made the instance, even when + // probe() handed back NULL (a plug-in with process-global state). + if (src->iface == NULL || src->iface == g_active_iface) { + continue; + } + if (src->iface->destroy != NULL) { + U_LOG_I("plugin loader: releasing display-claim source id=%s", + src->iface->id != NULL ? src->iface->id : "?"); + src->iface->destroy(src->inst); + } + src->inst = NULL; + } + // The next resolve re-collects (and re-probes) from scratch. The dlopen + // handles stay loaded, as everywhere else in this loader: a re-collection + // just takes another reference. + g_display_source_count = -1; + g_display_sources_active_only = false; + if (g_refresh_mutex_initialized) { + os_mutex_unlock(&g_refresh_mutex); + } +#endif +} diff --git a/src/xrt/targets/common/target_plugin_loader.h b/src/xrt/targets/common/target_plugin_loader.h index ca878c7c4..ab24f3805 100644 --- a/src/xrt/targets/common/target_plugin_loader.h +++ b/src/xrt/targets/common/target_plugin_loader.h @@ -402,6 +402,18 @@ struct xrt_plugin_display_info; void target_plugin_note_active_panel(const struct xrt_plugin_display_info *pdi); +/*! + * Release the plug-in instances that were loaded ONLY as display-claim sources + * (not the active plug-in), and drop the cached source set so the next + * resolve re-collects. Called at xrt_instance teardown, after the system (and + * with it the registry that referenced those sources) is gone. + * + * macOS/Linux only. Windows has always kept its claim sources for the process + * lifetime and is unchanged here; Android has no claim-only sources. + */ +void +target_plugin_release_claim_sources(void); + #ifdef __cplusplus } #endif From 7dca7215130c72901d36d5e9df3b385bdc793f38 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 13:57:11 -0700 Subject: [PATCH 7/8] test(linux): pin where the synthesized display claim lands Review of #1851, finding 3. The placement of the back-compat claim (for a plug-in without probe_displays) now comes from a pure, exported policy, target_plugin_backcompat_claim_index(). synth_primary_edid_claim feeds it the active plug-in's noted panel and the side-table records, so the loader and the tests run the same code. New tests_target_backcompat_claim (links target_lists, like tests_input_host_geometry): - no panel known gives the primary (or descriptor 0, or a primary that is not first); - the DS1 panel lands on the HDMI monitor, not the eDP primary, by connector device mode, and by pixel size when no device mode is known; - a plug-in origin inside a known monitor is trusted first and settles two same-size monitors; - an origin is never matched against DRM-only records (origin unknown); - a panel that matches nothing, or has no pixel size, falls back to the primary. The join cases from the same review item (NVIDIA off-by-one naming, identical panels, disabled connector) landed with the join fix in the previous commits. Co-Authored-By: Claude Opus 5.5 --- src/xrt/targets/common/target_plugin_loader.c | 121 ++++++++------ src/xrt/targets/common/target_plugin_loader.h | 26 +++ tests/CMakeLists.txt | 9 ++ tests/tests_target_backcompat_claim.cpp | 150 ++++++++++++++++++ 4 files changed, 259 insertions(+), 47 deletions(-) create mode 100644 tests/tests_target_backcompat_claim.cpp diff --git a/src/xrt/targets/common/target_plugin_loader.c b/src/xrt/targets/common/target_plugin_loader.c index 1571bd5c8..57de1f070 100644 --- a/src/xrt/targets/common/target_plugin_loader.c +++ b/src/xrt/targets/common/target_plugin_loader.c @@ -3340,33 +3340,21 @@ monitor_side_find(uint64_t monitor_id) #endif /*! - * Off-Windows: which descriptor is the ACTIVE plug-in's panel, by the same - * ADR-033 rules the desktop-rect resolver uses (origin, then connector mode, - * then pixel size; ties on physical size). Windows keeps the primary-monitor - * back-compat claim exactly as before. - * - * Why it exists: a 3D panel is a SECOND monitor on essentially every real - * deployment, so "the primary" lands the vendor's back-compat claim on the - * laptop screen — and the active plug-in wins every monitor it claims (#1521), - * so the registry would route the laptop screen to the vendor DP and the - * panel to the fallback. Caller holds @ref g_refresh_mutex. + * Off-Windows: which descriptor is the panel described by @p hint, by the + * same ADR-033 rules the desktop-rect resolver uses (origin, then connector + * mode, then pixel size; ties on physical size). Pure. */ +#ifndef XRT_OS_WINDOWS static bool -pick_active_panel_monitor(const struct xrt_display_descriptor *descs, uint32_t n, uint32_t *out_pick) +panel_monitor_index(const struct xrt_display_descriptor *descs, + uint32_t n, + const struct os_display_edid_monitor *const *monitors, + const struct os_display_panel_hint *hint, + uint32_t *out_pick) { -#ifdef XRT_OS_WINDOWS - (void)descs; - (void)n; - (void)out_pick; - return false; -#else - if (!g_active_panel_valid || n == 0) { - return false; - } if (n > XRT_DP_REGISTRY_MAX_ENTRIES) { n = XRT_DP_REGISTRY_MAX_ENTRIES; } - const struct os_display_panel_hint *hint = &g_active_panel_hint; struct os_display_desktop_info mons[XRT_DP_REGISTRY_MAX_ENTRIES]; memset(mons, 0, sizeof(mons)); @@ -3381,7 +3369,7 @@ pick_active_panel_monitor(const struct xrt_display_descriptor *descs, uint32_t n m->height_in_caller_dpi = m->height; m->is_primary = (descs[i].flags & 1u) != 0; origin_known[i] = true; - const struct os_display_edid_monitor *side = monitor_side_find(descs[i].monitor_id); + const struct os_display_edid_monitor *side = monitors != NULL ? monitors[i] : NULL; if (side != NULL) { m->physical_width_mm = side->physical_width_mm; m->physical_height_mm = side->physical_height_mm; @@ -3415,18 +3403,52 @@ pick_active_panel_monitor(const struct xrt_display_descriptor *descs, uint32_t n return true; } return false; +} #endif + +uint32_t +target_plugin_backcompat_claim_index(const struct xrt_display_descriptor *descs, + uint32_t n, + const struct os_display_edid_monitor *const *monitors, + const struct os_display_panel_hint *panel) +{ + if (descs == NULL || n == 0) { + return 0; + } + uint32_t pick = 0; + for (uint32_t i = 0; i < n; i++) { + if (descs[i].flags & 1u) { + pick = i; + break; + } + } +#ifndef XRT_OS_WINDOWS + uint32_t at_panel = 0; + if (panel != NULL && panel->pixel_width > 0 && panel->pixel_height > 0 && + panel_monitor_index(descs, n, monitors, panel, &at_panel)) { + pick = at_panel; + } +#else + (void)monitors; + (void)panel; +#endif + return pick; } /*! * Synthesize the back-compat claim for a loaded plug-in that has no * `probe_displays` but whose binary `probe()` succeeded: one - * @ref XRT_DISPLAY_CLAIM_EDID claim on the primary monitor (or the first - * descriptor if none is flagged primary) — except off-Windows for the ACTIVE - * plug-in once its panel is known, where it lands on that panel's monitor - * (@ref pick_active_panel_monitor). `supported_apis` is set to all - * bits — the actual factory set is masked against the plug-in's non-NULL - * factory pointers at fill time. + * @ref XRT_DISPLAY_CLAIM_EDID claim on the monitor + * @ref target_plugin_backcompat_claim_index picks — the primary monitor, or, + * off-Windows for the ACTIVE plug-in once its panel is known, that panel's + * monitor. `supported_apis` is set to all bits — the actual factory set is + * masked against the plug-in's non-NULL factory pointers at fill time. + * + * Why the panel and not the primary: a 3D panel is a SECOND monitor on + * essentially every real deployment, so "the primary" lands the vendor's + * claim on the laptop screen — and the active plug-in wins every monitor it + * claims (#1521), so the registry would route the laptop screen to the vendor + * DP and the panel to the fallback. Caller holds @ref g_refresh_mutex. */ static uint32_t synth_primary_edid_claim(const struct xrt_display_descriptor *descs, @@ -3438,27 +3460,32 @@ synth_primary_edid_claim(const struct xrt_display_descriptor *descs, if (n == 0 || max == 0) { return 0; } - uint32_t pick = 0; - for (uint32_t i = 0; i < n; i++) { - if (descs[i].flags & 1u) { - pick = i; - break; + if (n > XRT_DP_REGISTRY_MAX_ENTRIES) { + n = XRT_DP_REGISTRY_MAX_ENTRIES; + } + const bool is_active = iface != NULL && iface == g_active_iface; + const uint32_t primary = target_plugin_backcompat_claim_index(descs, n, NULL, NULL); + uint32_t pick = primary; +#ifndef XRT_OS_WINDOWS + // A plug-in that ships `probe_displays` never reaches here, so its own + // claims always win; only the ACTIVE plug-in's panel is known. + if (is_active && g_active_panel_valid) { + const struct os_display_edid_monitor *side[XRT_DP_REGISTRY_MAX_ENTRIES]; + for (uint32_t i = 0; i < n; i++) { + side[i] = monitor_side_find(descs[i].monitor_id); } + pick = target_plugin_backcompat_claim_index(descs, n, side, &g_active_panel_hint); } - // Off-Windows, the ACTIVE plug-in's back-compat claim follows its panel - // rather than the primary monitor (multi-screen M0). A plug-in that ships - // `probe_displays` never reaches here, so its own claims always win. - uint32_t panel = 0; - if (iface != NULL && iface == g_active_iface && pick_active_panel_monitor(descs, n, &panel)) { - if (panel != pick) { - U_LOG_I( - "plugin loader: '%s' has no probe_displays — back-compat claim placed on its panel " - "(monitor 0x%016llx, %ux%u at (%d,%d)) instead of the primary monitor", - iface->id != NULL ? iface->id : "?", (unsigned long long)descs[panel].monitor_id, - descs[panel].pixel_width, descs[panel].pixel_height, (int)descs[panel].screen_left, - (int)descs[panel].screen_top); - } - pick = panel; +#else + (void)is_active; +#endif + if (pick != primary) { + U_LOG_I( + "plugin loader: '%s' has no probe_displays — back-compat claim placed on its panel " + "(monitor 0x%016llx, %ux%u at (%d,%d)) instead of the primary monitor", + iface->id != NULL ? iface->id : "?", (unsigned long long)descs[pick].monitor_id, + descs[pick].pixel_width, descs[pick].pixel_height, (int)descs[pick].screen_left, + (int)descs[pick].screen_top); } out[0].monitor_id = descs[pick].monitor_id; out[0].confidence = (uint32_t)XRT_DISPLAY_CLAIM_EDID; diff --git a/src/xrt/targets/common/target_plugin_loader.h b/src/xrt/targets/common/target_plugin_loader.h index ab24f3805..9011d6bcb 100644 --- a/src/xrt/targets/common/target_plugin_loader.h +++ b/src/xrt/targets/common/target_plugin_loader.h @@ -388,6 +388,32 @@ target_plugin_resolve_displays(const struct xrt_display_descriptor *descriptors, struct xrt_dp_factory_registry *out_registry); struct xrt_plugin_display_info; +struct os_display_edid_monitor; +struct os_display_panel_hint; + +/*! + * Which descriptor a plug-in WITHOUT `probe_displays` gets its synthesized + * back-compat claim on. Pure — the policy behind the loader's synthesized + * claim, exposed so it can be pinned by tests. + * + * The primary monitor (flags bit 0), else descriptor 0. Off-Windows, when + * @p panel describes the plug-in's panel (non-zero pixel size), the monitor + * that panel matches instead, by the ADR-033 rules: a non-(0,0) origin inside + * a monitor whose own origin is known, then the connector's device mode, then + * the pixel size, ties broken on physical size. The loader passes the ACTIVE + * plug-in's noted panel (@ref target_plugin_note_active_panel) and nothing + * for any other plug-in. Windows always gets the primary. + * + * @param monitors Per-descriptor enumeration records (mm, device mode, + * origin_unknown), aligned with @p descs; NULL, or NULL + * entries, for "not known". + * @param panel The plug-in's panel; NULL = not known. + */ +uint32_t +target_plugin_backcompat_claim_index(const struct xrt_display_descriptor *descs, + uint32_t n, + const struct os_display_edid_monitor *const *monitors, + const struct os_display_panel_hint *panel); /*! * Record what the ACTIVE plug-in reported about its panel diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 998c1ba0d..834d9cec3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -88,6 +88,12 @@ endif() if(TARGET target_lists) list(APPEND tests tests_input_host_geometry) endif() +# Multi-screen M0 (#69) -- where a plug-in without probe_displays gets its +# synthesized display claim (its panel's monitor, not the primary). Pure +# policy in the loader, so it needs target_lists like the test above. +if(TARGET target_lists) + list(APPEND tests tests_target_backcompat_claim) +endif() if(XRT_HAVE_D3D11) list(APPEND tests tests_aux_d3d_d3d11 tests_aux_d3d_render_adapter tests_comp_client_d3d11 tests_comp_xbridge_plane_policy tests_comp_split_gate tests_comp_eye_dp_policy @@ -318,6 +324,9 @@ target_compile_definitions( if(TARGET tests_input_host_geometry) target_link_libraries(tests_input_host_geometry PRIVATE target_lists xrt-interfaces) endif() +if(TARGET tests_target_backcompat_claim) + target_link_libraries(tests_target_backcompat_claim PRIVATE target_lists xrt-interfaces aux_os) +endif() # #1380 rig composer -- the head device's pose source. Compiled straight into # the unit (like tests_comp_rear_budget) rather than linked from target_lists, diff --git a/tests/tests_target_backcompat_claim.cpp b/tests/tests_target_backcompat_claim.cpp new file mode 100644 index 000000000..7ded5b110 --- /dev/null +++ b/tests/tests_target_backcompat_claim.cpp @@ -0,0 +1,150 @@ +// Copyright 2026, The DisplayXR Project +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Pins where a plug-in without `probe_displays` gets its synthesized + * display claim (multi-screen M0, #69): the monitor its panel + * matches, not the primary monitor + * (`target_plugin_backcompat_claim_index`). + */ + +#include "catch_amalgamated.hpp" + +#include "xrt/xrt_plugin.h" +#include "os/os_display_desktop.h" +#include "os/os_display_edid.h" +#include "target_plugin_loader.h" + +#include + +namespace { + +xrt_display_descriptor +desc(uint64_t id, int32_t left, uint32_t w, uint32_t h, bool primary) +{ + xrt_display_descriptor d = {}; + d.struct_size = sizeof(d); + d.monitor_id = id; + d.screen_left = left; + d.pixel_width = w; + d.pixel_height = h; + d.flags = primary ? 1u : 0u; + return d; +} + +os_display_edid_monitor +mon(uint32_t native_w, uint32_t native_h, uint32_t mm_w, uint32_t mm_h, bool origin_unknown = false) +{ + os_display_edid_monitor m = {}; + m.native_width = native_w; + m.native_height = native_h; + m.physical_width_mm = mm_w; + m.physical_height_mm = mm_h; + m.origin_unknown = origin_unknown; + return m; +} + +//! The DS1 as the Leia plug-in reports it: 3840x2160, 0.344 x 0.193 m, no origin. +os_display_panel_hint +ds1_panel(int32_t left = 0, int32_t top = 0) +{ + os_display_panel_hint h = {}; + h.screen_left = left; + h.screen_top = top; + h.pixel_width = 3840; + h.pixel_height = 2160; + h.width_m = 0.344f; + h.height_m = 0.193f; + return h; +} + +/* + * ds1-linux: eDP-1 primary at (0,0), 3456x2160 X11 rect over a 2880x1800 + * device mode; the DS1 on HDMI at (3456,0), 3840x2160. + */ +struct Box +{ + xrt_display_descriptor d[2] = { + desc(0xA, 0, 3456, 2160, true), + desc(0xB, 3456, 3840, 2160, false), + }; + os_display_edid_monitor m[2] = { + mon(2880, 1800, 300, 190), + mon(3840, 2160, 344, 193), + }; + const os_display_edid_monitor *side[2] = {&m[0], &m[1]}; +}; + +} // namespace + +TEST_CASE("backcompat claim: no panel known -> the primary monitor") +{ + Box b; + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, nullptr) == 0); + + // No primary flagged at all -> descriptor 0. + b.d[0].flags = 0; + CHECK(target_plugin_backcompat_claim_index(b.d, 2, nullptr, nullptr) == 0); + + // Primary second -> the second. + b.d[1].flags = 1; + CHECK(target_plugin_backcompat_claim_index(b.d, 2, nullptr, nullptr) == 1); +} + +#ifndef _WIN32 +TEST_CASE("backcompat claim: the panel's monitor wins over the primary (connector mode)") +{ + Box b; + const os_display_panel_hint p = ds1_panel(); + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &p) == 1); +} + +TEST_CASE("backcompat claim: pixel size alone places it when no device mode is known") +{ + Box b; + const os_display_panel_hint p = ds1_panel(); + CHECK(target_plugin_backcompat_claim_index(b.d, 2, nullptr, &p) == 1); +} + +TEST_CASE("backcompat claim: a panel origin inside a known monitor is trusted first") +{ + Box b; + // Both monitors 3840x2160, so a size match would be ambiguous; the + // plug-in's own origin settles it. + b.d[0] = desc(0xA, 0, 3840, 2160, true); + b.m[0] = mon(3840, 2160, 344, 193); + b.d[1].screen_left = 3840; + const os_display_panel_hint p = ds1_panel(100, 50); + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &p) == 0); + + const os_display_panel_hint q = ds1_panel(4000, 10); + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &q) == 1); +} + +TEST_CASE("backcompat claim: an origin is never matched against a DRM-only record") +{ + Box b; + // No X server: both records sit at (0,0) with an unknown origin. A + // plug-in origin of (10,10) must not pick monitor 0 by containment; the + // connector-mode rule still finds the DS1. + b.d[1].screen_left = 0; + b.m[0].origin_unknown = true; + b.m[1].origin_unknown = true; + const os_display_panel_hint p = ds1_panel(10, 10); + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &p) == 1); +} + +TEST_CASE("backcompat claim: a panel that matches nothing falls back to the primary") +{ + Box b; + os_display_panel_hint p = ds1_panel(); + p.pixel_width = 1920; + p.pixel_height = 1080; + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &p) == 0); + + // A panel with no pixel size is "not known". + p.pixel_width = 0; + p.pixel_height = 0; + CHECK(target_plugin_backcompat_claim_index(b.d, 2, b.side, &p) == 0); +} +#endif From be5490ffdb00422cb6587e28b6575d3fdf447c78 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 7 Oct 2026 13:57:34 -0700 Subject: [PATCH 8/8] fix(linux): report monitor refresh from the compositor's current mode Review of #1851, finding 4. Refresh came only from the EDID's first detailed timing, and only when that timing matched the mode in use. eDP panels often carry no detailed timing in the base block, so eDP-1 printed 0Hz. Mutter's DisplayConfig already returns each connector's current mode with its refresh. os_display_connector_annotate now keeps it (os_display_desktop_info::native_refresh_mhz, compositor source only; DRM sysfs has no refresh). The join prefers it over the EDID guess. ds1-linux now reports eDP-1 @ 60Hz. Co-Authored-By: Claude Opus 5.5 --- .../auxiliary/os/os_display_connector_linux.c | 7 ++++++- src/xrt/auxiliary/os/os_display_desktop.h | 4 ++++ src/xrt/auxiliary/os/os_display_edid_linux.c | 6 ++++++ tests/tests_aux_display_edid_linux.cpp | 17 ++++++++++++++++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/xrt/auxiliary/os/os_display_connector_linux.c b/src/xrt/auxiliary/os/os_display_connector_linux.c index 9b68b1d99..3d62e9eb0 100644 --- a/src/xrt/auxiliary/os/os_display_connector_linux.c +++ b/src/xrt/auxiliary/os/os_display_connector_linux.c @@ -73,7 +73,8 @@ struct connector_mode { char name[64]; // normalised uint32_t w, h; - double scale; // 0 = unknown + double scale; // 0 = unknown + uint32_t refresh_mhz; // current mode's refresh, 0 = unknown }; // Local declarations of the libdbus ABI used here (stable since dbus 1.0). @@ -297,6 +298,7 @@ query_mutter(struct connector_mode *out, uint32_t max) if (read_spec_connector(&f, &mon, &connector) && connector != NULL) { f.message_iter_next(&mon); // past the spec, to the modes array uint32_t cw = 0, ch = 0; + double crefresh = 0.0; if (f.message_iter_get_arg_type(&mon) == OS_DBUS_TYPE_ARRAY) { os_dbus_iter modes; f.message_iter_recurse(&mon, &modes); @@ -315,6 +317,7 @@ query_mutter(struct connector_mode *out, uint32_t max) if (dict_bool(&f, &m, "is-current") && w > 0 && h > 0) { cw = (uint32_t)w; ch = (uint32_t)h; + crefresh = refresh; } } f.message_iter_next(&modes); @@ -326,6 +329,7 @@ query_mutter(struct connector_mode *out, uint32_t max) os_display_connector_normalise(connector, c->name, sizeof(c->name)); c->w = cw; c->h = ch; + c->refresh_mhz = crefresh > 0.0 ? (uint32_t)(crefresh * 1000.0 + 0.5) : 0u; } } f.message_iter_next(&mons); @@ -493,6 +497,7 @@ os_display_connector_annotate(struct os_display_desktop_info *mons, uint32_t cou m->native_width = mc->w; m->native_height = mc->h; m->scale = mc->scale; + m->native_refresh_mhz = mc->refresh_mhz; m->native_source = OS_DISPLAY_NATIVE_SOURCE_COMPOSITOR; continue; } diff --git a/src/xrt/auxiliary/os/os_display_desktop.h b/src/xrt/auxiliary/os/os_display_desktop.h index 0a3fc3624..d59c8ef2b 100644 --- a/src/xrt/auxiliary/os/os_display_desktop.h +++ b/src/xrt/auxiliary/os/os_display_desktop.h @@ -157,6 +157,10 @@ struct os_display_desktop_info //! Where @ref native_width / @ref native_height came from. enum os_display_native_source native_source; + + //! Refresh of that device mode in milli-Hz, when the source reports it + //! (Mutter DisplayConfig does; DRM sysfs does not). 0 = unknown. + uint32_t native_refresh_mhz; /*! @} */ }; diff --git a/src/xrt/auxiliary/os/os_display_edid_linux.c b/src/xrt/auxiliary/os/os_display_edid_linux.c index f0276bbc8..a038a66cf 100644 --- a/src/xrt/auxiliary/os/os_display_edid_linux.c +++ b/src/xrt/auxiliary/os/os_display_edid_linux.c @@ -506,6 +506,12 @@ os_display_edid_linux_join(const struct os_display_desktop_info *randr, if (randr_id != NULL && randr_id[i].valid) { apply_edid(m, &randr_id[i].edid); } + // The compositor's current mode carries its refresh (Mutter + // DisplayConfig); it beats the EDID's preferred timing, which is + // only a guess at the mode in use and absent on many eDP panels. + if (r->native_refresh_mhz > 0) { + m->refresh_hz = (r->native_refresh_mhz + 500u) / 1000u; + } } return rn; } diff --git a/tests/tests_aux_display_edid_linux.cpp b/tests/tests_aux_display_edid_linux.cpp index 0d497c806..546d1a50e 100644 --- a/tests/tests_aux_display_edid_linux.cpp +++ b/tests/tests_aux_display_edid_linux.cpp @@ -203,7 +203,7 @@ TEST_CASE("join: RandR output names tie to DRM connectors by name") CHECK(std::string(out[0].connector) == "card1-eDP-1"); CHECK(out[0].physical_width_mm == 300); CHECK(out[0].native_width == 2880); - CHECK(out[0].refresh_hz == 0); // no detailed timing to read it from + CHECK(out[0].refresh_hz == 0); // no detailed timing, no compositor refresh CHECK(out[1].join == OS_EDID_JOIN_NAME); CHECK(out[1].manufacturer_id == ACR_RAW); @@ -220,6 +220,21 @@ TEST_CASE("join: RandR output names tie to DRM connectors by name") CHECK_FALSE(out[1].origin_unknown); } +TEST_CASE("join: refresh comes from the compositor's current mode when it reports one") +{ + os_display_desktop_info randr[2]; + box_randr(randr); + randr[0].native_refresh_mhz = 120000; // eDP: no detailed timing in its EDID + randr[1].native_refresh_mhz = 59940; // beats the DTD's 60 Hz guess + os_display_drm_connector drm[2]; + box_drm(drm); + + os_display_edid_monitor out[2] = {}; + REQUIRE(os_display_edid_linux_join(randr, nullptr, 2, drm, 2, out, 2) == 2); + CHECK(out[0].refresh_hz == 120); + CHECK(out[1].refresh_hz == 60); +} + TEST_CASE("join: unrelated output names fall back to physical size") { os_display_desktop_info randr[2];