[Intel-gfx] [PATCH 16/37] drm/i915: Propagate read/write errors whilst setting SDVO.

Chris Wilson chris at chris-wilson.co.uk
Wed Mar 10 23:45:03 CET 2010


Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h       |    4 +-
 drivers/gpu/drm/i915/intel_sdvo.c      |  808 +++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_sdvo_regs.h |    1 +
 3 files changed, 426 insertions(+), 387 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3a467ca..990e01e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -198,8 +198,8 @@ extern void intel_release_load_detect_pipe(struct intel_output *intel_output,
 					   int dpms_mode);
 
 extern struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB);
-extern int intel_sdvo_supports_hotplug(struct drm_connector *connector);
-extern void intel_sdvo_set_hotplug(struct drm_connector *connector, int enable);
+extern bool intel_sdvo_supports_hotplug(struct drm_connector *connector);
+extern int intel_sdvo_set_hotplug(struct drm_connector *connector, int enable);
 extern int intelfb_probe(struct drm_device *dev);
 extern int intelfb_remove(struct drm_device *dev, struct drm_framebuffer *fb);
 extern int intelfb_resize(struct drm_device *dev, struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 48daee5..1da56ba 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -47,7 +47,7 @@ static char *tv_format_names[] = {
 	"SECAM_60"
 };
 
-#define TV_FORMAT_NUM  (sizeof(tv_format_names) / sizeof(*tv_format_names))
+#define TV_FORMAT_NUM  ARRAY_SIZE(tv_format_names)
 
 struct intel_sdvo_priv {
 	u8 slave_addr;
@@ -248,11 +248,7 @@ static bool intel_sdvo_write_byte(struct intel_output *intel_output, int addr,
 	out_buf[0] = addr;
 	out_buf[1] = ch;
 
-	if (i2c_transfer(intel_output->i2c_bus, msgs, 1) == 1)
-	{
-		return true;
-	}
-	return false;
+	return i2c_transfer(intel_output->i2c_bus, msgs, 1) == 1;
 }
 
 #define SDVO_CMD_NAME_ENTRY(cmd) {cmd, #cmd}
@@ -378,7 +374,7 @@ static void intel_sdvo_debug_write(struct intel_output *intel_output, u8 cmd,
 	DRM_LOG_KMS("\n");
 }
 
-static void intel_sdvo_write_cmd(struct intel_output *intel_output, u8 cmd,
+static bool intel_sdvo_write_cmd(struct intel_output *intel_output, u8 cmd,
 				 void *args, int args_len)
 {
 	int i;
@@ -386,11 +382,12 @@ static void intel_sdvo_write_cmd(struct intel_output *intel_output, u8 cmd,
 	intel_sdvo_debug_write(intel_output, cmd, args, args_len);
 
 	for (i = 0; i < args_len; i++) {
-		intel_sdvo_write_byte(intel_output, SDVO_I2C_ARG_0 - i,
-				      ((u8*)args)[i]);
+		if (!intel_sdvo_write_byte(intel_output, SDVO_I2C_ARG_0 - i,
+					   ((u8*)args)[i]))
+			return false;
 	}
 
-	intel_sdvo_write_byte(intel_output, SDVO_I2C_OPCODE, cmd);
+	return intel_sdvo_write_byte(intel_output, SDVO_I2C_OPCODE, cmd);
 }
 
 static const char *cmd_status_names[] = {
@@ -415,7 +412,7 @@ static void intel_sdvo_debug_response(struct intel_output *intel_output,
 		DRM_LOG_KMS("%02X ", ((u8 *)response)[i]);
 	for (; i < 8; i++)
 		DRM_LOG_KMS("   ");
-	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
+	if (status <= ARRAY_SIZE(cmd_status_names))
 		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
 	else
 		DRM_LOG_KMS("(??? %d)", status);
@@ -432,14 +429,16 @@ static u8 intel_sdvo_read_response(struct intel_output *intel_output,
 	while (retry--) {
 		/* Read the command response */
 		for (i = 0; i < response_len; i++) {
-			intel_sdvo_read_byte(intel_output,
-					     SDVO_I2C_RETURN_0 + i,
-					     &((u8 *)response)[i]);
+			if (!intel_sdvo_read_byte(intel_output,
+						  SDVO_I2C_RETURN_0 + i,
+						  &((u8 *)response)[i]))
+				return SDVO_CMD_STATUS_READ_FAILED;
 		}
 
 		/* read the return status */
-		intel_sdvo_read_byte(intel_output, SDVO_I2C_CMD_STATUS,
-				     &status);
+		if (!intel_sdvo_read_byte(intel_output, SDVO_I2C_CMD_STATUS,
+					  &status))
+			return SDVO_CMD_STATUS_READ_FAILED;
 
 		intel_sdvo_debug_response(intel_output, response, response_len,
 					  status);
@@ -469,7 +468,7 @@ static int intel_sdvo_get_pixel_multiplier(struct drm_display_mode *mode)
  * another I2C transaction after issuing the DDC bus switch, it will be
  * switched to the internal SDVO register.
  */
-static void intel_sdvo_set_control_bus_switch(struct intel_output *intel_output,
+static bool intel_sdvo_set_control_bus_switch(struct intel_output *intel_output,
 					      u8 target)
 {
 	struct intel_sdvo_priv *sdvo_priv = intel_output->dev_priv;
@@ -499,7 +498,8 @@ static void intel_sdvo_set_control_bus_switch(struct intel_output *intel_output,
 	intel_sdvo_debug_write(intel_output, SDVO_CMD_SET_CONTROL_BUS_SWITCH,
 					&target, 1);
 	/* write the DDC switch command argument */
-	intel_sdvo_write_byte(intel_output, SDVO_I2C_ARG_0, target);
+	if (!intel_sdvo_write_byte(intel_output, SDVO_I2C_ARG_0, target))
+		return false;
 
 	out_buf[0] = SDVO_I2C_OPCODE;
 	out_buf[1] = SDVO_CMD_SET_CONTROL_BUS_SWITCH;
@@ -512,14 +512,14 @@ static void intel_sdvo_set_control_bus_switch(struct intel_output *intel_output,
 	if (ret != 3) {
 		/* failure in I2C transfer */
 		DRM_DEBUG_KMS("I2c transfer returned %d\n", ret);
-		return;
+		return false;
 	}
 	if (ret_value[0] != SDVO_CMD_STATUS_SUCCESS) {
 		DRM_DEBUG_KMS("DDC switch command returns response %d\n",
 					ret_value[0]);
-		return;
+		return false;
 	}
-	return;
+	return true;
 }
 
 static bool intel_sdvo_set_target_input(struct intel_output *intel_output, bool target_0, bool target_1)
@@ -533,12 +533,11 @@ static bool intel_sdvo_set_target_input(struct intel_output *intel_output, bool
 	if (target_1)
 		targets.target_1 = 1;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_TARGET_INPUT, &targets,
-			     sizeof(targets));
-
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
-
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_TARGET_INPUT,
+				 &targets, sizeof(targets)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 /**
@@ -552,8 +551,9 @@ static bool intel_sdvo_get_trained_inputs(struct intel_output *intel_output, boo
 	struct intel_sdvo_get_trained_inputs_response response;
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_TRAINED_INPUTS, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &response, sizeof(response));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_TRAINED_INPUTS, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &response, sizeof(response));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
@@ -567,10 +567,10 @@ static bool intel_sdvo_get_active_outputs(struct intel_output *intel_output,
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ACTIVE_OUTPUTS, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, outputs, sizeof(*outputs));
-
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ACTIVE_OUTPUTS, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, outputs, sizeof(*outputs));
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_set_active_outputs(struct intel_output *intel_output,
@@ -578,10 +578,11 @@ static bool intel_sdvo_set_active_outputs(struct intel_output *intel_output,
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_OUTPUTS, &outputs,
-			     sizeof(outputs));
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_OUTPUTS,
+				 &outputs, sizeof(outputs)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_set_encoder_power_state(struct intel_output *intel_output,
@@ -604,11 +605,11 @@ static bool intel_sdvo_set_encoder_power_state(struct intel_output *intel_output
 		break;
 	}
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ENCODER_POWER_STATE, &state,
-			     sizeof(state));
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
-
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ENCODER_POWER_STATE,
+				 &state, sizeof(state)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_get_input_pixel_clock_range(struct intel_output *intel_output,
@@ -618,11 +619,10 @@ static bool intel_sdvo_get_input_pixel_clock_range(struct intel_output *intel_ou
 	struct intel_sdvo_pixel_clock_range clocks;
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE,
-			     NULL, 0);
-
-	status = intel_sdvo_read_response(intel_output, &clocks, sizeof(clocks));
-
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_INPUT_PIXEL_CLOCK_RANGE,
+				 NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &clocks, sizeof(clocks));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
@@ -638,11 +638,11 @@ static bool intel_sdvo_set_target_output(struct intel_output *intel_output,
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_TARGET_OUTPUT, &outputs,
-			     sizeof(outputs));
-
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_TARGET_OUTPUT,
+				 &outputs, sizeof(outputs)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_get_timing(struct intel_output *intel_output, u8 cmd,
@@ -650,15 +650,17 @@ static bool intel_sdvo_get_timing(struct intel_output *intel_output, u8 cmd,
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, cmd, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &dtd->part1,
-					  sizeof(dtd->part1));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, cmd, NULL, 0))
+		status = intel_sdvo_read_response(intel_output,
+						  &dtd->part1, sizeof(dtd->part1));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
-	intel_sdvo_write_cmd(intel_output, cmd + 1, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &dtd->part2,
-					  sizeof(dtd->part2));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, cmd + 1, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &dtd->part2,
+						  sizeof(dtd->part2));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
@@ -684,13 +686,15 @@ static bool intel_sdvo_set_timing(struct intel_output *intel_output, u8 cmd,
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, cmd, &dtd->part1, sizeof(dtd->part1));
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, cmd, &dtd->part1, sizeof(dtd->part1)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
-	intel_sdvo_write_cmd(intel_output, cmd + 1, &dtd->part2, sizeof(dtd->part2));
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, cmd + 1, &dtd->part2, sizeof(dtd->part2)))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
@@ -719,7 +723,7 @@ intel_sdvo_create_preferred_input_timing(struct intel_output *output,
 {
 	struct intel_sdvo_preferred_input_timing_args args;
 	struct intel_sdvo_priv *sdvo_priv = output->dev_priv;
-	uint8_t status;
+	u8 status;
 
 	memset(&args, 0, sizeof(args));
 	args.clock = clock;
@@ -732,33 +736,31 @@ intel_sdvo_create_preferred_input_timing(struct intel_output *output,
 	    sdvo_priv->sdvo_lvds_fixed_mode->vdisplay != height))
 		args.scaled = 1;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING,
-			     &args, sizeof(args));
-	status = intel_sdvo_read_response(output, NULL, 0);
-	if (status != SDVO_CMD_STATUS_SUCCESS)
-		return false;
-
-	return true;
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_CREATE_PREFERRED_INPUT_TIMING,
+				 &args, sizeof(args)))
+		status = intel_sdvo_read_response(output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_get_preferred_input_timing(struct intel_output *output,
 						  struct intel_sdvo_dtd *dtd)
 {
-	bool status;
-
-	intel_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1,
-			     NULL, 0);
+	u8 status;
 
-	status = intel_sdvo_read_response(output, &dtd->part1,
-					  sizeof(dtd->part1));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART1,
+				 NULL, 0))
+		status = intel_sdvo_read_response(output, &dtd->part1,
+						  sizeof(dtd->part1));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2,
-			     NULL, 0);
-
-	status = intel_sdvo_read_response(output, &dtd->part2,
-					  sizeof(dtd->part2));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_GET_PREFERRED_INPUT_TIMING_PART2,
+				 NULL, 0))
+		status = intel_sdvo_read_response(output, &dtd->part2,
+						  sizeof(dtd->part2));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return false;
 
@@ -769,29 +771,26 @@ static int intel_sdvo_get_clock_rate_mult(struct intel_output *intel_output)
 {
 	u8 response, status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_CLOCK_RATE_MULT, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &response, 1);
-
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_CLOCK_RATE_MULT, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &response, 1);
 	if (status != SDVO_CMD_STATUS_SUCCESS) {
 		DRM_DEBUG_KMS("Couldn't get SDVO clock rate multiplier\n");
 		return SDVO_CLOCK_RATE_MULT_1X;
 	} else {
 		DRM_DEBUG_KMS("Current clock rate multiplier: %d\n", response);
+		return response;
 	}
-
-	return response;
 }
 
 static bool intel_sdvo_set_clock_rate_mult(struct intel_output *intel_output, u8 val)
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1);
-	status = intel_sdvo_read_response(intel_output, NULL, 0);
-	if (status != SDVO_CMD_STATUS_SUCCESS)
-		return false;
-
-	return true;
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1))
+		status = intel_sdvo_read_response(intel_output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
@@ -879,10 +878,11 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
 static bool intel_sdvo_get_supp_encode(struct intel_output *output,
 				       struct intel_sdvo_encode *encode)
 {
-	uint8_t status;
+	u8 status;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_GET_SUPP_ENCODE, NULL, 0);
-	status = intel_sdvo_read_response(output, encode, sizeof(*encode));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_GET_SUPP_ENCODE, NULL, 0))
+		status = intel_sdvo_read_response(output, encode, sizeof(*encode));
 	if (status != SDVO_CMD_STATUS_SUCCESS) { /* non-support means DVI */
 		memset(encode, 0, sizeof(*encode));
 		return false;
@@ -893,23 +893,23 @@ static bool intel_sdvo_get_supp_encode(struct intel_output *output,
 
 static bool intel_sdvo_set_encode(struct intel_output *output, uint8_t mode)
 {
-	uint8_t status;
-
-	intel_sdvo_write_cmd(output, SDVO_CMD_SET_ENCODE, &mode, 1);
-	status = intel_sdvo_read_response(output, NULL, 0);
+	u8 status;
 
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_SET_ENCODE, &mode, 1))
+		status = intel_sdvo_read_response(output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_set_colorimetry(struct intel_output *output,
 				       uint8_t mode)
 {
-	uint8_t status;
-
-	intel_sdvo_write_cmd(output, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
-	status = intel_sdvo_read_response(output, NULL, 0);
+	u8 status;
 
-	return (status == SDVO_CMD_STATUS_SUCCESS);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_SET_COLORIMETRY, &mode, 1))
+		status = intel_sdvo_read_response(output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 #if 0
@@ -943,22 +943,24 @@ static void intel_sdvo_dump_hdmi_buf(struct intel_output *output)
 }
 #endif
 
-static void intel_sdvo_set_hdmi_buf(struct intel_output *output, int index,
-				uint8_t *data, int8_t size, uint8_t tx_rate)
+static bool intel_sdvo_set_hdmi_buf(struct intel_output *output, int index,
+				    uint8_t *data, int8_t size, uint8_t tx_rate)
 {
     uint8_t set_buf_index[2];
 
     set_buf_index[0] = index;
     set_buf_index[1] = 0;
 
-    intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_INDEX, set_buf_index, 2);
+    if (!intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_INDEX, set_buf_index, 2))
+	    return false;
 
     for (; size > 0; size -= 8) {
-	intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_DATA, data, 8);
+	if (!intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_DATA, data, 8))
+		return false;
 	data += 8;
     }
 
-    intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_TXRATE, &tx_rate, 1);
+    return intel_sdvo_write_cmd(output, SDVO_CMD_SET_HBUF_TXRATE, &tx_rate, 1);
 }
 
 static uint8_t intel_sdvo_calc_hbuf_csum(uint8_t *data, uint8_t size)
@@ -1033,7 +1035,7 @@ struct dip_infoframe {
 	} __attribute__ ((packed)) u;
 } __attribute__((packed));
 
-static void intel_sdvo_set_avi_infoframe(struct intel_output *output,
+static bool intel_sdvo_set_avi_infoframe(struct intel_output *output,
 					 struct drm_display_mode * mode)
 {
 	struct dip_infoframe avi_if = {
@@ -1044,17 +1046,17 @@ static void intel_sdvo_set_avi_infoframe(struct intel_output *output,
 
 	avi_if.checksum = intel_sdvo_calc_hbuf_csum((uint8_t *)&avi_if,
 						    4 + avi_if.len);
-	intel_sdvo_set_hdmi_buf(output, 1, (uint8_t *)&avi_if, 4 + avi_if.len,
-				SDVO_HBUF_TX_VSYNC);
+	return intel_sdvo_set_hdmi_buf(output, 1, (uint8_t *)&avi_if, 4 + avi_if.len,
+				       SDVO_HBUF_TX_VSYNC);
 }
 
-static void intel_sdvo_set_tv_format(struct intel_output *output)
+static bool intel_sdvo_set_tv_format(struct intel_output *output)
 {
 
 	struct intel_sdvo_tv_format format;
 	struct intel_sdvo_priv *sdvo_priv = output->dev_priv;
 	uint32_t format_map, i;
-	uint8_t status;
+	u8 status;
 
 	for (i = 0; i < TV_FORMAT_NUM; i++)
 		if (tv_format_names[i] == sdvo_priv->tv_format_name)
@@ -1065,13 +1067,11 @@ static void intel_sdvo_set_tv_format(struct intel_output *output)
 	memcpy(&format, &format_map, sizeof(format_map) > sizeof(format) ?
 			sizeof(format) : sizeof(format_map));
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_SET_TV_FORMAT, &format_map,
-			     sizeof(format));
-
-	status = intel_sdvo_read_response(output, NULL, 0);
-	if (status != SDVO_CMD_STATUS_SUCCESS)
-		DRM_DEBUG_KMS("%s: Failed to set TV format\n",
-			  SDVO_NAME(sdvo_priv));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_SET_TV_FORMAT,
+				 &format_map, sizeof(format)))
+		status = intel_sdvo_read_response(output, NULL, 0);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
@@ -1081,9 +1081,9 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 	struct intel_output *output = enc_to_intel_output(encoder);
 	struct intel_sdvo_priv *dev_priv = output->dev_priv;
 
-	if (dev_priv->is_tv) {
+	if (dev_priv->is_tv | dev_priv->is_lvds) {
 		struct intel_sdvo_dtd output_dtd;
-		bool success;
+		struct intel_sdvo_dtd input_dtd;
 
 		/* We need to construct preferred input timings based on our
 		 * output timings.  To do that, we have to set the output
@@ -1091,85 +1091,48 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 		 * the sequence to do it. Oh well.
 		 */
 
-
-		/* Set output timings */
-		intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
-		intel_sdvo_set_target_output(output,
-					     dev_priv->controlled_output);
-		intel_sdvo_set_output_timing(output, &output_dtd);
-
-		/* Set the input timing to the screen. Assume always input 0. */
-		intel_sdvo_set_target_input(output, true, false);
-
-
-		success = intel_sdvo_create_preferred_input_timing(output,
-								   mode->clock / 10,
-								   mode->hdisplay,
-								   mode->vdisplay);
-		if (success) {
-			struct intel_sdvo_dtd input_dtd;
-
-			intel_sdvo_get_preferred_input_timing(output,
-							     &input_dtd);
-			intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
-			dev_priv->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);
+		if (dev_priv->is_tv) {
+			intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
 		} else {
-			return false;
+			drm_mode_set_crtcinfo(dev_priv->sdvo_lvds_fixed_mode, 0);
+			intel_sdvo_get_dtd_from_mode(&output_dtd,
+						     dev_priv->sdvo_lvds_fixed_mode);
 		}
-	} else if (dev_priv->is_lvds) {
-		struct intel_sdvo_dtd output_dtd;
-		bool success;
 
-		drm_mode_set_crtcinfo(dev_priv->sdvo_lvds_fixed_mode, 0);
 		/* Set output timings */
-		intel_sdvo_get_dtd_from_mode(&output_dtd,
-				dev_priv->sdvo_lvds_fixed_mode);
+		if (!intel_sdvo_set_target_output(output,
+						  dev_priv->controlled_output))
+			return false;
 
-		intel_sdvo_set_target_output(output,
-					     dev_priv->controlled_output);
-		intel_sdvo_set_output_timing(output, &output_dtd);
+		if (!intel_sdvo_set_output_timing(output, &output_dtd))
+			return false;
 
 		/* Set the input timing to the screen. Assume always input 0. */
 		intel_sdvo_set_target_input(output, true, false);
 
 
-		success = intel_sdvo_create_preferred_input_timing(
-				output,
-				mode->clock / 10,
-				mode->hdisplay,
-				mode->vdisplay);
-
-		if (success) {
-			struct intel_sdvo_dtd input_dtd;
-
-			intel_sdvo_get_preferred_input_timing(output,
-							     &input_dtd);
-			intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
-			dev_priv->sdvo_flags = input_dtd.part2.sdvo_flags;
+		if (!intel_sdvo_create_preferred_input_timing(output,
+							      mode->clock / 10,
+							      mode->hdisplay,
+							      mode->vdisplay))
+			return false;
 
-			drm_mode_set_crtcinfo(adjusted_mode, 0);
+		if (!intel_sdvo_get_preferred_input_timing(output,
+							   &input_dtd))
+			return false;
 
-			mode->clock = adjusted_mode->clock;
+		intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd);
+		dev_priv->sdvo_flags = input_dtd.part2.sdvo_flags;
 
-			adjusted_mode->clock *=
-				intel_sdvo_get_pixel_multiplier(mode);
-		} else {
-			return false;
-		}
+		drm_mode_set_crtcinfo(adjusted_mode, 0);
 
-	} 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);
+		mode->clock = adjusted_mode->clock;
 	}
+
+	/* 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;
 }
 
@@ -1187,6 +1150,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	int sdvo_pixel_multiply;
 	struct intel_sdvo_in_out_map in_out;
 	struct intel_sdvo_dtd input_dtd;
+	u8 rate;
 	u8 status;
 
 	if (!mode)
@@ -1201,12 +1165,16 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	in_out.in0 = sdvo_priv->controlled_output;
 	in_out.in1 = 0;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_SET_IN_OUT_MAP,
-			     &in_out, sizeof(in_out));
-	status = intel_sdvo_read_response(output, NULL, 0);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_SET_IN_OUT_MAP,
+				 &in_out, sizeof(in_out)))
+		status = intel_sdvo_read_response(output, NULL, 0);
+	if (status != SDVO_CMD_STATUS_SUCCESS)
+		return;
 
 	if (sdvo_priv->is_hdmi) {
-		intel_sdvo_set_avi_infoframe(output, mode);
+		if (!intel_sdvo_set_avi_infoframe(output, mode))
+			return;
 		sdvox |= SDVO_AUDIO_ENABLE;
 	}
 
@@ -1223,16 +1191,21 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	 */
 	if (!sdvo_priv->is_tv && !sdvo_priv->is_lvds) {
 		/* Set the output timing to the screen */
-		intel_sdvo_set_target_output(output,
-					     sdvo_priv->controlled_output);
-		intel_sdvo_set_output_timing(output, &input_dtd);
+		if (!intel_sdvo_set_target_output(output,
+					     sdvo_priv->controlled_output))
+			return;
+
+		if (!intel_sdvo_set_output_timing(output, &input_dtd))
+			return;
 	}
 
 	/* Set the input timing to the screen. Assume always input 0. */
 	intel_sdvo_set_target_input(output, true, false);
 
-	if (sdvo_priv->is_tv)
-		intel_sdvo_set_tv_format(output);
+	if (sdvo_priv->is_tv) {
+		if (!intel_sdvo_set_tv_format(output))
+			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
@@ -1249,23 +1222,25 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 		intel_sdvo_set_input_timing(output, &input_dtd);
 	}
 #else
-	intel_sdvo_set_input_timing(output, &input_dtd);
+	if (!intel_sdvo_set_input_timing(output, &input_dtd))
+		return;
 #endif
 
+	rate = SDVO_CLOCK_RATE_MULT_1X;
 	switch (intel_sdvo_get_pixel_multiplier(mode)) {
+	default:
 	case 1:
-		intel_sdvo_set_clock_rate_mult(output,
-					       SDVO_CLOCK_RATE_MULT_1X);
+		rate = SDVO_CLOCK_RATE_MULT_1X;
 		break;
 	case 2:
-		intel_sdvo_set_clock_rate_mult(output,
-					       SDVO_CLOCK_RATE_MULT_2X);
+		rate = SDVO_CLOCK_RATE_MULT_2X;
 		break;
 	case 4:
-		intel_sdvo_set_clock_rate_mult(output,
-					       SDVO_CLOCK_RATE_MULT_4X);
+		rate = SDVO_CLOCK_RATE_MULT_4X;
 		break;
 	}
+	if (!intel_sdvo_set_clock_rate_mult(output, rate))
+		return;
 
 	/* Set the SDVO control regs. */
 	if (IS_I965G(dev)) {
@@ -1476,12 +1451,10 @@ static bool intel_sdvo_get_capabilities(struct intel_output *intel_output, struc
 {
 	u8 status;
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_DEVICE_CAPS, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, caps, sizeof(*caps));
-	if (status != SDVO_CMD_STATUS_SUCCESS)
-		return false;
-
-	return true;
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_DEVICE_CAPS, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, caps, sizeof(*caps));
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB)
@@ -1510,7 +1483,7 @@ struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB)
 	return NULL;
 }
 
-int intel_sdvo_supports_hotplug(struct drm_connector *connector)
+bool intel_sdvo_supports_hotplug(struct drm_connector *connector)
 {
 	u8 response[2];
 	u8 status;
@@ -1518,41 +1491,52 @@ int intel_sdvo_supports_hotplug(struct drm_connector *connector)
 	DRM_DEBUG_KMS("\n");
 
 	if (!connector)
-		return 0;
+		return false;
 
 	intel_output = to_intel_output(connector);
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &response, 2);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &response, 2);
 
-	if (response[0] !=0)
-		return 1;
+	if (status != SDVO_CMD_STATUS_SUCCESS)
+		return false;
 
-	return 0;
+	return response[0] != 0;
 }
 
-void intel_sdvo_set_hotplug(struct drm_connector *connector, int on)
+int intel_sdvo_set_hotplug(struct drm_connector *connector, int on)
 {
 	u8 response[2];
 	u8 status;
 	struct intel_output *intel_output = to_intel_output(connector);
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0);
-	intel_sdvo_read_response(intel_output, &response, 2);
+	if (!intel_sdvo_supports_hotplug (connector))
+		return -ENODEV;
 
 	if (on) {
-		intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL, 0);
-		status = intel_sdvo_read_response(intel_output, &response, 2);
-
-		intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response, 2);
+		status = SDVO_CMD_STATUS_READ_FAILED;
+		if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL, 0))
+			status = intel_sdvo_read_response(intel_output, &response, 2);
+		if (status != SDVO_CMD_STATUS_SUCCESS)
+			return -EIO;
+
+		if (!intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response, 2))
+			return -EIO;
 	} else {
 		response[0] = 0;
 		response[1] = 0;
-		intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response, 2);
+		if (!intel_sdvo_write_cmd(intel_output, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response, 2))
+			return -EIO;
 	}
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0);
-	intel_sdvo_read_response(intel_output, &response, 2);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0))
+		status = intel_sdvo_read_response(intel_output, &response, 2);
+	if (status != SDVO_CMD_STATUS_SUCCESS)
+		return -EIO;
+
+	return 0;
 }
 
 static bool
@@ -1592,22 +1576,20 @@ static struct drm_connector *
 intel_find_analog_connector(struct drm_device *dev)
 {
 	struct drm_connector *connector;
-	struct intel_output *intel_output;
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		intel_output = to_intel_output(connector);
-		if (intel_output->type == INTEL_OUTPUT_ANALOG)
+		if (to_intel_output(connector)->type == INTEL_OUTPUT_ANALOG)
 			return connector;
 	}
 	return NULL;
 }
 
-static int
+static bool
 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;
 
@@ -1691,12 +1673,15 @@ static enum drm_connector_status intel_sdvo_detect(struct drm_connector *connect
 	struct intel_output *intel_output = to_intel_output(connector);
 	struct intel_sdvo_priv *sdvo_priv = intel_output->dev_priv;
 
-	intel_sdvo_write_cmd(intel_output,
-			     SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0);
+	if (!intel_sdvo_write_cmd(intel_output,
+				  SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0))
+		return connector_status_unknown;
+
 	if (sdvo_priv->is_tv) {
 		/* add 30ms delay when the output type is SDVO-TV */
 		mdelay(30);
 	}
+
 	status = intel_sdvo_read_response(intel_output, &response, 2);
 
 	DRM_DEBUG_KMS("SDVO response %d %d\n", response & 0xff, response >> 8);
@@ -1820,7 +1805,7 @@ 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;
+	u8 status;
 
 
 	/* Read the list of supported input resolutions for the selected TV
@@ -1836,11 +1821,13 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
 	       sizeof(format_map) ? sizeof(format_map) :
 	       sizeof(struct intel_sdvo_sdtv_resolution_request));
 
-	intel_sdvo_set_target_output(output, sdvo_priv->controlled_output);
+	if (!intel_sdvo_set_target_output(output, sdvo_priv->controlled_output))
+		return;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
-			     &tv_res, sizeof(tv_res));
-	status = intel_sdvo_read_response(output, &reply, 3);
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
+				 &tv_res, sizeof(tv_res)))
+		status = intel_sdvo_read_response(output, &reply, 3);
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		return;
 
@@ -1906,9 +1893,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
@@ -2094,9 +2079,10 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			sdvo_priv->cur_brightness = temp_value;
 		}
 		if (cmd) {
-			intel_sdvo_write_cmd(intel_output, cmd, &temp_value, 2);
-			status = intel_sdvo_read_response(intel_output,
-								NULL, 0);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output, cmd, &temp_value, 2))
+				status = intel_sdvo_read_response(intel_output,
+								  NULL, 0);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO command \n");
 				return -EINVAL;
@@ -2104,9 +2090,11 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			changed = true;
 		}
 	}
-	if (changed && crtc)
-		drm_crtc_helper_set_mode(crtc, &crtc->mode, crtc->x,
-				crtc->y, crtc->fb);
+	if (changed && crtc) {
+		if (!drm_crtc_helper_set_mode(crtc, &crtc->mode, crtc->x,
+					      crtc->y, crtc->fb))
+			return -EINVAL;
+	}
 out:
 	return ret;
 }
@@ -2193,15 +2181,15 @@ static bool
 intel_sdvo_get_digital_encoding_mode(struct intel_output *output)
 {
 	struct intel_sdvo_priv *sdvo_priv = output->dev_priv;
-	uint8_t status;
-
-	intel_sdvo_set_target_output(output, sdvo_priv->controlled_output);
+	u8 status;
 
-	intel_sdvo_write_cmd(output, SDVO_CMD_GET_ENCODE, NULL, 0);
-	status = intel_sdvo_read_response(output, &sdvo_priv->is_hdmi, 1);
-	if (status != SDVO_CMD_STATUS_SUCCESS)
+	if (!intel_sdvo_set_target_output(output, sdvo_priv->controlled_output))
 		return false;
-	return true;
+
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(output, SDVO_CMD_GET_ENCODE, NULL, 0))
+		status = intel_sdvo_read_response(output, &sdvo_priv->is_hdmi, 1);
+	return status == SDVO_CMD_STATUS_SUCCESS;
 }
 
 static struct intel_output *
@@ -2239,7 +2227,9 @@ static int intel_sdvo_master_xfer(struct i2c_adapter *i2c_adap,
 	sdvo_priv = intel_output->dev_priv;
 	algo = intel_output->i2c_bus->algo;
 
-	intel_sdvo_set_control_bus_switch(intel_output, sdvo_priv->ddc_bus);
+	if (!intel_sdvo_set_control_bus_switch(intel_output, sdvo_priv->ddc_bus))
+		return -EIO;
+
 	return algo->master_xfer(i2c_adap, msgs, num);
 }
 
@@ -2335,9 +2325,13 @@ intel_sdvo_output_setup(struct intel_output *intel_output, uint16_t flags)
 		    intel_sdvo_get_digital_encoding_mode(intel_output) &&
 		    sdvo_priv->is_hdmi) {
 			/* enable hdmi encoding mode if supported */
-			intel_sdvo_set_encode(intel_output, SDVO_ENCODE_HDMI);
-			intel_sdvo_set_colorimetry(intel_output,
-						   SDVO_COLORIMETRY_RGB256);
+			if (!intel_sdvo_set_encode(intel_output, SDVO_ENCODE_HDMI))
+				return false;
+
+			if (!intel_sdvo_set_colorimetry(intel_output,
+							SDVO_COLORIMETRY_RGB256))
+				return false;
+
 			connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
 			intel_output->clone_mask =
 					(1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
@@ -2411,99 +2405,108 @@ intel_sdvo_output_setup(struct intel_output *intel_output, uint16_t flags)
 
 }
 
-static void intel_sdvo_tv_create_property(struct drm_connector *connector)
+static bool intel_sdvo_tv_create_property(struct drm_connector *connector)
 {
-      struct intel_output *intel_output = to_intel_output(connector);
+	struct intel_output *intel_output = to_intel_output(connector);
 	struct intel_sdvo_priv *sdvo_priv = intel_output->dev_priv;
 	struct intel_sdvo_tv_format format;
 	uint32_t format_map, i;
-	uint8_t status;
+	u8 status;
 
-	intel_sdvo_set_target_output(intel_output,
-				     sdvo_priv->controlled_output);
+	if (!intel_sdvo_set_target_output(intel_output,
+				     sdvo_priv->controlled_output))
+		return false;
 
-	intel_sdvo_write_cmd(intel_output,
-			     SDVO_CMD_GET_SUPPORTED_TV_FORMATS, NULL, 0);
-	status = intel_sdvo_read_response(intel_output,
-					  &format, sizeof(format));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output,
+				 SDVO_CMD_GET_SUPPORTED_TV_FORMATS, NULL, 0))
+		status = intel_sdvo_read_response(intel_output,
+						  &format, sizeof(format));
 	if (status != SDVO_CMD_STATUS_SUCCESS)
