Re: [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We have a very nice infrastructure for this now!
>
> Note that the multifunction sdvo support is pretty neatly broken: We
> completely ignore userspace's request for which connector to wire up
> with the encoder and just use whatever the last detect callback has
> seen.
>
> Not something I'll fix in this patch, but unfortunately something
> which is also broken in the DDI code ...
>
> v2: Don't call sdvo_tv_clock twice.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 ------------------------------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1128f83..f10f094 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4286,30 +4286,6 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>  	return refclk;
>  }
>  
> -static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
> -{
> -	unsigned dotclock = crtc->config.adjusted_mode.clock;
> -	struct dpll *clock = &crtc->config.dpll;
> -
> -	/* SDVO TV has fixed PLL values depend on its clock range,
> -	   this mirrors vbios setting. */
> -	if (dotclock >= 100000 && dotclock < 140500) {
> -		clock->p1 = 2;
> -		clock->p2 = 10;
> -		clock->n = 3;
> -		clock->m1 = 16;
> -		clock->m2 = 8;
> -	} else if (dotclock >= 140500 && dotclock <= 200000) {
> -		clock->p1 = 1;
> -		clock->p2 = 10;
> -		clock->n = 6;
> -		clock->m1 = 12;
> -		clock->m2 = 8;
> -	}
> -
> -	crtc->config.clock_set = true;
> -}
> -
>  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
>  {
>  	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
> @@ -4926,9 +4902,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		intel_crtc->config.dpll.p2 = clock.p2;
>  	}
>  
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(intel_crtc);
> -
>  	if (IS_GEN2(dev))
>  		i8xx_update_pll(intel_crtc, adjusted_mode,
>  				has_reduced_clock ? &reduced_clock : NULL,
> @@ -5538,9 +5511,6 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  						     reduced_clock);
>  	}
>  
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
> -

With this change, is_sdvo and is_tv are no longer used, and could be
dropped from the function. All the same,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d154284..f6bf9fc 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1041,6 +1041,32 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>  	return true;
>  }
>  
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config)
> +{
> +	unsigned dotclock = pipe_config->adjusted_mode.clock;
> +	struct dpll *clock = &pipe_config->dpll;
> +
> +	/* SDVO TV has fixed PLL values depend on its clock range,
> +	   this mirrors vbios setting. */
> +	if (dotclock >= 100000 && dotclock < 140500) {
> +		clock->p1 = 2;
> +		clock->p2 = 10;
> +		clock->n = 3;
> +		clock->m1 = 16;
> +		clock->m2 = 8;
> +	} else if (dotclock >= 140500 && dotclock <= 200000) {
> +		clock->p1 = 1;
> +		clock->p2 = 10;
> +		clock->n = 6;
> +		clock->m1 = 12;
> +		clock->m2 = 8;
> +	} else {
> +		WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
> +	}
> +
> +	pipe_config->clock_set = true;
> +}
> +
>  static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  				      struct intel_crtc_config *pipe_config)
>  {
> @@ -1097,6 +1123,10 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  	if (intel_sdvo->color_range)
>  		pipe_config->limited_color_range = true;
>  
> +	/* Clock computation needs to happen after pixel multiplier. */
> +	if (intel_sdvo->is_tv)
> +		i9xx_adjust_sdvo_tv_clock(pipe_config);
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx