Clean up the SDVO code.

The main change is to send SDVO commands using data passed into the send
command function, and receive responses into memory passed into the read
response function, rather than stuff things in/out through dev_priv->sdvo_regs.
This lets us use structures to represent some arguments, which results in a
nice cleanup (and 100% fewer arguments named magicN as a side effect).

Also, the mode set path is changed to not do any preferred input timing
work.  We weren't doing anything legitimate with the results, since we didn't
modify the CRTC timing appropriately, so now we just stuff the CRTC timing into
both and hope for the best.  This should probably be revisited later.

1 files changed, 261 insertions, 421 deletions

diff --git a/src/i830_sdvo.c b/src/i830_sdvo.c
index 523eed27..ac5ae48e 100644
--- a/src/i830_sdvo.c
+++ b/src/i830_sdvo.c
@@ -54,8 +54,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 struct i830_sdvo_priv {
     /** SDVO device on SDVO I2C bus. */
     I2CDevRec d;
-    /** Temporary storage for reg read/writes */
-    unsigned char sdvo_regs[20];
     /** Register for the SDVO device: SDVOB or SDVOC */
     int output_device;
     /**
@@ -69,14 +67,12 @@ struct i830_sdvo_priv {
     /** @{ */
     int save_sdvo_mult;
     Bool save_sdvo_active_1, save_sdvo_active_2;
-    i830_sdvo_dtd save_input_dtd_1, save_input_dtd_2;
-    i830_sdvo_dtd save_output_dtd_1, save_output_dtd_2;
+    struct i830_sdvo_dtd save_input_dtd_1, save_input_dtd_2;
+    struct i830_sdvo_dtd save_output_dtd_1, save_output_dtd_2;
     CARD32 save_SDVOX;
     /** @} */
 };
 
-CARD16 curr_table[6];
-
 /** Read a single byte from the given address on the SDVO device. */
 static Bool i830_sdvo_read_byte(I830OutputPtr output, int addr,
 				unsigned char *ch)
@@ -153,33 +149,36 @@ const struct _sdvo_cmd_name {
     SDVO_CMD_NAME_ENTRY(SDVO_CMD_SET_CONTROL_BUS_SWITCH),
 };
 
-/* following on from tracing the intel BIOS i2c routines */
+/**
+ * Writes out the data given in args (up to 8 bytes), followed by the opcode.
+ */
 static void
-i830_sdvo_write_outputs(I830OutputPtr output, int num_out)
+i830_sdvo_write_cmd(I830OutputPtr output, CARD8 cmd, void *args, int args_len)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
     int i;
 
-    ErrorF("SDVO: W: %02X ", dev_priv->sdvo_regs[SDVO_I2C_OPCODE]);
-    for (i = SDVO_I2C_ARG_0; i > SDVO_I2C_ARG_0 - num_out; i--)
-	ErrorF("%02X ", dev_priv->sdvo_regs[i]);
-    for (; i > SDVO_I2C_ARG_7; i--)
-	ErrorF("   ");
+    /* Write the SDVO command logging */
+    xf86DrvMsg(output->pI2CBus->scrnIndex, X_INFO, "SDVO: W: %02X ", cmd);
+    for (i = 0; i < args_len; i++)
+	LogWrite(1, "%02X ", ((CARD8 *)args)[i]);
+    for (; i < 8; i++)
+	LogWrite(1, "   ");
     for (i = 0; i < sizeof(sdvo_cmd_names) / sizeof(sdvo_cmd_names[0]); i++) {
-	if (dev_priv->sdvo_regs[SDVO_I2C_OPCODE] == sdvo_cmd_names[i].cmd) {
-	    ErrorF("(%s)", sdvo_cmd_names[i].name);
+	if (cmd == sdvo_cmd_names[i].cmd) {
+	    LogWrite(1, "(%s)", sdvo_cmd_names[i].name);
 	    break;
 	}
     }
-    ErrorF("\n");
+    if (i == sizeof(sdvo_cmd_names) / sizeof(sdvo_cmd_names[0]))
+	LogWrite(1, "(%02X)", cmd);
+    LogWrite(1, "\n");
 
-    /* blast the output regs */
-    for (i = SDVO_I2C_ARG_0; i > SDVO_I2C_ARG_0 - num_out; i--) {
-	i830_sdvo_write_byte(output, i, dev_priv->sdvo_regs[i]);
+    /* send the output regs */
+    for (i = 0; i < args_len; i++) {
+	i830_sdvo_write_byte(output, SDVO_I2C_ARG_0 - i, ((CARD8 *)args)[i]);
     }
     /* blast the command reg */
-    i830_sdvo_write_byte(output, SDVO_I2C_OPCODE,
-			 dev_priv->sdvo_regs[SDVO_I2C_OPCODE]);
+    i830_sdvo_write_byte(output, SDVO_I2C_OPCODE, cmd);
 }
 
 static const char *cmd_status_names[] = {
@@ -192,45 +191,40 @@ static const char *cmd_status_names[] = {
 	"Scaling not supported"
 };
 
-static void
-i830_sdvo_read_input_regs(I830OutputPtr output)
+/**
+ * Reads back response_len bytes from the SDVO device, and returns the status.
+ */
+static CARD8
+i830_sdvo_read_response(I830OutputPtr output, void *response, int response_len)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
     int i;
+    CARD8 status;
 
-    /* follow BIOS ordering */
-    i830_sdvo_read_byte(output, SDVO_I2C_CMD_STATUS,
-			&dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS]);
-
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_3,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_3]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_2,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_2]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_1,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_1]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_0,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_0]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_7,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_7]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_6,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_6]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_5,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_5]);
-    i830_sdvo_read_byte(output, SDVO_I2C_RETURN_4,
-			&dev_priv->sdvo_regs[SDVO_I2C_RETURN_4]);
-
-    ErrorF("SDVO: R: ");
-    for (i = SDVO_I2C_RETURN_0; i <= SDVO_I2C_RETURN_7; i++)
-	ErrorF("%02X ", dev_priv->sdvo_regs[i]);
-    if (dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS] <=
-	SDVO_CMD_STATUS_SCALING_NOT_SUPP)
+    /* Read the command response */
+    for (i = 0; i < response_len; i++) {
+	i830_sdvo_read_byte(output, SDVO_I2C_RETURN_0 + i,
+			    &((CARD8 *)response)[i]);
+    }
+
+    /* Read the return status */
+    i830_sdvo_read_byte(output, SDVO_I2C_CMD_STATUS, &status);
+
+    /* Write the SDVO command logging */
+    xf86DrvMsg(output->pI2CBus->scrnIndex, X_INFO,
+	       "SDVO: R: ");
+    for (i = 0; i < response_len; i++)
+	LogWrite(1, "%02X ", ((CARD8 *)response)[i]);
+    for (; i < 8; i++)
+	LogWrite(1, "   ");
+    if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
     {
-	ErrorF("(%s)",
-	       cmd_status_names[dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS]]);
+	LogWrite(1, "(%s)", cmd_status_names[status]);
     } else {
-	ErrorF("(??? %d)", dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS]);
+	LogWrite(1, "(??? %d)", status);
     }
-    ErrorF("\n");
+    LogWrite(1, "\n");
+
+    return status;
 }
 
 int
@@ -248,49 +242,47 @@ i830_sdvo_get_pixel_multiplier(DisplayModePtr pMode)
  * PROM.  It resets from the DDC bus back to internal registers at the next I2C
  * STOP.  PROM access is terminated by accessing an internal register.
  */
-static Bool
+static void
 i830_sdvo_set_control_bus_switch(I830OutputPtr output, CARD8 target)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_SET_CONTROL_BUS_SWITCH;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = target;
-
-    i830_sdvo_write_outputs(output, 1);
-    return TRUE;
+    i830_sdvo_write_cmd(output, SDVO_CMD_SET_CONTROL_BUS_SWITCH, &target, 1);
 }
 
 static Bool
 i830_sdvo_set_target_input(I830OutputPtr output, Bool target_1, Bool target_2)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 targets[2];
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
+    targets[0] = target_1;
+    targets[1] = target_2;
+    i830_sdvo_write_cmd(output, SDVO_CMD_SET_TARGET_INPUT, &targets, 2);
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_SET_TARGET_INPUT;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = target_1;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = target_2;
+    status = i830_sdvo_read_response(output, NULL, 0);
 
-    i830_sdvo_write_outputs(output, 2);
-
-    i830_sdvo_read_input_regs(output);
-
-    return TRUE;
+    return (status == SDVO_CMD_STATUS_SUCCESS);
 }
 
+/**
+ * Return whether each input is trained.
+ *
+ * This function is making an assumption about the layout of the response,
+ * which should be checked against the docs.
+ */
 static Bool
-i830_sdvo_get_trained_inputs(I830OutputPtr output)
+i830_sdvo_get_trained_inputs(I830OutputPtr output, Bool *input_1, Bool *input_2)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 response[2];
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_TRAINED_INPUTS, NULL, 0);
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_GET_TRAINED_INPUTS;
+    i830_sdvo_read_response(output, response, 2);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    *input_1 = response[0];
+    *input_2 = response[1];
 
     return TRUE;
 }
@@ -298,56 +290,50 @@ i830_sdvo_get_trained_inputs(I830OutputPtr output)
 static Bool
 i830_sdvo_get_active_outputs(I830OutputPtr output, Bool *on_1, Bool *on_2)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
+    CARD8 response[2];
+    CARD8 status;
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_GET_ACTIVE_OUTPUTS;
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_ACTIVE_OUTPUTS, NULL, 0);
 
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    status = i830_sdvo_read_response(output, &response, 2);
 
-    *on_1 = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0];
-    *on_2 = dev_priv->sdvo_regs[SDVO_I2C_RETURN_1];
+    *on_1 = response[0];
+    *on_2 = response[1];
 
-    return TRUE;
+    return (status == SDVO_CMD_STATUS_SUCCESS);
 }
 
 static Bool
 i830_sdvo_set_active_outputs(I830OutputPtr output, Bool on_1, Bool on_2)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 outputs[2];
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
+    outputs[0] = on_1;
+    outputs[1] = on_2;
+    i830_sdvo_write_cmd(output, SDVO_CMD_SET_ACTIVE_OUTPUTS, &outputs, 2);
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_SET_ACTIVE_OUTPUTS;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = on_1;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = on_2;
+    status = i830_sdvo_read_response(output, NULL, 0);
 
-    i830_sdvo_write_outputs(output, 2);
-    i830_sdvo_read_input_regs(output);
-
-    return TRUE;
+    return (status == SDVO_CMD_STATUS_SUCCESS);
 }
 
 static Bool
 i830_sdvo_get_input_pixel_clock_range(I830OutputPtr output, CARD16 *clock_min,
 				      CARD16 *clock_max)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    struct i830_sdvo_pixel_clock_range clocks;
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE, NULL, 0);
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] =
-	SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE;
+    status = i830_sdvo_read_response(output, &clocks, sizeof(clocks));
 
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-    *clock_min = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0] |
-		 (dev_priv->sdvo_regs[SDVO_I2C_RETURN_1] << 8);
-    *clock_max = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2] |
-		 (dev_priv->sdvo_regs[SDVO_I2C_RETURN_3] << 8);
+    *clock_min = clocks.min;
+    *clock_max = clocks.max;
 
     return TRUE;
 }
@@ -355,282 +341,164 @@ i830_sdvo_get_input_pixel_clock_range(I830OutputPtr output, CARD16 *clock_min,
 static Bool
 i830_sdvo_set_target_output(I830OutputPtr output, Bool target_1, Bool target_2)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 targets[2];
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
+    targets[0] = target_1;
+    targets[1] = target_2;
+    i830_sdvo_write_cmd(output, SDVO_CMD_SET_TARGET_OUTPUT, &targets, 2);
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_SET_TARGET_OUTPUT;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = target_1;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = target_2;
+    status = i830_sdvo_read_response(output, NULL, 0);
 
-    i830_sdvo_write_outputs(output, 2);
-    i830_sdvo_read_input_regs(output);
-
-    return TRUE;
+    return (status == SDVO_CMD_STATUS_SUCCESS);
 }
 
-/* Fetches either input or output timings to *dtd, depending on cmd. */
+/** Fetches either input or output timings to *dtd, depending on cmd. */
 static Bool
-i830_sdvo_get_timings(I830OutputPtr output, i830_sdvo_dtd *dtd, CARD8 cmd)
+i830_sdvo_get_timing(I830OutputPtr output, CARD8 cmd, struct i830_sdvo_dtd *dtd)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd;
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
-
-    dtd->clock = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0] |
-		 (dev_priv->sdvo_regs[SDVO_I2C_RETURN_1] << 8);
-    dtd->h_active = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2];
-    dtd->h_blank = dev_priv->sdvo_regs[SDVO_I2C_RETURN_3];
-    dtd->h_high = dev_priv->sdvo_regs[SDVO_I2C_RETURN_4];
-    dtd->v_active = dev_priv->sdvo_regs[SDVO_I2C_RETURN_5];
-    dtd->v_blank = dev_priv->sdvo_regs[SDVO_I2C_RETURN_6];
-    dtd->v_high = dev_priv->sdvo_regs[SDVO_I2C_RETURN_7];
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd + 1;
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
-
-    dtd->h_sync_off = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0];
-    dtd->h_sync_width = dev_priv->sdvo_regs[SDVO_I2C_RETURN_1];
-    dtd->v_sync_off_width = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2];
-    dtd->sync_off_width_high = dev_priv->sdvo_regs[SDVO_I2C_RETURN_3];
-    dtd->dtd_flags = dev_priv->sdvo_regs[SDVO_I2C_RETURN_4];
-    dtd->sdvo_flags = dev_priv->sdvo_regs[SDVO_I2C_RETURN_5];
-    dtd->v_sync_off_high = dev_priv->sdvo_regs[SDVO_I2C_RETURN_6];
-    dtd->reserved = dev_priv->sdvo_regs[SDVO_I2C_RETURN_7];
+    i830_sdvo_write_cmd(output, cmd, NULL, 0);
 
-    return TRUE;
-}
+    status = i830_sdvo_read_response(output, &dtd->part1, sizeof(dtd->part1));
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-/* Sets either input or output timings to *dtd, depending on cmd. */
-static Bool
-i830_sdvo_set_timings(I830OutputPtr output, i830_sdvo_dtd *dtd, CARD8 cmd)
-{
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    i830_sdvo_write_cmd(output, cmd + 1, NULL, 0);
 
-    memset(dev_priv->sdvo_regs, 0, 9);
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = dtd->clock & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = dtd->clock >> 8;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_2] = dtd->h_active;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_3] = dtd->h_blank;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_4] = dtd->h_high;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_5] = dtd->v_active;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_6] = dtd->v_blank;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_7] = dtd->v_high;
-    i830_sdvo_write_outputs(output, 8);
-    i830_sdvo_read_input_regs(output);
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd + 1;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = dtd->h_sync_off;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = dtd->h_sync_width;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_2] = dtd->v_sync_off_width;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_3] = dtd->sync_off_width_high;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_4] = dtd->dtd_flags;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_5] = dtd->sdvo_flags;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_6] = dtd->v_sync_off_high;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_7] = dtd->reserved;
-    i830_sdvo_write_outputs(output, 7);
-    i830_sdvo_read_input_regs(output);
+    status = i830_sdvo_read_response(output, &dtd->part2, sizeof(dtd->part2));
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
     return TRUE;
 }
 
 static Bool
-i830_sdvo_set_timings_part1(I830OutputPtr output, char cmd, CARD16 clock,
-			    CARD16 magic1, CARD16 magic2, CARD16 magic3)
+i830_sdvo_get_input_timing(I830OutputPtr output, struct i830_sdvo_dtd *dtd)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd;
-
-    /* set clock regs */
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = clock & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = (clock >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_2] = magic3 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_3] = (magic3 >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_4] = magic2 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_5] = (magic2 >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_6] = magic1 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_7] = (magic1 >> 8) & 0xff;
-
-    i830_sdvo_write_outputs(output, 8);
-    i830_sdvo_read_input_regs(output);
-
-    return TRUE;
+    return i830_sdvo_get_timing(output, SDVO_CMD_GET_INPUT_TIMINGS_PART1, dtd);
 }
 
 static Bool
-i830_sdvo_set_input_timings_part1(I830OutputPtr output, CARD16 clock,
-				  CARD16 magic1, CARD16 magic2, CARD16 magic3)
+i830_sdvo_get_output_timing(I830OutputPtr output, struct i830_sdvo_dtd *dtd)
 {
-    return i830_sdvo_set_timings_part1(output,
-				       SDVO_CMD_SET_INPUT_TIMINGS_PART1,
-				       clock, magic1, magic2, magic3);
+    return i830_sdvo_get_timing(output, SDVO_CMD_GET_OUTPUT_TIMINGS_PART1, dtd);
 }
 
+/** Sets either input or output timings from *dtd, depending on cmd. */
 static Bool
-i830_sdvo_set_output_timings_part1(I830OutputPtr output, CARD16 clock,
-				   CARD16 magic1, CARD16 magic2, CARD16 magic3)
+i830_sdvo_set_timing(I830OutputPtr output, CARD8 cmd, struct i830_sdvo_dtd *dtd)
 {
-    return i830_sdvo_set_timings_part1(output,
-				       SDVO_CMD_SET_OUTPUT_TIMINGS_PART1,
-				       clock, magic1, magic2, magic3);
-}
+    CARD8 status;
 
-static Bool
-i830_sdvo_set_timings_part2(I830OutputPtr output, CARD8 cmd,
-			    CARD16 magic4, CARD16 magic5, CARD16 magic6)
-{
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = cmd;
-
-    /* set clock regs */
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = magic4 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = (magic4 >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_2] = magic5 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_3] = (magic5 >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_4] = magic6 & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_5] = (magic6 >> 8) & 0xff;
+    i830_sdvo_write_cmd(output, cmd, &dtd->part1, sizeof(dtd->part1));
+    status = i830_sdvo_read_response(output, NULL, 0);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-    i830_sdvo_write_outputs(output, 8);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, cmd + 1, &dtd->part2, sizeof(dtd->part2));
+    status = i830_sdvo_read_response(output, NULL, 0);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
     return TRUE;
 }
 
 static Bool
-i830_sdvo_set_input_timings_part2(I830OutputPtr output,
-				  CARD16 magic4, CARD16 magic5, CARD16 magic6)
+i830_sdvo_set_input_timing(I830OutputPtr output, struct i830_sdvo_dtd *dtd)
 {
-    return i830_sdvo_set_timings_part2(output,
-				       SDVO_CMD_SET_INPUT_TIMINGS_PART2,
-				       magic4, magic5, magic6);
+    return i830_sdvo_set_timing(output, SDVO_CMD_SET_INPUT_TIMINGS_PART1, dtd);
 }
 
 static Bool
-i830_sdvo_set_output_timings_part2(I830OutputPtr output,
-			      CARD16 magic4, CARD16 magic5, CARD16 magic6)
+i830_sdvo_set_output_timing(I830OutputPtr output, struct i830_sdvo_dtd *dtd)
 {
-    return i830_sdvo_set_timings_part2(output,
-				       SDVO_CMD_SET_OUTPUT_TIMINGS_PART2,
-				       magic4, magic5, magic6);
+    return i830_sdvo_set_timing(output, SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd);
 }
 
+#if 0
 static Bool
 i830_sdvo_create_preferred_input_timing(I830OutputPtr output, CARD16 clock,
 					CARD16 width, CARD16 height)
 {
     struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] =
-	SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING;
-
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = clock & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_1] = (clock >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_2] = width & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_3] = (width >> 8) & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_4] = height & 0xff;
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_5] = (height >> 8) & 0xff;
-
-    i830_sdvo_write_outputs(output, 7);
-    i830_sdvo_read_input_regs(output);
+    struct i830_sdvo_preferred_input_timing_args args;
+
+    args.clock = clock;
+    args.width = width;
+    args.height = height;
+    i830_sdvo_write_cmd(output, SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING,
+			&args, sizeof(args));
+    status = i830_sdvo_read_response(output, NULL, 0);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
     return TRUE;
 }
 
 static Bool
-i830_sdvo_get_preferred_input_timing_part1(I830OutputPtr output)
+i830_sdvo_get_preferred_input_timing(I830OutputPtr output,
+				     struct i830_sdvo_dtd *dtd)
 {
     struct i830_sdvo_priv *dev_priv = output->dev_priv;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] =
-	SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1;
-
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
-
-    curr_table[0] = dev_priv->sdvo_regs[SDVO_I2C_RETURN_6] |
-		    (dev_priv->sdvo_regs[SDVO_I2C_RETURN_7] << 8);
-    curr_table[1] = dev_priv->sdvo_regs[SDVO_I2C_RETURN_4] |
-		    (dev_priv->sdvo_regs[SDVO_I2C_RETURN_5] << 8);
-    curr_table[2] = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2] |
-		    (dev_priv->sdvo_regs[SDVO_I2C_RETURN_3] << 8);
-
-    return TRUE;
-}
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1,
+			NULL, 0);
 
-static Bool
-i830_sdvo_get_preferred_input_timing_part2(I830OutputPtr output)
-{
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] =
-	SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2;
+    status = i830_sdvo_read_response(output, &dtd->part1, sizeof(dtd->part1));
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2,
+			NULL, 0);
 
-    curr_table[3] = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0] |
-		    (dev_priv->sdvo_regs[SDVO_I2C_RETURN_1] << 8);
-    curr_table[4] = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2] |
-		    (dev_priv->sdvo_regs[SDVO_I2C_RETURN_3] << 8);
-    curr_table[5] = 0x1e;
+    status = i830_sdvo_read_response(output, &dtd->part2, sizeof(dtd->part2));
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
     return TRUE;
 }
+#endif
 
+/** Returns the SDVO_CLOCK_RATE_MULT_* for the current clock multiplier */
 static int
 i830_sdvo_get_clock_rate_mult(I830OutputPtr output)
 {
     struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 response;
+    CARD8 status;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
-
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_GET_CLOCK_RATE_MULT;
-
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_CLOCK_RATE_MULT, NULL, 0);
+    status = i830_sdvo_read_response(output, &response, 1);
 
-    if (dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS] != SDVO_CMD_STATUS_SUCCESS) {
+    if (status != SDVO_CMD_STATUS_SUCCESS) {
 	xf86DrvMsg(dev_priv->d.pI2CBus->scrnIndex, X_ERROR,
 		   "Couldn't get SDVO clock rate multiplier\n");
 	return SDVO_CLOCK_RATE_MULT_1X;
     } else {
 	xf86DrvMsg(dev_priv->d.pI2CBus->scrnIndex, X_INFO,
-		   "Current clock rate multiplier: %d\n",
-		   dev_priv->sdvo_regs[SDVO_I2C_RETURN_0]);
+		   "Current clock rate multiplier: %d\n", response);
     }
 
-    return dev_priv->sdvo_regs[SDVO_I2C_RETURN_0];
+    return response;
 }
 
+/**
+ * Sets the current clock multiplier.
+ *
+ * This has to match with the settings in the DPLL/SDVO reg when the output
+ * is actually turned on.
+ */
 static Bool
 i830_sdvo_set_clock_rate_mult(I830OutputPtr output, CARD8 val)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-
-    memset(dev_priv->sdvo_regs, 0, 9);
+    CARD8 status;
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_SET_CLOCK_RATE_MULT;
-
-    dev_priv->sdvo_regs[SDVO_I2C_ARG_0] = val;
-    i830_sdvo_write_outputs(output, 1);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1);
+    status = i830_sdvo_read_response(output, NULL, 0);
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
     return TRUE;
 }
@@ -640,15 +508,11 @@ i830_sdvo_pre_set_mode(ScrnInfoPtr pScrn, I830OutputPtr output,
 		       DisplayModePtr mode)
 {
     I830Ptr pI830 = I830PTR(pScrn);
-    CARD16 clock = mode->Clock/10, width = mode->CrtcHDisplay;
+    CARD16 width = mode->CrtcHDisplay;
     CARD16 height = mode->CrtcVDisplay;
     CARD16 h_blank_len, h_sync_len, v_blank_len, v_sync_len;
     CARD16 h_sync_offset, v_sync_offset;
-    CARD16 sync_flags;
-    CARD8 c16a[8];
-    CARD8 c17a[8];
-    CARD16 out_timings[6];
-    Bool out1, out2;
+    struct i830_sdvo_dtd output_dtd;
 
     /* do some mode translations */
     h_blank_len = mode->CrtcHBlankEnd - mode->CrtcHBlankStart;
@@ -660,62 +524,57 @@ i830_sdvo_pre_set_mode(ScrnInfoPtr pScrn, I830OutputPtr output,
     h_sync_offset = mode->CrtcHSyncStart - mode->CrtcHBlankStart;
     v_sync_offset = mode->CrtcVSyncStart - mode->CrtcVBlankStart;
 
-    sync_flags = 0x18;
+    output_dtd.part1.clock = mode->Clock / 10;
+    output_dtd.part1.h_active = width & 0xff;
+    output_dtd.part1.h_blank = h_blank_len & 0xff;
+    output_dtd.part1.h_high = (((width >> 8) & 0xf) << 4) |
+	((h_blank_len >> 8) & 0xf);
+    output_dtd.part1.v_active = height & 0xff;
+    output_dtd.part1.v_blank = v_blank_len & 0xff;
+    output_dtd.part1.v_high = (((height >> 8) & 0xf) << 4) |
+	((v_blank_len >> 8) & 0xf);
+
+    output_dtd.part2.h_sync_off = h_sync_offset;
+    output_dtd.part2.h_sync_width = h_sync_len & 0xff;
+    output_dtd.part2.v_sync_off_width = (v_sync_offset & 0xf) << 4 |
+	(v_sync_len & 0xf);
+    output_dtd.part2.sync_off_width_high = 0;
+    output_dtd.part2.dtd_flags = 0x18;
+    output_dtd.part2.sdvo_flags = 0;
+    output_dtd.part2.v_sync_off_width = 0;
+    output_dtd.part2.reserved = 0;
     if (mode->Flags & V_PHSYNC)
-	sync_flags |= 0x2;
+	output_dtd.part2.dtd_flags |= 0x2;
     if (mode->Flags & V_PVSYNC)
-	sync_flags |= 0x4;
-    /* high bits of 0 */
-    c16a[7] = clock & 0xff;
-    c16a[6] = (clock >> 8) & 0xff;
-    c16a[5] = (width & 0xff);
-    c16a[4] = (h_blank_len & 0xff);
-    c16a[3] = (((width >> 8) & 0xf) << 4) | ((h_blank_len >> 8) & 0xf);
-    c16a[2] = (height & 0xff);
-    c16a[1] = (v_blank_len & 0xff);
-    c16a[0] = (((height >> 8) & 0xf) << 4) | ((v_blank_len >> 8) & 0xf);
-
-    c17a[7] = h_sync_offset;
-    c17a[6] = h_sync_len & 0xff;
-    c17a[5] = (v_sync_offset & 0xf) << 4 | (v_sync_len & 0xf);
-    c17a[4] = 0;
-    c17a[3] = sync_flags;
-    c17a[2] = 0;
-    out_timings[0] = c16a[1] | ((short)c16a[0] << 8);
-    out_timings[1] = c16a[3] | ((short)c16a[2] << 8);
-    out_timings[2] = c16a[5] | ((short)c16a[4] << 8);
-    out_timings[3] = c17a[7] | ((short)c17a[6] << 8);
-    out_timings[4] = c17a[5] | ((short)c17a[4] << 8);
-    out_timings[5] = c17a[3] | ((short)c17a[2] << 8);
-
-    i830_sdvo_set_target_input(output, FALSE, FALSE);
-
-    i830_sdvo_get_active_outputs(output, &out1, &out2);
+	output_dtd.part2.dtd_flags |= 0x4;
 
+    /* Turn off the screens before adjusting timings */
     i830_sdvo_set_active_outputs(output, FALSE, FALSE);
 
+    /* Set the output timing to the screen */
     i830_sdvo_set_target_output(output, TRUE, FALSE);
-    i830_sdvo_set_output_timings_part1(output, clock,
-				       out_timings[0], out_timings[1],
-				       out_timings[2]);
-    i830_sdvo_set_output_timings_part2(output, out_timings[3], out_timings[4],
-				       out_timings[5]);
-
-    i830_sdvo_set_target_input(output, FALSE, FALSE);
+    i830_sdvo_set_output_timing(output, &output_dtd);
 
-    i830_sdvo_create_preferred_input_timing(output, clock, width, height);
-    i830_sdvo_get_preferred_input_timing_part1(output);
-    i830_sdvo_get_preferred_input_timing_part2(output);
+    /* Set the input timing to the screen */
+    i830_sdvo_set_target_input(output, TRUE, FALSE);
 
-    i830_sdvo_set_target_input(output, FALSE, FALSE);
-
-    i830_sdvo_set_input_timings_part1(output, clock,
-				      curr_table[0], curr_table[1],
-				      curr_table[2]);
-    i830_sdvo_set_input_timings_part2(output, curr_table[3], curr_table[4],
-				      out_timings[5]);
-
-    i830_sdvo_set_target_input(output, FALSE, FALSE);
+    /* We would like to use i830_sdvo_create_preferred_input_timing() to
+     * provide the device with a timing it can support, if it supports that
+     * feature.  However, presumably we would need to adjust the CRTC to output
+     * the preferred timing, and we don't support that currently.
+     */
+#if 0
+    success = i830_sdvo_create_preferred_input_timing(output, clock,
+						      width, height);
+    if (success) {
+	struct i830_sdvo_dtd *input_dtd;
+
+	i830_sdvo_get_preferred_input_timing(output, &input_dtd);
+	i830_sdvo_set_input_timing(output, &input_dtd);
+    }
+#else
+    i830_sdvo_set_input_timing(output, &output_dtd);
+#endif
 
     switch (i830_sdvo_get_pixel_multiplier(mode)) {
     case 1:
@@ -738,26 +597,22 @@ i830_sdvo_post_set_mode(ScrnInfoPtr pScrn, I830OutputPtr output,
 			DisplayModePtr mode)
 {
     I830Ptr pI830 = I830PTR(pScrn);
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
-    Bool ret = TRUE;
-    Bool out1, out2;
+    Bool out1, out2, input1, input2;
     CARD32 dpll, sdvob, sdvoc;
     int dpll_reg = (output->pipe == 0) ? DPLL_A : DPLL_B;
     int sdvo_pixel_multiply;
+    CARD8 status;
 
     /* the BIOS writes out 6 commands post mode set */
     /* two 03s, 04 05, 10, 1d */
     /* these contain the height and mode clock / 10 by the looks of it */
 
-    i830_sdvo_get_trained_inputs(output);
+    i830_sdvo_get_trained_inputs(output, &input1, &input2);
 
-    /* THIS IS A DIRTY HACK - sometimes for some reason on startup
-     * the BIOS doesn't find my DVI monitor -
-     * without this hack the driver doesn't work.. this causes the modesetting
-     * to be re-run
-     */
-    if (dev_priv->sdvo_regs[SDVO_I2C_RETURN_0] != 0x1) {
-	ret = FALSE;
+    /* Warn if the device reported failure to sync. */
+    if (status == SDVO_CMD_STATUS_SUCCESS && !input1) {
+	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+		   "First SDVO output reported failure to sync\n");
     }
 
     i830_sdvo_get_active_outputs(output, &out1, &out2);
@@ -810,28 +665,24 @@ i830_sdvo_save(ScrnInfoPtr pScrn, I830OutputPtr output)
     i830_sdvo_get_active_outputs(output, &dev_priv->save_sdvo_active_1,
 				 &dev_priv->save_sdvo_active_2);
 
-    if (dev_priv->caps.caps & 0x1) {
+    if (dev_priv->caps.sdvo_inputs_mask & 0x1) {
        i830_sdvo_set_target_input(output, FALSE, FALSE);
-       i830_sdvo_get_timings(output, &dev_priv->save_input_dtd_1,
-			     SDVO_CMD_GET_INPUT_TIMINGS_PART1);
+       i830_sdvo_get_input_timing(output, &dev_priv->save_input_dtd_1);
     }
 
-    if (dev_priv->caps.caps & 0x2) {
+    if (dev_priv->caps.sdvo_inputs_mask & 0x2) {
        i830_sdvo_set_target_input(output, FALSE, TRUE);
-       i830_sdvo_get_timings(output, &dev_priv->save_input_dtd_2,
-			     SDVO_CMD_GET_INPUT_TIMINGS_PART1);
+       i830_sdvo_get_input_timing(output, &dev_priv->save_input_dtd_2);
     }
 
     if (dev_priv->caps.output_0_supported) {
        i830_sdvo_set_target_output(output, TRUE, FALSE);
-       i830_sdvo_get_timings(output, &dev_priv->save_output_dtd_1,
-			     SDVO_CMD_GET_OUTPUT_TIMINGS_PART1);
+       i830_sdvo_get_output_timing(output, &dev_priv->save_output_dtd_1);
     }
 
     if (dev_priv->caps.output_1_supported) {
        i830_sdvo_set_target_output(output, FALSE, TRUE);
-       i830_sdvo_get_timings(output, &dev_priv->save_output_dtd_2,
-			     SDVO_CMD_GET_OUTPUT_TIMINGS_PART1);
+       i830_sdvo_get_output_timing(output, &dev_priv->save_output_dtd_2);
     }
 
     dev_priv->save_SDVOX = INREG(dev_priv->output_device);
@@ -843,28 +694,24 @@ i830_sdvo_restore(ScrnInfoPtr pScrn, I830OutputPtr output)
     I830Ptr pI830 = I830PTR(pScrn);
     struct i830_sdvo_priv *dev_priv = output->dev_priv;
 
-    if (dev_priv->caps.caps & 0x1) {
+    if (dev_priv->caps.sdvo_inputs_mask & 0x1) {
        i830_sdvo_set_target_input(output, FALSE, FALSE);
-       i830_sdvo_set_timings(output, &dev_priv->save_input_dtd_1,
-			     SDVO_CMD_SET_INPUT_TIMINGS_PART1);
+       i830_sdvo_set_input_timing(output, &dev_priv->save_input_dtd_1);
     }
 
-    if (dev_priv->caps.caps & 0x2) {
+    if (dev_priv->caps.sdvo_inputs_mask & 0x2) {
        i830_sdvo_set_target_input(output, FALSE, TRUE);
-       i830_sdvo_set_timings(output, &dev_priv->save_input_dtd_2,
-			     SDVO_CMD_SET_INPUT_TIMINGS_PART1);
+       i830_sdvo_set_input_timing(output, &dev_priv->save_input_dtd_2);
     }
 
     if (dev_priv->caps.output_0_supported) {
        i830_sdvo_set_target_output(output, TRUE, FALSE);
-       i830_sdvo_set_timings(output, &dev_priv->save_output_dtd_1,
-			     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1);
+       i830_sdvo_set_output_timing(output, &dev_priv->save_output_dtd_1);
     }
 
     if (dev_priv->caps.output_1_supported) {
        i830_sdvo_set_target_output(output, FALSE, TRUE);
-       i830_sdvo_set_timings(output, &dev_priv->save_output_dtd_2,
-			     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1);
+       i830_sdvo_set_output_timing(output, &dev_priv->save_output_dtd_2);
     }
 
     i830_sdvo_set_clock_rate_mult(output, dev_priv->save_sdvo_mult);
@@ -890,24 +737,17 @@ i830_sdvo_mode_valid(ScrnInfoPtr pScrn, I830OutputPtr output,
     return MODE_OK;
 }
 
-static void
+static Bool
 i830_sdvo_get_capabilities(I830OutputPtr output, i830_sdvo_caps *caps)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 status;
+
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_DEVICE_CAPS, NULL, 0);
+    status = i830_sdvo_read_response(output, caps, sizeof(*caps));
+    if (status != SDVO_CMD_STATUS_SUCCESS)
+	return FALSE;
 
-    memset(dev_priv->sdvo_regs, 0, 9);
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_GET_DEVICE_CAPS;
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
-
-    caps->vendor_id = dev_priv->sdvo_regs[SDVO_I2C_RETURN_0];
-    caps->device_id = dev_priv->sdvo_regs[SDVO_I2C_RETURN_1];
-    caps->device_rev_id = dev_priv->sdvo_regs[SDVO_I2C_RETURN_2];
-    caps->sdvo_version_major = dev_priv->sdvo_regs[SDVO_I2C_RETURN_3];
-    caps->sdvo_version_minor = dev_priv->sdvo_regs[SDVO_I2C_RETURN_4];
-    caps->caps = dev_priv->sdvo_regs[SDVO_I2C_RETURN_5];
-    caps->output_0_supported = dev_priv->sdvo_regs[SDVO_I2C_RETURN_6];
-    caps->output_1_supported = dev_priv->sdvo_regs[SDVO_I2C_RETURN_7];
+    return TRUE;
 }
 
 /** Forces the device over to the real I2C bus and uses its GetByte */
@@ -1001,12 +841,10 @@ i830_sdvo_ddc_i2c_address(I2CDevPtr d, I2CSlaveAddr addr)
 static void
 i830_sdvo_dump_cmd(I830OutputPtr output, int opcode)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 response[8];
 
-    memset(dev_priv->sdvo_regs, 0, sizeof(dev_priv->sdvo_regs));
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = opcode;
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, opcode, NULL, 0);
+    i830_sdvo_read_response(output, response, 8);
 }
 
 static void
@@ -1062,17 +900,16 @@ i830_sdvo_dump(ScrnInfoPtr pScrn)
 Bool
 i830_sdvo_detect_displays(ScrnInfoPtr pScrn, I830OutputPtr output)
 {
-    struct i830_sdvo_priv *dev_priv = output->dev_priv;
+    CARD8 response[2];
+    CARD8 status;
 
-    dev_priv->sdvo_regs[SDVO_I2C_OPCODE] = SDVO_CMD_GET_ATTACHED_DISPLAYS;
-    i830_sdvo_write_outputs(output, 0);
-    i830_sdvo_read_input_regs(output);
+    i830_sdvo_write_cmd(output, SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0);
+    status = i830_sdvo_read_response(output, &response, 2);
 
-    if (dev_priv->sdvo_regs[SDVO_I2C_CMD_STATUS] != SDVO_CMD_STATUS_SUCCESS)
+    if (status != SDVO_CMD_STATUS_SUCCESS)
 	return FALSE;
 
-    return (dev_priv->sdvo_regs[SDVO_I2C_RETURN_0] != 0 ||
-	    dev_priv->sdvo_regs[SDVO_I2C_RETURN_1] != 0);
+    return (response[0] != 0 || response[1] != 0);
 }
 
 void
@@ -1182,10 +1019,13 @@ i830_sdvo_init(ScrnInfoPtr pScrn, int output_device)
 					  &dev_priv->pixel_clock_max);
 
     xf86DrvMsg(pScrn->scrnIndex, X_INFO,
-	       "SDVO device VID/DID: %02X:%02X.%02X, %02X,"
+	       "SDVO device VID/DID: %02X:%02X.%02X, "
+	       "input 1: %c, input 2: %c, "
 	       "output 1: %c, output 2: %c\n",
 	       dev_priv->caps.vendor_id, dev_priv->caps.device_id,
-	       dev_priv->caps.device_rev_id, dev_priv->caps.caps,
+	       dev_priv->caps.device_rev_id,
+	       (dev_priv->caps.sdvo_inputs_mask & 0x1) ? 'Y' : 'N',
+	       (dev_priv->caps.sdvo_inputs_mask & 0x2) ? 'Y' : 'N',
 	       dev_priv->caps.output_0_supported ? 'Y' : 'N',
 	       dev_priv->caps.output_1_supported ? 'Y' : 'N');