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

2015-10-20 Thread Kumar, Shobhit 

On 10/20/2015 04:49 PM, Ville Syrjälä wrote:

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 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
   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().


Yeah, I misunderstood your comment earlier.




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_ini

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

2015-10-20 Thread Ville Syrjälä 
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 
> >> Cc: Ville Syrjälä 
> >> Signed-off-by: Shobhit Kumar 
> >> ---
> >>   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/i91

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

2015-10-20 Thread Kumar, Shobhit 

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 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
  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,

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

2015-10-19 Thread Ville Syrjälä 
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 
> Cc: Ville Syrjälä 
> Signed-off-by: Shobhit Kumar 
> ---
>  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?

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

> + 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-10-19 Thread Kumar, Shobhit 

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 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
  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

[Intel-gfx] [v3] drm/i915/skl: If needed sanitize bios programmed cdclk

2015-10-16 Thread Shobhit Kumar 
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 
Cc: Ville Syrjälä 
Signed-off-by: Shobhit Kumar 
---
 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);
-- 
2.4.3

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