Re: [v2] drm/i915/skl: Init cdclk in the driver rather than relying on pre-os

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On to, 2015-10-08 at 09:58 +0530, Shobhit Kumar wrote:
> Reuse what is programmed by pre-os, but in case there is no pre-os
> initialization, init the cdclk with the max value by default untill
> dynamic cdclk support comes.
> 
> v2: Check if BIOS programmed correctly rather than always calling init
>     - Do validation of programmed cdctl and what it is expected
>     - Only do slk_init_cdclk if validation failed else reuse BIOS
>       programmed value
> 
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++-----
>  drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++---------
>  2 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2d3cc82..3ec5618 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2946,11 +2946,19 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  		int cdclk_freq;
>  
>  		cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> -		dev_priv->skl_boot_cdclk = cdclk_freq;
> -		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> -			DRM_ERROR("LCPLL1 is disabled\n");
> -		else
> -			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +
> +		/* Invalid CDCLK from BIOS ? */
> +		if (cdclk_freq < 0) {
> +			/* program to maximum cdclk till we have dynamic cdclk support */
> +			dev_priv->skl_boot_cdclk = 675000;
> +			skl_init_cdclk(dev_priv);

This would still try to reprogram CDCLK if BIOS enabled an output with
an incorrect CDCLK decimal part. Isn't this the exact scenario you're
seeing? As said before reprogramming CDCLK in that case would require
disabling the outputs first.

We would also need to call skl_init_cdclk if the BIOS hasn't enabled the
PLL for some reason. But I guess that could be a separate change.

> +		} else {
> +			dev_priv->skl_boot_cdclk = cdclk_freq;
> +			if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> +				DRM_ERROR("LCPLL1 is disabled\n");
> +			else
> +				intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +		}
>  	} else if (IS_BROXTON(dev)) {
>  		broxton_init_cdclk(dev);
>  		broxton_ddi_phy_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bbeb6d3..f734410 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6634,12 +6634,15 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
>  	uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
>  	uint32_t cdctl = I915_READ(CDCLK_CTL);
>  	uint32_t linkrate;
> +	int freq;
>  
>  	if (!(lcpll1 & LCPLL_PLL_ENABLE))
>  		return 24000; /* 24MHz is the cd freq with NSSC ref */
>  
> -	if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540)
> -		return 540000;
> +	if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540) {
> +		freq = 540000;
> +		goto verify;
> +	}
>  
>  	linkrate = (I915_READ(DPLL_CTRL1) &
>  		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
> @@ -6649,30 +6652,46 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
>  		/* vco 8640 */
>  		switch (cdctl & CDCLK_FREQ_SEL_MASK) {
>  		case CDCLK_FREQ_450_432:
> -			return 432000;
> +			freq = 432000;
> +			break;
>  		case CDCLK_FREQ_337_308:
> -			return 308570;
> +			freq = 308570;
> +			break;
>  		case CDCLK_FREQ_675_617:
> -			return 617140;
> +			freq = 617140;
> +			break;
>  		default:
>  			WARN(1, "Unknown cd freq selection\n");
> +			return -EINVAL;
>  		}
>  	} else {
>  		/* vco 8100 */
>  		switch (cdctl & CDCLK_FREQ_SEL_MASK) {
>  		case CDCLK_FREQ_450_432:
> -			return 450000;
> +			freq = 450000;
> +			break;
>  		case CDCLK_FREQ_337_308:
> -			return 337500;
> +			freq = 337500;
> +			break;
>  		case CDCLK_FREQ_675_617:
> -			return 675000;
> +			freq = 675000;
> +			break;
>  		default:
>  			WARN(1, "Unknown cd freq selection\n");
> +			return -EINVAL;
>  		}
>  	}
>  
> -	/* error case, do as if DPLL0 isn't enabled */
> -	return 24000;
> +verify:
> +	/*
> +	 * Noticed in some instances that the freq selection is correct but
> +	 * decimal part is programmed wrong from BIOS where pre-os does not
> +	 * enable display. Verify the same as well.
> +	 */
> +	if (cdctl == ((cdctl & CDCLK_FREQ_SEL_MASK) | skl_cdclk_decimal(freq)))
> +		return freq;
> +	else
> +		return -EINVAL;
>  }
>  
>  static int broxton_get_display_clock_speed(struct drm_device *dev)


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux