[PATCH] drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

[PATCH] drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

[Gary Wang]

Since BIOS RC 1.4 it would enable CDCLK PLL during BIOS S3 resume, then
driver needs to set CDCLK to avoid display corruption if DPLL0 enabled.

References: https://bugs.freedesktop.org/show_bug.cgi?id=91697
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
Reviewed-by: Wei Shun Chang <wei.shun.chang@intel.com>
Tested-by: Gary Wang <gary.c.wang@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Gavin Hindman <gavin.hindman@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Xiong Y Zhang <xiong.y.zhang@intel.com>
Signed-off-by: Gary Wang <gary.c.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
 mode change 100644 => 100755 drivers/gpu/drm/i915/intel_display.c

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
old mode 100644
new mode 100755
index f604ce1..617d1d8
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5707,16 +5707,13 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
 	/* enable PG1 and Misc I/O */
 	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
 
-	/* DPLL0 already enabed !? */
-	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
-		DRM_DEBUG_DRIVER("DPLL0 already running\n");
-		return;
+	/* DPLL0 not enabed !? */
+	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
+		/* enable DPLL0 */
+		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
+		skl_dpll0_enable(dev_priv, required_vco);
 	}
 
-	/* enable DPLL0 */
-	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
-	skl_dpll0_enable(dev_priv, required_vco);
-
 	/* set CDCLK to the frequency the BIOS chose */
 	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

[Damien Lespiau]

On Fri, Aug 28, 2015 at 04:40:34PM +0800, Gary Wang wrote:
> Since BIOS RC 1.4 it would enable CDCLK PLL during BIOS S3 resume, then
> driver needs to set CDCLK to avoid display corruption if DPLL0 enabled.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91697
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
> Reviewed-by: Wei Shun Chang <wei.shun.chang@intel.com>
> Tested-by: Gary Wang <gary.c.wang@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Gavin Hindman <gavin.hindman@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Xiong Y Zhang <xiong.y.zhang@intel.com>
> Signed-off-by: Gary Wang <gary.c.wang@intel.com>

Hum I had not given my r-b tag before this mail, r-b tags cannot be
forged by the patch author but needs to be explicitly given by someone.
In any case:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

We could probably remove the '!?' as this is now the expected behaviour
with recent firmware.

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/i915/intel_display.c
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> old mode 100644
> new mode 100755
> index f604ce1..617d1d8
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5707,16 +5707,13 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  	/* enable PG1 and Misc I/O */
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  
> -	/* DPLL0 already enabed !? */
> -	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
> -		DRM_DEBUG_DRIVER("DPLL0 already running\n");
> -		return;
> +	/* DPLL0 not enabed !? */
> +	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
> +		/* enable DPLL0 */
> +		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> +		skl_dpll0_enable(dev_priv, required_vco);
>  	}
>  
> -	/* enable DPLL0 */
> -	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> -	skl_dpll0_enable(dev_priv, required_vco);
> -
>  	/* set CDCLK to the frequency the BIOS chose */
>  	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 3097 bytes --]

On Fri, Aug 28, 2015 at 6:12 AM Damien Lespiau <damien.lespiau@intel.com>
wrote:

> On Fri, Aug 28, 2015 at 04:40:34PM +0800, Gary Wang wrote:
> > Since BIOS RC 1.4 it would enable CDCLK PLL during BIOS S3 resume, then
> > driver needs to set CDCLK to avoid display corruption if DPLL0 enabled.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=91697
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
> > Reviewed-by: Wei Shun Chang <wei.shun.chang@intel.com>
> > Tested-by: Gary Wang <gary.c.wang@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Gavin Hindman <gavin.hindman@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Xiong Y Zhang <xiong.y.zhang@intel.com>
> > Signed-off-by: Gary Wang <gary.c.wang@intel.com>
>
> Hum I had not given my r-b tag before this mail, r-b tags cannot be
> forged by the patch author but needs to be explicitly given by someone.
> In any case:
>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>
> We could probably remove the '!?' as this is now the expected behaviour
> with recent firmware.
>

totally agree with Damien and here it is my one:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


>
> --
> Damien
>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> >  mode change 100644 => 100755 drivers/gpu/drm/i915/intel_display.c
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > old mode 100644
> > new mode 100755
> > index f604ce1..617d1d8
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5707,16 +5707,13 @@ void skl_init_cdclk(struct drm_i915_private
> *dev_priv)
> >       /* enable PG1 and Misc I/O */
> >       intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> >
> > -     /* DPLL0 already enabed !? */
> > -     if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
> > -             DRM_DEBUG_DRIVER("DPLL0 already running\n");
> > -             return;
> > +     /* DPLL0 not enabed !? */
> > +     if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
> > +             /* enable DPLL0 */
> > +             required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> > +             skl_dpll0_enable(dev_priv, required_vco);
> >       }
> >
> > -     /* enable DPLL0 */
> > -     required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> > -     skl_dpll0_enable(dev_priv, required_vco);
> > -
> >       /* set CDCLK to the frequency the BIOS chose */
> >       skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 5241 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: set CDCLK if DPLL0 enabled during resuming from S3

[Jani Nikula]

On Fri, 28 Aug 2015, Gary Wang <gary.c.wang@intel.com> wrote:
> Since BIOS RC 1.4 it would enable CDCLK PLL during BIOS S3 resume, then
> driver needs to set CDCLK to avoid display corruption if DPLL0 enabled.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91697
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
> Reviewed-by: Wei Shun Chang <wei.shun.chang@intel.com>
> Tested-by: Gary Wang <gary.c.wang@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Gavin Hindman <gavin.hindman@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Xiong Y Zhang <xiong.y.zhang@intel.com>
> Signed-off-by: Gary Wang <gary.c.wang@intel.com>

Pushed to drm-intel-next-fixes, with the comment fixed per Damien's
comment. Also fixed the accidental mode change on the file; you can
avoid errors like this by running checkpatch. Thanks for the patch and
review.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/i915/intel_display.c
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> old mode 100644
> new mode 100755
> index f604ce1..617d1d8
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5707,16 +5707,13 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  	/* enable PG1 and Misc I/O */
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  
> -	/* DPLL0 already enabed !? */
> -	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
> -		DRM_DEBUG_DRIVER("DPLL0 already running\n");
> -		return;
> +	/* DPLL0 not enabed !? */
> +	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
> +		/* enable DPLL0 */
> +		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> +		skl_dpll0_enable(dev_priv, required_vco);
>  	}
>  
> -	/* enable DPLL0 */
> -	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> -	skl_dpll0_enable(dev_priv, required_vco);
> -
>  	/* set CDCLK to the frequency the BIOS chose */
>  	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
>  
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx