Re: [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 21, 2016 at 06:00:10PM +0200, Ander Conselvan de Oliveira wrote:
> When calculating clocks, just pass a pointer to crtc_state->dpll
> directly to the find_dpll() hook. Back when this was introduced in
> commit f47709a9502f3 ("drm/i915: create pipe_config->dpll for clock
> state") there was no staged crtc config or atomic crtc state, so it was
> possible to overwrite the current configuration on error. That hasn't
> been the case for a while now, so finally make it "disappear".
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

We'll populate the derived valeus in the crtc_state->dpll now too, but
IIRC the shared dpll code and state checks only use the dpll_hw_state
so this should totally OK.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++------------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0af1e7d..0b6eabf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7871,7 +7871,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int refclk;
> -	intel_clock_t clock;
>  	bool ok;
>  	const intel_limit_t *limit;
>  
> @@ -7893,18 +7892,12 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		limit = intel_limit(crtc_state, refclk);
>  		ok = dev_priv->display.find_dpll(limit, crtc_state,
>  						 crtc_state->port_clock,
> -						 refclk, NULL, &clock);
> +						 refclk, NULL,
> +						 &crtc_state->dpll);
>  		if (!ok) {
>  			DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  			return -EINVAL;
>  		}
> -
> -		/* Compat-code for transition, will disappear. */
> -		crtc_state->dpll.n = clock.n;
> -		crtc_state->dpll.m1 = clock.m1;
> -		crtc_state->dpll.m2 = clock.m2;
> -		crtc_state->dpll.p1 = clock.p1;
> -		crtc_state->dpll.p2 = clock.p2;
>  	}
>  
>  	if (IS_GEN2(dev)) {
> @@ -8925,7 +8918,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *crtc_state)
>  {
> -	intel_clock_t clock, reduced_clock;
> +	intel_clock_t reduced_clock;
>  	u32 dpll = 0, fp = 0, fp2 = 0;
>  	bool has_reduced_clock = false;
>  	struct intel_shared_dpll *pll;
> @@ -8939,20 +8932,13 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  	if (!crtc_state->has_pch_encoder)
>  		return 0;
>  
> -	if (!crtc_state->clock_set) {
> -		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
> -					     &has_reduced_clock,
> -					     &reduced_clock)) {
> -			DRM_ERROR("Couldn't find PLL settings for mode!\n");
> -			return -EINVAL;
> -		}
> -
> -		/* Compat-code for transition, will disappear. */
> -		crtc_state->dpll.n = clock.n;
> -		crtc_state->dpll.m1 = clock.m1;
> -		crtc_state->dpll.m2 = clock.m2;
> -		crtc_state->dpll.p1 = clock.p1;
> -		crtc_state->dpll.p2 = clock.p2;
> +	if (!crtc_state->clock_set &&
> +	    !ironlake_compute_clocks(&crtc->base, crtc_state,
> +				     &crtc_state->dpll,
> +				     &has_reduced_clock,
> +				     &reduced_clock)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
>  	}
>  
>  	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx