[PATCH] DDC: Add EDID caching

Adam Jackson ajax at nwnk.net
Fri Mar 20 12:15:50 PDT 2009


Attempt to detect when we're DDC'ing the same monitor on the same output
as we did last time.  The first 16 bytes of EDID is almost certainly
sufficient to identify a monitor uniquely, as it contains the vendor ID,
model ID, and serial number fields.

This takes 'xrandr -q' on my machine down from ~170ms to ~50ms.  That's
still three frames, but at least it's not ten.  However, the win isn't
perfectly reliable, occasionally it still takes a fair amount of time.
This might be as much to do with DDC timing bugs and retries as anything
else.

Patch is against 1.6.0, but applies with only minor massaging to master.

---
 hw/xfree86/ddc/xf86DDC.c    |  176 ++++++++++++++++++++++++++++++++-----------
 hw/xfree86/ddc/xf86DDC.h    |    1 +
 hw/xfree86/modes/xf86Crtc.c |   10 ++-
 3 files changed, 140 insertions(+), 47 deletions(-)

diff --git a/hw/xfree86/ddc/xf86DDC.c b/hw/xfree86/ddc/xf86DDC.c
index 0d86776..ac80241 100644
--- a/hw/xfree86/ddc/xf86DDC.c
+++ b/hw/xfree86/ddc/xf86DDC.c
@@ -150,64 +150,115 @@ EEDIDStop(I2CDevPtr d)
 {
 }
 
-/* block is the EDID block number.  a segment is two blocks. */
+enum {
+    DDC2_FAILURE,
+    DDC2_SUCCESS,
+    DDC2_CACHED,
+};
+
 static Bool
-DDC2Read(I2CDevPtr dev, int block, unsigned char *R_Buffer)
+DDC2SetSegment(I2CDevPtr dev, int block)
 {
-    unsigned char W_Buffer[1];
-    int i, segment;
+    unsigned char W_Buffer;
     I2CDevPtr seg;
     void (*stop)(I2CDevPtr);
+    int segment = block >> 1;
 
-    for (i = 0; i < RETRIES; i++) {
-	/* Stop bits reset the segment pointer to 0, so be careful here. */
-	segment = block >> 1;
-	if (segment) {
-	    Bool b;
-	    
-	    if (!(seg = xf86I2CFindDev(dev->pI2CBus, 0x0060)))
-		return FALSE;
+    /* Stop bits reset the segment pointer to 0, so be careful here. */
+    if (segment) {
+	Bool b;
 
-	    W_Buffer[0] = segment;
+	if (!(seg = xf86I2CFindDev(dev->pI2CBus, 0x0060)))
+	    return FALSE;
 
-	    stop = dev->pI2CBus->I2CStop;
-	    dev->pI2CBus->I2CStop = EEDIDStop;
+	W_Buffer = segment;
 
-	    b = xf86I2CWriteRead(seg, W_Buffer, 1, NULL, 0);
+	stop = dev->pI2CBus->I2CStop;
+	dev->pI2CBus->I2CStop = EEDIDStop;
 
-	    dev->pI2CBus->I2CStop = stop;
-	    if (!b) {
-		dev->pI2CBus->I2CStop(dev);
-		continue;
+	b = xf86I2CWriteRead(seg, &W_Buffer, 1, NULL, 0);
+
+	dev->pI2CBus->I2CStop = stop;
+	if (!b) {
+	    dev->pI2CBus->I2CStop(dev);
+	    return FALSE;
+	}
+    }
+
+    return TRUE;
+}
+
+/* Hacked version of I2CWriteRead for early bailout */
+int
+DDC2WriteRead(I2CDevPtr d, I2CByte *WriteBuffer, int nWrite,
+	      I2CByte *ReadBuffer, int nRead, xf86MonPtr old)
+{
+    I2CByte *rb = ReadBuffer;
+    int r = TRUE;
+    I2CBusPtr b = d->pI2CBus;
+    int s = 0;
+
+    if (r && nWrite > 0) {
+	r = b->I2CAddress(d, d->SlaveAddr & ~1);
+	if (r) {
+	    for (; nWrite > 0; WriteBuffer++, nWrite--)
+		if (!(r = b->I2CPutByte(d, *WriteBuffer))) 
+		    break;
+	    s++;
+	}
+    }
+
+    if (r && nRead > 0) {
+	r = b->I2CAddress(d, d->SlaveAddr | 1);
+	if (r) {
+	    for (; nRead > 0; ReadBuffer++, nRead--) {
+		if (!(r = b->I2CGetByte(d, ReadBuffer, nRead == 1))) 
+		    break;
+		if ((nRead == EDID1_LEN - 16) && old && old->rawData &&
+		    !memcmp(old->rawData, rb, 16)) {
+		    r = DDC2_CACHED;
+		    break;
+		}
 	    }
+	    s++;
 	}
+    }
+
+    if (s) b->I2CStop(d);
+
+    return r;
+}
+
+/* block is the EDID block number.  a segment is two blocks. */
+static int
+DDC2GetBlock(I2CDevPtr dev, int block, unsigned char *R_Buffer, xf86MonPtr old)
+{
+    unsigned char W_Buffer[1];
+    int i, ret;
+
+    if (block != 0)
+	old = NULL;
+
+    for (i = 0; i < RETRIES; i++) {
+	if (!DDC2SetSegment(dev, block))
+	    return FALSE;
 
 	W_Buffer[0] = (block & 0x01) * EDID1_LEN;
 
-	if (xf86I2CWriteRead(dev, W_Buffer, 1, R_Buffer, EDID1_LEN)) {
-	    if (!DDC_checksum(R_Buffer, EDID1_LEN))
-		return TRUE;
-	}
+	ret = DDC2WriteRead(dev, W_Buffer, 1, R_Buffer, EDID1_LEN, old);
+
+	if (ret == DDC2_CACHED)
+	    return DDC2_CACHED;
+	if (ret == DDC2_SUCCESS && !DDC_checksum(R_Buffer, EDID1_LEN))
+	    return DDC2_SUCCESS;
+	/* else continue */
     }
  
     return FALSE;
 }
 
-/**
- * Attempts to probe the monitor for EDID information, if NoDDC and NoDDC2 are
- * unset.  EDID information blocks are interpreted and the results returned in
- * an xf86MonPtr.  Unlike xf86DoEDID_DDC[12](), this function will return
- * the complete EDID data, including all extension blocks, if the 'complete'
- * parameter is TRUE;
- *
- * This function does not affect the list of modes used by drivers -- it is up
- * to the driver to decide policy on what to do with EDID information.
- *
- * @return pointer to a new xf86MonPtr containing the EDID information.
- * @return NULL if no monitor attached or failure to interpret the EDID.
- */
-xf86MonPtr
-xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
+static xf86MonPtr
+doEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete, xf86MonPtr old)
 {
     ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
     unsigned char *EDID_block = NULL;
@@ -216,6 +267,7 @@ xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
     /* Default DDC and DDC2 to enabled. */
     Bool noddc = FALSE, noddc2 = FALSE;
     OptionInfoPtr options;
+    int r;
 
     options = xalloc(sizeof(DDCOptions));
     if (!options)
@@ -237,14 +289,19 @@ xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
     if (!EDID_block)
 	return NULL;
 
-    if (DDC2Read(dev, 0, EDID_block)) {
+    r = DDC2GetBlock(dev, 0, EDID_block, old);
+
+    if (r == DDC2_CACHED) {
+	xfree(EDID_block);
+	return old;
+    } else if (r == DDC2_SUCCESS) {
 	int i, n = EDID_block[0x7e];
 
 	if (complete && n) {
 	    EDID_block = xrealloc(EDID_block, EDID1_LEN * (1+n));
 
 	    for (i = 0; i < n; i++)
-		DDC2Read(dev, i+1, EDID_block + (EDID1_LEN * (1+i)));
+		DDC2GetBlock(dev, i+1, EDID_block + (EDID1_LEN * (1+i)), NULL);
 	}
 
 	tmp = xf86InterpretEEDID(scrnIndex, EDID_block);
@@ -259,10 +316,39 @@ xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
 /**
  * Attempts to probe the monitor for EDID information, if NoDDC and NoDDC2 are
  * unset.  EDID information blocks are interpreted and the results returned in
- * an xf86MonPtr.
+ * an xf86MonPtr.  Unlike xf86DoEDID_DDC[12](), this function will return
+ * the complete EDID data, including all extension blocks, if the 'complete'
+ * parameter is TRUE;
  *
- * This function does not affect the list of modes used by drivers -- it is up
- * to the driver to decide policy on what to do with EDID information.
+ * @return pointer to a new xf86MonPtr containing the EDID information.
+ * @return NULL if no monitor attached or failure to interpret the EDID.
+ */
+xf86MonPtr
+xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
+{
+    return doEEDID(scrnIndex, pBus, complete, NULL);
+}
+
+/**
+ * Attempts to probe the monitor for EDID information, if NoDDC and NoDDC2 are
+ * unset.  EDID information blocks are interpreted and the results returned in
+ * an xf86MonPtr.  All extension blocks are returned.  If 'old' is non-NULL,
+ * an additional check is performed after the first 16 bytes are read to see
+ * if they match the contents of 'old'; if so, 'old' is returned unmodified.
+ *
+ * @return pointer to a new xf86MonPtr containing the EDID information.
+ * @return NULL if no monitor attached or failure to interpret the EDID.
+ */
+xf86MonPtr
+xf86DoFastEEDID(int scrnIndex, I2CBusPtr pBus, xf86MonPtr old)
+{
+    return doEEDID(scrnIndex, pBus, TRUE, old);
+}
+
+/**
+ * Attempts to probe the monitor for EDID information, if NoDDC and NoDDC2 are
+ * unset.  EDID information blocks are interpreted and the results returned in
+ * an xf86MonPtr.
  *
  * @return pointer to a new xf86MonPtr containing the EDID information.
  * @return NULL if no monitor attached or failure to interpret the EDID.
@@ -270,7 +356,7 @@ xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool complete)
 xf86MonPtr
 xf86DoEDID_DDC2(int scrnIndex, I2CBusPtr pBus)
 {
-    return xf86DoEEDID(scrnIndex, pBus, FALSE);
+    return doEEDID(scrnIndex, pBus, FALSE, NULL);
 }
 
 /* 
diff --git a/hw/xfree86/ddc/xf86DDC.h b/hw/xfree86/ddc/xf86DDC.h
index 3172b55..54ca6d4 100644
--- a/hw/xfree86/ddc/xf86DDC.h
+++ b/hw/xfree86/ddc/xf86DDC.h
@@ -36,6 +36,7 @@ extern xf86MonPtr xf86DoEDID_DDC2(
 );
 
 extern xf86MonPtr xf86DoEEDID(int scrnIndex, I2CBusPtr pBus, Bool);
+extern xf86MonPtr xf86DoFastEEDID(int scrnIndex, I2CBusPtr pBus, xf86MonPtr);
 
 extern xf86MonPtr xf86PrintEDID(
     xf86MonPtr monPtr
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index ad6ca98..c372d8a 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -2777,7 +2777,13 @@ xf86OutputSetEDID (xf86OutputPtr output, xf86MonPtr edid_mon)
 #ifdef RANDR_12_INTERFACE
     int			size;
 #endif
-    
+
+    if (output->MonInfo == edid_mon) {
+	xf86DrvMsg(scrn->scrnIndex, X_INFO, "EDID for output %s unchanged\n",
+		   output->name);
+	return;
+    }
+
     if (output->MonInfo != NULL)
 	xfree(output->MonInfo);
     
@@ -2854,7 +2860,7 @@ xf86OutputGetEDID (xf86OutputPtr output, I2CBusPtr pDDCBus)
     ScrnInfoPtr	scrn = output->scrn;
     xf86MonPtr mon;
 
-    mon = xf86DoEEDID(scrn->scrnIndex, pDDCBus, TRUE);
+    mon = xf86DoFastEEDID(scrn->scrnIndex, pDDCBus, output->MonInfo);
     if (mon)
         xf86DDCApplyQuirks(scrn->scrnIndex, mon);
 
-- 
1.6.2

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : http://lists.x.org/archives/xorg-devel/attachments/20090320/a7583c38/attachment.pgp 


More information about the xorg-devel mailing list