Re: [PATCH 4/7] drm/i915: extend lpt_enable_clkout_dp

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, Jul 12, 2013 at 02:19:39PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Now it implements 3 different sequences from BSpec and also has
> support for ULT.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 41 +++++++++++++++++++++++++-----------
>  2 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc3d6a7..be6164f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4863,6 +4863,8 @@
>  #define   SBI_SSCAUXDIV_FINALDIV2SEL(x)		((x)<<4)
>  #define  SBI_DBUFF0				0x2a00
>  #define   SBI_DBUFF0_ENABLE			(1<<0)
> +#define  SBI_GEN0				0x1f00
> +#define   SBI_GEN0_ENABLE			(1<<0)

bikeshed: I wouldn't haved bothered defining SBI_GEN0_ENABLE

>  
>  /* LPT PIXCLK_GATE */
>  #define PIXCLK_GATE			0xC6020
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f4c5263..5f3b636 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5258,12 +5258,20 @@ static void lpt_program_fdi_mphy(struct drm_i915_private *dev_priv)
>  	intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY);
>  }
>  
> -/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
> -static void lpt_enable_clkout_dp(struct drm_device *dev)
> +/* Implements 3 different sequences from BSpec chapter "Display iCLK
> + * Programming" based on the parameters passed:
> + * - Sequence to enable CLKOUT_DP
> + * - Sequence to enable CLKOUT_DP without spread
> + * - Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O
> + */
> +static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
> +				 bool with_fdi)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t tmp;
>  
> +	WARN(with_fdi && !with_spread, "FDI requires downspread\n");
> +

WARN_ON(with_fdi && IS_ULT(dev))?
Should we proceed after the WARN, or break out early?

>  	mutex_lock(&dev_priv->dpio_lock);
>  
>  	tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
> @@ -5273,17 +5281,26 @@ static void lpt_enable_clkout_dp(struct drm_device *dev)
>  
>  	udelay(24);
>  
> -	tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
> -	tmp &= ~SBI_SSCCTL_PATHALT;
> -	intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
> +	if (with_spread) {
> +		tmp = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
> +		tmp &= ~SBI_SSCCTL_PATHALT;
> +		intel_sbi_write(dev_priv, SBI_SSCCTL, tmp, SBI_ICLK);
>  
> -	lpt_reset_fdi_mphy(dev_priv);
> -	lpt_program_fdi_mphy(dev_priv);
> +		if (with_fdi) {
> +			lpt_reset_fdi_mphy(dev_priv);
> +			lpt_program_fdi_mphy(dev_priv);
> +		}
> +	}
>  
> -	/* ULT uses SBI_GEN0, but ULT doesn't have VGA, so we don't care. */
> -	tmp = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
> -	tmp |= SBI_DBUFF0_ENABLE;
> -	intel_sbi_write(dev_priv, SBI_DBUFF0, tmp, SBI_ICLK);
> +	if (IS_ULT(dev)) {
> +		tmp = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
> +		tmp |= SBI_GEN0_ENABLE;
> +		intel_sbi_write(dev_priv, SBI_GEN0, tmp, SBI_ICLK);
> +	} else {
> +		tmp = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
> +		tmp |= SBI_DBUFF0_ENABLE;
> +		intel_sbi_write(dev_priv, SBI_DBUFF0, tmp, SBI_ICLK);
> +	}
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
> @@ -5305,7 +5322,7 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>  	if (!has_vga)
>  		return;
>  
> -	lpt_enable_clkout_dp(dev);
> +	lpt_enable_clkout_dp(dev, true, true);
>  }
>  
>  /*
> -- 
> 1.8.1.2
-- 
Ben Widawsky, Intel Open Source Technology Center