drm/i915: implement multifunction SDVO device support

With new intel_encoder/intel_connector structure change, each supported
connector type on SDVO device will be created as a new 'intel_connector',
and all attached to one 'intel_encoder' for its SDVO port.

The SDVO encoder will handle SDVO protocol stuff, and each connector does
its own part of work now, like detection is only to check if current active
output is itself, etc.

Update since last submit:
- Fixed SDVO TV property creation failure by incorrect set target output call

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

drivers/gpu/drm/i915/intel_sdvo.c

@@ -37,6 +37,18 @@
 #include "intel_sdvo_regs.h"
 #include <linux/dmi.h>
 
+#define SDVO_TMDS_MASK (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)
+#define SDVO_RGB_MASK  (SDVO_OUTPUT_RGB0 | SDVO_OUTPUT_RGB1)
+#define SDVO_LVDS_MASK (SDVO_OUTPUT_LVDS0 | SDVO_OUTPUT_LVDS1)
+#define SDVO_TV_MASK   (SDVO_OUTPUT_CVBS0 | SDVO_OUTPUT_SVID0)
+
+#define SDVO_OUTPUT_MASK (SDVO_TMDS_MASK | SDVO_RGB_MASK | SDVO_LVDS_MASK |\
+                         SDVO_TV_MASK)
+
+#define IS_TV(c)	(c->output_flag & SDVO_TV_MASK)
+#define IS_LVDS(c)	(c->output_flag & SDVO_LVDS_MASK)
+
+
 static char *tv_format_names[] = {
 	"NTSC_M"   , "NTSC_J"  , "NTSC_443",
 	"PAL_B"    , "PAL_D"   , "PAL_G"   ,
@@ -85,12 +97,6 @@ struct intel_sdvo_priv {
 	/* This is for current tv format name */
 	char *tv_format_name;
 
-	/* This contains all current supported TV format */
-	char *tv_format_supported[TV_FORMAT_NUM];
-	int   format_supported_num;
-	struct drm_property *tv_format_property;
-	struct drm_property *tv_format_name_property[TV_FORMAT_NUM];
-
 	/**
 	 * This is set if we treat the device as HDMI, instead of DVI.
 	 */
@@ -111,12 +117,6 @@ struct intel_sdvo_priv {
 	 */
 	struct drm_display_mode *sdvo_lvds_fixed_mode;
 
-	/**
-	 * Returned SDTV resolutions allowed for the current format, if the
-	 * device reported it.
-	 */
-	struct intel_sdvo_sdtv_resolution_reply sdtv_resolutions;
-
 	/*
 	 * supported encoding mode, used to determine whether HDMI is
 	 * supported
@@ -129,6 +129,24 @@ struct intel_sdvo_priv {
 	/* Mac mini hack -- use the same DDC as the analog connector */
 	struct i2c_adapter *analog_ddc_bus;
 
+};
+
+struct intel_sdvo_connector {
+	/* Mark the type of connector */
+	uint16_t output_flag;
+
+	/* This contains all current supported TV format */
+	char *tv_format_supported[TV_FORMAT_NUM];
+	int   format_supported_num;
+	struct drm_property *tv_format_property;
+	struct drm_property *tv_format_name_property[TV_FORMAT_NUM];
+
+	/**
+	 * Returned SDTV resolutions allowed for the current format, if the
+	 * device reported it.
+	 */
+	struct intel_sdvo_sdtv_resolution_reply sdtv_resolutions;
+
 	/* add the property for the SDVO-TV */
 	struct drm_property *left_property;
 	struct drm_property *right_property;
@@ -157,8 +175,11 @@ struct intel_sdvo_priv {
 
 static bool
 intel_sdvo_output_setup(struct intel_encoder *intel_encoder,
-			struct intel_connector *intel_connector,
 			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);
 
 /**
  * Writes the SDVOB or SDVOC with the given value, but always writes both
@@ -1035,7 +1056,7 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 		/* Set output timings */
 		intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
 		intel_sdvo_set_target_output(intel_encoder,
-					     dev_priv->controlled_output);
+					     dev_priv->attached_output);
 		intel_sdvo_set_output_timing(intel_encoder, &output_dtd);
 
 		/* Set the input timing to the screen. Assume always input 0. */
@@ -1073,7 +1094,7 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 				dev_priv->sdvo_lvds_fixed_mode);
 
 		intel_sdvo_set_target_output(intel_encoder,
-					     dev_priv->controlled_output);
+					     dev_priv->attached_output);
 		intel_sdvo_set_output_timing(intel_encoder, &output_dtd);
 
 		/* Set the input timing to the screen. Assume always input 0. */
@@ -1138,7 +1159,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	 * channel on the motherboard.  In a two-input device, the first input
 	 * will be SDVOB and the second SDVOC.
 	 */
-	in_out.in0 = sdvo_priv->controlled_output;
+	in_out.in0 = sdvo_priv->attached_output;
 	in_out.in1 = 0;
 
 	intel_sdvo_write_cmd(intel_encoder, SDVO_CMD_SET_IN_OUT_MAP,
@@ -1164,7 +1185,7 @@ 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(intel_encoder,
-					     sdvo_priv->controlled_output);
+					     sdvo_priv->attached_output);
 		intel_sdvo_set_output_timing(intel_encoder, &input_dtd);
 	}
 
@@ -1286,7 +1307,7 @@ static void intel_sdvo_dpms(struct drm_encoder *encoder, int mode)
 
 		if (0)
 			intel_sdvo_set_encoder_power_state(intel_encoder, mode);
-		intel_sdvo_set_active_outputs(intel_encoder, sdvo_priv->controlled_output);
+		intel_sdvo_set_active_outputs(intel_encoder, sdvo_priv->attached_output);
 	}
 	return;
 }
@@ -1550,6 +1571,8 @@ static enum drm_connector_status intel_sdvo_detect(struct drm_connector *connect
 	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+	struct intel_sdvo_connector *sdvo_connector = intel_connector->dev_priv;
+	enum drm_connector_status ret;
 
 	intel_sdvo_write_cmd(intel_encoder,
 			     SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0);
@@ -1567,15 +1590,30 @@ static enum drm_connector_status intel_sdvo_detect(struct drm_connector *connect
 	if (response == 0)
 		return connector_status_disconnected;
 
-	if (intel_sdvo_multifunc_encoder(intel_encoder) &&
-		sdvo_priv->attached_output != response) {
-		if (sdvo_priv->controlled_output != response &&
-			intel_sdvo_output_setup(intel_encoder, intel_connector,
-						response) != true)
-			return connector_status_unknown;
-		sdvo_priv->attached_output = response;
+	sdvo_priv->attached_output = response;
+
+	if ((sdvo_connector->output_flag & response) == 0)
+		ret = connector_status_disconnected;
+	else if (response & (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1))
+		ret = intel_sdvo_hdmi_sink_detect(connector, response);
+	else
+		ret = connector_status_connected;
+
+	/* May update encoder flag for like clock for SDVO TV, etc.*/
+	if (ret == connector_status_connected) {
+		sdvo_priv->is_tv = false;
+		sdvo_priv->is_lvds = false;
+		intel_encoder->needs_tv_clock = false;
+
+		if (response & SDVO_TV_MASK) {
+			sdvo_priv->is_tv = true;
+			intel_encoder->needs_tv_clock = true;
+		}
+		if (response & SDVO_LVDS_MASK)
+			sdvo_priv->is_lvds = true;
 	}
-	return intel_sdvo_hdmi_sink_detect(connector, response);
+
+	return ret;
 }
 
 static void intel_sdvo_get_ddc_modes(struct drm_connector *connector)
@@ -1692,7 +1730,7 @@ 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(intel_encoder, sdvo_priv->controlled_output);
+	intel_sdvo_set_target_output(intel_encoder, sdvo_priv->attached_output);
 
 	intel_sdvo_write_cmd(intel_encoder, SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
 			     &tv_res, sizeof(tv_res));
@@ -1753,13 +1791,12 @@ end:
 
 static int intel_sdvo_get_modes(struct drm_connector *connector)
 {
-	struct drm_encoder *encoder = intel_attached_encoder(connector);
-	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
-	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_connector = intel_connector->dev_priv;
 
-	if (sdvo_priv->is_tv)
+	if (IS_TV(sdvo_connector))
 		intel_sdvo_get_tv_modes(connector);
-	else if (sdvo_priv->is_lvds == true)
+	else if (IS_LVDS(sdvo_connector))
 		intel_sdvo_get_lvds_modes(connector);
 	else
 		intel_sdvo_get_ddc_modes(connector);
@@ -1772,12 +1809,11 @@ static int intel_sdvo_get_modes(struct drm_connector *connector)
 static
 void intel_sdvo_destroy_enhance_property(struct drm_connector *connector)
 {
-	struct drm_encoder *encoder = intel_attached_encoder(connector);
-	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
-	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
-	struct drm_device *dev = encoder->dev;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_priv = intel_connector->dev_priv;
+	struct drm_device *dev = connector->dev;
 
-	if (sdvo_priv->is_tv) {
+	if (IS_TV(sdvo_priv)) {
 		if (sdvo_priv->left_property)
 			drm_property_destroy(dev, sdvo_priv->left_property);
 		if (sdvo_priv->right_property)
@@ -1790,8 +1826,6 @@ void intel_sdvo_destroy_enhance_property(struct drm_connector *connector)
 			drm_property_destroy(dev, sdvo_priv->hpos_property);
 		if (sdvo_priv->vpos_property)
 			drm_property_destroy(dev, sdvo_priv->vpos_property);
-	}
-	if (sdvo_priv->is_tv) {
 		if (sdvo_priv->saturation_property)
 			drm_property_destroy(dev,
 					sdvo_priv->saturation_property);
@@ -1801,7 +1835,7 @@ void intel_sdvo_destroy_enhance_property(struct drm_connector *connector)
 		if (sdvo_priv->hue_property)
 			drm_property_destroy(dev, sdvo_priv->hue_property);
 	}
-	if (sdvo_priv->is_tv || sdvo_priv->is_lvds) {
+	if (IS_TV(sdvo_priv) || IS_LVDS(sdvo_priv)) {
 		if (sdvo_priv->brightness_property)
 			drm_property_destroy(dev,
 					sdvo_priv->brightness_property);
@@ -1811,6 +1845,13 @@ void intel_sdvo_destroy_enhance_property(struct drm_connector *connector)
 
 static void intel_sdvo_destroy(struct drm_connector *connector)
 {
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_connector = intel_connector->dev_priv;
+
+	if (sdvo_connector->tv_format_property)
+		drm_property_destroy(connector->dev,
+				     sdvo_connector->tv_format_property);
+
 	intel_sdvo_destroy_enhance_property(connector);
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
@@ -1825,6 +1866,8 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	struct drm_encoder *encoder = intel_attached_encoder(connector);
 	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
 	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_connector = intel_connector->dev_priv;
 	struct drm_crtc *crtc = encoder->crtc;
 	int ret = 0;
 	bool changed = false;
@@ -1835,101 +1878,101 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	if (ret < 0)
 		goto out;
 
-	if (property == sdvo_priv->tv_format_property) {
+	if (property == sdvo_connector->tv_format_property) {
 		if (val >= TV_FORMAT_NUM) {
 			ret = -EINVAL;
 			goto out;
 		}
 		if (sdvo_priv->tv_format_name ==
-		    sdvo_priv->tv_format_supported[val])
+		    sdvo_connector->tv_format_supported[val])
 			goto out;
 
-		sdvo_priv->tv_format_name = sdvo_priv->tv_format_supported[val];
+		sdvo_priv->tv_format_name = sdvo_connector->tv_format_supported[val];
 		changed = true;
 	}
 
-	if (sdvo_priv->is_tv || sdvo_priv->is_lvds) {
+	if (IS_TV(sdvo_connector) || IS_LVDS(sdvo_connector)) {
 		cmd = 0;
 		temp_value = val;
-		if (sdvo_priv->left_property == property) {
+		if (sdvo_connector->left_property == property) {
 			drm_connector_property_set_value(connector,
-				sdvo_priv->right_property, val);
-			if (sdvo_priv->left_margin == temp_value)
+				sdvo_connector->right_property, val);
+			if (sdvo_connector->left_margin == temp_value)
 				goto out;
 
-			sdvo_priv->left_margin = temp_value;
-			sdvo_priv->right_margin = temp_value;
-			temp_value = sdvo_priv->max_hscan -
-					sdvo_priv->left_margin;
+			sdvo_connector->left_margin = temp_value;
+			sdvo_connector->right_margin = temp_value;
+			temp_value = sdvo_connector->max_hscan -
+					sdvo_connector->left_margin;
 			cmd = SDVO_CMD_SET_OVERSCAN_H;
-		} else if (sdvo_priv->right_property == property) {
+		} else if (sdvo_connector->right_property == property) {
 			drm_connector_property_set_value(connector,
-				sdvo_priv->left_property, val);
-			if (sdvo_priv->right_margin == temp_value)
+				sdvo_connector->left_property, val);
+			if (sdvo_connector->right_margin == temp_value)
 				goto out;
 
-			sdvo_priv->left_margin = temp_value;
-			sdvo_priv->right_margin = temp_value;
-			temp_value = sdvo_priv->max_hscan -
-				sdvo_priv->left_margin;
+			sdvo_connector->left_margin = temp_value;
+			sdvo_connector->right_margin = temp_value;
+			temp_value = sdvo_connector->max_hscan -
+				sdvo_connector->left_margin;
 			cmd = SDVO_CMD_SET_OVERSCAN_H;
-		} else if (sdvo_priv->top_property == property) {
+		} else if (sdvo_connector->top_property == property) {
 			drm_connector_property_set_value(connector,
-				sdvo_priv->bottom_property, val);
-			if (sdvo_priv->top_margin == temp_value)
+				sdvo_connector->bottom_property, val);
+			if (sdvo_connector->top_margin == temp_value)
 				goto out;
 
-			sdvo_priv->top_margin = temp_value;
-			sdvo_priv->bottom_margin = temp_value;
-			temp_value = sdvo_priv->max_vscan -
-					sdvo_priv->top_margin;
+			sdvo_connector->top_margin = temp_value;
+			sdvo_connector->bottom_margin = temp_value;
+			temp_value = sdvo_connector->max_vscan -
+					sdvo_connector->top_margin;
 			cmd = SDVO_CMD_SET_OVERSCAN_V;
-		} else if (sdvo_priv->bottom_property == property) {
+		} else if (sdvo_connector->bottom_property == property) {
 			drm_connector_property_set_value(connector,
-				sdvo_priv->top_property, val);
-			if (sdvo_priv->bottom_margin == temp_value)
+				sdvo_connector->top_property, val);
+			if (sdvo_connector->bottom_margin == temp_value)
 				goto out;
-			sdvo_priv->top_margin = temp_value;
-			sdvo_priv->bottom_margin = temp_value;
-			temp_value = sdvo_priv->max_vscan -
-					sdvo_priv->top_margin;
+			sdvo_connector->top_margin = temp_value;
+			sdvo_connector->bottom_margin = temp_value;
+			temp_value = sdvo_connector->max_vscan -
+					sdvo_connector->top_margin;
 			cmd = SDVO_CMD_SET_OVERSCAN_V;
-		} else if (sdvo_priv->hpos_property == property) {
-			if (sdvo_priv->cur_hpos == temp_value)
+		} else if (sdvo_connector->hpos_property == property) {
+			if (sdvo_connector->cur_hpos == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_POSITION_H;
-			sdvo_priv->cur_hpos = temp_value;
-		} else if (sdvo_priv->vpos_property == property) {
-			if (sdvo_priv->cur_vpos == temp_value)
+			sdvo_connector->cur_hpos = temp_value;
+		} else if (sdvo_connector->vpos_property == property) {
+			if (sdvo_connector->cur_vpos == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_POSITION_V;
-			sdvo_priv->cur_vpos = temp_value;
-		} else if (sdvo_priv->saturation_property == property) {
-			if (sdvo_priv->cur_saturation == temp_value)
+			sdvo_connector->cur_vpos = temp_value;
+		} else if (sdvo_connector->saturation_property == property) {
+			if (sdvo_connector->cur_saturation == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_SATURATION;
-			sdvo_priv->cur_saturation = temp_value;
-		} else if (sdvo_priv->contrast_property == property) {
-			if (sdvo_priv->cur_contrast == temp_value)
+			sdvo_connector->cur_saturation = temp_value;
+		} else if (sdvo_connector->contrast_property == property) {
+			if (sdvo_connector->cur_contrast == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_CONTRAST;
-			sdvo_priv->cur_contrast = temp_value;
-		} else if (sdvo_priv->hue_property == property) {
-			if (sdvo_priv->cur_hue == temp_value)
+			sdvo_connector->cur_contrast = temp_value;
+		} else if (sdvo_connector->hue_property == property) {
+			if (sdvo_connector->cur_hue == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_HUE;
-			sdvo_priv->cur_hue = temp_value;
-		} else if (sdvo_priv->brightness_property == property) {
-			if (sdvo_priv->cur_brightness == temp_value)
+			sdvo_connector->cur_hue = temp_value;
+		} else if (sdvo_connector->brightness_property == property) {
+			if (sdvo_connector->cur_brightness == temp_value)
 				goto out;
 
 			cmd = SDVO_CMD_SET_BRIGHTNESS;
-			sdvo_priv->cur_brightness = temp_value;
+			sdvo_connector->cur_brightness = temp_value;
 		}
 		if (cmd) {
 			intel_sdvo_write_cmd(intel_encoder, cmd, &temp_value, 2);
@@ -1987,10 +2030,6 @@ static void intel_sdvo_enc_destroy(struct drm_encoder *encoder)
 		drm_mode_destroy(encoder->dev,
 				 sdvo_priv->sdvo_lvds_fixed_mode);
 
-	if (sdvo_priv->tv_format_property)
-		drm_property_destroy(encoder->dev,
-				     sdvo_priv->tv_format_property);
-
 	drm_encoder_cleanup(encoder);
 	kfree(intel_encoder);
 }
@@ -2045,12 +2084,15 @@ intel_sdvo_select_ddc_bus(struct intel_sdvo_priv *dev_priv)
 }
 
 static bool
-intel_sdvo_get_digital_encoding_mode(struct intel_encoder *output)
+intel_sdvo_get_digital_encoding_mode(struct intel_encoder *output, int device)
 {
 	struct intel_sdvo_priv *sdvo_priv = output->dev_priv;
 	uint8_t status;
 
-	intel_sdvo_set_target_output(output, sdvo_priv->controlled_output);
+	if (device == 0)
+		intel_sdvo_set_target_output(output, SDVO_OUTPUT_TMDS0);
+	else
+		intel_sdvo_set_target_output(output, SDVO_OUTPUT_TMDS1);
 
 	intel_sdvo_write_cmd(output, SDVO_CMD_GET_ENCODE, NULL, 0);
 	status = intel_sdvo_read_response(output, &sdvo_priv->is_hdmi, 1);
@@ -2157,96 +2199,228 @@ static struct dmi_system_id intel_sdvo_bad_tv[] = {
 };
 
 static bool
-intel_sdvo_output_setup(struct intel_encoder *intel_encoder,
-			struct intel_connector *intel_connector,
-			uint16_t flags)
+intel_sdvo_connector_alloc (struct intel_connector **ret)
+{
+	struct intel_connector *intel_connector;
+	struct intel_sdvo_connector *sdvo_connector;
+
+	*ret = kzalloc(sizeof(*intel_connector) +
+			sizeof(*sdvo_connector), GFP_KERNEL);
+	if (!*ret)
+		return false;
+
+	intel_connector = *ret;
+	sdvo_connector = (struct intel_sdvo_connector *)(intel_connector + 1);
+	intel_connector->dev_priv = sdvo_connector;
+
+	return true;
+}
+
+static void
+intel_sdvo_connector_create (struct drm_encoder *encoder,
+			     struct drm_connector *connector)
+{
+	drm_connector_init(encoder->dev, connector, &intel_sdvo_connector_funcs,
+			   connector->connector_type);
+
+	drm_connector_helper_add(connector, &intel_sdvo_connector_helper_funcs);
+
+	connector->interlace_allowed = 0;
+	connector->doublescan_allowed = 0;
+	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
+
+	drm_mode_connector_attach_encoder(connector, encoder);
+	drm_sysfs_connector_add(connector);
+}
+
+static bool
+intel_sdvo_dvi_init(struct intel_encoder *intel_encoder, int device)
 {
-	struct drm_connector *connector = &intel_connector->base;
 	struct drm_encoder *encoder = &intel_encoder->enc;
 	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
-	bool ret = true, registered = false;
+	struct drm_connector *connector;
+	struct intel_connector *intel_connector;
+	struct intel_sdvo_connector *sdvo_connector;
+
+	if (!intel_sdvo_connector_alloc(&intel_connector))
+		return false;
+
+	sdvo_connector = intel_connector->dev_priv;
+
+	if (device == 0) {
+		sdvo_priv->controlled_output |= SDVO_OUTPUT_TMDS0;
+		sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
+	} else if (device == 1) {
+		sdvo_priv->controlled_output |= SDVO_OUTPUT_TMDS1;
+		sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
+	}
+
+	connector = &intel_connector->base;
+	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
+	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
+
+	if (intel_sdvo_get_supp_encode(intel_encoder, &sdvo_priv->encode)
+		&& intel_sdvo_get_digital_encoding_mode(intel_encoder, device)
+		&& sdvo_priv->is_hdmi) {
+		/* enable hdmi encoding mode if supported */
+		intel_sdvo_set_encode(intel_encoder, SDVO_ENCODE_HDMI);
+		intel_sdvo_set_colorimetry(intel_encoder,
+					   SDVO_COLORIMETRY_RGB256);
+		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
+	}
+	intel_encoder->clone_mask = (1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
+				    (1 << INTEL_ANALOG_CLONE_BIT);
+
+	intel_sdvo_connector_create(encoder, connector);
+
+	return true;
+}
+
+static bool
+intel_sdvo_tv_init(struct intel_encoder *intel_encoder, int type)
+{
+        struct drm_encoder *encoder = &intel_encoder->enc;
+        struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+        struct drm_connector *connector;
+        struct intel_connector *intel_connector;
+        struct intel_sdvo_connector *sdvo_connector;
+
+        if (!intel_sdvo_connector_alloc(&intel_connector))
+                return false;
+
+        connector = &intel_connector->base;
+        encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
+        connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
+        sdvo_connector = intel_connector->dev_priv;
+
+        sdvo_priv->controlled_output |= type;
+        sdvo_connector->output_flag = type;
+
+        sdvo_priv->is_tv = true;
+        intel_encoder->needs_tv_clock = true;
+        intel_encoder->clone_mask = 1 << INTEL_SDVO_TV_CLONE_BIT;
+
+        intel_sdvo_connector_create(encoder, connector);
+
+        intel_sdvo_tv_create_property(connector, type);
+
+        intel_sdvo_create_enhance_property(connector);
+
+        return true;
+}
+
+static bool
+intel_sdvo_analog_init(struct intel_encoder *intel_encoder, int device)
+{
+        struct drm_encoder *encoder = &intel_encoder->enc;
+        struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+        struct drm_connector *connector;
+        struct intel_connector *intel_connector;
+        struct intel_sdvo_connector *sdvo_connector;
+
+        if (!intel_sdvo_connector_alloc(&intel_connector))
+                return false;
+
+        connector = &intel_connector->base;
+        encoder->encoder_type = DRM_MODE_ENCODER_DAC;
+        connector->connector_type = DRM_MODE_CONNECTOR_VGA;
+        sdvo_connector = intel_connector->dev_priv;
+
+        if (device == 0) {
+                sdvo_priv->controlled_output |= SDVO_OUTPUT_RGB0;
+                sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
+        } else if (device == 1) {
+                sdvo_priv->controlled_output |= SDVO_OUTPUT_RGB1;
+                sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
+        }
+
+        intel_encoder->clone_mask = (1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
+                                    (1 << INTEL_ANALOG_CLONE_BIT);
+
+        intel_sdvo_connector_create(encoder, connector);
+        return true;
+}
+
+static bool
+intel_sdvo_lvds_init(struct intel_encoder *intel_encoder, int device)
+{
+        struct drm_encoder *encoder = &intel_encoder->enc;
+        struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+        struct drm_connector *connector;
+        struct intel_connector *intel_connector;
+        struct intel_sdvo_connector *sdvo_connector;
+
+        if (!intel_sdvo_connector_alloc(&intel_connector))
+                return false;
+
+        connector = &intel_connector->base;
+        encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
+        connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
+        sdvo_connector = intel_connector->dev_priv;
+
+        sdvo_priv->is_lvds = true;
+
+        if (device == 0) {
+                sdvo_priv->controlled_output |= SDVO_OUTPUT_LVDS0;
+                sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
+        } else if (device == 1) {
+                sdvo_priv->controlled_output |= SDVO_OUTPUT_LVDS1;
+                sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
+        }
+
+        intel_encoder->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;
+}
+
+static bool
+intel_sdvo_output_setup(struct intel_encoder *intel_encoder, uint16_t flags)
+{
+	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
 
 	sdvo_priv->is_tv = false;
 	intel_encoder->needs_tv_clock = false;
 	sdvo_priv->is_lvds = false;
 
-	if (device_is_registered(&connector->kdev)) {
-		drm_sysfs_connector_remove(connector);
-		registered = true;
-	}
+	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
 
-	if (flags &
-	    (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)) {
-		if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_TMDS0)
-			sdvo_priv->controlled_output = SDVO_OUTPUT_TMDS0;
-		else
-			sdvo_priv->controlled_output = SDVO_OUTPUT_TMDS1;
-
-		encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
-		connector->connector_type = DRM_MODE_CONNECTOR_DVID;
-
-		if (intel_sdvo_get_supp_encode(intel_encoder,
-					       &sdvo_priv->encode) &&
-		    intel_sdvo_get_digital_encoding_mode(intel_encoder) &&
-		    sdvo_priv->is_hdmi) {
-			/* enable hdmi encoding mode if supported */
-			intel_sdvo_set_encode(intel_encoder, SDVO_ENCODE_HDMI);
-			intel_sdvo_set_colorimetry(intel_encoder,
-						   SDVO_COLORIMETRY_RGB256);
-			connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
-			intel_encoder->clone_mask =
-					(1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
-					(1 << INTEL_ANALOG_CLONE_BIT);
-		}
-	} else if ((flags & SDVO_OUTPUT_SVID0) &&
-		   !dmi_check_system(intel_sdvo_bad_tv)) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_SVID0;
-		encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
-		connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
-		sdvo_priv->is_tv = true;
-		intel_encoder->needs_tv_clock = true;
-		intel_encoder->clone_mask = 1 << INTEL_SDVO_TV_CLONE_BIT;
-	} else if (flags & SDVO_OUTPUT_RGB0) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_RGB0;
-		encoder->encoder_type = DRM_MODE_ENCODER_DAC;
-		connector->connector_type = DRM_MODE_CONNECTOR_VGA;
-		intel_encoder->clone_mask = (1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
-					(1 << INTEL_ANALOG_CLONE_BIT);
-	} else if (flags & SDVO_OUTPUT_RGB1) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_RGB1;
-		encoder->encoder_type = DRM_MODE_ENCODER_DAC;
-		connector->connector_type = DRM_MODE_CONNECTOR_VGA;
-		intel_encoder->clone_mask = (1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
-					(1 << INTEL_ANALOG_CLONE_BIT);
-	} else if (flags & SDVO_OUTPUT_CVBS0) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_CVBS0;
-		encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
-		connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
-		sdvo_priv->is_tv = true;
-		intel_encoder->needs_tv_clock = true;
-		intel_encoder->clone_mask = 1 << INTEL_SDVO_TV_CLONE_BIT;
-	} else if (flags & SDVO_OUTPUT_LVDS0) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_LVDS0;
-		encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
-		connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
-		sdvo_priv->is_lvds = true;
-		intel_encoder->clone_mask = (1 << INTEL_ANALOG_CLONE_BIT) |
-					(1 << INTEL_SDVO_LVDS_CLONE_BIT);
-	} else if (flags & SDVO_OUTPUT_LVDS1) {
-
-		sdvo_priv->controlled_output = SDVO_OUTPUT_LVDS1;
-		encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
-		connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
-		sdvo_priv->is_lvds = true;
-		intel_encoder->clone_mask = (1 << INTEL_ANALOG_CLONE_BIT) |
-					(1 << INTEL_SDVO_LVDS_CLONE_BIT);
-	} else {
+	if (flags & SDVO_OUTPUT_TMDS0)
+		if (!intel_sdvo_dvi_init(intel_encoder, 0))
+			return false;
+
+	if ((flags & SDVO_TMDS_MASK) == SDVO_TMDS_MASK)
+		if (!intel_sdvo_dvi_init(intel_encoder, 1))
+			return false;
+
+	/* TV has no XXX1 function block */
+	if ((flags & SDVO_OUTPUT_SVID0) && !dmi_check_system(intel_sdvo_bad_tv))
+		if (!intel_sdvo_tv_init(intel_encoder, SDVO_OUTPUT_SVID0))
+			return false;
+
+	if (flags & SDVO_OUTPUT_CVBS0)
+		if (!intel_sdvo_tv_init(intel_encoder, SDVO_OUTPUT_CVBS0))
+			return false;
+
+	if (flags & SDVO_OUTPUT_RGB0)
+		if (!intel_sdvo_analog_init(intel_encoder, 0))
+			return false;
+
+	if ((flags & SDVO_RGB_MASK) == SDVO_RGB_MASK)
+		if (!intel_sdvo_analog_init(intel_encoder, 1))
+			return false;
+
+	if (flags & SDVO_OUTPUT_LVDS0)
+		if (!intel_sdvo_lvds_init(intel_encoder, 0))
+			return false;
+
+	if ((flags & SDVO_LVDS_MASK) == SDVO_LVDS_MASK)
+		if (!intel_sdvo_lvds_init(intel_encoder, 1))
+			return false;
 
+	if ((flags & SDVO_OUTPUT_MASK) == 0) {
 		unsigned char bytes[2];
 
 		sdvo_priv->controlled_output = 0;
@@ -2254,29 +2428,25 @@ intel_sdvo_output_setup(struct intel_encoder *intel_encoder,
 		DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
 			      SDVO_NAME(sdvo_priv),
 			      bytes[0], bytes[1]);
-		ret = false;
+		return false;
 	}
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 
-	if (ret && registered)
-		ret = drm_sysfs_connector_add(connector) == 0 ? true : false;
-
-
-	return ret;
-
+	return true;
 }
 
-static void intel_sdvo_tv_create_property(struct drm_connector *connector)
+static void intel_sdvo_tv_create_property(struct drm_connector *connector, int type)
 {
 	struct drm_encoder *encoder = intel_attached_encoder(connector);
 	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
 	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_connector = intel_connector->dev_priv;
 	struct intel_sdvo_tv_format format;
 	uint32_t format_map, i;
 	uint8_t status;
 
-	intel_sdvo_set_target_output(intel_encoder,
-				     sdvo_priv->controlled_output);
+	intel_sdvo_set_target_output(intel_encoder, type);
 
 	intel_sdvo_write_cmd(intel_encoder,
 			     SDVO_CMD_GET_SUPPORTED_TV_FORMATS, NULL, 0);
@@ -2291,28 +2461,28 @@ static void intel_sdvo_tv_create_property(struct drm_connector *connector)
 	if (format_map == 0)
 		return;
 
-	sdvo_priv->format_supported_num = 0;
+	sdvo_connector->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++] =
+			sdvo_connector->tv_format_supported
+			[sdvo_connector->format_supported_num++] =
 			tv_format_names[i];
 		}
 
 
-	sdvo_priv->tv_format_property =
+	sdvo_connector->tv_format_property =
 			drm_property_create(
 				connector->dev, DRM_MODE_PROP_ENUM,
-				"mode", sdvo_priv->format_supported_num);
+				"mode", sdvo_connector->format_supported_num);
 
-	for (i = 0; i < sdvo_priv->format_supported_num; i++)
+	for (i = 0; i < sdvo_connector->format_supported_num; i++)
 		drm_property_add_enum(
-				sdvo_priv->tv_format_property, i,
-				i, sdvo_priv->tv_format_supported[i]);
+				sdvo_connector->tv_format_property, i,
+				i, sdvo_connector->tv_format_supported[i]);
 
-	sdvo_priv->tv_format_name = sdvo_priv->tv_format_supported[0];
+	sdvo_priv->tv_format_name = sdvo_connector->tv_format_supported[0];
 	drm_connector_attach_property(
-			connector, sdvo_priv->tv_format_property, 0);
+			connector, sdvo_connector->tv_format_property, 0);
 
 }
 
@@ -2320,7 +2490,8 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder = intel_attached_encoder(connector);
 	struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
-	struct intel_sdvo_priv *sdvo_priv = intel_encoder->dev_priv;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_sdvo_connector *sdvo_priv = intel_connector->dev_priv;
 	struct intel_sdvo_enhancements_reply sdvo_data;
 	struct drm_device *dev = connector->dev;
 	uint8_t status;
@@ -2339,7 +2510,7 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 		DRM_DEBUG_KMS("No enhancement is supported\n");
 		return;
 	}
-	if (sdvo_priv->is_tv) {
+	if (IS_TV(sdvo_priv)) {
 		/* when horizontal overscan is supported, Add the left/right
 		 * property
 		 */
@@ -2487,8 +2658,6 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 					"default %d, current %d\n",
 					data_value[0], data_value[1], response);
 		}
-	}
-	if (sdvo_priv->is_tv) {
 		if (sdvo_data.saturation) {
 			intel_sdvo_write_cmd(intel_encoder,
 				SDVO_CMD_GET_MAX_SATURATION, NULL, 0);
@@ -2584,7 +2753,7 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 					data_value[0], data_value[1], response);
 		}
 	}
-	if (sdvo_priv->is_tv || sdvo_priv->is_lvds) {
+	if (IS_TV(sdvo_priv) || IS_LVDS(sdvo_priv)) {
 		if (sdvo_data.brightness) {
 			intel_sdvo_write_cmd(intel_encoder,
 				SDVO_CMD_GET_MAX_BRIGHTNESS, NULL, 0);
@@ -2624,11 +2793,8 @@ static void intel_sdvo_create_enhance_property(struct drm_connector *connector)
 bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_connector *connector;
 	struct intel_encoder *intel_encoder;
-	struct intel_connector *intel_connector;
 	struct intel_sdvo_priv *sdvo_priv;
-
 	u8 ch[0x40];
 	int i;
 
@@ -2637,12 +2803,6 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
 		return false;
 	}
 
-	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
-	if (!intel_connector) {
-		kfree(intel_encoder);
-		return false;
-	}
-
 	sdvo_priv = (struct intel_sdvo_priv *)(intel_encoder + 1);
 	sdvo_priv->sdvo_reg = sdvo_reg;
 
@@ -2691,40 +2851,20 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
 	/* Wrap with our custom algo which switches to DDC mode */
 	intel_encoder->ddc_bus->algo = &intel_sdvo_i2c_bit_algo;
 
+	/* encoder type will be decided later */
+	drm_encoder_init(dev, &intel_encoder->enc, &intel_sdvo_enc_funcs, 0);
+	drm_encoder_helper_add(&intel_encoder->enc, &intel_sdvo_helper_funcs);
+
 	/* In default case sdvo lvds is false */
 	intel_sdvo_get_capabilities(intel_encoder, &sdvo_priv->caps);
 
-	if (intel_sdvo_output_setup(intel_encoder, intel_connector,
+	if (intel_sdvo_output_setup(intel_encoder,
 				    sdvo_priv->caps.output_flags) != true) {
 		DRM_DEBUG_KMS("SDVO output failed to setup on SDVO%c\n",
 			  sdvo_reg == SDVOB ? 'B' : 'C');
 		goto err_i2c;
 	}
 
-
-	connector = &intel_connector->base;
-	drm_connector_init(dev, connector, &intel_sdvo_connector_funcs,
-			   connector->connector_type);
-
-	drm_connector_helper_add(connector, &intel_sdvo_connector_helper_funcs);
-	connector->interlace_allowed = 0;
-	connector->doublescan_allowed = 0;
-	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
-
-	drm_encoder_init(dev, &intel_encoder->enc,
-			&intel_sdvo_enc_funcs, intel_encoder->enc.encoder_type);
-
-	drm_encoder_helper_add(&intel_encoder->enc, &intel_sdvo_helper_funcs);
-
-	drm_mode_connector_attach_encoder(&intel_connector->base, &intel_encoder->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);
-
-	drm_sysfs_connector_add(connector);
-
 	intel_sdvo_select_ddc_bus(sdvo_priv);
 
 	/* Set the input timing to the screen. Assume always input 0. */
@@ -2763,7 +2903,6 @@ err_i2c:
 		intel_i2c_destroy(intel_encoder->i2c_bus);
 err_inteloutput:
 	kfree(intel_encoder);
-	kfree(intel_connector);
 
 	return false;
 }