From 2d6dcf7ee52796f5d285e474313fc80fe5c75b7b Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 30 Oct 2015 10:27:00 +0000 Subject: [PATCH] sna: Always query the fresh EDID id There's a time-to-open-time-to-use race between querying the EDID id (in detect as part of the getconnector ioctl) and querying the property value in attach_edid as the kernel may replace the EDID at any time (it assigns a new blob even if the EDID doesn't actually change). Therefore we need to delay querying the blob id for the EDID until just before we use and hope we are not interrupted. Signed-off-by: Chris Wilson References: https://bugs.freedesktop.org/show_bug.cgi?id=92728 --- src/sna/sna_display.c | 76 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index e2969b6..4f250f1 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -3122,7 +3122,7 @@ sna_output_detect(xf86OutputPtr output) uint32_t now; DBG(("%s(%s:%d)\n", __FUNCTION__, output->name, sna_output->id)); - sna_output->update_properties = false; + sna_output->update_properties = true; if (!sna_output->id) { DBG(("%s(%s) hiding due to lost connection\n", __FUNCTION__, output->name)); @@ -3136,7 +3136,6 @@ sna_output_detect(xf86OutputPtr output) DBG(("%s(%s) reporting cached status (since %dms): %d\n", __FUNCTION__, output->name, now - sna_output->last_detect, sna_output->status)); - sna_output->update_properties = true; return sna_output->status; } @@ -3144,9 +3143,7 @@ sna_output_detect(xf86OutputPtr output) compat_conn.conn.connector_id = sna_output->id; sna_output->num_modes = compat_conn.conn.count_modes = 0; /* reprobe */ compat_conn.conn.count_encoders = 0; - compat_conn.conn.count_props = sna_output->num_props; - compat_conn.conn.props_ptr = (uintptr_t)sna_output->prop_ids; - compat_conn.conn.prop_values_ptr = (uintptr_t)sna_output->prop_values; + compat_conn.conn.count_props = 0; if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETCONNECTOR, &compat_conn.conn)) return XF86OutputStatusUnknown; @@ -3233,6 +3230,7 @@ sna_output_attach_edid(xf86OutputPtr output) { struct sna *sna = to_sna(output->scrn); struct sna_output *sna_output = output->driver_private; + struct drm_mode_get_property prop; struct drm_mode_get_blob blob; void *old, *raw = NULL; xf86MonPtr mon = NULL; @@ -3240,6 +3238,20 @@ sna_output_attach_edid(xf86OutputPtr output) if (sna_output->edid_idx == -1) return; + DBG(("%s(%s): querying EDID property value\n", + __FUNCTION__, output->name)); + + memset(&prop, 0, sizeof(prop)); + prop.prop_id = sna_output->prop_ids[sna_output->edid_idx]; + prop.values_ptr = (uintptr_t)&sna_output->prop_values[sna_output->edid_idx]; + prop.count_values = 1; + + if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, &prop)) { + DBG(("%s(%s): failed to update EDID property\n", + __FUNCTION__, output->name)); + return; + } + raw = sna_output->edid_raw; blob.length = sna_output->edid_len; @@ -3250,8 +3262,9 @@ sna_output_attach_edid(xf86OutputPtr output) old = NULL; blob.blob_id = sna_output->prop_values[sna_output->edid_idx]; - DBG(("%s: attaching EDID id=%d, current=%d\n", - __FUNCTION__, blob.blob_id, sna_output->edid_blob_id)); + DBG(("%s(%s): attaching EDID id=%d, current=%d\n", + __FUNCTION__, output->name, + blob.blob_id, sna_output->edid_blob_id)); if (blob.blob_id == sna_output->edid_blob_id && 0) { /* sigh */ if (output->MonInfo) { /* XXX the property keeps on disappearing... */ @@ -3272,8 +3285,8 @@ sna_output_attach_edid(xf86OutputPtr output) if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) goto done; - DBG(("%s: retrieving blob id=%d, length=%d\n", - __FUNCTION__, blob.blob_id, blob.length)); + DBG(("%s(%s): retrieving blob id=%d, length=%d\n", + __FUNCTION__, output->name, blob.blob_id, blob.length)); if (blob.length > sna_output->edid_len) { raw = realloc(raw, blob.length); @@ -3285,14 +3298,19 @@ sna_output_attach_edid(xf86OutputPtr output) } if (blob.length != sna_output->edid_len && - drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) + drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) { + DBG(("%s(%s): failed to read EDID blob\n", __FUNCTION__, output->name)); goto done; + } - if (blob.length < 128) + if (blob.length < 128) { + DBG(("%s(%s): EDID too short, %d\n", __FUNCTION__, output->name, blob.length)); goto done; + } if (blob.length & 127) { /* Truncated EDID! Make sure no one reads too far */ + DBG(("%s(%s): EDID truncated, %d\n", __FUNCTION__, output->name, blob.length)); *SECTION(NO_EDID, (uint8_t*)raw) = blob.length/128 - 1; blob.length &= -128; } @@ -3300,6 +3318,7 @@ sna_output_attach_edid(xf86OutputPtr output) if (old && blob.length == sna_output->edid_len && memcmp(old, raw, blob.length) == 0) { + DBG(("%s(%s): EDID unchanged\n", __FUNCTION__, output->name)); assert(sna_output->edid_raw == raw); sna_output->edid_blob_id = blob.blob_id; RRChangeOutputProperty(output->randr_output, @@ -3333,31 +3352,46 @@ sna_output_attach_tile(xf86OutputPtr output) #if XF86_OUTPUT_VERSION >= 3 struct sna *sna = to_sna(output->scrn); struct sna_output *sna_output = output->driver_private; + struct drm_mode_get_property prop; struct drm_mode_get_blob blob; struct xf86CrtcTileInfo tile_info, *set = NULL; char *tile; - int id; + int idx; - id = find_property(sna, sna_output, "TILE"); - DBG(("%s: found? TILE=%d\n", __FUNCTION__, id)); - if (id == -1) + idx = find_property(sna, sna_output, "TILE"); + DBG(("%s: found? TILE=%d\n", __FUNCTION__, idx)); + if (idx == -1) goto out; + DBG(("%s(%s): querying TILE property value\n", + __FUNCTION__, output->name)); + + memset(&prop, 0, sizeof(prop)); + prop.prop_id = sna_output->prop_ids[idx]; + prop.values_ptr = (uintptr_t)&sna_output->prop_values[idx] + prop.count_values = 1; + + if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, &prop)) { + DBG(("%s(%s): failed to update TILE property\n", + __FUNCTION__, output->name)); + goto out; + } + VG_CLEAR(blob); - blob.blob_id = sna_output->prop_values[id]; + blob.blob_id = sna_output->prop_values[idx]; blob.length = 0; if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) goto out; do { - id = blob.length; - tile = alloca(id + 1); + idx = blob.length; + tile = alloca(idx + 1); blob.data = (uintptr_t)tile; - VG(memset(tile, 0, id)); - DBG(("%s: reading %d bytes for TILE blob\n", __FUNCTION__, id)); + VG(memset(tile, 0, idx)); + DBG(("%s: reading %d bytes for TILE blob\n", __FUNCTION__, idx)); if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) goto out; - } while (id != blob.length); + } while (idx != blob.length); tile[blob.length] = '\0'; /* paranoia */ DBG(("%s: TILE='%s'\n", __FUNCTION__, tile)); -- 2.6.2