[Intel-gfx] [PATCH 1/2] drm/i915: Create crtc_clock_get function pointers

[Daniel Vetter]

On Thu, Oct 15, 2015 at 06:52:48PM +0530, Vandana Kannan wrote:
> There are separate functions i9xx_crtc_clock_get(), vlv_crtc_clock_get(),
> chv_crtc_clock_get(). instead of calling these using if-else, making
> func pointers. This will also be useful going forward when the implementation
> for BXT is done.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan at intel.com>

I general I much prefer direct callers if there's only one caller ever.
The per-platform modeset code is already really hard to follow, no need to
insert even more indirection imo.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++--------
>  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8afda45..773f507 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -607,6 +607,8 @@ struct intel_limit;
>  struct dpll;
>  
>  struct drm_i915_display_funcs {
> +	void (*crtc_clock_get)(struct intel_crtc *crtc,
> +			struct intel_crtc_state *pipe_config);
>  	int (*get_display_clock_speed)(struct drm_device *dev);
>  	int (*get_fifo_size)(struct drm_device *dev, int plane);
>  	/**
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c28fb6a..d98385e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8130,12 +8130,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  						     DPLL_PORTB_READY_MASK);
>  	}
>  
> -	if (IS_CHERRYVIEW(dev))
> -		chv_crtc_clock_get(crtc, pipe_config);
> -	else if (IS_VALLEYVIEW(dev))
> -		vlv_crtc_clock_get(crtc, pipe_config);
> -	else
> -		i9xx_crtc_clock_get(crtc, pipe_config);
> +	dev_priv->display.crtc_clock_get(crtc, pipe_config);
>  
>  	/*
>  	 * Normally the dotclock is filled in by the encoder .get_config()
> @@ -10579,9 +10574,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	/* read out port_clock from the DPLL */
> -	i9xx_crtc_clock_get(crtc, pipe_config);
> +	dev_priv->display.crtc_clock_get(crtc, &pipe_config);
>  
>  	/*
>  	 * This value does not include pixel_multiplier.
> @@ -10625,7 +10621,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
>  	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
>  	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> -	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> +	dev_priv->display.crtc_clock_get(intel_crtc, &pipe_config);
>  
>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
>  	mode->hdisplay = (htot & 0xffff) + 1;
> @@ -14413,6 +14409,7 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_disable = haswell_crtc_disable;
>  		dev_priv->display.update_primary_plane =
>  			skylake_update_primary_plane;
> +		dev_priv->display.crtc_clock_get = i9xx_crtc_clock_get;
>  	} else if (HAS_DDI(dev)) {
>  		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14423,6 +14420,7 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_disable = haswell_crtc_disable;
>  		dev_priv->display.update_primary_plane =
>  			ironlake_update_primary_plane;
> +		dev_priv->display.crtc_clock_get = i9xx_crtc_clock_get;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14433,6 +14431,7 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
>  		dev_priv->display.update_primary_plane =
>  			ironlake_update_primary_plane;
> +		dev_priv->display.crtc_clock_get = i9xx_crtc_clock_get;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14442,6 +14441,10 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  		dev_priv->display.update_primary_plane =
>  			i9xx_update_primary_plane;
> +		if (IS_CHERRYVIEW(dev))
> +			dev_priv->display.crtc_clock_get = chv_crtc_clock_get;
> +		else
> +			dev_priv->display.crtc_clock_get = vlv_crtc_clock_get;
>  	} else {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14451,6 +14454,7 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  		dev_priv->display.update_primary_plane =
>  			i9xx_update_primary_plane;
> +		dev_priv->display.crtc_clock_get = i9xx_crtc_clock_get;
>  	}
>  
>  	/* Returns the core display clock speed */
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch