i2c: add convenience function xf86I2CMaskByte.

I love mask functions, they make code clear and therefor directly
contribute to less bugs.

10 files changed, 117 insertions, 98 deletions

diff --git a/src/Imakefile b/src/Imakefile
index 663019e..b964714 100644
--- a/src/Imakefile
+++ b/src/Imakefile
@@ -192,7 +192,7 @@ DependTarget()
 InstallDriverSDKNonExecFile(Imakefile,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_accel.c,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_ch7xxx.c,$(DRIVERSDKDIR)/drivers/via)
-InstallDriverSDKNonExecFile(via_ch7xxx.h,$(DRIVERSDKDIR)/drivers/via)
+InstallDriverSDKNonExecFile(via_ch7011_tables.c,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_crt.c,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_crtc.c,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_crtc.h,$(DRIVERSDKDIR)/drivers/via)
@@ -224,6 +224,6 @@ InstallDriverSDKNonExecFile(via_video.c,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_video.h,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_videoregs.h,$(DRIVERSDKDIR)/drivers/via)
 InstallDriverSDKNonExecFile(via_vt162x.c,$(DRIVERSDKDIR)/drivers/via)
-InstallDriverSDKNonExecFile(via_vt162x.h,$(DRIVERSDKDIR)/drivers/via)
+InstallDriverSDKNonExecFile(via_vt162x_tables.c,$(DRIVERSDKDIR)/drivers/via)
 
 InstallDriverSDKObjectModule(unichrome,$(DRIVERSDKMODULEDIR),drivers)
diff --git a/src/Makefile.am b/src/Makefile.am
index 65f4e2f..b39b553 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -39,7 +39,6 @@ via_driver.c: $(top_srcdir)/git_version.h
 unichrome_drv_la_SOURCES = \
          via_accel.c \
          via_ch7xxx.c \
-         via_ch7xxx.h \
          via_crt.c \
 	 via_crtc.c \
          via_crtc.h \
@@ -63,8 +62,7 @@ unichrome_drv_la_SOURCES = \
          via_sii16x.c \
          via_vgahw.c \
          via_vgahw.h \
-         via_vt162x.c \
-         via_vt162x.h
+         via_vt162x.c
 
 if XV
 unichrome_drv_la_SOURCES += \
diff --git a/src/via_ch7xxx.h b/src/via_ch7011_tables.c
index d6ffb17..13e497d 100644
--- a/src/via_ch7xxx.h
+++ b/src/via_ch7011_tables.c
@@ -21,9 +21,6 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
-#ifndef _VIA_CH7XXX_H_
-#define _VIA_CH7XXX_H_ 1
-
 /*
  * All the info here is nicely documented in Chrontel datasheets.
  * Chrontel makes these freely available. Don't you just love 'em?
@@ -158,4 +155,3 @@ static DisplayModeRec CH7011TVModesPAL[] = {
     { MODEPREFIX(NULL), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,0, 0,0,0,0,0,0,0, 0,0,0,0,0,0,FALSE,FALSE, 0, NULL, 0,0.0,0.0},
 };
 
-#endif /* _VIA_CH7XXX_H_ */
diff --git a/src/via_ch7xxx.c b/src/via_ch7xxx.c
index c53b6fb..0c8b2a4 100644
--- a/src/via_ch7xxx.c
+++ b/src/via_ch7xxx.c
@@ -37,7 +37,9 @@
 #include "via_output.h"
 #include "via_mode.h"
 
-#include "via_ch7xxx.h"
+#include "via_i2c.h"
+
+#include "via_ch7011_tables.c"
 
 /*
  *
@@ -236,7 +238,7 @@ CH7011Restore(struct ViaOutput *Output)
 static CARD8
 CH7xxxTVDACSense(struct ViaOutput *Output)
 {
-    CARD8 dacsave, save, sense;
+    CARD8 dacsave, sense;
 
     VIAFUNC(Output);
 
@@ -245,18 +247,15 @@ CH7xxxTVDACSense(struct ViaOutput *Output)
     xf86I2CWriteByte(Output->I2CDev, 0x49, 0x20);
 
     /* Disable bypass mode: DACBP = 0 */
-    xf86I2CReadByte(Output->I2CDev, 0x21, &save);
-    xf86I2CWriteByte(Output->I2CDev, 0x21, save & 0xFE);
+    xf86I2CMaskByte(Output->I2CDev, 0x21, 0, 0x01);
 
     /* SENSE = 1 */
-    xf86I2CReadByte(Output->I2CDev, 0x20, &save);
-    xf86I2CWriteByte(Output->I2CDev, 0x20, save | 0x01);
+    xf86I2CMaskByte(Output->I2CDev, 0x20, 0x01, 0x01);
 
     Output->I2CDev->pI2CBus->I2CUDelay(Output->I2CDev->pI2CBus, 10);
 
     /* Restore SENSE */
-    xf86I2CReadByte(Output->I2CDev, 0x20, &save);
-    xf86I2CWriteByte(Output->I2CDev, 0x20, save & 0xFE);
+    xf86I2CMaskByte(Output->I2CDev, 0x20, 0, 0x01);
 
     /* Read DAC0-3 */
     xf86I2CReadByte(Output->I2CDev, 0x20, &sense);
@@ -427,16 +426,13 @@ CH7011TVClock(I2CDevPtr I2CDev, int clock)
  * ModeValid should ensure that only modes with proper privates are passed here.
  */
 static void
-CH7011TVMode(struct ViaOutput *Output, DisplayModePtr mode)
+CH7011TVMode(struct ViaOutput *Output, DisplayModePtr Mode)
 {
-    struct CH7011TableRec *Table = NULL;
-    CARD8 tmp;
-    int  i;
+    struct CH7011TableRec *Table = (struct CH7011TableRec *) Mode->Private;
+    CARD32 Active;
 
     VIAFUNC(Output);
 
-    Table = (struct CH7011TableRec *) mode->Private;
-
     xf86I2CWriteByte(Output->I2CDev, 0x00, Table->Mode);
     xf86I2CWriteByte(Output->I2CDev, 0x01, 0x3F);
 
@@ -447,23 +443,19 @@ CH7011TVMode(struct ViaOutput *Output, DisplayModePtr mode)
         xf86I2CWriteByte(Output->I2CDev, 0x02, 0xE0); /* default */
 
     /* Text Enhancement */
-    xf86I2CReadByte(Output->I2CDev, 0x03, &tmp);
-    xf86I2CWriteByte(Output->I2CDev, 0x03, tmp | 0x07); /* max */
+    xf86I2CMaskByte(Output->I2CDev, 0x03, 0x07, 0x07); /* max */
 
     /* Start Active Video */
-    i = mode->CrtcHTotal - mode->CrtcHSyncEnd + 1;
-    xf86I2CReadByte(Output->I2CDev, 0x03, &tmp);
-    xf86I2CWriteByte(Output->I2CDev, 0x03, (tmp & 0xDF) | ((i >> 3) & 0x20));
-    xf86I2CWriteByte(Output->I2CDev, 0x04, i & 0xFF);
+    Active = Mode->CrtcHTotal - Mode->CrtcHSyncEnd + 1;
+    xf86I2CMaskByte(Output->I2CDev, 0x03, Active >> 3, 0x20);
+    xf86I2CWriteByte(Output->I2CDev, 0x04, Active & 0xFF);
 
     /* Horizontal Position */
-    xf86I2CReadByte(Output->I2CDev, 0x03, &tmp);
-    xf86I2CWriteByte(Output->I2CDev, 0x03, (tmp & 0xEF) | ((Table->HPosition >> 4) & 0x10));
+    xf86I2CMaskByte(Output->I2CDev, 0x03, Table->HPosition >> 4, 0x10);
     xf86I2CWriteByte(Output->I2CDev, 0x05, Table->HPosition & 0xFF);
 
     /* Vertical Position */
-    xf86I2CReadByte(Output->I2CDev, 0x03, &tmp);
-    xf86I2CWriteByte(Output->I2CDev, 0x03, (tmp & 0xF7) | ((Table->VPosition >> 5) & 0x08));
+    xf86I2CMaskByte(Output->I2CDev, 0x03, Table->VPosition >> 5, 0x08);
     xf86I2CWriteByte(Output->I2CDev, 0x06, Table->VPosition & 0xFF);
 
     /* Black level */
@@ -475,8 +467,7 @@ CH7011TVMode(struct ViaOutput *Output, DisplayModePtr mode)
     /* contrast */
     xf86I2CWriteByte(Output->I2CDev, 0x08, 0x03); /* default to 3 */
 
-
-    CH7011TVClock(Output->I2CDev, mode->Clock);
+    CH7011TVClock(Output->I2CDev, Mode->Clock);
 
     xf86I2CWriteByte(Output->I2CDev, 0x0C, (Table->FSCI >> 24) & 0xFF);
     xf86I2CWriteByte(Output->I2CDev, 0x0D, (Table->FSCI >> 16) & 0xFF);
@@ -486,8 +477,7 @@ CH7011TVMode(struct ViaOutput *Output, DisplayModePtr mode)
     xf86I2CWriteByte(Output->I2CDev, 0x10, 0x00);
 
     /* stop dark band in lower half (undocumented register) */
-    xf86I2CReadByte(Output->I2CDev, 0x15, &tmp);
-    xf86I2CWriteByte(Output->I2CDev, 0x15, tmp & 0xF8);
+    xf86I2CMaskByte(Output->I2CDev, 0x15, 0, 0x07);
 
     xf86I2CWriteByte(Output->I2CDev, 0x1C, 0x48);
     xf86I2CWriteByte(Output->I2CDev, 0x1D, 0x40);
@@ -507,14 +497,11 @@ CH7011TVMode(struct ViaOutput *Output, DisplayModePtr mode)
 static void
 CH7xxxTVPower(struct ViaOutput *Output, Bool On)
 {
-    CARD8 save;
-
     VIAFUNC(Output);
 
     if (On) {
         /* POUTE */
-        xf86I2CReadByte(Output->I2CDev, 0x1E, &save);
-        xf86I2CWriteByte(Output->I2CDev, 0x1E, save | 0x02);
+        xf86I2CMaskByte(Output->I2CDev, 0x1E, 0x02, 0x02);
 
 	/* Plainly enables all DACs - should be more specific. */
 	xf86I2CWriteByte(Output->I2CDev, 0x49, 0x20);
@@ -522,8 +509,7 @@ CH7xxxTVPower(struct ViaOutput *Output, Bool On)
 	xf86I2CWriteByte(Output->I2CDev, 0x49, 0x3E);
 
         /* !POUTE */
-        xf86I2CReadByte(Output->I2CDev, 0x1E, &save);
-        xf86I2CWriteByte(Output->I2CDev, 0x1E, save & ~0x02);
+        xf86I2CMaskByte(Output->I2CDev, 0x1E, 0, 0x02);
     }
 }
 
diff --git a/src/via_i2c.c b/src/via_i2c.c
index 086320a..898d836 100644
--- a/src/via_i2c.c
+++ b/src/via_i2c.c
@@ -357,3 +357,22 @@ ViaI2CInit(ScrnInfoPtr pScrn)
 	pVia->I2CBus[4] = I2CBus5Init(pScrn->scrnIndex);
     }
 }
+
+/*
+ * Sure, it is polluting namespace, but this one is quite useful.
+ */
+Bool
+xf86I2CMaskByte(I2CDevPtr d, I2CByte subaddr, I2CByte value, I2CByte mask)
+{
+    I2CByte tmp;
+    Bool ret;
+
+    ret = xf86I2CReadByte(d, subaddr, &tmp);
+    if (!ret)
+	return FALSE;
+
+    tmp &= ~mask;
+    tmp |= (value & mask);
+
+    return xf86I2CWriteByte(d, subaddr, tmp);
+}
diff --git a/src/via_i2c.h b/src/via_i2c.h
new file mode 100644
index 0000000..bef73a4
--- /dev/null
+++ b/src/via_i2c.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2009 Luc Verhaegen.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#ifndef HAVE_VIA_I2C_H
+#define HAVE_VIA_I2C_H
+
+/*
+ * Really useful convenience function that maybe should live in the X i2c
+ * infrastructure.
+ */
+Bool xf86I2CMaskByte(I2CDevPtr d, I2CByte subaddr, I2CByte value, I2CByte mask);
+
+#endif /* HAVE_VIA_I2C_H */
diff --git a/src/via_output.c b/src/via_output.c
index 825f401..4357e67 100644
--- a/src/via_output.c
+++ b/src/via_output.c
@@ -192,8 +192,8 @@ ViaVT1631Init(ScrnInfoPtr pScrn, I2CDevPtr pDev)
         return NULL;
     default:
         xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "%s: Unknown VIA device "
-                   "detected on %s:0x%02X: 0x%08lX.\n", __func__,
-                   pDev->pI2CBus->BusName, pDev->SlaveAddr, ID);
+                   "detected on %s:0x%02X: 0x%08X.\n", __func__,
+                   pDev->pI2CBus->BusName, pDev->SlaveAddr, (unsigned int) ID);
         return NULL;
     }
 
diff --git a/src/via_sii16x.c b/src/via_sii16x.c
index 3f8c767..1b43ec4 100644
--- a/src/via_sii16x.c
+++ b/src/via_sii16x.c
@@ -32,6 +32,7 @@
 #endif
 
 #include "via_output.h"
+#include "via_i2c.h"
 
 /*
  *
@@ -178,15 +179,12 @@ SiI16xMode(struct ViaOutput *Output, DisplayModePtr mode)
 static void
 SiI16xPower(struct ViaOutput *Output, Bool On)
 {
-    CARD8 buf;
-
     VIAFUNC(Output);
 
-    xf86I2CReadByte(Output->I2CDev, 0x08, &buf);
     if (On)
-	xf86I2CWriteByte(Output->I2CDev, 0x08, buf | 0x01);
+	xf86I2CMaskByte(Output->I2CDev, 0x08, 0x01, 0x01);
     else
-	xf86I2CWriteByte(Output->I2CDev, 0x08, buf & 0xFE);
+	xf86I2CMaskByte(Output->I2CDev, 0x08, 0, 0x01);
 }
 
 /*
@@ -278,8 +276,8 @@ ViaSiI16xInit(ScrnInfoPtr pScrn, I2CDevPtr pDev)
 
     default:
         xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "%s: Unknown SiI16x compatible"
-                   " device detected on %s:0x%02X: 0x%08lX.\n", __func__,
-                   pDev->pI2CBus->BusName, pDev->SlaveAddr, ID);
+                   " device detected on %s:0x%02X: 0x%08X.\n", __func__,
+                   pDev->pI2CBus->BusName, pDev->SlaveAddr, (unsigned int) ID);
         return NULL;
     }
 
diff --git a/src/via_vt162x.c b/src/via_vt162x.c
index 8920962..b4aec8f 100644
--- a/src/via_vt162x.c
+++ b/src/via_vt162x.c
@@ -38,8 +38,9 @@
 #include "via_id.h"
 #include "via_output.h"
 #include "via_mode.h"
+#include "via_i2c.h"
 
-#include "via_vt162x.h"
+#include "via_vt162x_tables.c"
 
 /*
  *
@@ -387,7 +388,7 @@ VT1621TVStandardSet(struct ViaOutput *Output, int Standard)
  *
  */
 static CARD8
-VT1621ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
+VT1621ModeIndex(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
     int i;
@@ -395,14 +396,14 @@ VT1621ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
     VIAFUNC(Output);
 
     for (i = 0; VT1621Table[i].Width; i++) {
-	if ((VT1621Table[i].Width == mode->CrtcHDisplay) &&
-	    (VT1621Table[i].Height == mode->CrtcVDisplay) &&
+	if ((VT1621Table[i].Width == Mode->CrtcHDisplay) &&
+	    (VT1621Table[i].Height == Mode->CrtcVDisplay) &&
 	    (VT1621Table[i].Standard == Private->Standard) &&
-	    !(strcmp(VT1621Table[i].name, mode->name)))
+	    !(strcmp(VT1621Table[i].name, Mode->name)))
 	    return i;
     }
     xf86DrvMsg(Output->scrnIndex, X_WARNING, "%s: Mode \"%s\" not found in "
-               "Table\n", __func__, mode->name);
+               "Table\n", __func__, Mode->name);
     return 0xFF;
 }
 
@@ -410,25 +411,25 @@ VT1621ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
  *
  */
 static ModeStatus
-VT1621ModeValid(struct ViaOutput *Output, DisplayModePtr mode)
+VT1621ModeValid(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
 
     VIAFUNC(Output);
 
-    if ((mode->PrivSize != sizeof(struct VT162xModePrivate)) ||
-	((mode->Private != (void *) &VT162xModePrivateNTSC) &&
-	 (mode->Private != (void *) &VT162xModePrivatePAL)))
+    if ((Mode->PrivSize != sizeof(struct VT162xModePrivate)) ||
+	((Mode->Private != (void *) &VT162xModePrivateNTSC) &&
+	 (Mode->Private != (void *) &VT162xModePrivatePAL)))
 	return MODE_OUTPUT_UNDEF;
 
     if ((Private->Standard == TVSTANDARD_NTSC) &&
-	(mode->Private != (void *) &VT162xModePrivateNTSC))
+	(Mode->Private != (void *) &VT162xModePrivateNTSC))
 	return MODE_NOT_NTSC;
     else if ((Private->Standard == TVSTANDARD_PAL) &&
-             (mode->Private != (void *) &VT162xModePrivatePAL))
+             (Mode->Private != (void *) &VT162xModePrivatePAL))
 	return MODE_NOT_PAL;
 
-    if (VT1621ModeIndex(Output, mode) != 0xFF)
+    if (VT1621ModeIndex(Output, Mode) != 0xFF)
 	return MODE_OK;
     return MODE_ERROR;
 }
@@ -533,7 +534,7 @@ VT1623TVStandardSet(struct ViaOutput *Output, int Standard)
  *
  */
 static CARD8
-VT1622ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
+VT1622ModeIndex(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
     struct VT162XTableRec *Table;
@@ -547,14 +548,14 @@ VT1622ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
 	Table = VT1623Table;
 
     for (i = 0; Table[i].Width; i++) {
-	if ((Table[i].Width == mode->CrtcHDisplay) &&
-	    (Table[i].Height == mode->CrtcVDisplay) &&
+	if ((Table[i].Width == Mode->CrtcHDisplay) &&
+	    (Table[i].Height == Mode->CrtcVDisplay) &&
 	    (Table[i].Standard == Private->Standard) &&
-	    !strcmp(Table[i].name, mode->name))
+	    !strcmp(Table[i].name, Mode->name))
 	    return i;
     }
     xf86DrvMsg(Output->scrnIndex, X_WARNING, "%s: Mode \"%s\" not found in "
-               "Table\n", __func__, mode->name);
+               "Table\n", __func__, Mode->name);
     return 0xFF;
 }
 
@@ -562,25 +563,25 @@ VT1622ModeIndex(struct ViaOutput *Output, DisplayModePtr mode)
  *
  */
 static ModeStatus
-VT1622ModeValid(struct ViaOutput *Output, DisplayModePtr mode)
+VT1622ModeValid(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
 
     VIAFUNC(Output);
 
-    if ((mode->PrivSize != sizeof(struct VT162xModePrivate)) ||
-	((mode->Private != (void *) &VT162xModePrivateNTSC) &&
-	 (mode->Private != (void *) &VT162xModePrivatePAL)))
+    if ((Mode->PrivSize != sizeof(struct VT162xModePrivate)) ||
+	((Mode->Private != (void *) &VT162xModePrivateNTSC) &&
+	 (Mode->Private != (void *) &VT162xModePrivatePAL)))
 	return MODE_OUTPUT_UNDEF;
 
     if ((Private->Standard == TVSTANDARD_NTSC) &&
-	(mode->Private != (void *) &VT162xModePrivateNTSC))
+	(Mode->Private != (void *) &VT162xModePrivateNTSC))
 	return MODE_NOT_NTSC;
      else if ((Private->Standard == TVSTANDARD_PAL) &&
-              (mode->Private != (void *) &VT162xModePrivatePAL))
+              (Mode->Private != (void *) &VT162xModePrivatePAL))
 	return MODE_NOT_PAL;
 
-    if (VT1622ModeIndex(Output, mode) != 0xFF)
+    if (VT1622ModeIndex(Output, Mode) != 0xFF)
 	return MODE_OK;
     return MODE_ERROR;
 }
@@ -601,10 +602,10 @@ VT162xSetSubCarrier(I2CDevPtr pDev, CARD32 SubCarrier)
  *
  */
 static void
