[PATCH 00/10] drm/i915/skl: fix display core init/uninit sequence

[PATCH 00/10] drm/i915/skl: fix display core init/uninit sequence

[Imre Deak]

Atm we toggle HW resources handled automatically by the DMC firmware.
This is redundant and also interferes with the firmware's functionality.
This patchset fixes this and also an old existing issue leaving RPM
disabled all the time (see Damien's patch).

The patchset depends on Mika's firmware version blacklisting/capture
[1] and Animesh' firmware loading redesign [2] patchset. Both of these
are reviewed now.

This patchset also relates to Patrik's DC5/DC6 rework patchset [3], but
it's not dependent on it. After discussing with him on IRC I'd suggest the
following merge order:

Patchset [1], patchset [2], Patrik's firmware programming fix from
his patchset [4], this patchset, the rest of Patrik's patchset [3]. Feel
free to suggest a different order.

I tested this on top of [1], [2], [4] on SKL-Y with eDP and DP outputs,
DC5/6, PC9/10 residencies and S3/S4 suspend/resume seemed to work as
expected. The basic D3 igt tests are also passing, as claimed by
Damien's patch. 

[1]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/078898.html

[2]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/079041.html

[3]
http://lists.freedesktop.org/archives/intel-gfx/2015-November/079343.html

[4]
http://lists.freedesktop.org/archives/intel-gfx/2015-November/079349.html


Damien Lespiau (1):
  drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini
    sequences

Imre Deak (9):
  drm/i915: fix the power well ID for always on wells
  drm/i915: fix lookup_power_well for power wells without any domain
  drm/i915: rename intel_power_domains_resume to *_sync_hw
  drm/i915/skl: init/uninit display core as part of the HW power domain
    state
  drm/i915/skl: don't toggle PW1 and MISC power wells on-demand
  drm/i915/gen9: simplify DC toggling code
  drm/i915/skl: disable DC states before display core init/uninit
  drm/i915/skl: make sure LCPLL is disabled when uniniting CDCLK
  drm/i915/skl: remove redundant DDI/IRQ reinitialization during PW1
    enabling

 drivers/gpu/drm/i915/i915_dma.c         |   2 +-
 drivers/gpu/drm/i915/i915_drv.c         |   9 +-
 drivers/gpu/drm/i915/i915_reg.h         |   5 +-
 drivers/gpu/drm/i915/intel_ddi.c        |   4 +-
 drivers/gpu/drm/i915/intel_display.c    |  24 +---
 drivers/gpu/drm/i915/intel_drv.h        |   5 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 208 ++++++++++++++++++++------------
 7 files changed, 149 insertions(+), 108 deletions(-)

-- 
2.1.4

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

[PATCH 01/10] drm/i915: fix the power well ID for always on wells

[Imre Deak]

lookup_power_well() expects uniq power well IDs, but atm we have
uninitialized IDs which would clash with those power wells with a 0
ID. This wasn't a problem so far since nothing looked up such a power
well, but an upcoming patch will (Misc IO for SKL), so fix this up on
platforms where this matters.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 4 +++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72bbed2..c103f8d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -621,7 +621,7 @@ enum punit_power_well {
 	PUNIT_POWER_WELL_DPIO_RX1		= 11,
 	PUNIT_POWER_WELL_DPIO_CMN_D		= 12,
 
-	PUNIT_POWER_WELL_NUM,
+	PUNIT_POWER_WELL_ALWAYS_ON,
 };
 
 enum skl_disp_power_wells {
@@ -632,6 +632,8 @@ enum skl_disp_power_wells {
 	SKL_DISP_PW_DDI_D,
 	SKL_DISP_PW_1 = 14,
 	SKL_DISP_PW_2,
+
+	SKL_DISP_PW_ALWAYS_ON,
 };
 
 #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3a989a7..fc5552c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1633,6 +1633,7 @@ static struct i915_power_well vlv_power_wells[] = {
 		.always_on = 1,
 		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
 		.ops = &i9xx_always_on_power_well_ops,
+		.data = PUNIT_POWER_WELL_ALWAYS_ON,
 	},
 	{
 		.name = "display",
@@ -1734,6 +1735,7 @@ static struct i915_power_well skl_power_wells[] = {
 		.always_on = 1,
 		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
 		.ops = &i9xx_always_on_power_well_ops,
+		.data = SKL_DISP_PW_ALWAYS_ON,
 	},
 	{
 		.name = "power well 1",
-- 
2.1.4

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

Re: [PATCH 01/10] drm/i915: fix the power well ID for always on wells

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:10PM +0200, Imre Deak wrote:
> lookup_power_well() expects uniq power well IDs, but atm we have
> uninitialized IDs which would clash with those power wells with a 0
> ID. This wasn't a problem so far since nothing looked up such a power
> well, but an upcoming patch will (Misc IO for SKL), so fix this up on
> platforms where this matters.

I think the concept of using these bit positions as IDs for lookup_power_well()
is a bit confusing. "Always on" and later on "DC off" are not bits in any
register so IMO they don't belong in ->data. With that said, I don't think it's
worth fixing right here and now and since we add some comments about this in
later patches I'm ok with this.

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 4 +++-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 72bbed2..c103f8d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -621,7 +621,7 @@ enum punit_power_well {
>  	PUNIT_POWER_WELL_DPIO_RX1		= 11,
>  	PUNIT_POWER_WELL_DPIO_CMN_D		= 12,
>  
> -	PUNIT_POWER_WELL_NUM,
> +	PUNIT_POWER_WELL_ALWAYS_ON,
>  };
>  
>  enum skl_disp_power_wells {
> @@ -632,6 +632,8 @@ enum skl_disp_power_wells {
>  	SKL_DISP_PW_DDI_D,
>  	SKL_DISP_PW_1 = 14,
>  	SKL_DISP_PW_2,
> +
> +	SKL_DISP_PW_ALWAYS_ON,
>  };
>  
>  #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3a989a7..fc5552c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1633,6 +1633,7 @@ static struct i915_power_well vlv_power_wells[] = {
>  		.always_on = 1,
>  		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
>  		.ops = &i9xx_always_on_power_well_ops,
> +		.data = PUNIT_POWER_WELL_ALWAYS_ON,
>  	},
>  	{
>  		.name = "display",
> @@ -1734,6 +1735,7 @@ static struct i915_power_well skl_power_wells[] = {
>  		.always_on = 1,
>  		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
>  		.ops = &i9xx_always_on_power_well_ops,
> +		.data = SKL_DISP_PW_ALWAYS_ON,
>  	},
>  	{
>  		.name = "power well 1",
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915: fix the power well ID for always on wells

[Ville Syrjälä]

On Wed, Nov 04, 2015 at 07:24:10PM +0200, Imre Deak wrote:
> lookup_power_well() expects uniq power well IDs, but atm we have
> uninitialized IDs which would clash with those power wells with a 0
> ID. This wasn't a problem so far since nothing looked up such a power
> well, but an upcoming patch will (Misc IO for SKL), so fix this up on
> platforms where this matters.

I thought we were moving the MISCIO and PW1 out from the power well
framework? I think I'd rather do that than to fabricate stuff for .data.

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 4 +++-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 72bbed2..c103f8d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -621,7 +621,7 @@ enum punit_power_well {
>  	PUNIT_POWER_WELL_DPIO_RX1		= 11,
>  	PUNIT_POWER_WELL_DPIO_CMN_D		= 12,
>  
> -	PUNIT_POWER_WELL_NUM,
> +	PUNIT_POWER_WELL_ALWAYS_ON,
>  };
>  
>  enum skl_disp_power_wells {
> @@ -632,6 +632,8 @@ enum skl_disp_power_wells {
>  	SKL_DISP_PW_DDI_D,
>  	SKL_DISP_PW_1 = 14,
>  	SKL_DISP_PW_2,
> +
> +	SKL_DISP_PW_ALWAYS_ON,
>  };
>  
>  #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3a989a7..fc5552c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1633,6 +1633,7 @@ static struct i915_power_well vlv_power_wells[] = {
>  		.always_on = 1,
>  		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
>  		.ops = &i9xx_always_on_power_well_ops,
> +		.data = PUNIT_POWER_WELL_ALWAYS_ON,
>  	},
>  	{
>  		.name = "display",
> @@ -1734,6 +1735,7 @@ static struct i915_power_well skl_power_wells[] = {
>  		.always_on = 1,
>  		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
>  		.ops = &i9xx_always_on_power_well_ops,
> +		.data = SKL_DISP_PW_ALWAYS_ON,
>  	},
>  	{
>  		.name = "power well 1",
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915: fix the power well ID for always on wells

[Imre Deak]

On to, 2015-11-12 at 15:39 +0200, Ville Syrjälä wrote:
> On Wed, Nov 04, 2015 at 07:24:10PM +0200, Imre Deak wrote:
> > lookup_power_well() expects uniq power well IDs, but atm we have
> > uninitialized IDs which would clash with those power wells with a 0
> > ID. This wasn't a problem so far since nothing looked up such a
> > power
> > well, but an upcoming patch will (Misc IO for SKL), so fix this up
> > on
> > platforms where this matters.
> 
> I thought we were moving the MISCIO and PW1 out from the power well
> framework? I think I'd rather do that than to fabricate stuff for
> .data.

That's a cleaner way yes, but imo we could do that refactoring as a
follow-up, thinking about the other users of lookup_power_well() at the
same time. This change makes sense in any case, since whenever you use
the lookup_power_well function you need to have unique IDs in place.

> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h         | 4 +++-
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 72bbed2..c103f8d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -621,7 +621,7 @@ enum punit_power_well {
> >  	PUNIT_POWER_WELL_DPIO_RX1		= 11,
> >  	PUNIT_POWER_WELL_DPIO_CMN_D		= 12,
> >  
> > -	PUNIT_POWER_WELL_NUM,
> > +	PUNIT_POWER_WELL_ALWAYS_ON,
> >  };
> >  
> >  enum skl_disp_power_wells {
> > @@ -632,6 +632,8 @@ enum skl_disp_power_wells {
> >  	SKL_DISP_PW_DDI_D,
> >  	SKL_DISP_PW_1 = 14,
> >  	SKL_DISP_PW_2,
> > +
> > +	SKL_DISP_PW_ALWAYS_ON,
> >  };
> >  
> >  #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 3a989a7..fc5552c 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1633,6 +1633,7 @@ static struct i915_power_well
> > vlv_power_wells[] = {
> >  		.always_on = 1,
> >  		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
> >  		.ops = &i9xx_always_on_power_well_ops,
> > +		.data = PUNIT_POWER_WELL_ALWAYS_ON,
> >  	},
> >  	{
> >  		.name = "display",
> > @@ -1734,6 +1735,7 @@ static struct i915_power_well
> > skl_power_wells[] = {
> >  		.always_on = 1,
> >  		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> >  		.ops = &i9xx_always_on_power_well_ops,
> > +		.data = SKL_DISP_PW_ALWAYS_ON,
> >  	},
> >  	{
> >  		.name = "power well 1",
> > -- 
> > 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/10] drm/i915: fix lookup_power_well for power wells without any domain

[Imre Deak]

The current lookup code wouldn't find a power well if it's not in any
power domain. There wasn't any power wells before but an upcoming patch
will detach the power domains from power well#1 and the MISC IO power
wells, so fix things up accordingly.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index fc5552c..868c0f2 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -962,10 +962,12 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
 						 int power_well_id)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
-	struct i915_power_well *power_well;
 	int i;
 
-	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
+	for (i = 0; i < power_domains->power_well_count; i++) {
+		struct i915_power_well *power_well;
+
+		power_well = &power_domains->power_wells[i];
 		if (power_well->data == power_well_id)
 			return power_well;
 	}
-- 
2.1.4

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

Re: [PATCH 02/10] drm/i915: fix lookup_power_well for power wells without any domain

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:11PM +0200, Imre Deak wrote:
> The current lookup code wouldn't find a power well if it's not in any
> power domain. There wasn't any power wells before but an upcoming patch
> will detach the power domains from power well#1 and the MISC IO power
> wells, so fix things up accordingly.
> 

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index fc5552c..868c0f2 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -962,10 +962,12 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
>  						 int power_well_id)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> -	struct i915_power_well *power_well;
>  	int i;
>  
> -	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> +	for (i = 0; i < power_domains->power_well_count; i++) {
> +		struct i915_power_well *power_well;
> +
> +		power_well = &power_domains->power_wells[i];
>  		if (power_well->data == power_well_id)
>  			return power_well;
>  	}
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/10] drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini sequences

[Imre Deak]

From: Damien Lespiau <damien.lespiau@intel.com>

Before this patch, we used the intel_display_power_{get,put} functions
to make sure the PW1 and Misc I/O power wells were enabled all the
time while LCPLL was enabled. We called a get() at
intel_ddi_pll_init() when we discovered that LCPLL was enabled, then
we would call put/get at skl_{un,}init_cdclk().

The problem is that skl_uninit_cdclk() is indirectly called by
intel_runtime_suspend(). So it will only release its power well
_after_ we already decided to runtime suspend. But since we only
decide to runtime suspend after all power wells and refcounts are
released, that basically means we will never decide to runtime
suspend.

So what this patch does to fix that problem is move the PW1 + Misc I/O
power well handling out of the runtime PM mechanism: instead of
calling intel_display_power_{get_put} - functions that touch the
refcount -, we'll call the low level intel_power_well_{en,dis}able,
which don't change the refcount. This way, it is now possible for the
refcount to actually reach zero, and we'll now start runtime
suspending/resuming.

v2 (from Paulo):
  - Write a commit message since the original patch left it empty.
  - Rebase after the intel_power_well_{en,dis}able rename.
  - Use lookup_power_well() instead of hardcoded indexes.

Testcase: igt/pm_rpm/rte (and every other rpm test)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c        |  4 ++--
 drivers/gpu/drm/i915/intel_display.c    |  5 +++--
 drivers/gpu/drm/i915/intel_drv.h        |  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b164122..cacc406 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2951,8 +2951,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
 		dev_priv->skl_boot_cdclk = cdclk_freq;
 		if (skl_sanitize_cdclk(dev_priv))
 			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
-		else
-			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
+			DRM_ERROR("LCPLL1 is disabled\n");
 	} 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 7b3cfb6..ef2ebcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5702,7 +5702,8 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 			DRM_ERROR("Couldn't disable DPLL0\n");
 	}
 
-	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
+	/* disable PG1 and Misc I/O */
+	skl_pw1_misc_io_fini(dev_priv);
 }
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
@@ -5715,7 +5716,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
 	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
 
 	/* enable PG1 and Misc I/O */
-	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+	skl_pw1_misc_io_init(dev_priv);
 
 	/* DPLL0 not enabled (happens on early BIOS versions) */
 	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bc744a..3d9d1d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1381,6 +1381,8 @@ void intel_psr_single_frame_update(struct drm_device *dev,
 int intel_power_domains_init(struct drm_i915_private *);
 void intel_power_domains_fini(struct drm_i915_private *);
 void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
+void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
+void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
 
 bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 868c0f2..16691a3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1783,6 +1783,35 @@ static struct i915_power_well skl_power_wells[] = {
 	},
 };
 
+void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_well *well;
+
+	if (!IS_SKYLAKE(dev_priv))
+		return;
+
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
+	intel_power_well_enable(dev_priv, well);
+
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
+	intel_power_well_enable(dev_priv, well);
+}
+
+void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_well *well;
+
+	if (!IS_SKYLAKE(dev_priv))
+		return;
+
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
+	intel_power_well_disable(dev_priv, well);
+
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
+	intel_power_well_disable(dev_priv, well);
+
+}
+
 static struct i915_power_well bxt_power_wells[] = {
 	{
 		.name = "always-on",
-- 
2.1.4

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

Re: [PATCH 03/10] drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini sequences

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:12PM +0200, Imre Deak wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> Before this patch, we used the intel_display_power_{get,put} functions
> to make sure the PW1 and Misc I/O power wells were enabled all the
> time while LCPLL was enabled. We called a get() at
> intel_ddi_pll_init() when we discovered that LCPLL was enabled, then
> we would call put/get at skl_{un,}init_cdclk().
> 
> The problem is that skl_uninit_cdclk() is indirectly called by
> intel_runtime_suspend(). So it will only release its power well
> _after_ we already decided to runtime suspend. But since we only
> decide to runtime suspend after all power wells and refcounts are
> released, that basically means we will never decide to runtime
> suspend.
> 
> So what this patch does to fix that problem is move the PW1 + Misc I/O
> power well handling out of the runtime PM mechanism: instead of
> calling intel_display_power_{get_put} - functions that touch the
> refcount -, we'll call the low level intel_power_well_{en,dis}able,
> which don't change the refcount. This way, it is now possible for the
> refcount to actually reach zero, and we'll now start runtime
> suspending/resuming.
> 
> v2 (from Paulo):
>   - Write a commit message since the original patch left it empty.
>   - Rebase after the intel_power_well_{en,dis}able rename.
>   - Use lookup_power_well() instead of hardcoded indexes.
> 
> Testcase: igt/pm_rpm/rte (and every other rpm test)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c        |  4 ++--
>  drivers/gpu/drm/i915/intel_display.c    |  5 +++--
>  drivers/gpu/drm/i915/intel_drv.h        |  2 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++++++++++++++++
>  4 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b164122..cacc406 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2951,8 +2951,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  		dev_priv->skl_boot_cdclk = cdclk_freq;
>  		if (skl_sanitize_cdclk(dev_priv))
>  			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
> -		else
> -			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> +			DRM_ERROR("LCPLL1 is disabled\n");
>  	} 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 7b3cfb6..ef2ebcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5702,7 +5702,8 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  			DRM_ERROR("Couldn't disable DPLL0\n");
>  	}
>  
> -	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +	/* disable PG1 and Misc I/O */
> +	skl_pw1_misc_io_fini(dev_priv);
>  }
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
> @@ -5715,7 +5716,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
>  
>  	/* enable PG1 and Misc I/O */
> -	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +	skl_pw1_misc_io_init(dev_priv);
>  
>  	/* DPLL0 not enabled (happens on early BIOS versions) */
>  	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5bc744a..3d9d1d3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1381,6 +1381,8 @@ void intel_psr_single_frame_update(struct drm_device *dev,
>  int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
> +void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
> +void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
>  
>  bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 868c0f2..16691a3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1783,6 +1783,35 @@ static struct i915_power_well skl_power_wells[] = {
>  	},
>  };
>  
> +void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_well *well;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_enable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_enable(dev_priv, well);
> +}
> +
> +void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_well *well;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_disable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_disable(dev_priv, well);
> +
> +}
> +
>  static struct i915_power_well bxt_power_wells[] = {
>  	{
>  		.name = "always-on",
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini sequences

[Imre Deak]

On ke, 2015-11-04 at 19:24 +0200, Imre Deak wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> Before this patch, we used the intel_display_power_{get,put}
> functions
> to make sure the PW1 and Misc I/O power wells were enabled all the
> time while LCPLL was enabled. We called a get() at
> intel_ddi_pll_init() when we discovered that LCPLL was enabled, then
> we would call put/get at skl_{un,}init_cdclk().
> 
> The problem is that skl_uninit_cdclk() is indirectly called by
> intel_runtime_suspend(). So it will only release its power well
> _after_ we already decided to runtime suspend. But since we only
> decide to runtime suspend after all power wells and refcounts are
> released, that basically means we will never decide to runtime
> suspend.
> 
> So what this patch does to fix that problem is move the PW1 + Misc
> I/O
> power well handling out of the runtime PM mechanism: instead of
> calling intel_display_power_{get_put} - functions that touch the
> refcount -, we'll call the low level intel_power_well_{en,dis}able,
> which don't change the refcount. This way, it is now possible for the
> refcount to actually reach zero, and we'll now start runtime
> suspending/resuming.
> 
> v2 (from Paulo):
>   - Write a commit message since the original patch left it empty.
>   - Rebase after the intel_power_well_{en,dis}able rename.
>   - Use lookup_power_well() instead of hardcoded indexes.
> 
> Testcase: igt/pm_rpm/rte (and every other rpm test)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92211
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92605

> ---
>  drivers/gpu/drm/i915/intel_ddi.c        |  4 ++--
>  drivers/gpu/drm/i915/intel_display.c    |  5 +++--
>  drivers/gpu/drm/i915/intel_drv.h        |  2 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 29
> +++++++++++++++++++++++++++++
>  4 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index b164122..cacc406 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2951,8 +2951,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  		dev_priv->skl_boot_cdclk = cdclk_freq;
>  		if (skl_sanitize_cdclk(dev_priv))
>  			DRM_DEBUG_KMS("Sanitized cdclk programmed by
> pre-os\n");
> -		else
> -			intel_display_power_get(dev_priv,
> POWER_DOMAIN_PLLS);
> +		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> +			DRM_ERROR("LCPLL1 is disabled\n");
>  	} 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 7b3cfb6..ef2ebcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5702,7 +5702,8 @@ void skl_uninit_cdclk(struct drm_i915_private
> *dev_priv)
>  			DRM_ERROR("Couldn't disable DPLL0\n");
>  	}
>  
> -	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +	/* disable PG1 and Misc I/O */
> +	skl_pw1_misc_io_fini(dev_priv);
>  }
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
> @@ -5715,7 +5716,7 @@ void skl_init_cdclk(struct drm_i915_private
> *dev_priv)
>  	I915_WRITE(HSW_NDE_RSTWRN_OPT, val |
> RESET_PCH_HANDSHAKE_ENABLE);
>  
>  	/* enable PG1 and Misc I/O */
> -	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +	skl_pw1_misc_io_init(dev_priv);
>  
>  	/* DPLL0 not enabled (happens on early BIOS versions) */
>  	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5bc744a..3d9d1d3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1381,6 +1381,8 @@ void intel_psr_single_frame_update(struct
> drm_device *dev,
>  int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
> +void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
> +void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
>  
>  bool intel_display_power_is_enabled(struct drm_i915_private
> *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 868c0f2..16691a3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1783,6 +1783,35 @@ static struct i915_power_well
> skl_power_wells[] = {
>  	},
>  };
>  
> +void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_well *well;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_enable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_enable(dev_priv, well);
> +}
> +
> +void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_well *well;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_disable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_disable(dev_priv, well);
> +
> +}
> +
>  static struct i915_power_well bxt_power_wells[] = {
>  	{
>  		.name = "always-on",
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/10] drm/i915: rename intel_power_domains_resume to *_sync_hw

[Imre Deak]

Give a more proper name to this function.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 16691a3..63ad315 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1907,7 +1907,7 @@ void intel_power_domains_fini(struct drm_i915_private *dev_priv)
 	intel_display_set_init_power(dev_priv, true);
 }
 
-static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
+static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	struct i915_power_well *power_well;
@@ -2063,7 +2063,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
 
 	/* For now, we need the power well to be always enabled. */
 	intel_display_set_init_power(dev_priv, true);
-	intel_power_domains_resume(dev_priv);
+	intel_power_domains_sync_hw(dev_priv);
 	power_domains->initializing = false;
 }
 
-- 
2.1.4

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

Re: [PATCH 04/10] drm/i915: rename intel_power_domains_resume to *_sync_hw

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:13PM +0200, Imre Deak wrote:
> Give a more proper name to this function.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 16691a3..63ad315 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1907,7 +1907,7 @@ void intel_power_domains_fini(struct drm_i915_private *dev_priv)
>  	intel_display_set_init_power(dev_priv, true);
>  }
>  
> -static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
> +static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *power_well;
> @@ -2063,7 +2063,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
>  
>  	/* For now, we need the power well to be always enabled. */
>  	intel_display_set_init_power(dev_priv, true);
> -	intel_power_domains_resume(dev_priv);
> +	intel_power_domains_sync_hw(dev_priv);
>  	power_domains->initializing = false;
>  }
>  
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/10] drm/i915/skl: init/uninit display core as part of the HW power domain state

[Imre Deak]

We need to initialize the display core part early, before initializing
the rest of the display power state. This is also described in the bspec
termed "Display initialization sequence". Atm we run this sequence
during driver loading after power domain HW state initialization which
is too late and during runtime suspend/resume which is unneeded and can
interere with DMC functionality which handles HW resources toggled
by this init/uninit sequence automatically. The init sequence must be
run as the first step of HW power state initialization and during
system resume. The uninit sequence must be run during system suspend.

To address the above move the init sequence to the initial HW power
state setup and the uninit sequence to a new power domains suspend
function called during system suspend.

As part of the init sequence we also have to reprogram the DMC firmware
as it's lost across a system suspend/resume cycle.

After this change CD clock initialization during driver loading will
happen only later after other dependent HW/SW parts are initialized,
while during system resume it will get initialized as the last step of
the init sequence. This distinction can be removed by some refactoring
of platform independent parts. I left this refactoring out from this
series since I didn't want to change non-SKL parts. This is a TODO for
later.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         |  2 +-
 drivers/gpu/drm/i915/i915_drv.c         |  9 ++----
 drivers/gpu/drm/i915/intel_display.c    | 11 -------
 drivers/gpu/drm/i915/intel_drv.h        |  3 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 55 +++++++++++++++++++++++++++++++--
 5 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 71e1323..6e9337f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -396,7 +396,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_switcheroo;
 
-	intel_power_domains_init_hw(dev_priv);
+	intel_power_domains_init_hw(dev_priv, false);
 
 	intel_csr_ucode_init(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 858d58c..5a63f9a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -704,6 +704,8 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
 	struct drm_i915_private *dev_priv = drm_dev->dev_private;
 	int ret;
 
+	intel_power_domains_suspend(dev_priv);
+
 	ret = intel_suspend_complete(dev_priv);
 
 	if (ret) {
@@ -861,7 +863,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 		hsw_disable_pc8(dev_priv);
 
 	intel_uncore_sanitize(dev);
-	intel_power_domains_init_hw(dev_priv);
+	intel_power_domains_init_hw(dev_priv, true);
 
 	return ret;
 }
@@ -1070,8 +1072,6 @@ static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
-	skl_uninit_cdclk(dev_priv);
-
 	if (dev_priv->csr.dmc_payload)
 		skl_enable_dc6(dev_priv);
 
@@ -1122,9 +1122,6 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 	if (dev_priv->csr.dmc_payload)
 		skl_disable_dc6(dev_priv);
 
-	skl_init_cdclk(dev_priv);
-	intel_csr_load_program(dev_priv);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ef2ebcd..d9ed128 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5701,23 +5701,12 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 		if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
 			DRM_ERROR("Couldn't disable DPLL0\n");
 	}
