On 10/19/2015 07:18 PM, Ville Syrjälä wrote:
On Fri, Oct 16, 2015 at 06:48:53PM +0530, 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)

Cc: Imre Deak <imre.d...@intel.com>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Shobhit Kumar <shobhit.ku...@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");

Since skl_sanitize_cdclk() will enable the PLL this shouldn't
happen anymore, right?


Yeah right. Will remove.

+                       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;

Should be '= dev_priv->max_cdclk'

I think we end up doing the intel_update_cdclk() before this gets
called, so max_cdclk should already contain something sensible. The
whole init sequence is a bit messy currently, but I think we can put
off cleaning it up after the dc6 stuff gets sorted.


Actually at the end of skl_init_cdclock, intel_update_cdclk will be called and initialize the max_cdclk correctly. In case there was no sanitization needed, the BIOS programmed cdclk is stored in skl_boot_cdclk and that is used inside the skl_init_cdclk. Same is invoked resume time, so we need to update this variable or else change the logic.

+       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);
--
2.4.3

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

Reply via email to