drm/i915/sdvo: Propagate errors from reading/writing control bus.
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 4 Aug 2010 12:50:25 +0000 (13:50 +0100)
committerEric Anholt <eric@anholt.net>
Mon, 9 Aug 2010 18:24:28 +0000 (11:24 -0700)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eric Anholt <eric@anholt.net>
drivers/gpu/drm/i915/intel_sdvo.c
drivers/gpu/drm/i915/intel_sdvo_regs.h

index dca123527d5f034ebdd8a73ad2a7097e39abda43..300f110fd180be1ff4e1c542e12249940a3100ba 100644 (file)
@@ -47,6 +47,7 @@
 
 #define IS_TV(c)       (c->output_flag & SDVO_TV_MASK)
 #define IS_LVDS(c)     (c->output_flag & SDVO_LVDS_MASK)
+#define IS_TV_OR_LVDS(c) (c->output_flag & (SDVO_TV_MASK | SDVO_LVDS_MASK))
 
 
 static char *tv_format_names[] = {
@@ -189,10 +190,13 @@ static struct intel_sdvo_connector *to_intel_sdvo_connector(struct drm_connector
 
 static bool
 intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, uint16_t flags);
-static void
-intel_sdvo_tv_create_property(struct drm_connector *connector, int type);
-static void
-intel_sdvo_create_enhance_property(struct drm_connector *connector);
+static bool
+intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
+                             struct intel_sdvo_connector *intel_sdvo_connector,
+                             int type);
+static bool
+intel_sdvo_create_enhance_property(struct intel_sdvo *intel_sdvo,
+                                  struct intel_sdvo_connector *intel_sdvo_connector);
 
 /**
  * Writes the SDVOB or SDVOC with the given value, but always writes both
@@ -231,13 +235,10 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
        }
 }
 
-static bool intel_sdvo_read_byte(struct intel_sdvo *intel_sdvo, u8 addr,
-                                u8 *ch)
+static bool intel_sdvo_read_byte(struct intel_sdvo *intel_sdvo, u8 addr, u8 *ch)
 {
-       u8 out_buf[2];
+       u8 out_buf[2] = { addr, 0 };
        u8 buf[2];
-       int ret;
-
        struct i2c_msg msgs[] = {
                {
                        .addr = intel_sdvo->slave_addr >> 1,
@@ -252,9 +253,7 @@ static bool intel_sdvo_read_byte(struct intel_sdvo *intel_sdvo, u8 addr,
                        .buf = buf,
                }
        };
-
-       out_buf[0] = addr;
-       out_buf[1] = 0;
+       int ret;
 
        if ((ret = i2c_transfer(intel_sdvo->base.i2c_bus, msgs, 2)) == 2)
        {
@@ -266,10 +265,9 @@ static bool intel_sdvo_read_byte(struct intel_sdvo *intel_sdvo, u8 addr,
        return false;
 }
 
-static bool intel_sdvo_write_byte(struct intel_sdvo *intel_sdvo, int addr,
-                                 u8 ch)
+static bool intel_sdvo_write_byte(struct intel_sdvo *intel_sdvo, int addr, u8 ch)
 {
-       u8 out_buf[2];
+       u8 out_buf[2] = { addr, ch };
        struct i2c_msg msgs[] = {
                {
                        .addr = intel_sdvo->slave_addr >> 1,
@@ -279,14 +277,7 @@ static bool intel_sdvo_write_byte(struct intel_sdvo *intel_sdvo, int addr,
                }
        };
 
-       out_buf[0] = addr;
-       out_buf[1] = ch;
-
-       if (i2c_transfer(intel_sdvo->base.i2c_bus, msgs, 1) == 1)
-       {
-               return true;
-       }
-       return false;
+       return i2c_transfer(intel_sdvo->base.i2c_bus, msgs, 1) == 1;
 }
 
 #define SDVO_CMD_NAME_ENTRY(cmd) {cmd, #cmd}
@@ -390,7 +381,7 @@ static const struct _sdvo_cmd_name {
 #define SDVO_NAME(svdo) (IS_SDVOB((svdo)->sdvo_reg) ? "SDVOB" : "SDVOC")
 
 static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
-                                  void *args, int args_len)
+                                  const void *args, int args_len)
 {
        int i;
 
@@ -411,19 +402,20 @@ static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
        DRM_LOG_KMS("\n");
 }
 
-static void intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
-                                void *args, int args_len)
+static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
+                                const void *args, int args_len)
 {
        int i;
 
        intel_sdvo_debug_write(intel_sdvo, cmd, args, args_len);
 
        for (i = 0; i < args_len; i++) {
-               intel_sdvo_write_byte(intel_sdvo, SDVO_I2C_ARG_0 - i,
-                                     ((u8*)args)[i]);
+               if (!intel_sdvo_write_byte(intel_sdvo, SDVO_I2C_ARG_0 - i,
+                                          ((u8*)args)[i]))
+                       return false;
        }
 
-       intel_sdvo_write_byte(intel_sdvo, SDVO_I2C_OPCODE, cmd);
+       return intel_sdvo_write_byte(intel_sdvo, SDVO_I2C_OPCODE, cmd);
 }
 
 static const char *cmd_status_names[] = {
@@ -454,8 +446,8 @@ static void intel_sdvo_debug_response(struct intel_sdvo *intel_sdvo,
        DRM_LOG_KMS("\n");
 }
 
-static u8 intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
-                                  void *response, int response_len)
+static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
+                                    void *response, int response_len)
 {
        int i;
        u8 status;
@@ -464,24 +456,26 @@ static u8 intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
        while (retry--) {
                /* Read the command response */
                for (i = 0; i < response_len; i++) {
-                       intel_sdvo_read_byte(intel_sdvo,
-                                            SDVO_I2C_RETURN_0 + i,
-                                            &((u8 *)response)[i]);
+                       if (!intel_sdvo_read_byte(intel_sdvo,
+                                                 SDVO_I2C_RETURN_0 + i,
+                                                 &((u8 *)response)[i]))
+                               return false;
                }
 
                /* read the return status */
-               intel_sdvo_read_byte(intel_sdvo, SDVO_I2C_CMD_STATUS,
-                                    &status);
+               if (!intel_sdvo_read_byte(intel_sdvo, SDVO_I2C_CMD_STATUS,
+                                         &status))
+                       return false;
 
                intel_sdvo_debug_response(intel_sdvo, response, response_len,
                                          status);
                if (status != SDVO_CMD_STATUS_PENDING)
-                       return status;
+                       break;
 
                mdelay(50);
        }
 
-       return status;
+       return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static int intel_sdvo_get_pixel_multiplier(struct drm_display_mode *mode)
@@ -553,23 +547,29 @@ static void intel_sdvo_set_control_bus_switch(struct intel_sdvo *intel_sdvo,
        return;
 }
 
-static bool intel_sdvo_set_target_input(struct intel_sdvo *intel_sdvo, bool target_0, bool target_1)
+static bool intel_sdvo_set_value(struct intel_sdvo *intel_sdvo, u8 cmd, const void *data, int len)
 {
-       struct intel_sdvo_set_target_input_args targets = {0};
-       u8 status;
-
-       if (target_0 && target_1)
-               return SDVO_CMD_STATUS_NOTSUPP;
+       if (!intel_sdvo_write_cmd(intel_sdvo, cmd, data, len))
+               return false;
 
-       if (target_1)
-               targets.target_1 = 1;
+       return intel_sdvo_read_response(intel_sdvo, NULL, 0);
+}
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_TARGET_INPUT, &targets,
-                            sizeof(targets));
+static bool
+intel_sdvo_get_value(struct intel_sdvo *intel_sdvo, u8 cmd, void *value, int len)
+{
+       if (!intel_sdvo_write_cmd(intel_sdvo, cmd, NULL, 0))
+               return false;
 
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
+       return intel_sdvo_read_response(intel_sdvo, value, len);
+}
 
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+static bool intel_sdvo_set_target_input(struct intel_sdvo *intel_sdvo)
+{
+       struct intel_sdvo_set_target_input_args targets = {0};
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_SET_TARGET_INPUT,
+                                   &targets, sizeof(targets));
 }
 
 /**
@@ -581,11 +581,9 @@ static bool intel_sdvo_set_target_input(struct intel_sdvo *intel_sdvo, bool targ
 static bool intel_sdvo_get_trained_inputs(struct intel_sdvo *intel_sdvo, bool *input_1, bool *input_2)
 {
        struct intel_sdvo_get_trained_inputs_response response;
-       u8 status;
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_TRAINED_INPUTS, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, &response, sizeof(response));
-       if (status != SDVO_CMD_STATUS_SUCCESS)
+       if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_TRAINED_INPUTS,
+                                 &response, sizeof(response)))
                return false;
 
        *input_1 = response.input0_trained;
@@ -596,18 +594,15 @@ static bool intel_sdvo_get_trained_inputs(struct intel_sdvo *intel_sdvo, bool *i
 static bool intel_sdvo_set_active_outputs(struct intel_sdvo *intel_sdvo,
                                          u16 outputs)
 {
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_OUTPUTS, &outputs,
-                            sizeof(outputs));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_SET_ACTIVE_OUTPUTS,
+                                   &outputs, sizeof(outputs));
 }
 
 static bool intel_sdvo_set_encoder_power_state(struct intel_sdvo *intel_sdvo,
                                               int mode)
 {
-       u8 status, state = SDVO_ENCODER_STATE_ON;
+       u8 state = SDVO_ENCODER_STATE_ON;
 
        switch (mode) {
        case DRM_MODE_DPMS_ON:
@@ -624,11 +619,8 @@ static bool intel_sdvo_set_encoder_power_state(struct intel_sdvo *intel_sdvo,
                break;
        }
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ENCODER_POWER_STATE, &state,
-                            sizeof(state));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_SET_ENCODER_POWER_STATE, &state, sizeof(state));
 }
 
 static bool intel_sdvo_get_input_pixel_clock_range(struct intel_sdvo *intel_sdvo,
@@ -636,51 +628,31 @@ static bool intel_sdvo_get_input_pixel_clock_range(struct intel_sdvo *intel_sdvo
                                                   int *clock_max)
 {
        struct intel_sdvo_pixel_clock_range clocks;
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE,
-                            NULL, 0);
 
-       status = intel_sdvo_read_response(intel_sdvo, &clocks, sizeof(clocks));
-
-       if (status != SDVO_CMD_STATUS_SUCCESS)
+       if (!intel_sdvo_get_value(intel_sdvo,
+                                 SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE,
+                                 &clocks, sizeof(clocks)))
                return false;
 
        /* Convert the values from units of 10 kHz to kHz. */
        *clock_min = clocks.min * 10;
        *clock_max = clocks.max * 10;
-
        return true;
 }
 
 static bool intel_sdvo_set_target_output(struct intel_sdvo *intel_sdvo,
                                         u16 outputs)
 {
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_TARGET_OUTPUT, &outputs,
-                            sizeof(outputs));
-
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_SET_TARGET_OUTPUT,
+                                   &outputs, sizeof(outputs));
 }
 
 static bool intel_sdvo_set_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
                                  struct intel_sdvo_dtd *dtd)
 {
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       intel_sdvo_write_cmd(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       return true;
+       return intel_sdvo_set_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1)) &&
+               intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
 }
 
 static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo,
@@ -704,7 +676,6 @@ intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
                                         uint16_t height)
 {
        struct intel_sdvo_preferred_input_timing_args args;
-       uint8_t status;
 
        memset(&args, 0, sizeof(args));
        args.clock = clock;
@@ -717,54 +688,27 @@ intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
            intel_sdvo->sdvo_lvds_fixed_mode->vdisplay != height))
                args.scaled = 1;
 
-       intel_sdvo_write_cmd(intel_sdvo,
-                            SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING,
-                            &args, sizeof(args));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       return true;
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING,
+                                   &args, sizeof(args));
 }
 
 static bool intel_sdvo_get_preferred_input_timing(struct intel_sdvo *intel_sdvo,
                                                  struct intel_sdvo_dtd *dtd)
 {
-       bool status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1,
-                            NULL, 0);
-
-       status = intel_sdvo_read_response(intel_sdvo, &dtd->part1,
-                                         sizeof(dtd->part1));
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2,
-                            NULL, 0);
-
-       status = intel_sdvo_read_response(intel_sdvo, &dtd->part2,
-                                         sizeof(dtd->part2));
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       return false;
+       return intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1,
+                                   &dtd->part1, sizeof(dtd->part1)) &&
+               intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2,
+                                    &dtd->part2, sizeof(dtd->part2));
 }
 
 static bool intel_sdvo_set_clock_rate_mult(struct intel_sdvo *intel_sdvo, u8 val)
 {
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1);
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       return true;
+       return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1);
 }
 
 static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
-                                        struct drm_display_mode *mode)
+                                        const struct drm_display_mode *mode)
 {
        uint16_t width, height;
        uint16_t h_blank_len, h_sync_len, v_blank_len, v_sync_len;
@@ -813,7 +757,7 @@ static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
 }
 
 static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
-                                        struct intel_sdvo_dtd *dtd)
+                                        const struct intel_sdvo_dtd *dtd)
 {
        mode->hdisplay = dtd->part1.h_active;
        mode->hdisplay += ((dtd->part1.h_high >> 4) & 0x0f) << 8;
@@ -848,38 +792,26 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
 static bool intel_sdvo_get_supp_encode(struct intel_sdvo *intel_sdvo,
                                       struct intel_sdvo_encode *encode)
 {
-       uint8_t status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_SUPP_ENCODE, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, encode, sizeof(*encode));
-       if (status != SDVO_CMD_STATUS_SUCCESS) { /* non-support means DVI */
-               memset(encode, 0, sizeof(*encode));
-               return false;
-       }
+       if (intel_sdvo_get_value(intel_sdvo,
+                                 SDVO_CMD_GET_SUPP_ENCODE,
+                                 encode, sizeof(*encode)))
+               return true;
 
-       return true;
+       /* non-support means DVI */
+       memset(encode, 0, sizeof(*encode));
+       return false;
 }
 
 static bool intel_sdvo_set_encode(struct intel_sdvo *intel_sdvo,
                                  uint8_t mode)
 {
-       uint8_t status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ENCODE, &mode, 1);
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+       return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_ENCODE, &mode, 1);
 }
 
 static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
                                       uint8_t mode)
 {
-       uint8_t status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-
-       return (status == SDVO_CMD_STATUS_SUCCESS);
+       return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
 }
 
 #if 0
@@ -892,8 +824,7 @@ static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
        uint8_t buf[48];
        uint8_t *pos;
 
-       intel_sdvo_write_cmd(encoder, SDVO_CMD_GET_HBUF_AV_SPLIT, NULL, 0);
-       intel_sdvo_read_response(encoder, &av_split, 1);
+       intel_sdvo_get_value(encoder, SDVO_CMD_GET_HBUF_AV_SPLIT, &av_split, 1);
 
        for (i = 0; i <= av_split; i++) {
                set_buf_index[0] = i; set_buf_index[1] = 0;
@@ -913,7 +844,7 @@ static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
 }
 #endif
 
-static void intel_sdvo_set_hdmi_buf(struct intel_sdvo *intel_sdvo,
+static bool intel_sdvo_set_hdmi_buf(struct intel_sdvo *intel_sdvo,
                                    int index,
                                    uint8_t *data, int8_t size, uint8_t tx_rate)
 {
@@ -922,15 +853,18 @@ static void intel_sdvo_set_hdmi_buf(struct intel_sdvo *intel_sdvo,
     set_buf_index[0] = index;
     set_buf_index[1] = 0;
 
-    intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_INDEX,
-                        set_buf_index, 2);
+    if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_INDEX,
+                             set_buf_index, 2))
+           return false;
 
     for (; size > 0; size -= 8) {
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_DATA, data, 8);
+       if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_DATA, data, 8))
+               return false;
+
        data += 8;
     }
 
-    intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_TXRATE, &tx_rate, 1);
+    return intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_TXRATE, &tx_rate, 1);
 }
 
 static uint8_t intel_sdvo_calc_hbuf_csum(uint8_t *data, uint8_t size)
@@ -1005,7 +939,7 @@ struct dip_infoframe {
        } __attribute__ ((packed)) u;
 } __attribute__((packed));
 
-static void intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
+static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
                                         struct drm_display_mode * mode)
 {
        struct dip_infoframe avi_if = {
@@ -1016,17 +950,15 @@ static void intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 
        avi_if.checksum = intel_sdvo_calc_hbuf_csum((uint8_t *)&avi_if,
                                                    4 + avi_if.len);
-       intel_sdvo_set_hdmi_buf(intel_sdvo, 1, (uint8_t *)&avi_if,
-                               4 + avi_if.len,
-                               SDVO_HBUF_TX_VSYNC);
+       return intel_sdvo_set_hdmi_buf(intel_sdvo, 1, (uint8_t *)&avi_if,
+                                      4 + avi_if.len,
+                                      SDVO_HBUF_TX_VSYNC);
 }
 
-static void intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
+static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
 {
-
        struct intel_sdvo_tv_format format;
        uint32_t format_map, i;
-       uint8_t status;
 
        for (i = 0; i < TV_FORMAT_NUM; i++)
                if (tv_format_names[i] == intel_sdvo->tv_format_name)
@@ -1034,113 +966,93 @@ static void intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
 
        format_map = 1 << i;
        memset(&format, 0, sizeof(format));
-       memcpy(&format, &format_map, sizeof(format_map) > sizeof(format) ?
-                       sizeof(format) : sizeof(format_map));
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_TV_FORMAT, &format,
-                            sizeof(format));
+       memcpy(&format, &format_map, min(sizeof(format), sizeof(format_map)));
 
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               DRM_DEBUG_KMS("%s: Failed to set TV format\n",
-                         SDVO_NAME(intel_sdvo));
+       BUILD_BUG_ON(sizeof(format) != 6);
+       return intel_sdvo_set_value(intel_sdvo,
+                                   SDVO_CMD_SET_TV_FORMAT,
+                                   &format, sizeof(format));
 }
 
-static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
-                                 struct drm_display_mode *mode,
-                                 struct drm_display_mode *adjusted_mode)
+static bool
+intel_sdvo_set_output_timings_from_mode(struct intel_sdvo *intel_sdvo,
+                                       struct drm_display_mode *mode)
 {
-       struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
+       struct intel_sdvo_dtd output_dtd;
 
-       if (intel_sdvo->is_tv) {
-               struct intel_sdvo_dtd output_dtd;
-               bool success;
+       if (!intel_sdvo_set_target_output(intel_sdvo,
+                                         intel_sdvo->attached_output))
+               return false;
 
-               /* We need to construct preferred input timings based on our
-                * output timings.  To do that, we have to set the output
-                * timings, even though this isn't really the right place in
-                * the sequence to do it. Oh well.
-                */
+       intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
+       if (!intel_sdvo_set_output_timing(intel_sdvo, &output_dtd))
+               return false;
 
+       return true;
+}
+
+static bool
+intel_sdvo_set_input_timings_for_mode(struct intel_sdvo *intel_sdvo,
+                                       struct drm_display_mode *mode,
+                                       struct drm_display_mode *adjusted_mode)
+{
+       struct intel_sdvo_dtd input_dtd;
 
-               /* Set output timings */
-               intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
-               intel_sdvo_set_target_output(intel_sdvo,
-                                            intel_sdvo->attached_output);
-               intel_sdvo_set_output_timing(intel_sdvo, &output_dtd);
+       /* Reset the input timing to the screen. Assume always input 0. */
+       if (!intel_sdvo_set_target_input(intel_sdvo))
+               return false;
 
-               /* Set the input timing to the screen. Assume always input 0. */
-               intel_sdvo_set_target_input(intel_sdvo, true, false);
+       if (!intel_sdvo_create_preferred_input_timing(intel_sdvo,
+                                                     mode->clock / 10,
+                                                     mode->hdisplay,
+                                                     mode->vdisplay))
+               return false;
 
+       if (!intel_sdvo_get_preferred_input_timing(intel_sdvo,
+                                                  &input_dtd))
+               return false;
 
-               success = intel_sdvo_create_preferred_input_timing(intel_sdvo,
-                                                                  mode->clock / 10,
-                                                                  mode->hdisplay,
-                                                                  mode->vdisplay);
-               if (success) {
-                       struct intel_sdvo_dtd input_dtd;
+       intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
+       intel_sdvo->sdvo_flags = input_dtd.part2.sdvo_flags;
 
-                       intel_sdvo_get_preferred_input_timing(intel_sdvo,
-                                                            &input_dtd);
-                       intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
-                       intel_sdvo->sdvo_flags = input_dtd.part2.sdvo_flags;
+       drm_mode_set_crtcinfo(adjusted_mode, 0);
+       mode->clock = adjusted_mode->clock;
+       return true;
+}
 
-                       drm_mode_set_crtcinfo(adjusted_mode, 0);
+static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
+                                 struct drm_display_mode *mode,
+                                 struct drm_display_mode *adjusted_mode)
+{
+       struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
 
-                       mode->clock = adjusted_mode->clock;
+       /* We need to construct preferred input timings based on our
+        * output timings.  To do that, we have to set the output
+        * timings, even though this isn't really the right place in
+        * the sequence to do it. Oh well.
+        */
+       if (intel_sdvo->is_tv) {
+               if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo, mode))
+                       return false;
 
-                       adjusted_mode->clock *=
-                               intel_sdvo_get_pixel_multiplier(mode);
-               } else {
+               if (!intel_sdvo_set_input_timings_for_mode(intel_sdvo, mode, adjusted_mode))
                        return false;
-               }
        } else if (intel_sdvo->is_lvds) {
-               struct intel_sdvo_dtd output_dtd;
-               bool success;
-
                drm_mode_set_crtcinfo(intel_sdvo->sdvo_lvds_fixed_mode, 0);
-               /* Set output timings */
-               intel_sdvo_get_dtd_from_mode(&output_dtd,
-                               intel_sdvo->sdvo_lvds_fixed_mode);
-
-               intel_sdvo_set_target_output(intel_sdvo,
-                                            intel_sdvo->attached_output);
-               intel_sdvo_set_output_timing(intel_sdvo, &output_dtd);
-
-               /* Set the input timing to the screen. Assume always input 0. */
-               intel_sdvo_set_target_input(intel_sdvo, true, false);
-
-
-               success = intel_sdvo_create_preferred_input_timing(
-                               intel_sdvo,
-                               mode->clock / 10,
-                               mode->hdisplay,
-                               mode->vdisplay);
-
-               if (success) {
-                       struct intel_sdvo_dtd input_dtd;
-
-                       intel_sdvo_get_preferred_input_timing(intel_sdvo,
-                                                            &input_dtd);
-                       intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
-                       intel_sdvo->sdvo_flags = input_dtd.part2.sdvo_flags;
 
-                       drm_mode_set_crtcinfo(adjusted_mode, 0);
-
-                       mode->clock = adjusted_mode->clock;
-
-                       adjusted_mode->clock *=
-                               intel_sdvo_get_pixel_multiplier(mode);
-               } else {
+               if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
+                                                           intel_sdvo->sdvo_lvds_fixed_mode))
                        return false;
-               }
 
-       } else {
-               /* Make the CRTC code factor in the SDVO pixel multiplier.  The
-                * SDVO device will be told of the multiplier during mode_set.
-                */
-               adjusted_mode->clock *= intel_sdvo_get_pixel_multiplier(mode);
+               if (!intel_sdvo_set_input_timings_for_mode(intel_sdvo, mode, adjusted_mode))
+                       return false;
        }
+
+       /* Make the CRTC code factor in the SDVO pixel multiplier.  The
+        * SDVO device will be told of the multiplier during mode_set.
+        */
+       adjusted_mode->clock *= intel_sdvo_get_pixel_multiplier(mode);
+
        return true;
 }
 
@@ -1154,10 +1066,9 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
        struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
        struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
        u32 sdvox = 0;
-       int sdvo_pixel_multiply;
+       int sdvo_pixel_multiply, rate;
        struct intel_sdvo_in_out_map in_out;
        struct intel_sdvo_dtd input_dtd;
-       u8 status;
 
        if (!mode)
                return;
@@ -1171,12 +1082,15 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
        in_out.in0 = intel_sdvo->attached_output;
        in_out.in1 = 0;
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_IN_OUT_MAP,
-                            &in_out, sizeof(in_out));
-       status = intel_sdvo_read_response(intel_sdvo, NULL, 0);
+       if (!intel_sdvo_set_value(intel_sdvo,
+                                 SDVO_CMD_SET_IN_OUT_MAP,
+                                 &in_out, sizeof(in_out)))
+               return;
 
        if (intel_sdvo->is_hdmi) {
-               intel_sdvo_set_avi_infoframe(intel_sdvo, mode);
+               if (!intel_sdvo_set_avi_infoframe(intel_sdvo, mode))
+                       return;
+
                sdvox |= SDVO_AUDIO_ENABLE;
        }
 
@@ -1193,16 +1107,22 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
         */
        if (!intel_sdvo->is_tv && !intel_sdvo->is_lvds) {
                /* Set the output timing to the screen */
-               intel_sdvo_set_target_output(intel_sdvo,
-                                            intel_sdvo->attached_output);
-               intel_sdvo_set_output_timing(intel_sdvo, &input_dtd);
+               if (!intel_sdvo_set_target_output(intel_sdvo,
+                                                 intel_sdvo->attached_output))
+                       return;
+
+               if (!intel_sdvo_set_output_timing(intel_sdvo, &input_dtd))
+                       return;
        }
 
        /* Set the input timing to the screen. Assume always input 0. */
-       intel_sdvo_set_target_input(intel_sdvo, true, false);
+       if (!intel_sdvo_set_target_input(intel_sdvo))
+               return;
 
-       if (intel_sdvo->is_tv)
-               intel_sdvo_set_tv_format(intel_sdvo);
+       if (intel_sdvo->is_tv) {
+               if (!intel_sdvo_set_tv_format(intel_sdvo))
+                       return;
+       }
 
        /* We would like to use intel_sdvo_create_preferred_input_timing() to
         * provide the device with a timing it can support, if it supports that
@@ -1219,23 +1139,18 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
                intel_sdvo_set_input_timing(encoder, &input_dtd);
        }
 #else
-       intel_sdvo_set_input_timing(intel_sdvo, &input_dtd);
+       if (!intel_sdvo_set_input_timing(intel_sdvo, &input_dtd))
+               return;
 #endif
 
-       switch (intel_sdvo_get_pixel_multiplier(mode)) {
-       case 1:
-               intel_sdvo_set_clock_rate_mult(intel_sdvo,
-                                              SDVO_CLOCK_RATE_MULT_1X);
-               break;
-       case 2:
-               intel_sdvo_set_clock_rate_mult(intel_sdvo,
-                                              SDVO_CLOCK_RATE_MULT_2X);
-               break;
-       case 4:
-               intel_sdvo_set_clock_rate_mult(intel_sdvo,
-                                              SDVO_CLOCK_RATE_MULT_4X);
-               break;
+       sdvo_pixel_multiply = intel_sdvo_get_pixel_multiplier(mode);
+       switch (sdvo_pixel_multiply) {
+       case 1: rate = SDVO_CLOCK_RATE_MULT_1X; break;
+       case 2: rate = SDVO_CLOCK_RATE_MULT_2X; break;
+       case 4: rate = SDVO_CLOCK_RATE_MULT_4X; break;
        }
+       if (!intel_sdvo_set_clock_rate_mult(intel_sdvo, rate))
+               return;
 
        /* Set the SDVO control regs. */
        if (IS_I965G(dev)) {
@@ -1259,7 +1174,6 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
        if (intel_crtc->pipe == 1)
                sdvox |= SDVO_PIPE_B_SELECT;
 
-       sdvo_pixel_multiply = intel_sdvo_get_pixel_multiplier(mode);
        if (IS_I965G(dev)) {
                /* done in crtc_mode_set as the dpll_md reg must be written early */
        } else if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) {
@@ -1302,10 +1216,7 @@ static void intel_sdvo_dpms(struct drm_encoder *encoder, int mode)
                for (i = 0; i < 2; i++)
                  intel_wait_for_vblank(dev);
 
-               status = intel_sdvo_get_trained_inputs(intel_sdvo, &input1,
-                                                      &input2);
-
-
+               status = intel_sdvo_get_trained_inputs(intel_sdvo, &input1, &input2);
                /* Warn if the device reported failure to sync.
                 * A lot of SDVO devices fail to notify of sync, but it's
                 * a given it the status is a success, we succeeded.
@@ -1353,14 +1264,7 @@ static int intel_sdvo_mode_valid(struct drm_connector *connector,
 
 static bool intel_sdvo_get_capabilities(struct intel_sdvo *intel_sdvo, struct intel_sdvo_caps *caps)
 {
-       u8 status;
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_DEVICE_CAPS, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, caps, sizeof(*caps));
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-
-       return true;
+       return intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_DEVICE_CAPS, caps, sizeof(*caps));
 }
 
 /* No use! */
@@ -1403,13 +1307,8 @@ int intel_sdvo_supports_hotplug(struct drm_connector *connector)
 
        intel_sdvo = to_intel_sdvo(connector);
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, &response, 2);
-
-       if (response[0] !=0)
-               return 1;
-
-       return 0;
+       return intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HOT_PLUG_SUPPORT,
+                                   &response, 2) && response[0];
 }
 
 void intel_sdvo_set_hotplug(struct drm_connector *connector, int on)
@@ -1492,8 +1391,8 @@ static int
 intel_analog_is_connected(struct drm_device *dev)
 {
        struct drm_connector *analog_connector;
-       analog_connector = intel_find_analog_connector(dev);
 
+       analog_connector = intel_find_analog_connector(dev);
        if (!analog_connector)
                return false;
 
@@ -1569,25 +1468,23 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
 static enum drm_connector_status intel_sdvo_detect(struct drm_connector *connector)
 {
        uint16_t response;
-       u8 status;
        struct drm_encoder *encoder = intel_attached_encoder(connector);
        struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
        struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
        enum drm_connector_status ret;
 
-       intel_sdvo_write_cmd(intel_sdvo,
-                            SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0);
+       if (!intel_sdvo_write_cmd(intel_sdvo,
+                            SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0))
+               return connector_status_unknown;
        if (intel_sdvo->is_tv) {
                /* add 30ms delay when the output type is SDVO-TV */
                mdelay(30);
        }
-       status = intel_sdvo_read_response(intel_sdvo, &response, 2);
+       if (!intel_sdvo_read_response(intel_sdvo, &response, 2))
+               return connector_status_unknown;
 
        DRM_DEBUG_KMS("SDVO response %d %d\n", response & 0xff, response >> 8);
 
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return connector_status_unknown;
-
        if (response == 0)
                return connector_status_disconnected;
 
@@ -1713,8 +1610,6 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
        struct intel_sdvo_sdtv_resolution_request tv_res;
        uint32_t reply = 0, format_map = 0;
        int i;
-       uint8_t status;
-
 
        /* Read the list of supported input resolutions for the selected TV
         * format.
@@ -1725,23 +1620,23 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
 
        format_map = (1 << i);
        memcpy(&tv_res, &format_map,
-              sizeof(struct intel_sdvo_sdtv_resolution_request) >
-              sizeof(format_map) ? sizeof(format_map) :
-              sizeof(struct intel_sdvo_sdtv_resolution_request));
+              min(sizeof(format_map), sizeof(struct intel_sdvo_sdtv_resolution_request)));
 
-       intel_sdvo_set_target_output(intel_sdvo, intel_sdvo->attached_output);
+       if (!intel_sdvo_set_target_output(intel_sdvo, intel_sdvo->attached_output))
+               return;
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
-                            &tv_res, sizeof(tv_res));
-       status = intel_sdvo_read_response(intel_sdvo, &reply, 3);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
+       BUILD_BUG_ON(sizeof(tv_res) != 3);
+       if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
+                                 &tv_res, sizeof(tv_res)))
+               return;
+       if (!intel_sdvo_read_response(intel_sdvo, &reply, 3))
                return;
 
        for (i = 0; i < ARRAY_SIZE(sdvo_tv_modes); i++)
                if (reply & (1 << i)) {
                        struct drm_display_mode *nmode;
                        nmode = drm_mode_duplicate(connector->dev,
-                                       &sdvo_tv_modes[i]);
+                                                  &sdvo_tv_modes[i]);
                        if (nmode)
                                drm_mode_probed_add(connector, nmode);
                }
@@ -1798,9 +1693,7 @@ static int intel_sdvo_get_modes(struct drm_connector *connector)
        else
                intel_sdvo_get_ddc_modes(connector);
 
-       if (list_empty(&connector->probed_modes))
-               return 0;
-       return 1;
+       return !list_empty(&connector->probed_modes);
 }
 
 static
@@ -1831,7 +1724,7 @@ void intel_sdvo_destroy_enhance_property(struct drm_connector *connector)
                if (intel_sdvo_connector->hue_property)
                        drm_property_destroy(dev, intel_sdvo_connector->hue_property);
        }
-       if (IS_TV(intel_sdvo_connector) || IS_LVDS(intel_sdvo_connector)) {
+       if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
                if (intel_sdvo_connector->brightness_property)
                        drm_property_destroy(dev,
                                        intel_sdvo_connector->brightness_property);
@@ -1862,36 +1755,33 @@ intel_sdvo_set_property(struct drm_connector *connector,
        struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
        struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
        struct drm_crtc *crtc = encoder->crtc;
-       int ret = 0;
        bool changed = false;
-       uint8_t cmd, status;
        uint16_t temp_value;
+       uint8_t cmd;
+       int ret;
 
        ret = drm_connector_property_set_value(connector, property, val);
-       if (ret < 0)
-               goto out;
+       if (ret)
+               return ret;
 
        if (property == intel_sdvo_connector->tv_format_property) {
-               if (val >= TV_FORMAT_NUM) {
-                       ret = -EINVAL;
-                       goto out;
-               }
+               if (val >= TV_FORMAT_NUM)
+                       return -EINVAL;
+
                if (intel_sdvo->tv_format_name ==
                    intel_sdvo_connector->tv_format_supported[val])
-                       goto out;
+                       return 0;
 
                intel_sdvo->tv_format_name = intel_sdvo_connector->tv_format_supported[val];
                changed = true;
-       }
-
-       if (IS_TV(intel_sdvo_connector) || IS_LVDS(intel_sdvo_connector)) {
+       } else if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
                cmd = 0;
                temp_value = val;
                if (intel_sdvo_connector->left_property == property) {
                        drm_connector_property_set_value(connector,
                                intel_sdvo_connector->right_property, val);
                        if (intel_sdvo_connector->left_margin == temp_value)
-                               goto out;
+                               return 0;
 
                        intel_sdvo_connector->left_margin = temp_value;
                        intel_sdvo_connector->right_margin = temp_value;
@@ -1902,7 +1792,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
                        drm_connector_property_set_value(connector,
                                intel_sdvo_connector->left_property, val);
                        if (intel_sdvo_connector->right_margin == temp_value)
-                               goto out;
+                               return 0;
 
                        intel_sdvo_connector->left_margin = temp_value;
                        intel_sdvo_connector->right_margin = temp_value;
@@ -1913,7 +1803,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
                        drm_connector_property_set_value(connector,
                                intel_sdvo_connector->bottom_property, val);
                        if (intel_sdvo_connector->top_margin == temp_value)
-                               goto out;
+                               return 0;
 
                        intel_sdvo_connector->top_margin = temp_value;
                        intel_sdvo_connector->bottom_margin = temp_value;
@@ -1924,7 +1814,8 @@ intel_sdvo_set_property(struct drm_connector *connector,
                        drm_connector_property_set_value(connector,
                                intel_sdvo_connector->top_property, val);
                        if (intel_sdvo_connector->bottom_margin == temp_value)
-                               goto out;
+                               return 0;
+
                        intel_sdvo_connector->top_margin = temp_value;
                        intel_sdvo_connector->bottom_margin = temp_value;
                        temp_value = intel_sdvo_connector->max_vscan -
@@ -1932,57 +1823,52 @@ intel_sdvo_set_property(struct drm_connector *connector,
                        cmd = SDVO_CMD_SET_OVERSCAN_V;
                } else if (intel_sdvo_connector->hpos_property == property) {
                        if (intel_sdvo_connector->cur_hpos == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_POSITION_H;
                        intel_sdvo_connector->cur_hpos = temp_value;
                } else if (intel_sdvo_connector->vpos_property == property) {
                        if (intel_sdvo_connector->cur_vpos == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_POSITION_V;
                        intel_sdvo_connector->cur_vpos = temp_value;
                } else if (intel_sdvo_connector->saturation_property == property) {
                        if (intel_sdvo_connector->cur_saturation == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_SATURATION;
                        intel_sdvo_connector->cur_saturation = temp_value;
                } else if (intel_sdvo_connector->contrast_property == property) {
                        if (intel_sdvo_connector->cur_contrast == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_CONTRAST;
                        intel_sdvo_connector->cur_contrast = temp_value;
                } else if (intel_sdvo_connector->hue_property == property) {
                        if (intel_sdvo_connector->cur_hue == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_HUE;
                        intel_sdvo_connector->cur_hue = temp_value;
                } else if (intel_sdvo_connector->brightness_property == property) {
                        if (intel_sdvo_connector->cur_brightness == temp_value)
-                               goto out;
+                               return 0;
 
                        cmd = SDVO_CMD_SET_BRIGHTNESS;
                        intel_sdvo_connector->cur_brightness = temp_value;
                }
                if (cmd) {
-                       intel_sdvo_write_cmd(intel_sdvo, cmd, &temp_value, 2);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                                                               NULL, 0);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO command \n");
+                       if (!intel_sdvo_set_value(intel_sdvo, cmd, &temp_value, 2))
                                return -EINVAL;
-                       }
+
                        changed = true;
                }
        }
        if (changed && crtc)
                drm_crtc_helper_set_mode(crtc, &crtc->mode, crtc->x,
                                crtc->y, crtc->fb);
-out:
-       return ret;
+       return 0;
 }
 
 static const struct drm_encoder_helper_funcs intel_sdvo_helper_funcs = {
@@ -2050,18 +1936,10 @@ intel_sdvo_select_ddc_bus(struct drm_i915_private *dev_priv,
 static bool
 intel_sdvo_get_digital_encoding_mode(struct intel_sdvo *intel_sdvo, int device)
 {
-       uint8_t status;
-
-       if (device == 0)
-               intel_sdvo_set_target_output(intel_sdvo, SDVO_OUTPUT_TMDS0);
-       else
-               intel_sdvo_set_target_output(intel_sdvo, SDVO_OUTPUT_TMDS1);
-
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_ENCODE, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, &intel_sdvo->is_hdmi, 1);
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return false;
-       return true;
+       return intel_sdvo_set_target_output(intel_sdvo,
+                                           device == 0 ? SDVO_OUTPUT_TMDS0 : SDVO_OUTPUT_TMDS1) &&
+               intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
+                                    &intel_sdvo->is_hdmi, 1);
 }
 
 static struct intel_sdvo *
@@ -2076,7 +1954,7 @@ intel_sdvo_chan_to_intel_sdvo(struct intel_i2c_chan *chan)
                        return intel_sdvo;
        }
 
-       return NULL;;
+       return NULL;
 }
 
 static int intel_sdvo_master_xfer(struct i2c_adapter *i2c_adap,
@@ -2141,8 +2019,8 @@ intel_sdvo_get_slave_addr(struct drm_device *dev, int sdvo_reg)
 }
 
 static void
-intel_sdvo_connector_create (struct drm_encoder *encoder,
-                            struct drm_connector *connector)
+intel_sdvo_connector_init(struct drm_encoder *encoder,
+                         struct drm_connector *connector)
 {
        drm_connector_init(encoder->dev, connector, &intel_sdvo_connector_funcs,
                           connector->connector_type);
@@ -2195,7 +2073,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
        intel_sdvo->base.clone_mask = ((1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
                                       (1 << INTEL_ANALOG_CLONE_BIT));
 
-       intel_sdvo_connector_create(encoder, connector);
+       intel_sdvo_connector_init(encoder, connector);
 
        return true;
 }
@@ -2224,13 +2102,19 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
         intel_sdvo->base.needs_tv_clock = true;
         intel_sdvo->base.clone_mask = 1 << INTEL_SDVO_TV_CLONE_BIT;
 
-        intel_sdvo_connector_create(encoder, connector);
+        intel_sdvo_connector_init(encoder, connector);
 
-        intel_sdvo_tv_create_property(connector, type);
+        if (!intel_sdvo_tv_create_property(intel_sdvo, intel_sdvo_connector, type))
+               goto err;
 
-        intel_sdvo_create_enhance_property(connector);
+        if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
+               goto err;
 
         return true;
+
+err:
+       kfree(intel_sdvo_connector);
+       return false;
 }
 
 static bool
@@ -2262,7 +2146,7 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
         intel_sdvo->base.clone_mask = ((1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
                                       (1 << INTEL_ANALOG_CLONE_BIT));
 
-        intel_sdvo_connector_create(encoder, connector);
+        intel_sdvo_connector_init(encoder, connector);
         return true;
 }
 
@@ -2296,9 +2180,15 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
         intel_sdvo->base.clone_mask = ((1 << INTEL_ANALOG_CLONE_BIT) |
                                       (1 << INTEL_SDVO_LVDS_CLONE_BIT));
 
-        intel_sdvo_connector_create(encoder, connector);
-        intel_sdvo_create_enhance_property(connector);
-        return true;
+        intel_sdvo_connector_init(encoder, connector);
+        if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
+               goto err;
+
+       return true;
+
+err:
+       kfree(intel_sdvo_connector);
+       return false;
 }
 
 static bool
@@ -2358,29 +2248,26 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, uint16_t flags)
        return true;
 }
 
-static void intel_sdvo_tv_create_property(struct drm_connector *connector, int type)
+static bool intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
+                                         struct intel_sdvo_connector *intel_sdvo_connector,
+                                         int type)
 {
-       struct drm_encoder *encoder = intel_attached_encoder(connector);
-       struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
-       struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
+       struct drm_device *dev = intel_sdvo->base.enc.dev;
        struct intel_sdvo_tv_format format;
        uint32_t format_map, i;
-       uint8_t status;
 
-       intel_sdvo_set_target_output(intel_sdvo, type);
+       if (!intel_sdvo_set_target_output(intel_sdvo, type))
+               return false;
 
-       intel_sdvo_write_cmd(intel_sdvo,
-                            SDVO_CMD_GET_SUPPORTED_TV_FORMATS, NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo,
-                                         &format, sizeof(format));
-       if (status != SDVO_CMD_STATUS_SUCCESS)
-               return;
+       if (!intel_sdvo_get_value(intel_sdvo,
+                                 SDVO_CMD_GET_SUPPORTED_TV_FORMATS,
+                                 &format, sizeof(format)))
+               return false;
 
-       memcpy(&format_map, &format, sizeof(format) > sizeof(format_map) ?
-              sizeof(format_map) : sizeof(format));
+       memcpy(&format_map, &format, min(sizeof(format_map), sizeof(format)));
 
        if (format_map == 0)
-               return;
+               return false;
 
        intel_sdvo_connector->format_supported_num = 0;
        for (i = 0 ; i < TV_FORMAT_NUM; i++)
@@ -2392,9 +2279,8 @@ static void intel_sdvo_tv_create_property(struct drm_connector *connector, int t
 
 
        intel_sdvo_connector->tv_format_property =
-                       drm_property_create(
-                               connector->dev, DRM_MODE_PROP_ENUM,
-                               "mode", intel_sdvo_connector->format_supported_num);
+                       drm_property_create(dev, DRM_MODE_PROP_ENUM,
+                                           "mode", intel_sdvo_connector->format_supported_num);
 
        for (i = 0; i < intel_sdvo_connector->format_supported_num; i++)
                drm_property_add_enum(
@@ -2402,56 +2288,45 @@ static void intel_sdvo_tv_create_property(struct drm_connector *connector, int t
                                i, intel_sdvo_connector->tv_format_supported[i]);
 
        intel_sdvo->tv_format_name = intel_sdvo_connector->tv_format_supported[0];
-       drm_connector_attach_property(
-                       connector, intel_sdvo_connector->tv_format_property, 0);
+       drm_connector_attach_property(&intel_sdvo_connector->base.base,
+                                     intel_sdvo_connector->tv_format_property, 0);
+       return true;
 
 }
 
-static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
+static bool intel_sdvo_create_enhance_property(struct intel_sdvo *intel_sdvo,
+                                              struct intel_sdvo_connector *intel_sdvo_connector)
 {
-       struct drm_encoder *encoder = intel_attached_encoder(connector);
-       struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
-       struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
+       struct drm_device *dev = intel_sdvo->base.enc.dev;
+       struct drm_connector *connector = &intel_sdvo_connector->base.base;
        struct intel_sdvo_enhancements_reply sdvo_data;
-       struct drm_device *dev = connector->dev;
-       uint8_t status;
        uint16_t response, data_value[2];
 
-       intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_SUPPORTED_ENHANCEMENTS,
-                                               NULL, 0);
-       status = intel_sdvo_read_response(intel_sdvo, &sdvo_data,
-                                       sizeof(sdvo_data));
-       if (status != SDVO_CMD_STATUS_SUCCESS) {
-               DRM_DEBUG_KMS(" incorrect response is returned\n");
-               return;
-       }
+       if (!intel_sdvo_get_value(intel_sdvo,
+                                 SDVO_CMD_GET_SUPPORTED_ENHANCEMENTS,
+                                 &sdvo_data, sizeof(sdvo_data)))
+               return false;
+
        response = *((uint16_t *)&sdvo_data);
        if (!response) {
                DRM_DEBUG_KMS("No enhancement is supported\n");
-               return;
+               return true;
        }
        if (IS_TV(intel_sdvo_connector)) {
                /* when horizontal overscan is supported, Add the left/right
                 * property
                 */
                if (sdvo_data.overscan_h) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_OVERSCAN_H, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO max "
-                                               "h_overscan\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_OVERSCAN_H, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO h_overscan\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_OVERSCAN_H,
+                                                 &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_OVERSCAN_H,
+                                                 &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_hscan = data_value[0];
                        intel_sdvo_connector->left_margin = data_value[0] - response;
                        intel_sdvo_connector->right_margin = intel_sdvo_connector->left_margin;
@@ -2476,23 +2351,16 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.overscan_v) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_OVERSCAN_V, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO max "
-                                               "v_overscan\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_OVERSCAN_V, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO v_overscan\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_OVERSCAN_V,
+                                                 &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_OVERSCAN_V,
+                                                 &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_vscan = data_value[0];
                        intel_sdvo_connector->top_margin = data_value[0] - response;
                        intel_sdvo_connector->bottom_margin = intel_sdvo_connector->top_margin;
@@ -2517,22 +2385,16 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.position_h) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_POSITION_H, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max h_pos\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_POSITION_H, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get h_postion\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_POSITION_H,
+                                                 &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_POSITION_H,
+                                                 &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_hpos = data_value[0];
                        intel_sdvo_connector->cur_hpos = response;
                        intel_sdvo_connector->hpos_property =
@@ -2548,22 +2410,16 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.position_v) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_POSITION_V, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max v_pos\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_POSITION_V, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get v_postion\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_POSITION_V,
+                                                 &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_POSITION_V,
+                                                 &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_vpos = data_value[0];
                        intel_sdvo_connector->cur_vpos = response;
                        intel_sdvo_connector->vpos_property =
@@ -2579,22 +2435,16 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.saturation) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_SATURATION, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max sat\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_SATURATION, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get sat\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_SATURATION,
+                                                 &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_SATURATION,
+                                                 &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_saturation = data_value[0];
                        intel_sdvo_connector->cur_saturation = response;
                        intel_sdvo_connector->saturation_property =
@@ -2611,22 +2461,14 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.contrast) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_CONTRAST, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max contrast\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_CONTRAST, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get contrast\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_CONTRAST, &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_CONTRAST, &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_contrast = data_value[0];
                        intel_sdvo_connector->cur_contrast = response;
                        intel_sdvo_connector->contrast_property =
@@ -2642,22 +2484,14 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
                if (sdvo_data.hue) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_HUE, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max hue\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_HUE, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get hue\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_HUE, &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_HUE, &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_hue = data_value[0];
                        intel_sdvo_connector->cur_hue = response;
                        intel_sdvo_connector->hue_property =
@@ -2673,24 +2507,16 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
        }
-       if (IS_TV(intel_sdvo_connector) || IS_LVDS(intel_sdvo_connector)) {
+       if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
                if (sdvo_data.brightness) {
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_MAX_BRIGHTNESS, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &data_value, 4);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO Max bright\n");
-                               return;
-                       }
-                       intel_sdvo_write_cmd(intel_sdvo,
-                               SDVO_CMD_GET_BRIGHTNESS, NULL, 0);
-                       status = intel_sdvo_read_response(intel_sdvo,
-                               &response, 2);
-                       if (status != SDVO_CMD_STATUS_SUCCESS) {
-                               DRM_DEBUG_KMS("Incorrect SDVO get brigh\n");
-                               return;
-                       }
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_MAX_BRIGHTNESS, &data_value, 4))
+                               return false;
+
+                       if (!intel_sdvo_get_value(intel_sdvo,
+                                                 SDVO_CMD_GET_BRIGHTNESS, &response, 2))
+                               return false;
+
                        intel_sdvo_connector->max_brightness = data_value[0];
                        intel_sdvo_connector->cur_brightness = response;
                        intel_sdvo_connector->brightness_property =
@@ -2707,7 +2533,7 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
                                        data_value[0], data_value[1], response);
                }
        }
-       return;
+       return true;
 }
 
 bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
@@ -2773,8 +2599,7 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
                                                "SDVOC/VGA DDC BUS");
                dev_priv->hotplug_supported_mask |= SDVOC_HOTPLUG_INT_STATUS;
        }
-
-       if (intel_encoder->ddc_bus == NULL)
+       if (intel_encoder->ddc_bus == NULL || intel_sdvo->analog_ddc_bus == NULL)
                goto err_i2c;
 
        /* Wrap with our custom algo which switches to DDC mode */
@@ -2785,24 +2610,26 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
        drm_encoder_helper_add(&intel_encoder->enc, &intel_sdvo_helper_funcs);
 
        /* In default case sdvo lvds is false */
-       intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps);
+       if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
+               goto err_enc;
 
        if (intel_sdvo_output_setup(intel_sdvo,
                                    intel_sdvo->caps.output_flags) != true) {
                DRM_DEBUG_KMS("SDVO output failed to setup on SDVO%c\n",
                              IS_SDVOB(sdvo_reg) ? 'B' : 'C');
-               goto err_i2c;
+               goto err_enc;
        }
 
        intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
 
        /* Set the input timing to the screen. Assume always input 0. */
-       intel_sdvo_set_target_input(intel_sdvo, true, false);
-
-       intel_sdvo_get_input_pixel_clock_range(intel_sdvo,
-                                              &intel_sdvo->pixel_clock_min,
-                                              &intel_sdvo->pixel_clock_max);
+       if (!intel_sdvo_set_target_input(intel_sdvo))
+               goto err_enc;
 
+       if (!intel_sdvo_get_input_pixel_clock_range(intel_sdvo,
+                                                   &intel_sdvo->pixel_clock_min,
+                                                   &intel_sdvo->pixel_clock_max))
+               goto err_enc;
 
        DRM_DEBUG_KMS("%s device VID/DID: %02X:%02X.%02X, "
                        "clock range %dMHz - %dMHz, "
@@ -2820,9 +2647,10 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
                        (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_RGB0) ? 'Y' : 'N',
                        intel_sdvo->caps.output_flags &
                        (SDVO_OUTPUT_TMDS1 | SDVO_OUTPUT_RGB1) ? 'Y' : 'N');
-
        return true;
 
+err_enc:
+       drm_encoder_cleanup(&intel_encoder->enc);
 err_i2c:
        if (intel_sdvo->analog_ddc_bus != NULL)
                intel_i2c_destroy(intel_sdvo->analog_ddc_bus);
index ba5cdf8ae40bea4ab9d5b416fd5c3e10ac43facb..4988660f661b28b3a75b916408d92626154d50a1 100644 (file)
@@ -312,7 +312,7 @@ struct intel_sdvo_set_target_input_args {
 # define SDVO_CLOCK_RATE_MULT_4X                               (1 << 3)
 
 #define SDVO_CMD_GET_SUPPORTED_TV_FORMATS              0x27
-/** 5 bytes of bit flags for TV formats shared by all TV format functions */
+/** 6 bytes of bit flags for TV formats shared by all TV format functions */
 struct intel_sdvo_tv_format {
     unsigned int ntsc_m:1;
     unsigned int ntsc_j:1;