[Intel-gfx] [PATCH 2/5] drm/i915: Added a return type for the restore crtc mode function

akash.goel at intel.com akash.goel at intel.com
Mon Jun 23 10:34:26 CEST 2014


From: Akash Goel <akash.goel at intel.com>

This patch changes the return type of 'crtc_restore_mode'
function from 'void', so that an error could be returned back to
User space, from the set property ioctl call, if the configuation
is not valid.

Signed-off-by: Akash Goel <akash.goel at intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 ++--
 drivers/gpu/drm/i915/intel_dp.c      | 30 +++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c    | 28 +++++++++++++++---
 drivers/gpu/drm/i915/intel_lvds.c    |  7 ++++-
 drivers/gpu/drm/i915/intel_sdvo.c    | 57 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_tv.c      | 26 ++++++++++++++--
 7 files changed, 136 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fa77557..f39a538 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10817,9 +10817,10 @@ static int intel_set_mode(struct drm_crtc *crtc,
 	return ret;
 }
 
-void intel_crtc_restore_mode(struct drm_crtc *crtc)
+int intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
-	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
+	return intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+						crtc->primary->fb);
 }
 
 #undef for_each_intel_crtc_masked
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3e04e15..e0186d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3680,6 +3680,9 @@ intel_dp_set_property(struct drm_connector *connector,
 	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
 	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
 	int ret;
+	bool old_has_audio = 0, old_auto = 0;
+	uint32_t old_range = 0;
+	int old_force_audio = 0, old_fitting_mode = 0;
 
 	ret = drm_object_property_set_value(&connector->base, property, val);
 	if (ret)
@@ -3692,6 +3695,9 @@ intel_dp_set_property(struct drm_connector *connector,
 		if (i == intel_dp->force_audio)
 			return 0;
 
+		old_force_audio = intel_dp->force_audio;
+		old_has_audio = intel_dp->has_audio;
+
 		intel_dp->force_audio = i;
 
 		if (i == HDMI_AUDIO_AUTO)
@@ -3707,8 +3713,8 @@ intel_dp_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_dp->color_range_auto;
-		uint32_t old_range = intel_dp->color_range;
+		old_auto = intel_dp->color_range_auto;
+		old_range = intel_dp->color_range;
 
 		switch (val) {
 		case INTEL_BROADCAST_RGB_AUTO:
@@ -3744,6 +3750,7 @@ intel_dp_set_property(struct drm_connector *connector,
 			/* the eDP scaling property is not changed */
 			return 0;
 		}
+		old_fitting_mode = intel_connector->panel.fitting_mode;
 		intel_connector->panel.fitting_mode = val;
 
 		goto done;
@@ -3752,9 +3759,22 @@ intel_dp_set_property(struct drm_connector *connector,
 	return -EINVAL;
 
 done:
-	if (intel_encoder->base.crtc)
-		intel_crtc_restore_mode(intel_encoder->base.crtc);
-
+	if (intel_encoder->base.crtc) {
+		ret = intel_crtc_restore_mode(intel_encoder->base.crtc);
+		if (ret) {
+			if (property == dev_priv->force_audio_property) {
+				intel_dp->force_audio = old_force_audio;
+				intel_dp->has_audio = old_has_audio;
+			}
+			else if (property == dev_priv->broadcast_rgb_property) {
+				intel_dp->color_range_auto = old_auto;
+				intel_dp->color_range = old_range;
+			}
+			else if (property == connector->dev->mode_config.scaling_mode_property)
+				intel_connector->panel.fitting_mode = old_fitting_mode;
+		}
+		return ret;
+	}
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47d4827..074f6d8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -752,7 +752,7 @@ void intel_frontbuffer_flip(struct drm_device *dev,
 
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
 void intel_mark_idle(struct drm_device *dev);
-void intel_crtc_restore_mode(struct drm_crtc *crtc);
+int intel_crtc_restore_mode(struct drm_crtc *crtc);
 void intel_crtc_update_dpms(struct drm_crtc *crtc);
 void intel_encoder_destroy(struct drm_encoder *encoder);
 void intel_connector_dpms(struct drm_connector *, int mode);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 2422413..63c32a6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1071,6 +1071,9 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		hdmi_to_dig_port(intel_hdmi);
 	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	int ret;
+	enum hdmi_force_audio old_force_audio = 0;
+	bool old_has_audio = 0, old_has_hdmi_sink = 0, old_auto = 0;
+	uint32_t old_range = 0;
 
 	ret = drm_object_property_set_value(&connector->base, property, val);
 	if (ret)
@@ -1083,6 +1086,10 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		if (i == intel_hdmi->force_audio)
 			return 0;
 
+		old_force_audio = intel_hdmi->force_audio;
+		old_has_hdmi_sink = intel_hdmi->has_hdmi_sink;
+		old_has_audio = intel_hdmi->has_audio;
+
 		intel_hdmi->force_audio = i;
 
 		if (i == HDMI_AUDIO_AUTO)
@@ -1098,8 +1105,8 @@ intel_hdmi_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_hdmi->color_range_auto;
-		uint32_t old_range = intel_hdmi->color_range;
+		old_auto = intel_hdmi->color_range_auto;
+		old_range = intel_hdmi->color_range;
 
 		switch (val) {
 		case INTEL_BROADCAST_RGB_AUTO:
@@ -1127,8 +1134,21 @@ intel_hdmi_set_property(struct drm_connector *connector,
 	return -EINVAL;
 
 done:
-	if (intel_dig_port->base.base.crtc)
-		intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
+	if (intel_dig_port->base.base.crtc) {
+		ret = intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
+		if (ret) {
+			if (property == dev_priv->force_audio_property) {
+				intel_hdmi->force_audio = old_force_audio;
+				intel_hdmi->has_hdmi_sink = old_has_hdmi_sink;
+				intel_hdmi->has_audio = old_has_audio;
+			}
+			else if (property == dev_priv->broadcast_rgb_property) {
+				intel_hdmi->color_range_auto = old_auto;
+				intel_hdmi->color_range = old_range;
+			}
+		}
+		return ret;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index facd8cb..9850a38 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -479,6 +479,7 @@ static int intel_lvds_set_property(struct drm_connector *connector,
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = connector->dev;
+	int old_fitting_mode = 0;
 
 	if (property == dev->mode_config.scaling_mode_property) {
 		struct drm_crtc *crtc;
@@ -492,6 +493,7 @@ static int intel_lvds_set_property(struct drm_connector *connector,
 			/* the LVDS scaling property is not changed */
 			return 0;
 		}
+		old_fitting_mode = intel_connector->panel.fitting_mode;
 		intel_connector->panel.fitting_mode = value;
 
 		crtc = intel_attached_encoder(connector)->base.crtc;
@@ -500,7 +502,10 @@ static int intel_lvds_set_property(struct drm_connector *connector,
 			 * If the CRTC is enabled, the display will be changed
 			 * according to the new panel fitting mode.
 			 */
-			intel_crtc_restore_mode(crtc);
+			int ret = intel_crtc_restore_mode(crtc);
+			if (ret)
+				intel_connector->panel.fitting_mode = old_fitting_mode;
+			return ret;
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 9350edd..fe9b447 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2074,6 +2074,9 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	uint16_t temp_value;
 	uint8_t cmd;
 	int ret;
+	bool old_has_hdmi_audio = 0, old_auto = 0;
+	uint32_t old_range = 0, old_value = 0;
+	int old_force_audio = 0, old_tv_format_index = 0;
 
 	ret = drm_object_property_set_value(&connector->base, property, val);
 	if (ret)
@@ -2086,6 +2089,9 @@ intel_sdvo_set_property(struct drm_connector *connector,
 		if (i == intel_sdvo_connector->force_audio)
 			return 0;
 
+		old_force_audio = intel_sdvo_connector->force_audio;
+		old_has_hdmi_audio = intel_sdvo->has_hdmi_audio;
+
 		intel_sdvo_connector->force_audio = i;
 
 		if (i == HDMI_AUDIO_AUTO)
@@ -2101,8 +2107,8 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_sdvo->color_range_auto;
-		uint32_t old_range = intel_sdvo->color_range;
+		old_auto = intel_sdvo->color_range_auto;
+		old_range = intel_sdvo->color_range;
 
 		switch (val) {
 		case INTEL_BROADCAST_RGB_AUTO:
@@ -2133,6 +2139,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	if (intel_sdvo_connector->name == property) { \
 		if (intel_sdvo_connector->cur_##name == temp_value) return 0; \
 		if (intel_sdvo_connector->max_##name < temp_value) return -EINVAL; \
+		old_value = intel_sdvo_connector->cur_##name; \
 		cmd = SDVO_CMD_SET_##NAME; \
 		intel_sdvo_connector->cur_##name = temp_value; \
 		goto set_value; \
@@ -2146,6 +2153,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 		    intel_sdvo_connector->tv_format_supported[val])
 			return 0;
 
+		old_tv_format_index = intel_sdvo->tv_format_index;
 		intel_sdvo->tv_format_index = intel_sdvo_connector->tv_format_supported[val];
 		goto done;
 	} else if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
@@ -2156,6 +2164,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			if (intel_sdvo_connector->left_margin == temp_value)
 				return 0;
 
+			old_value = temp_value;
 			intel_sdvo_connector->left_margin = temp_value;
 			intel_sdvo_connector->right_margin = temp_value;
 			temp_value = intel_sdvo_connector->max_hscan -
@@ -2168,6 +2177,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			if (intel_sdvo_connector->right_margin == temp_value)
 				return 0;
 
+			old_value = temp_value;
 			intel_sdvo_connector->left_margin = temp_value;
 			intel_sdvo_connector->right_margin = temp_value;
 			temp_value = intel_sdvo_connector->max_hscan -
@@ -2180,6 +2190,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			if (intel_sdvo_connector->top_margin == temp_value)
 				return 0;
 
+			old_value = temp_value;
 			intel_sdvo_connector->top_margin = temp_value;
 			intel_sdvo_connector->bottom_margin = temp_value;
 			temp_value = intel_sdvo_connector->max_vscan -
@@ -2192,6 +2203,7 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			if (intel_sdvo_connector->bottom_margin == temp_value)
 				return 0;
 
+			old_value = temp_value;
 			intel_sdvo_connector->top_margin = temp_value;
 			intel_sdvo_connector->bottom_margin = temp_value;
 			temp_value = intel_sdvo_connector->max_vscan -
@@ -2222,11 +2234,48 @@ set_value:
 
 
 done:
-	if (intel_sdvo->base.base.crtc)
-		intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
+	if (intel_sdvo->base.base.crtc) {
+		int ret = intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
+
+#define UNDO_PROPERTY(name) \
+		else if (intel_sdvo_connector->name == property) \
+			intel_sdvo_connector->cur_##name = old_value;
+
+		if (property == dev_priv->force_audio_property) {
+			intel_sdvo_connector->force_audio = old_force_audio;
+			intel_sdvo->has_hdmi_audio = old_has_hdmi_audio;
+		} else if (property == dev_priv->broadcast_rgb_property) {
+			intel_sdvo->color_range_auto = old_auto;
+			intel_sdvo->color_range = old_range;
+		} else if ((intel_sdvo_connector->left == property) ||
+				   (intel_sdvo_connector->right == property)) {
+			intel_sdvo_connector->left_margin = old_value;
+			intel_sdvo_connector->right_margin = old_value;
+		} else if ((intel_sdvo_connector->top == property) ||
+				   (intel_sdvo_connector->bottom == property)) {
+			intel_sdvo_connector->top_margin = old_value;
+			intel_sdvo_connector->bottom_margin = old_value;
+		} else if (property == intel_sdvo_connector->tv_format)
+			intel_sdvo->tv_format_index = old_tv_format_index;
+		UNDO_PROPERTY(hpos)
+		UNDO_PROPERTY(vpos)
+		UNDO_PROPERTY(saturation)
+		UNDO_PROPERTY(contrast)
+		UNDO_PROPERTY(hue)
+		UNDO_PROPERTY(brightness)
+		UNDO_PROPERTY(sharpness)
+		UNDO_PROPERTY(flicker_filter)
+		UNDO_PROPERTY(flicker_filter_2d)
+		UNDO_PROPERTY(flicker_filter_adaptive)
+		UNDO_PROPERTY(tv_chroma_filter)
+		UNDO_PROPERTY(tv_luma_filter)
+		UNDO_PROPERTY(dot_crawl)
+		return ret;
+	}
 
 	return 0;
 #undef CHECK_PROPERTY
+#undef UNDO_PROPERTY
 }
 
 static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index e211eef..6d0f728 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1453,6 +1453,8 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 	struct drm_crtc *crtc = intel_tv->base.base.crtc;
 	int ret = 0;
 	bool changed = false;
+	int old_margin = 0;
+	const char *old_format = NULL;
 
 	ret = drm_object_property_set_value(&connector->base, property, val);
 	if (ret < 0)
@@ -1460,18 +1462,22 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 
 	if (property == dev->mode_config.tv_left_margin_property &&
 		intel_tv->margin[TV_MARGIN_LEFT] != val) {
+		old_margin = intel_tv->margin[TV_MARGIN_LEFT];
 		intel_tv->margin[TV_MARGIN_LEFT] = val;
 		changed = true;
 	} else if (property == dev->mode_config.tv_right_margin_property &&
 		intel_tv->margin[TV_MARGIN_RIGHT] != val) {
+		old_margin = intel_tv->margin[TV_MARGIN_RIGHT];
 		intel_tv->margin[TV_MARGIN_RIGHT] = val;
 		changed = true;
 	} else if (property == dev->mode_config.tv_top_margin_property &&
 		intel_tv->margin[TV_MARGIN_TOP] != val) {
+		old_margin = intel_tv->margin[TV_MARGIN_TOP];
 		intel_tv->margin[TV_MARGIN_TOP] = val;
 		changed = true;
 	} else if (property == dev->mode_config.tv_bottom_margin_property &&
 		intel_tv->margin[TV_MARGIN_BOTTOM] != val) {
+		old_margin = intel_tv->margin[TV_MARGIN_BOTTOM];
 		intel_tv->margin[TV_MARGIN_BOTTOM] = val;
 		changed = true;
 	} else if (property == dev->mode_config.tv_mode_property) {
@@ -1482,6 +1488,7 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 		if (!strcmp(intel_tv->tv_format, tv_modes[val].name))
 			goto out;
 
+		old_format = intel_tv->tv_format;
 		intel_tv->tv_format = tv_modes[val].name;
 		changed = true;
 	} else {
@@ -1489,8 +1496,23 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 		goto out;
 	}
 
-	if (changed && crtc)
-		intel_crtc_restore_mode(crtc);
+	if (changed && crtc) {
+		ret =  intel_crtc_restore_mode(crtc);
+		if (ret) {
+			if (property == dev->mode_config.tv_left_margin_property)
+				intel_tv->margin[TV_MARGIN_LEFT] = old_margin;
+			else if (property == dev->mode_config.tv_right_margin_property)
+				intel_tv->margin[TV_MARGIN_RIGHT] = old_margin;
+			else if (property == dev->mode_config.tv_top_margin_property)
+				intel_tv->margin[TV_MARGIN_TOP] = old_margin;
+			else if (property == dev->mode_config.tv_bottom_margin_property)
+				intel_tv->margin[TV_MARGIN_BOTTOM] = old_margin;
+			else if (property == dev->mode_config.tv_mode_property) {
+				intel_tv->tv_format = old_format;
+			}
+		}
+		return ret;
+	}
 out:
 	return ret;
 }
-- 
1.9.2




More information about the Intel-gfx mailing list