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

[Chris Wilson <chris@chris-wilson.co.uk>]

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

-- 
Chris Wilson, Intel Open Source Technology Centre