Re: [v3] drm/i915/skl: If needed sanitize bios programmed cdclk

["Kumar, Shobhit" <shobhit.kumar@linux.intel.com>]

On 10/16/2015 06:48 PM, Shobhit Kumar wrote:
> Especially in cases where pre-os does not enable display, cdclk might
> not be in sane state. During sanitization initialize cdclk with maximum
> value till we get dynamic cdclk support.
>
> 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
>
> v3: Move the validation logic in a separate sanitize function (Ville)

Ville, does  this come even close to what you had in mind ?

>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c     | 12 ++++++++----
>   drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_drv.h     |  1 +
>   3 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b25e99a..86d43e6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2949,10 +2949,14 @@ void intel_ddi_pll_init(struct drm_device *dev)
>
>   		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);
> +		if (skl_sanitize_cdclk(dev_priv))
> +			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
> +		else {
> +			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 5f37f84..98333d3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5784,6 +5784,37 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>   		DRM_ERROR("DBuf power enable timeout\n");
>   }
>
> +int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
> +	uint32_t cdctl = I915_READ(CDCLK_CTL);
> +	int freq = dev_priv->skl_boot_cdclk;
> +
> +	/* Is PLL enabled and locked ? */
> +	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
> +		goto sanitize;
> +
> +	/* DPLL okay; verify the cdclock
> +	 *
> +	 * 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)))
> +		/* All well; nothing to sanitize */
> +		return false;
> +sanitize:
> +	/*
> +	 * As of now initialize with max cdclk till
> +	 * we get dynamic cdclk support
> +	 * */
> +	dev_priv->skl_boot_cdclk = 675000;
> +	skl_init_cdclk(dev_priv);
> +
> +	/* we did have to sanitize */
> +	return true;
> +}
> +
>   /* Adjust CDclk dividers to allow high res or save power if possible */
>   static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>   {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0598932..ec10e6a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1152,6 +1152,7 @@ void broxton_ddi_phy_uninit(struct drm_device *dev);
>   void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>   void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>   void skl_init_cdclk(struct drm_i915_private *dev_priv);
> +int skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
>   void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
>   void intel_dp_get_m_n(struct intel_crtc *crtc,
>   		      struct intel_crtc_state *pipe_config);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx