The intel ddx sometimes doesn't return the edid on a XRRGetOutputProperty() call. The wrong seems to be due to sna_output->prop_values not being up to date. If I force the properties to be updated on sna_output_attach_edid() I always get a valid edid. Something like: @@ -3235,6 +3238,8 @@ sna_output_attach_edid(xf86OutputPtr output) if (sna_output->edid_idx == -1) return; + update_properties(sna, sna_output); + raw = sna_output->edid_raw; blob.length = sna_output->edid_len; that's unlikely to the the correct solution since it seems that the driver is trying to cache these values but I'm not sure where's the correct place to invalidate the cache.
So at first, I thought it was the loop in sna_output_detect() not filling in the properties, but the kernel should always fill in the property values, even when just querying the mode count. The expectation is that we call sna_output_detect() everytime we have reason to believe the output has changed (i.e. hotplug uevent or user reprobe). During the detect we then refresh the property values. The EDID property is rather special in terms of the connector properties as it gets explicitly updated by get_modes() (after detect). If the EDID was swapped (but the connector status remained the same) it could miss the update, but the modes would remain the same as well. It wouldn't be the same effect as not returning an EDID at all though. Hmm. The problem appears to be that the kernel changes the ID whenever, not on an actual EDID change. That means we always have the issue that the EDID value can be out of date after being set in detect by the time we reach attach_edid.
Created attachment 119297 [details] [review] sna: Always query the fresh EDID id
(In reply to Chris Wilson from comment #2) > Created attachment 119297 [details] [review] [review] > sna: Always query the fresh EDID id with the compilation error fixed, this doesn't seem to make it any better
What compilation error? It applies against xf86-video-intel.git and builds fine here (even with --enable-debug=full). Have a look at the debug=full Xorg.log output.
(In reply to Chris Wilson from comment #4) > What compilation error? CC sna_display.lo sna_display.c: In function ‘sna_output_attach_tile’: sna_display.c:3372:2: error: expected ‘;’ before ‘prop’ prop.count_values = 1; ^ I'll get a debug log
Ah, that requires XF86_OUTPUT_VERSION >= 3, ta. Can you grab the debug=full log? As we unconditionally call getproperty we should have the actual EDID blob id just before the call to getblob.
(In reply to Chris Wilson from comment #6) > Ah, that requires XF86_OUTPUT_VERSION >= 3, ta. Can you grab the debug=full > log? As we unconditionally call getproperty we should have the actual EDID > blob id just before the call to getblob. https://fedorapeople.org/~rtcm/Xorg.0.log.xz I unplugged and plugged a DVI cable two times there. On the second plug the EDID wasn't retrieved from mutter's point of view.
The kernel reports success and says there is no EDID. The kernel also thinks that the EDID is length 68 on occasion. I don't think this is an issue with the ddx not requesting the right property id. For sanity's sake though: @@ -3238,8 +3248,8 @@ sna_output_attach_edid(xf86OutputPtr output) if (sna_output->edid_idx == -1) return; - DBG(("%s(%s): querying EDID property value\n", - __FUNCTION__, output->name)); + DBG(("%s(%s): querying EDID property [%d] value\n", + __FUNCTION__, output->name, sna_output->prop_ids[sna_output->edid_idx])); memset(&prop, 0, sizeof(prop)); prop.prop_id = sna_output->prop_ids[sna_output->edid_idx]; @@ -3251,6 +3261,14 @@ sna_output_attach_edid(xf86OutputPtr output) __FUNCTION__, output->name)); return; } + DBG(("%s(%s): found property '%s' num_values=%d\n", + __FUNCTION__, output->name, prop.name, prop.count_values)); + + if (prop.count_values != 1) { + DBG(("%s(%s): unexpected number of values for EDID: %d\n", + __FUNCTION__, output->name, prop.count_values)); + return; + } raw = sna_output->edid_raw; blob.length = sna_output->edid_len;
https://fedorapeople.org/~rtcm/Xorg.0.log.1.xz This time I just unplugged the DVI cable once and immediately didn't get the EDID for the laptop's panel.
Bah, I managed to truly confuse myself with the property api.
Try: diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index e2969b6..6bb8250 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -3250,6 +3250,9 @@ sna_output_attach_edid(xf86OutputPtr output) old = NULL; blob.blob_id = sna_output->prop_values[sna_output->edid_idx]; + if (!blob.blob_id) + goto done; + DBG(("%s: attaching EDID id=%d, current=%d\n", __FUNCTION__, blob.blob_id, sna_output->edid_blob_id)); if (blob.blob_id == sna_output->edid_blob_id && 0) { /* sigh */ @@ -3270,7 +3273,7 @@ sna_output_attach_edid(xf86OutputPtr output) blob.data = (uintptr_t)raw; if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob)) - goto done; + goto skip_read; DBG(("%s: retrieving blob id=%d, length=%d\n", __FUNCTION__, blob.blob_id, blob.length))
https://fedorapeople.org/~rtcm/Xorg.0.log.2.xz Still not working
Created attachment 119302 [details] [review] Always refresh the blob property before reading
(In reply to Chris Wilson from comment #13) > Created attachment 119302 [details] [review] [review] > Always refresh the blob property before reading sna_display.c: In function ‘sna_output_attach_edid’: sna_display.c:3248:3: error: implicit declaration of function ‘update_properties’ [-Werror=implicit-function-declaration] update_properties(sna, sna_output); With that fixed seems fine after several tries, thanks.
Still not 100% sure about where the error creeps in, but for now commit d78200e53e6e5b889a71f79c103aa4e1ba148c95 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Oct 30 15:53:06 2015 +0000 sna: Always refresh the blob property before reading Ensure that the property value for the EDID is current before retreiving the blob. Reported-by: Rui Tiago Matos <tiagomatos@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92728 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> commit 0196fa2da8140853e9542bf41b63cbd345e857be Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Oct 30 15:26:02 2015 +0000 sna: Handle getblob failures gracefully As the EDID property blob may be lost at time after we perform the detection probe (as the kernel may recreate the EDID blob at any time with a new id), presume that if there is no matching property that we can simply keep using the last known EDID. References: https://bugs.freedesktop.org/show_bug.cgi?id=92728 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
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.