Re: [PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 31, 2012 at 05:50:17PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Split the i9xx clock stuff out from i9xx_compute_clocks().
> 
> Only compile tested!
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'm working on a massive overhaul of the clock computation madness, so
that we do all that stuff before we start to touch the hw (and so would
finally have a reasonable chance at getting global bw issues right).
Current wip pile is at:

http://cgit.freedesktop.org/~danvet/drm/log/?h=modeset-rework

Until we've figured out what's the best approach I prefere if we keep the
churn in this (rather convoluted code) low.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |  128 ++++++++++++++++++++--------------
>  1 files changed, 75 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6d3feea..b42637b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4557,6 +4557,76 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
>  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
>  }
>  
> +static bool i9xx_compute_clocks(struct drm_crtc *crtc,
> +				struct drm_display_mode *adjusted_mode,
> +				intel_clock_t *clock,
> +				bool *has_reduced_clock,
> +				intel_clock_t *reduced_clock,
> +				int *refclk, int *num_connectors, bool *is_dp)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *encoder;
> +	const intel_limit_t *limit;
> +	bool ok, is_sdvo = false, is_tv = false, is_lvds = false;
> +
> +	*num_connectors = 0;
> +
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		switch (encoder->type) {
> +		case INTEL_OUTPUT_LVDS:
> +			is_lvds = true;
> +			break;
> +		case INTEL_OUTPUT_SDVO:
> +		case INTEL_OUTPUT_HDMI:
> +			is_sdvo = true;
> +			if (encoder->needs_tv_clock)
> +				is_tv = true;
> +			break;
> +		case INTEL_OUTPUT_TVOUT:
> +			is_tv = true;
> +			break;
> +		case INTEL_OUTPUT_DISPLAYPORT:
> +			*is_dp = true;
> +			break;
> +		}
> +
> +		(*num_connectors)++;
> +	}
> +
> +	*refclk = i9xx_get_refclk(crtc, *num_connectors);
> +
> +	/*
> +	 * Returns a set of divisors for the desired target clock with the given
> +	 * refclk, or FALSE.  The returned values represent the clock equation:
> +	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> +	 */
> +	limit = intel_limit(crtc, *refclk);
> +	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, *refclk, NULL,
> +			      clock);
> +	if (!ok)
> +		return false;
> +
> +	if (is_lvds && dev_priv->lvds_downclock_avail) {
> +		/*
> +		 * Ensure we match the reduced clock's P to the target clock.
> +		 * If the clocks don't match, we can't switch the display clock
> +		 * by using the FP0/FP1. In such case we will disable the LVDS
> +		 * downclock feature.
> +		*/
> +		*has_reduced_clock = limit->find_pll(limit, crtc,
> +						     dev_priv->lvds_downclock,
> +						     *refclk,
> +						     clock,
> +						     reduced_clock);
> +	}
> +
> +	if (is_sdvo && is_tv)
> +		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> +
> +	return true;
> +}
> +
>  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  			      struct drm_display_mode *mode,
>  			      struct drm_display_mode *adjusted_mode,
> @@ -4571,44 +4641,13 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
>  	u32 dspcntr, pipeconf;
> -	bool ok, has_reduced_clock = false, is_sdvo = false;
> -	bool is_lvds = false, is_tv = false, is_dp = false;
> -	struct intel_encoder *encoder;
> -	const intel_limit_t *limit;
> +	bool ok, has_reduced_clock = false;
> +	bool is_dp = false;
>  	int ret;
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder) {
> -		switch (encoder->type) {
> -		case INTEL_OUTPUT_LVDS:
> -			is_lvds = true;
> -			break;
> -		case INTEL_OUTPUT_SDVO:
> -		case INTEL_OUTPUT_HDMI:
> -			is_sdvo = true;
> -			if (encoder->needs_tv_clock)
> -				is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_TVOUT:
> -			is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
> -		}
> -
> -		num_connectors++;
> -	}
> -
> -	refclk = i9xx_get_refclk(crtc, num_connectors);
> -
> -	/*
> -	 * Returns a set of divisors for the desired target clock with the given
> -	 * refclk, or FALSE.  The returned values represent the clock equation:
> -	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> -	 */
> -	limit = intel_limit(crtc, refclk);
> -	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, NULL,
> -			     &clock);
> +	ok = i9xx_compute_clocks(crtc, adjusted_mode, &clock,
> +				 &has_reduced_clock, &reduced_clock,
> +				 &refclk, &num_connectors, &is_dp);
>  	if (!ok) {
>  		DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  		return -EINVAL;
> @@ -4617,23 +4656,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	/* Ensure that the cursor is valid for the new mode before changing... */
>  	intel_crtc_update_cursor(crtc, true);
>  
> -	if (is_lvds && dev_priv->lvds_downclock_avail) {
> -		/*
> -		 * Ensure we match the reduced clock's P to the target clock.
> -		 * If the clocks don't match, we can't switch the display clock
> -		 * by using the FP0/FP1. In such case we will disable the LVDS
> -		 * downclock feature.
> -		*/
> -		has_reduced_clock = limit->find_pll(limit, crtc,
> -						    dev_priv->lvds_downclock,
> -						    refclk,
> -						    &clock,
> -						    &reduced_clock);
> -	}
> -
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> -
>  	if (IS_GEN2(dev))
>  		i8xx_update_pll(crtc, adjusted_mode, &clock,
>  				has_reduced_clock ? &reduced_clock : NULL,
> -- 
> 1.7.8.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch