Re: [PATCH 1/3] drm/i915: make FDI training a display function

[Ben Widawsky <ben@bwidawsk.net>]

On Thu, Apr 07, 2011 at 02:32:58PM -0700, Jesse Barnes wrote:
> Rather than branching in ironlake_pch_enable, add a new train_fdi
> function to the display function pointer struct and use it instead.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    7 +++----
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5004724..b4116ae 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -203,6 +203,7 @@ struct drm_i915_display_funcs {
>  	int (*get_display_clock_speed)(struct drm_device *dev);
>  	int (*get_fifo_size)(struct drm_device *dev, int plane);
>  	void (*update_wm)(struct drm_device *dev);
> +	void (*train_fdi)(struct drm_crtc *crtc);
>  	/* clock updates for mode set */
>  	/* cursor updates */
>  	/* render clock increase/decrease */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 432fc04..9055eff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2757,10 +2757,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	u32 reg, temp;
>  
>  	/* For PCH output, training FDI link */
> -	if (IS_GEN6(dev))
> -		gen6_fdi_link_train(crtc);
> -	else
> -		ironlake_fdi_link_train(crtc);
> +	dev_priv->display.train_fdi(crtc);
>  
>  	intel_enable_pch_pll(dev_priv, pipe);
>  
> @@ -7270,6 +7267,7 @@ static void intel_init_display(struct drm_device *dev)
>  					      "Disable CxSR\n");
>  				dev_priv->display.update_wm = NULL;
>  			}
> +			dev_priv->display.train_fdi = ironlake_fdi_link_train;
>  		} else if (IS_GEN6(dev)) {
>  			if (SNB_READ_WM0_LATENCY()) {
>  				dev_priv->display.update_wm = sandybridge_update_wm;
> @@ -7278,6 +7276,7 @@ static void intel_init_display(struct drm_device *dev)
>  					      "Disable CxSR\n");
>  				dev_priv->display.update_wm = NULL;
>  			}
> +			dev_priv->display.train_fdi = gen6_fdi_link_train;
>  		} else
>  			dev_priv->display.update_wm = NULL;
>  	} else if (IS_PINEVIEW(dev)) {

I prefer when the function pointer is named similarly to the function.
Makes it easier to read/find code.

(*fdi_link_train)(struct drm_crtc *crtc);
OR
(*link_train)(struct drm_crtc *crtc);