From patchwork Mon Jul 19 13:25:44 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 112669 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.4/8.14.3) with ESMTP id o6JDS5Hc009863 for ; Mon, 19 Jul 2010 13:28:40 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4CEEE9EEE1 for ; Mon, 19 Jul 2010 06:28:05 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id CCB079EDA2 for ; Mon, 19 Jul 2010 06:26:02 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 19 Jul 2010 06:23:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.55,226,1278313200"; d="scan'208";a="587576954" Received: from unknown (HELO broadwater.alporthouse.com) ([10.255.17.84]) by fmsmga002.fm.intel.com with ESMTP; 19 Jul 2010 06:25:22 -0700 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 19 Jul 2010 14:25:44 +0100 Message-Id: <1279545948-24378-4-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1279545948-24378-1-git-send-email-chris@chris-wilson.co.uk> References: <1279545948-24378-1-git-send-email-chris@chris-wilson.co.uk> Subject: [Intel-gfx] [PATCH 3/7] drm/i915/sdvo: Propagate errors from reading/writing control bus. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter.kernel.org [140.211.167.41]); Mon, 19 Jul 2010 13:28:40 +0000 (UTC) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 4c0b80e..ba50755 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -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)) { @@ -1257,7 +1172,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)) { @@ -1300,10 +1214,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. @@ -1351,14 +1262,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! */ @@ -1401,13 +1305,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) @@ -1490,8 +1389,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; @@ -1567,25 +1466,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; @@ -1711,8 +1608,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. @@ -1723,23 +1618,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); } @@ -1796,9 +1691,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 @@ -1829,7 +1722,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); @@ -1860,36 +1753,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; @@ -1900,7 +1790,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; @@ -1911,7 +1801,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; @@ -1922,7 +1812,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 - @@ -1930,57 +1821,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 = { @@ -2048,18 +1934,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 * @@ -2074,7 +1952,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, @@ -2139,8 +2017,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); @@ -2193,7 +2071,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; } @@ -2222,13 +2100,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 @@ -2260,7 +2144,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; } @@ -2294,9 +2178,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 @@ -2356,29 +2246,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++) @@ -2390,9 +2277,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( @@ -2400,56 +2286,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; @@ -2474,23 +2349,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; @@ -2515,22 +2383,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 = @@ -2546,22 +2408,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 = @@ -2577,22 +2433,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 = @@ -2609,22 +2459,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 = @@ -2640,22 +2482,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 = @@ -2671,24 +2505,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 = @@ -2705,7 +2531,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) @@ -2771,8 +2597,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 */ @@ -2783,24 +2608,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, " @@ -2818,9 +2645,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); diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h index ba5cdf8..4988660 100644 --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h @@ -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;