On Tue, Oct 20, 2015 at 03:27:24PM +0530, Kumar, Shobhit wrote:
> 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.deak@xxxxxxxxx>
> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
> >> ---
> >> 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.
It gets called already earlier from intel_modeset_init().
> 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
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