-VT1621Mode(struct ViaOutput *Output, DisplayModePtr mode)
+VT1621Mode(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
-    struct VT1621TableRec Table = VT1621Table[VT1621ModeIndex(Output, mode)];
+    struct VT1621TableRec Table = VT1621Table[VT1621ModeIndex(Output, Mode)];
     CARD8 i;
 
     VIAFUNC(Output);
@@ -637,9 +638,7 @@ VT1621Mode(struct ViaOutput *Output, DisplayModePtr mode)
 
     if (Private->DotCrawl) {
 	if (Table.DotCrawlSubCarrier) {
-	    xf86I2CReadByte(Output->I2CDev, 0x11, &i);
-	    xf86I2CWriteByte(Output->I2CDev, 0x11, i | 0x08);
-
+	    xf86I2CMaskByte(Output->I2CDev, 0x11, 0x08, 0x08);
 	    VT162xSetSubCarrier(Output->I2CDev, Table.DotCrawlSubCarrier);
 	} else
 	    xf86DrvMsg(Output->scrnIndex, X_INFO, "This mode does not currently "
@@ -651,18 +650,18 @@ VT1621Mode(struct ViaOutput *Output, DisplayModePtr mode)
  * also suited for VT1622A, VT1623
  */
 static void
-VT1622Mode(struct ViaOutput *Output, DisplayModePtr mode)
+VT1622Mode(struct ViaOutput *Output, DisplayModePtr Mode)
 {
     struct VT162xOutputPrivate *Private = Output->Private;
     struct VT162XTableRec Table;
-    CARD8 save, i;
+    CARD8 i;
 
     VIAFUNC(Output);
 
     if (Private->Device == VT1622)
-	Table = VT1622Table[VT1622ModeIndex(Output, mode)];
+	Table = VT1622Table[VT1622ModeIndex(Output, Mode)];
     else /* VT1622A/VT1623 */
-	Table = VT1623Table[VT1622ModeIndex(Output, mode)];
+	Table = VT1623Table[VT1622ModeIndex(Output, Mode)];
 
     /* TV Reset */
     xf86I2CWriteByte(Output->I2CDev, 0x1D, 0x00);
@@ -689,9 +688,7 @@ VT1622Mode(struct ViaOutput *Output, DisplayModePtr mode)
 
     if (Private->DotCrawl) {
 	if (Table.DotCrawlSubCarrier) {
-	    xf86I2CReadByte(Output->I2CDev, 0x11, &save);
-	    xf86I2CWriteByte(Output->I2CDev, 0x11, save | 0x08);
-
+	    xf86I2CMaskByte(Output->I2CDev, 0x11, 0x08, 0x08);
 	    VT162xSetSubCarrier(Output->I2CDev, Table.DotCrawlSubCarrier);
 	} else
 	    xf86DrvMsg(Output->scrnIndex, X_INFO, "This mode does not currently "
@@ -717,13 +714,10 @@ VT1622Mode(struct ViaOutput *Output, DisplayModePtr mode)
     }
 
     /* Configure flicker filter */
-    xf86I2CReadByte(Output->I2CDev, 0x03, &save);
-    save &= 0xFC;
     if (Private->Deflicker == 1)
-	save |= 0x01;
+	xf86I2CMaskByte(Output->I2CDev, 0x03, 0x01, 0x03);
     else if (Private->Deflicker == 2)
-        save |= 0x02;
-    xf86I2CWriteByte(Output->I2CDev, 0x03, save);
+	xf86I2CMaskByte(Output->I2CDev, 0x03, 0x02, 0x03);
 }
 
 /*
diff --git a/src/via_vt162x.h b/src/via_vt162x_tables.c
index 0920b16..74dabb5 100644
--- a/src/via_vt162x.h
+++ b/src/via_vt162x_tables.c
@@ -21,8 +21,6 @@
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  */
-#ifndef _VIA_VT162X_H_
-#define _VIA_VT162X_H_ 1
 
 struct VT162xModePrivate {
     char   id[12]; /* "VT162x" */
@@ -717,5 +715,3 @@ VT1623Table[] = {
       0, 0,
     }
 };
-
-#endif /* _VIA_VT162X_H_ */