Attachment 119297 Details for Bug 92728 – sna: Always query the fresh EDID id

[patch] sna: Always query the fresh EDID id

0001-sna-Always-query-the-fresh-EDID-id.patch (text/plain), 6.91 KB, created by Chris Wilson on 2015-10-30 10:30:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2015-10-30 10:30:31 UTC

Size: 6.91 KB

patch

obsolete

>From 2d6dcf7ee52796f5d285e474313fc80fe5c75b7b Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>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 <chris@chris-wilson.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 92728: 119297 | 119302

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.