diff mbox

[1/3] drm/i915: Added a return type for panel fitter config functions

Message ID 1403272554-20383-2-git-send-email-akash.goel@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

akash.goel@intel.com June 20, 2014, 1:55 p.m. UTC 
From: Akash Goel <akash.goel@intel.com>

This patch changes the return type of panel fitter configuration
functions from 'void', so that an error could be returned back to
User space, either during the modeset time or when the 'border' property
is being set, if the configuation is not valid.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 +++--
 drivers/gpu/drm/i915/intel_dp.c      | 17 ++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +++---
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c    | 13 +++++++------
 drivers/gpu/drm/i915/intel_panel.c   | 10 ++++++----
 drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
 drivers/gpu/drm/i915/intel_tv.c      |  2 +-
 8 files changed, 32 insertions(+), 25 deletions(-)

Comments

Chris Wilson June 20, 2014, 2:03 p.m. UTC | #1 
On Fri, Jun 20, 2014 at 07:25:52PM +0530, akash.goel@intel.com wrote:
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 912e9c4..6117639 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -824,12 +824,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>  				       adjusted_mode);
> -		if (!HAS_PCH_SPLIT(dev))
> -			intel_gmch_panel_fitting(intel_crtc, pipe_config,
> -						 intel_connector->panel.fitting_mode);
> -		else
> -			intel_pch_panel_fitting(intel_crtc, pipe_config,
> -						intel_connector->panel.fitting_mode);
> +		if (!HAS_PCH_SPLIT(dev)) {
> +			if (!intel_gmch_panel_fitting(intel_crtc, pipe_config,
> +						 intel_connector->panel.fitting_mode))
> +				return false;
> +		} else {
> +			if (!intel_pch_panel_fitting(intel_crtc, pipe_config,
> +						 intel_connector->panel.fitting_mode))
> +				return false;
> +		}
>  	}
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> @@ -3782,7 +3785,7 @@ intel_dp_set_property(struct drm_connector *connector,
>  
>  done:
>  	if (intel_encoder->base.crtc)
> -		intel_crtc_restore_mode(intel_encoder->base.crtc);
> +		return intel_crtc_restore_mode(intel_encoder->base.crtc);

But you don't unwind the property change after failure.

>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ab5962b..860d531 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -727,7 +727,7 @@ void intel_mark_busy(struct drm_device *dev);
>  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>  			struct intel_engine_cs *ring);
>  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);
> @@ -918,10 +918,10 @@ int intel_panel_init(struct intel_panel *panel,
>  void intel_panel_fini(struct intel_panel *panel);
>  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  			    struct drm_display_mode *adjusted_mode);
> -void intel_pch_panel_fitting(struct intel_crtc *crtc,
> +bool intel_pch_panel_fitting(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config,
>  			     int fitting_mode);
> -void intel_gmch_panel_fitting(struct intel_crtc *crtc,
> +bool intel_gmch_panel_fitting(struct intel_crtc *crtc,
>  			      struct intel_crtc_config *pipe_config,
>  			      int fitting_mode);

Only make significnt changes like this to one interface at a time.
-Chris
akash.goel@intel.com June 23, 2014, 8:27 a.m. UTC | #2 
On Fri, 2014-06-20 at 15:03 +0100, Chris Wilson wrote:
> On Fri, Jun 20, 2014 at 07:25:52PM +0530, akash.goel@intel.com wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 912e9c4..6117639 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -824,12 +824,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> >  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> >  				       adjusted_mode);
> > -		if (!HAS_PCH_SPLIT(dev))
> > -			intel_gmch_panel_fitting(intel_crtc, pipe_config,
> > -						 intel_connector->panel.fitting_mode);
> > -		else
> > -			intel_pch_panel_fitting(intel_crtc, pipe_config,
> > -						intel_connector->panel.fitting_mode);
> > +		if (!HAS_PCH_SPLIT(dev)) {
> > +			if (!intel_gmch_panel_fitting(intel_crtc, pipe_config,
> > +						 intel_connector->panel.fitting_mode))
> > +				return false;
> > +		} else {
> > +			if (!intel_pch_panel_fitting(intel_crtc, pipe_config,
> > +						 intel_connector->panel.fitting_mode))
> > +				return false;
> > +		}
> >  	}
> >  
> >  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > @@ -3782,7 +3785,7 @@ intel_dp_set_property(struct drm_connector *connector,
> >  
> >  done:
> >  	if (intel_encoder->base.crtc)
> > -		intel_crtc_restore_mode(intel_encoder->base.crtc);
> > +		return intel_crtc_restore_mode(intel_encoder->base.crtc);
> 
> But you don't unwind the property change after failure.
Sorry, will handle this in next version of the patch.
> 
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ab5962b..860d531 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -727,7 +727,7 @@ void intel_mark_busy(struct drm_device *dev);
> >  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >  			struct intel_engine_cs *ring);
> >  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);
> > @@ -918,10 +918,10 @@ int intel_panel_init(struct intel_panel *panel,
> >  void intel_panel_fini(struct intel_panel *panel);
> >  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> >  			    struct drm_display_mode *adjusted_mode);
> > -void intel_pch_panel_fitting(struct intel_crtc *crtc,
> > +bool intel_pch_panel_fitting(struct intel_crtc *crtc,
> >  			     struct intel_crtc_config *pipe_config,
> >  			     int fitting_mode);
> > -void intel_gmch_panel_fitting(struct intel_crtc *crtc,
> > +bool intel_gmch_panel_fitting(struct intel_crtc *crtc,
> >  			      struct intel_crtc_config *pipe_config,
> >  			      int fitting_mode);
> 
> Only make significnt changes like this to one interface at a time.
Fine will split this patch in 2 parts.
> -Chris
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..f764b74 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10631,9 +10631,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 912e9c4..6117639 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -824,12 +824,15 @@  intel_dp_compute_config(struct intel_encoder *encoder,
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
 				       adjusted_mode);
-		if (!HAS_PCH_SPLIT(dev))
-			intel_gmch_panel_fitting(intel_crtc, pipe_config,
-						 intel_connector->panel.fitting_mode);
-		else
-			intel_pch_panel_fitting(intel_crtc, pipe_config,
-						intel_connector->panel.fitting_mode);
+		if (!HAS_PCH_SPLIT(dev)) {
+			if (!intel_gmch_panel_fitting(intel_crtc, pipe_config,
+						 intel_connector->panel.fitting_mode))
+				return false;
+		} else {
+			if (!intel_pch_panel_fitting(intel_crtc, pipe_config,
+						 intel_connector->panel.fitting_mode))
+				return false;
+		}
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
@@ -3782,7 +3785,7 @@  intel_dp_set_property(struct drm_connector *connector,
 
 done:
 	if (intel_encoder->base.crtc)
-		intel_crtc_restore_mode(intel_encoder->base.crtc);
+		return intel_crtc_restore_mode(intel_encoder->base.crtc);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ab5962b..860d531 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -727,7 +727,7 @@  void intel_mark_busy(struct drm_device *dev);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_engine_cs *ring);
 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);
@@ -918,10 +918,10 @@  int intel_panel_init(struct intel_panel *panel,
 void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
-void intel_pch_panel_fitting(struct intel_crtc *crtc,
+bool intel_pch_panel_fitting(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config,
 			     int fitting_mode);
-void intel_gmch_panel_fitting(struct intel_crtc *crtc,
+bool intel_gmch_panel_fitting(struct intel_crtc *crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode);
 void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 318b150..2551b62 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1128,7 +1128,7 @@  intel_hdmi_set_property(struct drm_connector *connector,
 
 done:
 	if (intel_dig_port->base.base.crtc)
-		intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
+		return intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 2312602..ca251a0 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -303,12 +303,13 @@  static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	if (HAS_PCH_SPLIT(dev)) {
 		pipe_config->has_pch_encoder = true;
 
-		intel_pch_panel_fitting(intel_crtc, pipe_config,
-					intel_connector->panel.fitting_mode);
+		if (!intel_pch_panel_fitting(intel_crtc, pipe_config,
+					intel_connector->panel.fitting_mode))
+			return false;
 	} else {
-		intel_gmch_panel_fitting(intel_crtc, pipe_config,
-					 intel_connector->panel.fitting_mode);
-
+		if (!intel_gmch_panel_fitting(intel_crtc, pipe_config,
+					intel_connector->panel.fitting_mode))
+			return false;
 	}
 
 	/*
@@ -499,7 +500,7 @@  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);
+			return intel_crtc_restore_mode(crtc);
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 38a9857..e605006 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -96,7 +96,7 @@  intel_find_panel_downclock(struct drm_device *dev,
 }
 
 /* adjusted_mode has been preset to be the panel's fixed mode */
-void
+bool
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 			struct intel_crtc_config *pipe_config,
 			int fitting_mode)
@@ -158,13 +158,14 @@  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	default:
 		WARN(1, "bad panel fit mode: %d\n", fitting_mode);
-		return;
+		return false;
 	}
 
 done:
 	pipe_config->pch_pfit.pos = (x << 16) | y;
 	pipe_config->pch_pfit.size = (width << 16) | height;
 	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+	return true;
 }
 
 static void
@@ -300,7 +301,7 @@  static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
-void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
+bool intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
 {
@@ -352,7 +353,7 @@  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		break;
 	default:
 		WARN(1, "bad panel fit mode: %d\n", fitting_mode);
-		return;
+		return false;
 	}
 
 	/* 965+ wants fuzzy fitting */
@@ -374,6 +375,7 @@  out:
 	pipe_config->gmch_pfit.control = pfit_control;
 	pipe_config->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
 	pipe_config->gmch_pfit.lvds_border_bits = border;
+	return true;
 }
 
 enum drm_connector_status
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 20375cc..5628cfd 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2223,7 +2223,7 @@  set_value:
 
 done:
 	if (intel_sdvo->base.base.crtc)
-		intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
+		return intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
 
 	return 0;
 #undef CHECK_PROPERTY
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 67c6c9a..7c04066 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1490,7 +1490,7 @@  intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 	}
 
 	if (changed && crtc)
-		intel_crtc_restore_mode(crtc);
+		return intel_crtc_restore_mode(crtc);
 out:
 	return ret;
 }