-		return;
+		return false;
 
-	memcpy(&format_map, &format, sizeof(format) > sizeof(format_map) ?
-	       sizeof(format_map) : sizeof(format));
+	memcpy(&format_map, &format,
+	       sizeof(format) < sizeof(format_map) ? sizeof(format) : sizeof(format_map));
 
 	if (format_map == 0)
-		return;
+		return false;
 
 	sdvo_priv->format_supported_num = 0;
 	for (i = 0 ; i < TV_FORMAT_NUM; i++)
 		if (format_map & (1 << i)) {
 			sdvo_priv->tv_format_supported
-			[sdvo_priv->format_supported_num++] =
-			tv_format_names[i];
+				[sdvo_priv->format_supported_num++] =
+				tv_format_names[i];
 		}
 
 
 	sdvo_priv->tv_format_property =
-			drm_property_create(
-				connector->dev, DRM_MODE_PROP_ENUM,
-				"mode", sdvo_priv->format_supported_num);
+		drm_property_create(
+				    connector->dev, DRM_MODE_PROP_ENUM,
+				    "mode", sdvo_priv->format_supported_num);
 
 	for (i = 0; i < sdvo_priv->format_supported_num; i++)
 		drm_property_add_enum(
-				sdvo_priv->tv_format_property, i,
-				i, sdvo_priv->tv_format_supported[i]);
+				      sdvo_priv->tv_format_property, i,
+				      i, sdvo_priv->tv_format_supported[i]);
 
 	sdvo_priv->tv_format_name = sdvo_priv->tv_format_supported[0];
 	drm_connector_attach_property(
-			connector, sdvo_priv->tv_format_property, 0);
+				      connector, sdvo_priv->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 drm_connector *connector)
 {
 	struct intel_output *intel_output = to_intel_output(connector);
 	struct intel_sdvo_priv *sdvo_priv = intel_output->dev_priv;
 	struct intel_sdvo_enhancements_reply sdvo_data;
 	struct drm_device *dev = connector->dev;
-	uint8_t status;
+	u8 status;
 	uint16_t response, data_value[2];
 
-	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_SUPPORTED_ENHANCEMENTS,
-						NULL, 0);
-	status = intel_sdvo_read_response(intel_output, &sdvo_data,
-					sizeof(sdvo_data));
+	status = SDVO_CMD_STATUS_READ_FAILED;
+	if (intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_SUPPORTED_ENHANCEMENTS,
+				 NULL, 0))
+		status = intel_sdvo_read_response(intel_output,
+						  &sdvo_data, sizeof(sdvo_data));
 	if (status != SDVO_CMD_STATUS_SUCCESS) {
 		DRM_DEBUG_KMS(" incorrect response is returned\n");
-		return;
+		return false;
 	}
 	response = *((uint16_t *)&sdvo_data);
 	if (!response) {
 		DRM_DEBUG_KMS("No enhancement is supported\n");
-		return;
+		return false;
 	}
+
 	if (sdvo_priv->is_tv) {
 		/* when horizontal overscan is supported, Add the left/right
 		 * property
 		 */
 		if (sdvo_data.overscan_h) {
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_MAX_OVERSCAN_H, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_OVERSCAN_H, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO max "
 						"h_overscan\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_OVERSCAN_H, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_OVERSCAN_H, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO h_overscan\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_hscan = data_value[0];
 			sdvo_priv->left_margin = data_value[0] - response;
 			sdvo_priv->right_margin = sdvo_priv->left_margin;
@@ -2528,23 +2531,27 @@ 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_output,
-				SDVO_CMD_GET_MAX_OVERSCAN_V, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_OVERSCAN_V, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO max "
 						"v_overscan\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_OVERSCAN_V, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_OVERSCAN_V, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO v_overscan\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_vscan = data_value[0];
 			sdvo_priv->top_margin = data_value[0] - response;
 			sdvo_priv->bottom_margin = sdvo_priv->top_margin;
@@ -2569,22 +2576,26 @@ 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_output,
-				SDVO_CMD_GET_MAX_POSITION_H, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_POSITION_H, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max h_pos\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_POSITION_H, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_POSITION_H, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get h_postion\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_hpos = data_value[0];
 			sdvo_priv->cur_hpos = response;
 			sdvo_priv->hpos_property =
@@ -2600,22 +2611,26 @@ 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_output,
-				SDVO_CMD_GET_MAX_POSITION_V, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_POSITION_V, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max v_pos\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_POSITION_V, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_POSITION_V, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get v_postion\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_vpos = data_value[0];
 			sdvo_priv->cur_vpos = response;
 			sdvo_priv->vpos_property =
@@ -2633,22 +2648,26 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 	}
 	if (sdvo_priv->is_tv) {
 		if (sdvo_data.saturation) {
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_MAX_SATURATION, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_SATURATION, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max sat\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_SATURATION, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_SATURATION, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get sat\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_saturation = data_value[0];
 			sdvo_priv->cur_saturation = response;
 			sdvo_priv->saturation_property =
@@ -2665,22 +2684,26 @@ 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_output,
-				SDVO_CMD_GET_MAX_CONTRAST, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_CONTRAST, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max contrast\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_CONTRAST, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_CONTRAST, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get contrast\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_contrast = data_value[0];
 			sdvo_priv->cur_contrast = response;
 			sdvo_priv->contrast_property =
@@ -2696,55 +2719,63 @@ 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_output,
-				SDVO_CMD_GET_MAX_HUE, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_HUE, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max hue\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_HUE, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_HUE, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get hue\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_hue = data_value[0];
 			sdvo_priv->cur_hue = response;
 			sdvo_priv->hue_property =
 				drm_property_create(dev, DRM_MODE_PROP_RANGE,
-						"hue", 2);
+						    "hue", 2);
 			sdvo_priv->hue_property->values[0] = 0;
 			sdvo_priv->hue_property->values[1] =
-							data_value[0];
+				data_value[0];
 			drm_connector_attach_property(connector,
-						sdvo_priv->hue_property,
-						sdvo_priv->cur_hue);
+						      sdvo_priv->hue_property,
+						      sdvo_priv->cur_hue);
 			DRM_DEBUG_KMS("hue: max %d, default %d, current %d\n",
-					data_value[0], data_value[1], response);
+				      data_value[0], data_value[1], response);
 		}
 	}
 	if (sdvo_priv->is_tv || sdvo_priv->is_lvds) {
 		if (sdvo_data.brightness) {
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_MAX_BRIGHTNESS, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&data_value, 4);
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_MAX_BRIGHTNESS, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &data_value, 4);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO Max bright\n");
-				return;
+				return false;
 			}
-			intel_sdvo_write_cmd(intel_output,
-				SDVO_CMD_GET_BRIGHTNESS, NULL, 0);
-			status = intel_sdvo_read_response(intel_output,
-				&response, 2);
+
+			status = SDVO_CMD_STATUS_READ_FAILED;
+			if (intel_sdvo_write_cmd(intel_output,
+						 SDVO_CMD_GET_BRIGHTNESS, NULL, 0))
+				status = intel_sdvo_read_response(intel_output,
+								  &response, 2);
 			if (status != SDVO_CMD_STATUS_SUCCESS) {
 				DRM_DEBUG_KMS("Incorrect SDVO get brigh\n");
-				return;
+				return false;
 			}
+
 			sdvo_priv->max_brightness = data_value[0];
 			sdvo_priv->cur_brightness = response;
 			sdvo_priv->brightness_property =
@@ -2761,7 +2792,8 @@ 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 output_device)
@@ -2828,7 +2860,8 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
 	intel_output->ddc_bus->algo = &intel_sdvo_i2c_bit_algo;
 
 	/* In default case sdvo lvds is false */
-	intel_sdvo_get_capabilities(intel_output, &sdvo_priv->caps);
+	if (!intel_sdvo_get_capabilities(intel_output, &sdvo_priv->caps))
+		goto err_i2c;
 
 	if (intel_sdvo_output_setup(intel_output,
 				    sdvo_priv->caps.output_flags) != true) {
@@ -2853,23 +2886,28 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
 	drm_encoder_helper_add(&intel_output->enc, &intel_sdvo_helper_funcs);
 
 	drm_mode_connector_attach_encoder(&intel_output->base, &intel_output->enc);
-	if (sdvo_priv->is_tv)
-		intel_sdvo_tv_create_property(connector);
-
-	if (sdvo_priv->is_tv || sdvo_priv->is_lvds)
-		intel_sdvo_create_enhance_property(connector);
+	if (sdvo_priv->is_tv) {
+		if (!intel_sdvo_tv_create_property(connector))
+			goto err_i2c;
+	}
 
-	drm_sysfs_connector_add(connector);
+	if (sdvo_priv->is_tv || sdvo_priv->is_lvds) {
+		if (!intel_sdvo_create_enhance_property(connector))
+			goto err_i2c;
+	}
 
 	intel_sdvo_select_ddc_bus(sdvo_priv);
 
 	/* Set the input timing to the screen. Assume always input 0. */
-	intel_sdvo_set_target_input(intel_output, true, false);
+	if (!intel_sdvo_set_target_input(intel_output, true, false))
+		goto err_i2c;
 
-	intel_sdvo_get_input_pixel_clock_range(intel_output,
-					       &sdvo_priv->pixel_clock_min,
-					       &sdvo_priv->pixel_clock_max);
+	if (!intel_sdvo_get_input_pixel_clock_range(intel_output,
+						    &sdvo_priv->pixel_clock_min,
+						    &sdvo_priv->pixel_clock_max))
+		goto err_i2c;
 
+	drm_sysfs_connector_add(connector);
 
 	DRM_DEBUG_KMS("%s device VID/DID: %02X:%02X.%02X, "
 			"clock range %dMHz - %dMHz, "
diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
index ba5cdf8..da6d102 100644
--- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
+++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
@@ -134,6 +134,7 @@ struct intel_sdvo_preferred_input_timing_args {
 #define SDVO_CMD_STATUS_PENDING			0x4
 #define SDVO_CMD_STATUS_TARGET_NOT_SPECIFIED	0x5
 #define SDVO_CMD_STATUS_SCALING_NOT_SUPP	0x6
+#define SDVO_CMD_STATUS_READ_FAILED		0xff
 
 /* SDVO commands, argument/result registers */
 
-- 
1.7.0




More information about the Intel-gfx mailing list