[Intel-gfx] [PATCH 2/2]IGD: workaround i2c bus issue in gfx driver side

Shaohua Li shaohua.li at intel.com
Tue Apr 21 07:22:19 CEST 2009


On Tue, 2009-04-14 at 07:33 +0800, Eric Anholt wrote:
> On Tue, 2009-04-07 at 11:02 +0800, Shaohua Li wrote:
> > In IGD, DPCUNIT_CLOCK_GATE_DISABLE bit should be set, otherwise i2c
> > access will be wrong.
> 
> Not sure here, if it might be easier (and more obvious what's going on)
> to just wrap the native DDC bus that's implemented with getbits with a
> IGD wrapper that sets the gating between start and stop.  i830_sdvo.c
> has an example of doing this.
Ok, I updated the patch.
> Was the failure with having the gating off reproducible (wondering about
> that for both of these patches)
yes, it's always reproducible here.

In IGD, DPCUNIT_CLOCK_GATE_DISABLE bit should be set, otherwise i2c
access will be wrong.

Signed-off-by: Shaohua Li <shaohua.li at intel.com>
---
 src/i830.h      |    1 
 src/i830_crt.c  |    6 +-
 src/i830_dvo.c  |   14 ++---
 src/i830_hdmi.c |    2 
 src/i830_i2c.c  |  152 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 src/i830_lvds.c |    2 
 src/i830_sdvo.c |    4 -
 7 files changed, 149 insertions(+), 32 deletions(-)

Index: xf86_video_intel/src/i830_crt.c
===================================================================
--- xf86_video_intel.orig/src/i830_crt.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_crt.c	2009-04-21 12:00:27.000000000 +0800
@@ -376,7 +376,7 @@ i830_crt_detect_ddc(xf86OutputPtr output
 
     I830I2CInit(pScrn, &i830_output->pDDCBus, GPIOA, "CRTDDC_A");
     detect = xf86I2CProbeAddress(i830_output->pDDCBus, 0x00A0);
-    xf86DestroyI2CBusRec(i830_output->pDDCBus, TRUE, TRUE);
+    I830I2CDestroy(i830_output->pDDCBus, TRUE, TRUE);
 
     return detect;
 }
@@ -467,7 +467,7 @@ i830_get_edid(xf86OutputPtr output, int 
     edid_mon = xf86OutputGetEDID (output, intel_output->pDDCBus);
 
     if (!edid_mon || DIGITAL(edid_mon->features.input_type)) {
-	xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+	I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
 	intel_output->pDDCBus = NULL;
 	if (edid_mon) {
 	    xfree(edid_mon);
@@ -497,7 +497,7 @@ found:
     /* Destroy DDC bus after probe, so every other new probe will
        scan all ports again */
     if (intel_output->pDDCBus)
-	xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+	I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
 
     xf86OutputSetEDID (output, edid_mon);
 
Index: xf86_video_intel/src/i830_hdmi.c
===================================================================
--- xf86_video_intel.orig/src/i830_hdmi.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_hdmi.c	2009-04-21 12:00:27.000000000 +0800
@@ -214,7 +214,7 @@ i830_hdmi_destroy (xf86OutputPtr output)
     I830OutputPrivatePtr intel_output = output->driver_private;
 
     if (intel_output != NULL) {
-	xf86DestroyI2CBusRec(intel_output->pDDCBus, FALSE, FALSE);
+	I830I2CDestroy(intel_output->pDDCBus, FALSE, FALSE);
 	xfree(intel_output);
     }
 }
Index: xf86_video_intel/src/i830_lvds.c
===================================================================
--- xf86_video_intel.orig/src/i830_lvds.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_lvds.c	2009-04-21 12:00:27.000000000 +0800
@@ -1596,6 +1596,6 @@ found_mode:
     return;
 
 disable_exit:
-    xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+    I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
     xf86OutputDestroy(output);
 }
Index: xf86_video_intel/src/i830.h
===================================================================
--- xf86_video_intel.orig/src/i830.h	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830.h	2009-04-21 12:00:27.000000000 +0800
@@ -865,6 +865,7 @@ i830_pad_drawable_width(int width, int c
 
 extern Bool I830I2CInit(ScrnInfoPtr pScrn, I2CBusPtr *bus_ptr, int i2c_reg,
 			char *name);
+extern void I830I2CDestroy(I2CBusPtr b, Bool unalloc, Bool devs_too);
 
 /* return a mask of output indices matching outputs against type_mask */
 int i830_output_clones (ScrnInfoPtr pScrn, int type_mask);
Index: xf86_video_intel/src/i830_i2c.c
===================================================================
--- xf86_video_intel.orig/src/i830_i2c.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_i2c.c	2009-04-21 12:01:28.000000000 +0800
@@ -48,6 +48,17 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "shadow.h"
 #include "i830.h"
 
+struct i830_i2c_data {
+    Bool (*I2CGetByte) (I2CDevPtr d, I2CByte *data, Bool last);
+    Bool (*I2CPutByte) (I2CDevPtr d, I2CByte c);
+    Bool (*I2CStart)  (I2CBusPtr b, int timeout);
+    Bool (*I2CAddress)(I2CDevPtr d, I2CSlaveAddr);
+    void (*I2CStop)   (I2CDevPtr d);
+    int i2c_reg;
+};
+#define I2C_REG(b) \
+    (((struct i830_i2c_data *)b->DriverPrivate.ptr)->i2c_reg)
+
 #define AIRLIED_I2C	0
 
 #if AIRLIED_I2C
@@ -61,10 +72,10 @@ static void i830_setscl(I2CBusPtr b, int
     I830Ptr pI830 = I830PTR(pScrn);
     uint32_t val;
 
-    OUTREG(b->DriverPrivate.uval,
+    OUTREG(I2C_REG(b),
 	   (state ? GPIO_CLOCK_VAL_OUT : 0) | GPIO_CLOCK_DIR_OUT |
 	   GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK);
-    val = INREG(b->DriverPrivate.uval);
+    val = INREG(I2C_REG(b));
 }
 
 static void i830_setsda(I2CBusPtr b, int state)
@@ -73,10 +84,10 @@ static void i830_setsda(I2CBusPtr b, int
     I830Ptr pI830 = I830PTR(pScrn);
     uint32_t val;
 
-    OUTREG(b->DriverPrivate.uval,
+    OUTREG(I2C_REG(b),
 	   (state ? GPIO_DATA_VAL_OUT : 0) | GPIO_DATA_DIR_OUT |
 	   GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK);
-    val = INREG(b->DriverPrivate.uval);
+    val = INREG(I2C_REG(b));
 }
 
 static void i830_getscl(I2CBusPtr b, int *state)
@@ -85,9 +96,9 @@ static void i830_getscl(I2CBusPtr b, int
     I830Ptr pI830 = I830PTR(pScrn);
     uint32_t val;
 
-    OUTREG(b->DriverPrivate.uval, GPIO_CLOCK_DIR_IN | GPIO_CLOCK_DIR_MASK);
-    OUTREG(b->DriverPrivate.uval, 0);
-    val = INREG(b->DriverPrivate.uval);
+    OUTREG(I2C_REG(b), GPIO_CLOCK_DIR_IN | GPIO_CLOCK_DIR_MASK);
+    OUTREG(I2C_REG(b), 0);
+    val = INREG(I2C_REG(b));
     *state = ((val & GPIO_CLOCK_VAL_IN) != 0);
 }
 
@@ -97,9 +108,9 @@ static int i830_getsda(I2CBusPtr b)
      I830Ptr pI830 = I830PTR(pScrn);
      uint32_t val;
 
-     OUTREG(b->DriverPrivate.uval, GPIO_DATA_DIR_IN | GPIO_DATA_DIR_MASK);
-     OUTREG(b->DriverPrivate.uval, 0);
-     val = INREG(b->DriverPrivate.uval);
+     OUTREG(I2C_REG(b), GPIO_DATA_DIR_IN | GPIO_DATA_DIR_MASK);
+     OUTREG(I2C_REG(b), 0);
+     val = INREG(I2C_REG(b));
      return ((val & GPIO_DATA_VAL_IN) != 0);
 }
 
@@ -274,7 +285,7 @@ i830I2CGetBits(I2CBusPtr b, int *clock, 
     I830Ptr pI830 = I830PTR(pScrn);
     uint32_t val;
 
-    val = INREG(b->DriverPrivate.uval);
+    val = INREG(I2C_REG(b));
 
     /*
      * to read valid data, we must have written a 1 to
@@ -314,14 +325,14 @@ i830I2CPutBits(I2CBusPtr b, int clock, i
     }
 
     ErrorF("Setting %s 0x%08x to: %c %c\n", b->BusName,
-	   (int)b->DriverPrivate.uval,
+	   (int)(I2C_REG(b),
 	   clock ? '^' : 'v',
 	   data ? '^' : 'v');
 #endif
 
     if (!IS_I830(pI830) && !IS_845G(pI830)) {
 	/* On most chips, these bits must be preserved in software. */
-	reserved = INREG(b->DriverPrivate.uval) &
+	reserved = INREG(I2C_REG(b)) &
 	    (GPIO_DATA_PULLUP_DISABLE | GPIO_CLOCK_PULLUP_DISABLE);
     }
 
@@ -335,23 +346,104 @@ i830I2CPutBits(I2CBusPtr b, int clock, i
     else
 	clock_bits = GPIO_CLOCK_DIR_OUT|GPIO_CLOCK_DIR_MASK|GPIO_CLOCK_VAL_MASK;
     
-    OUTREG(b->DriverPrivate.uval, reserved | data_bits | clock_bits);
-    POSTING_READ(b->DriverPrivate.uval);
+    OUTREG(I2C_REG(b), reserved | data_bits | clock_bits);
+    POSTING_READ(I2C_REG(b));
 }
 
 #endif
 
+static void i830_i2c_quirk_set(I830Ptr pI830, Bool enable)
+{
+   if (!IS_IGD(pI830))
+	return;
+   if (enable)
+        /* Driver is using bit bashing for I2C, this bit should be set to 1 */
+        OUTREG(DSPCLK_GATE_D, INREG(DSPCLK_GATE_D) | DPCUNIT_CLOCK_GATE_DISABLE);
+   else
+        OUTREG(DSPCLK_GATE_D, INREG(DSPCLK_GATE_D) &(~DPCUNIT_CLOCK_GATE_DISABLE));
+}
+
+static Bool i830_i2c_get_byte(I2CDevPtr d, I2CByte *i2cdata, Bool last)
+{
+    I2CBusPtr b = d->pI2CBus;
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+    ScrnInfoPtr pScrn = xf86Screens[b->scrnIndex];
+    I830Ptr pI830 = I830PTR(pScrn);
+    Bool ret;
+
+    i830_i2c_quirk_set(pI830, TRUE);
+    ret = data->I2CGetByte(d, i2cdata, last);
+    i830_i2c_quirk_set(pI830, FALSE);
+    return ret;
+}
+
+static Bool i830_i2c_put_byte(I2CDevPtr d, I2CByte c)
+{
+    I2CBusPtr b = d->pI2CBus;
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+    ScrnInfoPtr pScrn = xf86Screens[b->scrnIndex];
+    I830Ptr pI830 = I830PTR(pScrn);
+    Bool ret;
+
+    i830_i2c_quirk_set(pI830, TRUE);
+    ret = data->I2CPutByte(d, c);
+    i830_i2c_quirk_set(pI830, FALSE);
+    return ret;
+}
+
+static Bool i830_i2c_start(I2CBusPtr b, int timeout)
+{
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+    ScrnInfoPtr pScrn = xf86Screens[b->scrnIndex];
+    I830Ptr pI830 = I830PTR(pScrn);
+
+    i830_i2c_quirk_set(pI830, TRUE);
+    return data->I2CStart(b, timeout);
+}
+
+static Bool i830_i2c_address(I2CDevPtr d, I2CSlaveAddr addr)
+{
+    I2CBusPtr b = d->pI2CBus;
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+    ScrnInfoPtr pScrn = xf86Screens[b->scrnIndex];
+    I830Ptr pI830 = I830PTR(pScrn);
+    Bool ret;
+
+    i830_i2c_quirk_set(pI830, TRUE);
+    ret = data->I2CAddress(d, addr);
+    i830_i2c_quirk_set(pI830, FALSE);
+    return ret;
+}
+
+static void i830_i2c_stop(I2CDevPtr d)
+{
+    I2CBusPtr b = d->pI2CBus;
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+    ScrnInfoPtr pScrn = xf86Screens[b->scrnIndex];
+    I830Ptr pI830 = I830PTR(pScrn);
+
+    data->I2CStop(d);
+    i830_i2c_quirk_set(pI830, FALSE);
+}
+
 /* the i830 has a number of I2C Buses */
 Bool
 I830I2CInit(ScrnInfoPtr pScrn, I2CBusPtr *bus_ptr, int i2c_reg, char *name)
 {
     I2CBusPtr pI2CBus;
     I830Ptr pI830 = I830PTR(pScrn);
+    struct i830_i2c_data *data;
+
+    data = xalloc(sizeof(*data));
+    if (!data)
+        return FALSE;
 
     pI2CBus = xf86CreateI2CBusRec();
 
-    if (!pI2CBus)
+    if (!pI2CBus) {
+        xfree(data);
 	return FALSE;
+    }
 
     pI2CBus->BusName = name;
     pI2CBus->scrnIndex = pScrn->scrnIndex;
@@ -365,7 +457,7 @@ I830I2CInit(ScrnInfoPtr pScrn, I2CBusPtr
     pI2CBus->I2CGetBits = i830I2CGetBits;
     pI2CBus->I2CPutBits = i830I2CPutBits;
 #endif
-    pI2CBus->DriverPrivate.uval = i2c_reg;
+    pI2CBus->DriverPrivate.ptr = data;
 
     /* Assume all busses are used for DDCish stuff */
     
@@ -386,9 +478,33 @@ I830I2CInit(ScrnInfoPtr pScrn, I2CBusPtr
      */
     OUTREG(GMBUS0, 0);
 
-    if (!xf86I2CBusInit(pI2CBus))
+    if (!xf86I2CBusInit(pI2CBus)) {
+        xfree(data);
 	return FALSE;
+    }
+
+    data->I2CGetByte = pI2CBus->I2CGetByte;
+    data->I2CPutByte = pI2CBus->I2CPutByte;
+    data->I2CStart = pI2CBus->I2CStart;
+    data->I2CAddress = pI2CBus->I2CAddress;
+    data->I2CStop = pI2CBus->I2CStop;
+    data->i2c_reg = i2c_reg;
+
+    pI2CBus->I2CGetByte = i830_i2c_get_byte;
+    pI2CBus->I2CPutByte = i830_i2c_put_byte;
+    pI2CBus->I2CStart = i830_i2c_start;
+    pI2CBus->I2CAddress = i830_i2c_address;
+    pI2CBus->I2CStop = i830_i2c_stop;
 
     *bus_ptr = pI2CBus;
     return TRUE;
 }
+
+void I830I2CDestroy(I2CBusPtr b, Bool unalloc, Bool devs_too)
+{
+    struct i830_i2c_data *data = b->DriverPrivate.ptr;
+
+    xf86DestroyI2CBusRec(b, unalloc, devs_too);
+    if (unalloc)
+        xfree(data);
+}
Index: xf86_video_intel/src/i830_dvo.c
===================================================================
--- xf86_video_intel.orig/src/i830_dvo.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_dvo.c	2009-04-21 12:00:27.000000000 +0800
@@ -326,9 +326,9 @@ i830_dvo_destroy (xf86OutputPtr output)
 	if (intel_output->i2c_drv->vid_rec->destroy)
 	    intel_output->i2c_drv->vid_rec->destroy (intel_output->i2c_drv->dev_priv);
 	if (intel_output->pI2CBus)
-	    xf86DestroyI2CBusRec (intel_output->pI2CBus, TRUE, TRUE);
+	    I830I2CDestroy(intel_output->pI2CBus, TRUE, TRUE);
 	if (intel_output->pDDCBus)
-	    xf86DestroyI2CBusRec (intel_output->pDDCBus, TRUE, TRUE);
+	    I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
 	xfree (intel_output);
     }
 }
@@ -473,7 +473,7 @@ i830_dvo_init(ScrnInfoPtr pScrn)
 	 */
 	if (gpio_inited != gpio) {
 	    if (pI2CBus != NULL)
-		xf86DestroyI2CBusRec(pI2CBus, TRUE, TRUE);
+		I830I2CDestroy(pI2CBus, TRUE, TRUE);
 	    if (!I830I2CInit(pScrn, &pI2CBus, gpio,
 			     gpio == GPIOB ? "DVOI2C_B" : "DVOI2C_E")) {
 		continue;
@@ -509,8 +509,8 @@ i830_dvo_init(ScrnInfoPtr pScrn)
 		break;
 	    }
 	    if (output == NULL) {
-		xf86DestroyI2CBusRec(pI2CBus, TRUE, TRUE);
-		xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+		I830I2CDestroy(pI2CBus, TRUE, TRUE);
+		I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
 		xfree(intel_output);
 		xf86UnloadSubModule(drv->modhandle);
 		return;
@@ -544,7 +544,7 @@ i830_dvo_init(ScrnInfoPtr pScrn)
 
     /* Didn't find a chip, so tear down. */
     if (pI2CBus != NULL)
-	xf86DestroyI2CBusRec(pI2CBus, TRUE, TRUE);
-    xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+	I830I2CDestroy(pI2CBus, TRUE, TRUE);
+    I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
     xfree(intel_output);
 }
Index: xf86_video_intel/src/i830_sdvo.c
===================================================================
--- xf86_video_intel.orig/src/i830_sdvo.c	2009-04-21 12:00:16.000000000 +0800
+++ xf86_video_intel/src/i830_sdvo.c	2009-04-21 12:00:27.000000000 +0800
@@ -1752,7 +1752,7 @@ i830_sdvo_get_ddc_modes(xf86OutputPtr ou
 	crt->funcs->detect(crt) == XF86OutputStatusDisconnected) {
 	I830I2CInit(pScrn, &intel_output->pDDCBus, GPIOA, "CRTDDC_A");
 	edid_mon = xf86OutputGetEDID(crt, intel_output->pDDCBus);
-	xf86DestroyI2CBusRec(intel_output->pDDCBus, TRUE, TRUE);
+	I830I2CDestroy(intel_output->pDDCBus, TRUE, TRUE);
     }
     if (edid_mon) {
 	xf86OutputSetEDID(output, edid_mon);
@@ -1905,7 +1905,7 @@ i830_sdvo_destroy (xf86OutputPtr output)
 
 	xf86DestroyI2CBusRec (intel_output->pDDCBus, FALSE, FALSE);
 	xf86DestroyI2CDevRec (&dev_priv->d, FALSE);
-	xf86DestroyI2CBusRec (dev_priv->d.pI2CBus, TRUE, TRUE);
+	I830I2CDestroy(dev_priv->d.pI2CBus, TRUE, TRUE);
 
 	if (output->randr_output) {
 	    RROutputPtr	randr_output = output->randr_output;





More information about the Intel-gfx mailing list