-
-	/* disable PG1 and Misc I/O */
-	skl_pw1_misc_io_fini(dev_priv);
 }
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
 {
-	u32 val;
 	unsigned int required_vco;
 
-	/* enable PCH reset handshake */
-	val = I915_READ(HSW_NDE_RSTWRN_OPT);
-	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
-
-	/* enable PG1 and Misc I/O */
-	skl_pw1_misc_io_init(dev_priv);
-
 	/* DPLL0 not enabled (happens on early BIOS versions) */
 	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
 		/* enable DPLL0 */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3d9d1d3..e0476c3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1380,7 +1380,8 @@ void intel_psr_single_frame_update(struct drm_device *dev,
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
 void intel_power_domains_fini(struct drm_i915_private *);
-void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
+void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
+void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
 void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
 void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 63ad315..dce76ff 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1922,6 +1922,42 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
 	mutex_unlock(&power_domains->lock);
 }
 
+static void skl_display_core_init(struct drm_i915_private *dev_priv,
+				  bool resume)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	uint32_t val;
+
+	/* enable PCH reset handshake */
+	val = I915_READ(HSW_NDE_RSTWRN_OPT);
+	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
+
+	/* enable PG1 and Misc I/O */
+	mutex_lock(&power_domains->lock);
+	skl_pw1_misc_io_init(dev_priv);
+	mutex_unlock(&power_domains->lock);
+
+	if (!resume)
+		return;
+
+	skl_init_cdclk(dev_priv);
+
+	intel_csr_load_program(dev_priv);
+}
+
+static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+
+	skl_uninit_cdclk(dev_priv);
+
+	/* The spec doesn't call for removing the reset handshake flag */
+	/* disable PG1 and Misc I/O */
+	mutex_lock(&power_domains->lock);
+	skl_pw1_misc_io_fini(dev_priv);
+	mutex_unlock(&power_domains->lock);
+}
+
 static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_well *cmn_bc =
@@ -2044,14 +2080,16 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
  * This function initializes the hardware power domain state and enables all
  * power domains using intel_display_set_init_power().
  */
-void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
+void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 {
 	struct drm_device *dev = dev_priv->dev;
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
 	power_domains->initializing = true;
 
-	if (IS_CHERRYVIEW(dev)) {
+	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
+		skl_display_core_init(dev_priv, resume);
+	} else if (IS_CHERRYVIEW(dev)) {
 		mutex_lock(&power_domains->lock);
 		chv_phy_control_init(dev_priv);
 		mutex_unlock(&power_domains->lock);
@@ -2068,6 +2106,19 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * intel_power_domains_suspend - suspend power domain state
+ * @dev_priv: i915 device instance
+ *
+ * This function prepares the hardware power domain state before entering
+ * system suspend. It must be paired with intel_power_domains_init_hw().
+ */
+void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
+{
+	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+		skl_display_core_uninit(dev_priv);
+}
+
+/**
  * intel_aux_display_runtime_get - grab an auxiliary power domain reference
  * @dev_priv: i915 device instance
  *
-- 
2.1.4

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

Re: [PATCH 05/10] drm/i915/skl: init/uninit display core as part of the HW power domain state

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:14PM +0200, Imre Deak wrote:
> We need to initialize the display core part early, before initializing
> the rest of the display power state. This is also described in the bspec
> termed "Display initialization sequence". Atm we run this sequence
> during driver loading after power domain HW state initialization which
> is too late and during runtime suspend/resume which is unneeded and can
> interere with DMC functionality which handles HW resources toggled
> by this init/uninit sequence automatically. The init sequence must be
> run as the first step of HW power state initialization and during
> system resume. The uninit sequence must be run during system suspend.
> 
> To address the above move the init sequence to the initial HW power
> state setup and the uninit sequence to a new power domains suspend
> function called during system suspend.
> 
> As part of the init sequence we also have to reprogram the DMC firmware
> as it's lost across a system suspend/resume cycle.
> 
> After this change CD clock initialization during driver loading will
> happen only later after other dependent HW/SW parts are initialized,
> while during system resume it will get initialized as the last step of
> the init sequence. This distinction can be removed by some refactoring
> of platform independent parts. I left this refactoring out from this
> series since I didn't want to change non-SKL parts. This is a TODO for
> later.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_dma.c         |  2 +-
>  drivers/gpu/drm/i915/i915_drv.c         |  9 ++----
>  drivers/gpu/drm/i915/intel_display.c    | 11 -------
>  drivers/gpu/drm/i915/intel_drv.h        |  3 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 55 +++++++++++++++++++++++++++++++--
>  5 files changed, 59 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 71e1323..6e9337f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -396,7 +396,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	if (ret)
>  		goto cleanup_vga_switcheroo;
>  
> -	intel_power_domains_init_hw(dev_priv);
> +	intel_power_domains_init_hw(dev_priv, false);
>  
>  	intel_csr_ucode_init(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 858d58c..5a63f9a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -704,6 +704,8 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>  	struct drm_i915_private *dev_priv = drm_dev->dev_private;
>  	int ret;
>  
> +	intel_power_domains_suspend(dev_priv);
> +
>  	ret = intel_suspend_complete(dev_priv);
>  
>  	if (ret) {
> @@ -861,7 +863,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  		hsw_disable_pc8(dev_priv);
>  
>  	intel_uncore_sanitize(dev);
> -	intel_power_domains_init_hw(dev_priv);
> +	intel_power_domains_init_hw(dev_priv, true);
>  
>  	return ret;
>  }
> @@ -1070,8 +1072,6 @@ static int i915_pm_resume(struct device *dev)
>  
>  static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  {
> -	skl_uninit_cdclk(dev_priv);
> -
>  	if (dev_priv->csr.dmc_payload)
>  		skl_enable_dc6(dev_priv);
>  
> @@ -1122,9 +1122,6 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  	if (dev_priv->csr.dmc_payload)
>  		skl_disable_dc6(dev_priv);
>  
> -	skl_init_cdclk(dev_priv);
> -	intel_csr_load_program(dev_priv);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ef2ebcd..d9ed128 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5701,23 +5701,12 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  		if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
>  			DRM_ERROR("Couldn't disable DPLL0\n");
>  	}
> -
> -	/* disable PG1 and Misc I/O */
> -	skl_pw1_misc_io_fini(dev_priv);
>  }
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	u32 val;
>  	unsigned int required_vco;
>  
> -	/* enable PCH reset handshake */
> -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
> -
> -	/* enable PG1 and Misc I/O */
> -	skl_pw1_misc_io_init(dev_priv);
> -
>  	/* DPLL0 not enabled (happens on early BIOS versions) */
>  	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
>  		/* enable DPLL0 */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3d9d1d3..e0476c3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1380,7 +1380,8 @@ void intel_psr_single_frame_update(struct drm_device *dev,
>  /* intel_runtime_pm.c */
>  int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
> -void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
> +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
> +void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
>  void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
>  void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 63ad315..dce76ff 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1922,6 +1922,42 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&power_domains->lock);
>  }
>  
> +static void skl_display_core_init(struct drm_i915_private *dev_priv,
> +				  bool resume)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	uint32_t val;
> +
> +	/* enable PCH reset handshake */
> +	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
> +
> +	/* enable PG1 and Misc I/O */
> +	mutex_lock(&power_domains->lock);
> +	skl_pw1_misc_io_init(dev_priv);
> +	mutex_unlock(&power_domains->lock);
> +
> +	if (!resume)
> +		return;
> +
> +	skl_init_cdclk(dev_priv);
> +
> +	intel_csr_load_program(dev_priv);
> +}
> +
> +static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +
> +	skl_uninit_cdclk(dev_priv);
> +
> +	/* The spec doesn't call for removing the reset handshake flag */
> +	/* disable PG1 and Misc I/O */
> +	mutex_lock(&power_domains->lock);
> +	skl_pw1_misc_io_fini(dev_priv);
> +	mutex_unlock(&power_domains->lock);
> +}
> +
>  static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_well *cmn_bc =
> @@ -2044,14 +2080,16 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
>   * This function initializes the hardware power domain state and enables all
>   * power domains using intel_display_set_init_power().
>   */
> -void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
> +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  
>  	power_domains->initializing = true;
>  
> -	if (IS_CHERRYVIEW(dev)) {
> +	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> +		skl_display_core_init(dev_priv, resume);
> +	} else if (IS_CHERRYVIEW(dev)) {
>  		mutex_lock(&power_domains->lock);
>  		chv_phy_control_init(dev_priv);
>  		mutex_unlock(&power_domains->lock);
> @@ -2068,6 +2106,19 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
>  }
>  
>  /**
> + * intel_power_domains_suspend - suspend power domain state
> + * @dev_priv: i915 device instance
> + *
> + * This function prepares the hardware power domain state before entering
> + * system suspend. It must be paired with intel_power_domains_init_hw().
> + */
> +void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> +		skl_display_core_uninit(dev_priv);
> +}
> +
> +/**
>   * intel_aux_display_runtime_get - grab an auxiliary power domain reference
>   * @dev_priv: i915 device instance
>   *
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 05/10] drm/i915/skl: init/uninit display core as part of the HW power domain state

[Imre Deak]

We need to initialize the display core part early, before initializing
the rest of the display power state. This is also described in the bspec
termed "Display initialization sequence". Atm we run this sequence
during driver loading after power domain HW state initialization which
is too late and during runtime suspend/resume which is unneeded and can
interere with DMC functionality which handles HW resources toggled
by this init/uninit sequence automatically. The init sequence must be
run as the first step of HW power state initialization and during
system resume. The uninit sequence must be run during system suspend.

To address the above move the init sequence to the initial HW power
state setup and the uninit sequence to a new power domains suspend
function called during system suspend.

As part of the init sequence we also have to reprogram the DMC firmware
as it's lost across a system suspend/resume cycle.

After this change CD clock initialization during driver loading will
happen only later after other dependent HW/SW parts are initialized,
while during system resume it will get initialized as the last step of
the init sequence. This distinction can be removed by some refactoring
of platform independent parts. I left this refactoring out from this
series since I didn't want to change non-SKL parts. This is a TODO for
later.

v2:
- fix error path in i915_drm_suspend_late()
- don't try to re-program the DMC firmware if it failed to load

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         |  2 +-
 drivers/gpu/drm/i915/i915_drv.c         | 10 +++---
 drivers/gpu/drm/i915/intel_display.c    | 11 -------
 drivers/gpu/drm/i915/intel_drv.h        |  3 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 56 +++++++++++++++++++++++++++++++--
 5 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 731cf31..a7289be 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -395,7 +395,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_switcheroo;
 
-	intel_power_domains_init_hw(dev_priv);
+	intel_power_domains_init_hw(dev_priv, false);
 
 	intel_csr_ucode_init(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 858d58c..8ea1896 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -704,10 +704,13 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
 	struct drm_i915_private *dev_priv = drm_dev->dev_private;
 	int ret;
 
+	intel_power_domains_suspend(dev_priv);
+
 	ret = intel_suspend_complete(dev_priv);
 
 	if (ret) {
 		DRM_ERROR("Suspend complete failed: %d\n", ret);
+		intel_power_domains_init_hw(dev_priv, true);
 
 		return ret;
 	}
@@ -861,7 +864,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 		hsw_disable_pc8(dev_priv);
 
 	intel_uncore_sanitize(dev);
-	intel_power_domains_init_hw(dev_priv);
+	intel_power_domains_init_hw(dev_priv, true);
 
 	return ret;
 }
@@ -1070,8 +1073,6 @@ static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
-	skl_uninit_cdclk(dev_priv);
-
 	if (dev_priv->csr.dmc_payload)
 		skl_enable_dc6(dev_priv);
 
@@ -1122,9 +1123,6 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 	if (dev_priv->csr.dmc_payload)
 		skl_disable_dc6(dev_priv);
 
-	skl_init_cdclk(dev_priv);
-	intel_csr_load_program(dev_priv);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2f0e44e..2e1e6a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5727,23 +5727,12 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 		if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
 			DRM_ERROR("Couldn't disable DPLL0\n");
 	}
-
-	/* disable PG1 and Misc I/O */
-	skl_pw1_misc_io_fini(dev_priv);
 }
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
 {
-	u32 val;
 	unsigned int required_vco;
 
-	/* enable PCH reset handshake */
-	val = I915_READ(HSW_NDE_RSTWRN_OPT);
-	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
-
-	/* enable PG1 and Misc I/O */
-	skl_pw1_misc_io_init(dev_priv);
-
 	/* DPLL0 not enabled (happens on early BIOS versions) */
 	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
 		/* enable DPLL0 */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6616378..91ec8ac 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1410,7 +1410,8 @@ void intel_psr_single_frame_update(struct drm_device *dev,
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
 void intel_power_domains_fini(struct drm_i915_private *);
-void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
+void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
+void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
 void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
 void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f76c214..266ba3c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1923,6 +1923,43 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
 	mutex_unlock(&power_domains->lock);
 }
 
+static void skl_display_core_init(struct drm_i915_private *dev_priv,
+				  bool resume)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	uint32_t val;
+
+	/* enable PCH reset handshake */
+	val = I915_READ(HSW_NDE_RSTWRN_OPT);
+	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
+
+	/* enable PG1 and Misc I/O */
+	mutex_lock(&power_domains->lock);
+	skl_pw1_misc_io_init(dev_priv);
+	mutex_unlock(&power_domains->lock);
+
+	if (!resume)
+		return;
+
+	skl_init_cdclk(dev_priv);
+
+	if (dev_priv->csr.dmc_payload)
+		intel_csr_load_program(dev_priv);
+}
+
+static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+
+	skl_uninit_cdclk(dev_priv);
+
+	/* The spec doesn't call for removing the reset handshake flag */
+	/* disable PG1 and Misc I/O */
+	mutex_lock(&power_domains->lock);
+	skl_pw1_misc_io_fini(dev_priv);
+	mutex_unlock(&power_domains->lock);
+}
+
 static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_well *cmn_bc =
@@ -2045,14 +2082,16 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
  * This function initializes the hardware power domain state and enables all
  * power domains using intel_display_set_init_power().
  */
-void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
+void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 {
 	struct drm_device *dev = dev_priv->dev;
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
 	power_domains->initializing = true;
 
-	if (IS_CHERRYVIEW(dev)) {
+	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
+		skl_display_core_init(dev_priv, resume);
+	} else if (IS_CHERRYVIEW(dev)) {
 		mutex_lock(&power_domains->lock);
 		chv_phy_control_init(dev_priv);
 		mutex_unlock(&power_domains->lock);
@@ -2069,6 +2108,19 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * intel_power_domains_suspend - suspend power domain state
+ * @dev_priv: i915 device instance
+ *
+ * This function prepares the hardware power domain state before entering
+ * system suspend. It must be paired with intel_power_domains_init_hw().
+ */
+void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
+{
+	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+		skl_display_core_uninit(dev_priv);
+}
+
+/**
  * intel_aux_display_runtime_get - grab an auxiliary power domain reference
  * @dev_priv: i915 device instance
  *
-- 
2.5.0

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

[PATCH 06/10] drm/i915/skl: don't toggle PW1 and MISC power wells on-demand

[Imre Deak]

With the DMC firmware installed we don't need to handle HW resources
that are handled automatically by the firmware. Besides beeing redundant
this can also interfere with the firmware, possibly getting it into a
broken/blocked state. The on-demand handling of PW1 was already half-way
removed, MISC IO was still handled in this way. After the last patch we
init/uninit these HW resources manually as part of the display core
init/uninit sequence, so we can now remove the on-demand handling for
these completely.

We still keep around the power wells (with no domains attached to them)
since the manual toggling during display core init/uninit happens via
the current API.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 36 +++++++++------------------------
 1 file changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index dce76ff..b9a0493 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -305,16 +305,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_AUDIO) |			\
 	BIT(POWER_DOMAIN_VGA) |				\
 	BIT(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
-	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
-	BIT(POWER_DOMAIN_PLLS) |			\
-	BIT(POWER_DOMAIN_PIPE_A) |			\
-	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
-	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
-	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
-	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
-	BIT(POWER_DOMAIN_AUX_A) |			\
-	BIT(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
@@ -332,18 +322,13 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
 	BIT(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
-	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
-	BIT(POWER_DOMAIN_PLLS) |			\
-	BIT(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
-	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
+	(POWER_DOMAIN_MASK & ~(				\
 	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
 	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
 	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
 	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
-	SKL_DISPLAY_DDI_D_POWER_DOMAINS |		\
-	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
+	SKL_DISPLAY_DDI_D_POWER_DOMAINS)) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
@@ -662,14 +647,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 		}
 	} else {
 		if (enable_requested) {
-			if (IS_SKYLAKE(dev) &&
-				(power_well->data == SKL_DISP_PW_1))
-				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
-			else {
-				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
-				POSTING_READ(HSW_PWR_WELL_DRIVER);
-				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
-			}
+			I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
+			POSTING_READ(HSW_PWR_WELL_DRIVER);
+			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
 
 			if (GEN9_ENABLE_DC5(dev) &&
 				power_well->data == SKL_DISP_PW_2)
@@ -1741,13 +1721,15 @@ static struct i915_power_well skl_power_wells[] = {
 	},
 	{
 		.name = "power well 1",
-		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
+		/* Handled by the DMC firmware */
+		.domains = 0,
 		.ops = &skl_power_well_ops,
 		.data = SKL_DISP_PW_1,
 	},
 	{
 		.name = "MISC IO power well",
-		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
+		/* Handled by the DMC firmware */
+		.domains = 0,
 		.ops = &skl_power_well_ops,
 		.data = SKL_DISP_PW_MISC_IO,
 	},
-- 
2.1.4

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

Re: [PATCH 06/10] drm/i915/skl: don't toggle PW1 and MISC power wells on-demand

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:15PM +0200, Imre Deak wrote:
> With the DMC firmware installed we don't need to handle HW resources
> that are handled automatically by the firmware. Besides beeing redundant
> this can also interfere with the firmware, possibly getting it into a
> broken/blocked state. The on-demand handling of PW1 was already half-way
> removed, MISC IO was still handled in this way. After the last patch we
> init/uninit these HW resources manually as part of the display core
> init/uninit sequence, so we can now remove the on-demand handling for
> these completely.
> 
> We still keep around the power wells (with no domains attached to them)
> since the manual toggling during display core init/uninit happens via
> the current API.

Did the "don't touch PW1 and Misc IO" also apply to BXT DMC? Need to make sure
we catch all of these fixes/changes in a BXT follow-up series later on.

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 36 +++++++++------------------------
>  1 file changed, 9 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index dce76ff..b9a0493 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -305,16 +305,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_AUDIO) |			\
>  	BIT(POWER_DOMAIN_VGA) |				\
>  	BIT(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
> -	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> -	BIT(POWER_DOMAIN_PLLS) |			\
> -	BIT(POWER_DOMAIN_PIPE_A) |			\
> -	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> -	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> -	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> -	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> -	BIT(POWER_DOMAIN_AUX_A) |			\
> -	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> @@ -332,18 +322,13 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
>  	BIT(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
> -	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
> -	BIT(POWER_DOMAIN_PLLS) |			\
> -	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
> -	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
> +	(POWER_DOMAIN_MASK & ~(				\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
>  	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
>  	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
>  	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
> -	SKL_DISPLAY_DDI_D_POWER_DOMAINS |		\
> -	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
> +	SKL_DISPLAY_DDI_D_POWER_DOMAINS)) |		\
>  	BIT(POWER_DOMAIN_INIT))
>  
>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
> @@ -662,14 +647,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  		}
>  	} else {
>  		if (enable_requested) {
> -			if (IS_SKYLAKE(dev) &&
> -				(power_well->data == SKL_DISP_PW_1))
> -				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
> -			else {
> -				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
> -				POSTING_READ(HSW_PWR_WELL_DRIVER);
> -				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
> -			}
> +			I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
> +			POSTING_READ(HSW_PWR_WELL_DRIVER);
> +			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>  
>  			if (GEN9_ENABLE_DC5(dev) &&
>  				power_well->data == SKL_DISP_PW_2)
> @@ -1741,13 +1721,15 @@ static struct i915_power_well skl_power_wells[] = {
>  	},
>  	{
>  		.name = "power well 1",
> -		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> +		/* Handled by the DMC firmware */
> +		.domains = 0,
>  		.ops = &skl_power_well_ops,
>  		.data = SKL_DISP_PW_1,
>  	},
>  	{
>  		.name = "MISC IO power well",
> -		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> +		/* Handled by the DMC firmware */
> +		.domains = 0,
>  		.ops = &skl_power_well_ops,
>  		.data = SKL_DISP_PW_MISC_IO,
>  	},
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/10] drm/i915/gen9: simplify DC toggling code

[Imre Deak]

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 66 ++++++++++++++-------------------
 2 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c103f8d..e6d88f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7491,6 +7491,7 @@ enum skl_disp_power_wells {
 
 /* GEN9 DC */
 #define DC_STATE_EN			0x45504
+#define  DC_STATE_DISABLE		0
 #define  DC_STATE_EN_UPTO_DC5		(1<<0)
 #define  DC_STATE_EN_DC9		(1<<3)
 #define  DC_STATE_EN_UPTO_DC6		(2<<0)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b9a0493..f5fb003 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -401,32 +401,43 @@ static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
 	  */
 }
 
+static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
+{
+	uint32_t val;
+	uint32_t mask;
+
+	mask = DC_STATE_EN_UPTO_DC5;
+	if (IS_BROXTON(dev_priv))
+		mask |= DC_STATE_EN_DC9;
+	else
+		mask |= DC_STATE_EN_UPTO_DC6;
+
+	WARN_ON_ONCE(state & ~mask);
+
+	val = I915_READ(DC_STATE_EN);
+	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n", val & mask, state);
+	val &= ~mask;
+	val |= state;
+	I915_WRITE(DC_STATE_EN, val);
+	POSTING_READ(DC_STATE_EN);
+}
+
 void bxt_enable_dc9(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_enable_dc9(dev_priv);
 
 	DRM_DEBUG_KMS("Enabling DC9\n");
 
-	val = I915_READ(DC_STATE_EN);
-	val |= DC_STATE_EN_DC9;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
 }
 
 void bxt_disable_dc9(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_disable_dc9(dev_priv);
 
 	DRM_DEBUG_KMS("Disabling DC9\n");
 
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_DC9;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 }
 
 static void gen9_set_dc_state_debugmask_memory_up(
@@ -487,33 +498,22 @@ static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
 
 static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_enable_dc5(dev_priv);
 
 	DRM_DEBUG_KMS("Enabling DC5\n");
 
 	gen9_set_dc_state_debugmask_memory_up(dev_priv);
 
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
-	val |= DC_STATE_EN_UPTO_DC5;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
 }
 
 static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_disable_dc5(dev_priv);
 
 	DRM_DEBUG_KMS("Disabling DC5\n");
 
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_UPTO_DC5;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 }
 
 static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
@@ -545,33 +545,23 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
 
 void skl_enable_dc6(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_enable_dc6(dev_priv);
 
 	DRM_DEBUG_KMS("Enabling DC6\n");
 
 	gen9_set_dc_state_debugmask_memory_up(dev_priv);
 
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
-	val |= DC_STATE_EN_UPTO_DC6;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
+
 }
 
 void skl_disable_dc6(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-
 	assert_can_disable_dc6(dev_priv);
 
 	DRM_DEBUG_KMS("Disabling DC6\n");
 
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_UPTO_DC6;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 }
 
 static void skl_set_power_well(struct drm_i915_private *dev_priv,
-- 
2.1.4

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

Re: [PATCH 07/10] drm/i915/gen9: simplify DC toggling code

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:16PM +0200, Imre Deak wrote:
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 66 ++++++++++++++-------------------
>  2 files changed, 29 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c103f8d..e6d88f5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7491,6 +7491,7 @@ enum skl_disp_power_wells {
>  
>  /* GEN9 DC */
>  #define DC_STATE_EN			0x45504
> +#define  DC_STATE_DISABLE		0
>  #define  DC_STATE_EN_UPTO_DC5		(1<<0)
>  #define  DC_STATE_EN_DC9		(1<<3)
>  #define  DC_STATE_EN_UPTO_DC6		(2<<0)
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b9a0493..f5fb003 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -401,32 +401,43 @@ static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
>  	  */
>  }
>  
> +static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
> +{
> +	uint32_t val;
> +	uint32_t mask;
> +
> +	mask = DC_STATE_EN_UPTO_DC5;
> +	if (IS_BROXTON(dev_priv))
> +		mask |= DC_STATE_EN_DC9;
> +	else
> +		mask |= DC_STATE_EN_UPTO_DC6;
> +
> +	WARN_ON_ONCE(state & ~mask);
> +

I took the liberty to move gen9_set_dc_state_debugmask_memory_up() here in my
patch series. It'll get called if we try to enable DC5 or DC6 so we only have to
care about it at one place in the code. Perhaps we could even move it to
skl_display_core_init() if we do additional testing on when it needs to be
reset. Either way, not a biggie so let's ignore it for now.

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> +	val = I915_READ(DC_STATE_EN);
> +	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n", val & mask, state);
> +	val &= ~mask;
> +	val |= state;
> +	I915_WRITE(DC_STATE_EN, val);
> +	POSTING_READ(DC_STATE_EN);
> +}
> +
>  void bxt_enable_dc9(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_enable_dc9(dev_priv);
>  
>  	DRM_DEBUG_KMS("Enabling DC9\n");
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val |= DC_STATE_EN_DC9;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
>  }
>  
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_disable_dc9(dev_priv);
>  
>  	DRM_DEBUG_KMS("Disabling DC9\n");
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_DC9;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  }
>  
>  static void gen9_set_dc_state_debugmask_memory_up(
> @@ -487,33 +498,22 @@ static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
>  
>  static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_enable_dc5(dev_priv);
>  
>  	DRM_DEBUG_KMS("Enabling DC5\n");
>  
>  	gen9_set_dc_state_debugmask_memory_up(dev_priv);
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
> -	val |= DC_STATE_EN_UPTO_DC5;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
>  }
>  
>  static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_disable_dc5(dev_priv);
>  
>  	DRM_DEBUG_KMS("Disabling DC5\n");
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC5;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  }
>  
>  static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
> @@ -545,33 +545,23 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>  
>  void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_enable_dc6(dev_priv);
>  
>  	DRM_DEBUG_KMS("Enabling DC6\n");
>  
>  	gen9_set_dc_state_debugmask_memory_up(dev_priv);
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
> -	val |= DC_STATE_EN_UPTO_DC6;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
> +
>  }
>  
>  void skl_disable_dc6(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
>  	assert_can_disable_dc6(dev_priv);
>  
>  	DRM_DEBUG_KMS("Disabling DC6\n");
>  
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC6;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  }
>  
>  static void skl_set_power_well(struct drm_i915_private *dev_priv,
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/10] drm/i915/skl: disable DC states before display core init/uninit

[Imre Deak]

We need to disable the DC states during display core init to sanitize
the HW state we inherit from the BIOS. We need to disable it during
display core uninit too, since the power well framework will leave it
enabled (since we get to the display core uninit step with all power
domains disabled already).

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f5fb003..3d500e1d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1900,6 +1900,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	uint32_t val;
 
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
+
 	/* enable PCH reset handshake */
 	val = I915_READ(HSW_NDE_RSTWRN_OPT);
 	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
@@ -1921,6 +1923,8 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
+
 	skl_uninit_cdclk(dev_priv);
 
 	/* The spec doesn't call for removing the reset handshake flag */
-- 
2.1.4

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

Re: [PATCH 08/10] drm/i915/skl: disable DC states before display core init/uninit

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:17PM +0200, Imre Deak wrote:
> We need to disable the DC states during display core init to sanitize
> the HW state we inherit from the BIOS. We need to disable it during
> display core uninit too, since the power well framework will leave it
> enabled (since we get to the display core uninit step with all power
> domains disabled already).
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f5fb003..3d500e1d 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1900,6 +1900,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	uint32_t val;
>  
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
>  	/* enable PCH reset handshake */
>  	val = I915_READ(HSW_NDE_RSTWRN_OPT);
>  	I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE);
> @@ -1921,6 +1923,8 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
>  	skl_uninit_cdclk(dev_priv);
>  
>  	/* The spec doesn't call for removing the reset handshake flag */
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/10] drm/i915/skl: make sure LCPLL is disabled when uniniting CDCLK

[Imre Deak]

Suppressing LCPLL disabling was added to avoid interfering with the DMC
firmware. It is not needed any more since we uninit CDCLK now with the
DMC deactivated (DC states disabled). We also must disable it during system
suspend as part of the Bspec "Display uninit sequence".

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d9ed128..d0fec07 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5691,16 +5691,10 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
 		DRM_ERROR("DBuf power disable timeout\n");
 
-	/*
-	 * DMC assumes ownership of LCPLL and will get confused if we touch it.
-	 */
-	if (dev_priv->csr.dmc_payload) {
-		/* disable DPLL0 */
-		I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
-					~LCPLL_PLL_ENABLE);
-		if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
-			DRM_ERROR("Couldn't disable DPLL0\n");
-	}
+	/* disable DPLL0 */
+	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
+	if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
+		DRM_ERROR("Couldn't disable DPLL0\n");
 }
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
-- 
2.1.4

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

Re: [PATCH 09/10] drm/i915/skl: make sure LCPLL is disabled when uniniting CDCLK

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:18PM +0200, Imre Deak wrote:
> Suppressing LCPLL disabling was added to avoid interfering with the DMC
> firmware. It is not needed any more since we uninit CDCLK now with the
> DMC deactivated (DC states disabled). We also must disable it during system
> suspend as part of the Bspec "Display uninit sequence".
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d9ed128..d0fec07 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5691,16 +5691,10 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
>  		DRM_ERROR("DBuf power disable timeout\n");
>  
> -	/*
> -	 * DMC assumes ownership of LCPLL and will get confused if we touch it.
> -	 */
> -	if (dev_priv->csr.dmc_payload) {
> -		/* disable DPLL0 */
> -		I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> -					~LCPLL_PLL_ENABLE);
> -		if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> -			DRM_ERROR("Couldn't disable DPLL0\n");
> -	}
> +	/* disable DPLL0 */
> +	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> +	if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> +		DRM_ERROR("Couldn't disable DPLL0\n");
>  }
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/10] drm/i915/skl: remove redundant DDI/IRQ reinitialization during PW1 enabling

[Imre Deak]

We don't need to reinit DDI and IRQs during PW1 enabling any more, since
we don't toggle PW1 on-demand any more. We enable PW1 only as part of
the display core init sequence and after this we initialize both DDI and
IRQs later in the init sequence. So remove these init steps from the
power well code.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3d500e1d..5a36dd5 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -244,12 +244,6 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
 		gen8_irq_power_well_post_enable(dev_priv,
 						1 << PIPE_C | 1 << PIPE_B);
 	}
-
-	if (power_well->data == SKL_DISP_PW_1) {
-		if (!dev_priv->power_domains.initializing)
-			intel_prepare_ddi(dev);
-		gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
-	}
 }
 
 static void hsw_set_power_well(struct drm_i915_private *dev_priv,
-- 
2.1.4

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

Re: [PATCH 10/10] drm/i915/skl: remove redundant DDI/IRQ reinitialization during PW1 enabling

[Patrik Jakobsson]

On Wed, Nov 04, 2015 at 07:24:19PM +0200, Imre Deak wrote:
> We don't need to reinit DDI and IRQs during PW1 enabling any more, since
> we don't toggle PW1 on-demand any more. We enable PW1 only as part of
> the display core init sequence and after this we initialize both DDI and
> IRQs later in the init sequence. So remove these init steps from the
> power well code.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3d500e1d..5a36dd5 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -244,12 +244,6 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
>  		gen8_irq_power_well_post_enable(dev_priv,
>  						1 << PIPE_C | 1 << PIPE_B);
>  	}
> -
> -	if (power_well->data == SKL_DISP_PW_1) {
> -		if (!dev_priv->power_domains.initializing)
> -			intel_prepare_ddi(dev);
> -		gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
> -	}
>  }
>  
>  static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/10] drm/i915/skl: fix display core init/uninit sequence

[Imre Deak]

On ke, 2015-11-04 at 19:24 +0200, Imre Deak wrote:
> Atm we toggle HW resources handled automatically by the DMC firmware.
> This is redundant and also interferes with the firmware's functionality.
> This patchset fixes this and also an old existing issue leaving RPM
> disabled all the time (see Damien's patch).
> 
> The patchset depends on Mika's firmware version blacklisting/capture
> [1] and Animesh' firmware loading redesign [2] patchset. Both of these
> are reviewed now.
> 
> This patchset also relates to Patrik's DC5/DC6 rework patchset [3], but
> it's not dependent on it. After discussing with him on IRC I'd suggest the
> following merge order:
> 
> Patchset [1], patchset [2], Patrik's firmware programming fix from
> his patchset [4], this patchset, the rest of Patrik's patchset [3]. Feel
> free to suggest a different order.
> 
> I tested this on top of [1], [2], [4] on SKL-Y with eDP and DP outputs,
> DC5/6, PC9/10 residencies and S3/S4 suspend/resume seemed to work as
> expected. The basic D3 igt tests are also passing, as claimed by
> Damien's patch. 


I pushed the series to dinq, thanks for the review.

> 
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2015-October/078898.html
> 
> [2]
> http://lists.freedesktop.org/archives/intel-gfx/2015-October/079041.html
> 
> [3]
> http://lists.freedesktop.org/archives/intel-gfx/2015-November/079343.html
> 
> [4]
> http://lists.freedesktop.org/archives/intel-gfx/2015-November/079349.html
> 
> 
> Damien Lespiau (1):
>   drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini
>     sequences
> 
> Imre Deak (9):
>   drm/i915: fix the power well ID for always on wells
>   drm/i915: fix lookup_power_well for power wells without any domain
>   drm/i915: rename intel_power_domains_resume to *_sync_hw
>   drm/i915/skl: init/uninit display core as part of the HW power domain
>     state
>   drm/i915/skl: don't toggle PW1 and MISC power wells on-demand
>   drm/i915/gen9: simplify DC toggling code
>   drm/i915/skl: disable DC states before display core init/uninit
>   drm/i915/skl: make sure LCPLL is disabled when uniniting CDCLK
>   drm/i915/skl: remove redundant DDI/IRQ reinitialization during PW1
>     enabling
> 
>  drivers/gpu/drm/i915/i915_dma.c         |   2 +-
>  drivers/gpu/drm/i915/i915_drv.c         |   9 +-
>  drivers/gpu/drm/i915/i915_reg.h         |   5 +-
>  drivers/gpu/drm/i915/intel_ddi.c        |   4 +-
>  drivers/gpu/drm/i915/intel_display.c    |  24 +---
>  drivers/gpu/drm/i915/intel_drv.h        |   5 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 208 ++++++++++++++++++++------------
>  7 files changed, 149 insertions(+), 108 deletions(-)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx