All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
@ 2015-12-30 22:56 Matt Roper
  2015-12-31  3:01 ` Kannan, Vandana
  2015-12-31  7:43 ` ✗ failure: Fi.CI.BAT Patchwork
  0 siblings, 2 replies; 7+ messages in thread
From: Matt Roper @ 2015-12-30 22:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: drm-intel-fixes

Broxton-specific panel power sequencing was added in commit

        commit b0a08bec96318be54db97c3f0b9e37b52561f9ea
        Author: Vandana Kannan <vandana.kannan@intel.com>
        Date:   Thu Jun 18 11:00:55 2015 +0530

            drm/i915/bxt: eDP Panel Power sequencing

As noted in that commit, Broxton has two sets of registers (and we're
supposed to read which to use from the VBT, although we're punting on
that part at the moment) and also doesn't have a divisor register.
Although we take the BXT-specific details into account at initial system
init, we fail to save/restore the appropriate registers around system
suspend/resume, which leads to unclaimed register warnings at the
moment, and may also fail to save the proper registers once we start
actually paying attention to the VBT.

Cc: Vandana Kannan <vandana.kannan@intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_suspend.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index bf582fe..61bec8e 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -44,7 +44,18 @@ static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(dev)) {
+		/*
+		 * TODO: BXT has 2 sets of PPS registers.
+		 * Correct Register for Broxton need to be identified
+		 * using VBT. hardcoding for now.
+		 *
+		 * There's also no divisor register on Broxton.
+		 */
+		dev_priv->regfile.savePP_CONTROL = I915_READ(BXT_PP_CONTROL(0));
+		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(BXT_PP_ON_DELAYS(0));
+		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(BXT_PP_OFF_DELAYS(0));
+	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
 		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
 		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
@@ -83,7 +94,16 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(dev)) {
+		/*
+		 * TODO: BXT has 2 sets of PPS registers.
+		 * Correct Register for Broxton need to be identified
+		 * using VBT. hardcoding for now
+		 */
+		I915_WRITE(BXT_PP_ON_DELAYS(0), dev_priv->regfile.savePP_ON_DELAYS);
+		I915_WRITE(BXT_PP_OFF_DELAYS(0), dev_priv->regfile.savePP_OFF_DELAYS);
+		I915_WRITE(BXT_PP_CONTROL(0), dev_priv->regfile.savePP_CONTROL);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
 		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
 		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
  2015-12-30 22:56 [PATCH] drm/i915/bxt: Fix eDP panel power save/restore Matt Roper
@ 2015-12-31  3:01 ` Kannan, Vandana
  2015-12-31 16:51   ` Matt Roper
  2015-12-31  7:43 ` ✗ failure: Fi.CI.BAT Patchwork
  1 sibling, 1 reply; 7+ messages in thread
From: Kannan, Vandana @ 2015-12-31  3:01 UTC (permalink / raw)
  To: Matt Roper, intel-gfx, Jani Nikula; +Cc: drm-intel-fixes

When I submitted the PPS patch in April, I got an input from Jani to not 
make changes in i915_suspend.c as it was to become obsolete. Below mail 
for your reference.

Jani,
Does your initial comment still hold good?

On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
 > Changes based on future platform readiness patches related to
 > HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
 >
 > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
 > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
 > ---
 >  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
 >  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
 >  2 files changed, 6 insertions(+), 6 deletions(-)
 >
 > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
 > b/drivers/gpu/drm/i915/i915_suspend.c
 > index cf67f82..e91d637 100644
 > --- a/drivers/gpu/drm/i915/i915_suspend.c
 > +++ b/drivers/gpu/drm/i915/i915_suspend.c
 > @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
 >  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
 >
 >  	/* Panel power sequencer */
 > -	if (HAS_PCH_SPLIT(dev)) {
 > +	if (!HAS_GMCH_DISPLAY(dev)) {
 >  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
 >  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
 >  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
 > @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device 
*dev)
 >  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
 >
 >  	/* Panel power sequencer */
 > -	if (HAS_PCH_SPLIT(dev)) {
 > +	if (!HAS_GMCH_DISPLAY(dev)) {
 >  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
 >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
 >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);

I don't think you should touch i915_suspend.c at all. We're trying to 
get rid of this blind register save/restore, and make everything work in 
the encoder/connector code. Do note that we already skip this for 
vlv/chv power sequencer registers.

 > diff --git a/drivers/gpu/drm/i915/intel_dp.c
 > b/drivers/gpu/drm/i915/intel_dp.c index 937ba31..68e10c1 100644
 > --- a/drivers/gpu/drm/i915/intel_dp.c
 > +++ b/drivers/gpu/drm/i915/intel_dp.c
 > @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 > {
 >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 >
 > -	if (HAS_PCH_SPLIT(dev))
 > +	if (!HAS_GMCH_DISPLAY(dev))
 >  		return PCH_PP_CONTROL;
 >  	else
 >  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
 > @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 > {
 >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 >
 > -	if (HAS_PCH_SPLIT(dev))
 > +	if (!HAS_GMCH_DISPLAY(dev))
 >  		return PCH_PP_STATUS;
 >  	else
 >  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
 > @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct 
drm_device *dev,
 >  	if (final->t11_t12 != 0)
 >  		return;
 >
 > -	if (HAS_PCH_SPLIT(dev)) {
 > +	if (!HAS_GMCH_DISPLAY(dev)) {
 >  		pp_ctrl_reg = PCH_PP_CONTROL;
 >  		pp_on_reg = PCH_PP_ON_DELAYS;
 >  		pp_off_reg = PCH_PP_OFF_DELAYS;
 > @@ -5063,7 +5063,7 @@
 > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 >
 >  	lockdep_assert_held(&dev_priv->pps_mutex);
 >
 > -	if (HAS_PCH_SPLIT(dev)) {
 > +	if (!HAS_GMCH_DISPLAY(dev)) {
 >  		pp_on_reg = PCH_PP_ON_DELAYS;
 >  		pp_off_reg = PCH_PP_OFF_DELAYS;
 >  		pp_div_reg = PCH_PP_DIVISOR;

For the rest, I'd like you to do what I suggested in my reply to patch 
3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of these, with
BXT_PP_WHATEVER(0) as the register. Later on, we can add code to choose 
the power sequencer, a bit similar to what vlv/chv do now (except there 
it's based on pipe).

BR,
Jani.

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

--
Jani Nikula, Intel Open Source Technology Center

- Vandana


On 12/31/2015 4:26 AM, Matt Roper wrote:
> Broxton-specific panel power sequencing was added in commit
>
>          commit b0a08bec96318be54db97c3f0b9e37b52561f9ea
>          Author: Vandana Kannan <vandana.kannan@intel.com>
>          Date:   Thu Jun 18 11:00:55 2015 +0530
>
>              drm/i915/bxt: eDP Panel Power sequencing
>
> As noted in that commit, Broxton has two sets of registers (and we're
> supposed to read which to use from the VBT, although we're punting on
> that part at the moment) and also doesn't have a divisor register.
> Although we take the BXT-specific details into account at initial system
> init, we fail to save/restore the appropriate registers around system
> suspend/resume, which leads to unclaimed register warnings at the
> moment, and may also fail to save the proper registers once we start
> actually paying attention to the VBT.
>
> Cc: Vandana Kannan <vandana.kannan@intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_suspend.c | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index bf582fe..61bec8e 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,18 @@ static void i915_save_display(struct drm_device *dev)
>   		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>
>   	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(dev)) {
> +		/*
> +		 * TODO: BXT has 2 sets of PPS registers.
> +		 * Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now.
> +		 *
> +		 * There's also no divisor register on Broxton.
> +		 */
> +		dev_priv->regfile.savePP_CONTROL = I915_READ(BXT_PP_CONTROL(0));
> +		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(BXT_PP_ON_DELAYS(0));
> +		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(BXT_PP_OFF_DELAYS(0));
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>   		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>   		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> @@ -83,7 +94,16 @@ static void i915_restore_display(struct drm_device *dev)
>   		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>
>   	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(dev)) {
> +		/*
> +		 * TODO: BXT has 2 sets of PPS registers.
> +		 * Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now
> +		 */
> +		I915_WRITE(BXT_PP_ON_DELAYS(0), dev_priv->regfile.savePP_ON_DELAYS);
> +		I915_WRITE(BXT_PP_OFF_DELAYS(0), dev_priv->regfile.savePP_OFF_DELAYS);
> +		I915_WRITE(BXT_PP_CONTROL(0), dev_priv->regfile.savePP_CONTROL);
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>   		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>   		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* ✗ failure: Fi.CI.BAT
  2015-12-30 22:56 [PATCH] drm/i915/bxt: Fix eDP panel power save/restore Matt Roper
  2015-12-31  3:01 ` Kannan, Vandana
@ 2015-12-31  7:43 ` Patchwork
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2015-12-31  7:43 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Summary ==

Built on 79686f613b3955a4ed09cee936e7f70ec4e61b67 drm-intel-nightly: 2015y-12m-30d-11h-59m-54s UTC integration manifest

Test gem_ctx_param_basic:
        Subgroup basic:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup invalid-param-set:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_mmap_gtt:
        Subgroup basic-write:
                pass       -> DMESG-WARN (skl-i7k-2)
Test gem_storedw_loop:
        Subgroup basic-render:
                dmesg-warn -> PASS       (skl-i5k-2)
Test kms_addfb_basic:
        Subgroup too-wide:
                pass       -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (bsw-nuc-2)
                dmesg-warn -> PASS       (hsw-xps12)
                dmesg-warn -> PASS       (hsw-brixbox)
                dmesg-warn -> PASS       (bdw-nuci7)
                dmesg-warn -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-FAIL (skl-i7k-2)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                dmesg-warn -> PASS       (snb-dellxps)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (byt-nuc)

bdw-nuci7        total:132  pass:122  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:121  dwarn:1   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
hsw-xps12        total:132  pass:126  dwarn:2   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:124  dwarn:3   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:119  dwarn:7   dfail:1   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:122  dwarn:1   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_970/

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
  2015-12-31  3:01 ` Kannan, Vandana
@ 2015-12-31 16:51   ` Matt Roper
  2016-01-02 13:41     ` Jani Nikula
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Roper @ 2015-12-31 16:51 UTC (permalink / raw)
  To: Kannan, Vandana; +Cc: intel-gfx, drm-intel-fixes

On Thu, Dec 31, 2015 at 08:31:45AM +0530, Kannan, Vandana wrote:
> When I submitted the PPS patch in April, I got an input from Jani to
> not make changes in i915_suspend.c as it was to become obsolete.
> Below mail for your reference.
> 
> Jani,
> Does your initial comment still hold good?

I don't think not touching i915_suspend.c at all is an option since that
means that the current code will try to read/write registers that simply
don't exist on the platform.  Based on Jani's comment, maybe we should
just be adding BXT to the list of platforms we don't do any PP
save/restore on?

        diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
        index 61bec8e..69d5965 100644
        --- a/drivers/gpu/drm/i915/i915_suspend.c
        +++ b/drivers/gpu/drm/i915/i915_suspend.c
        @@ -60,7 +60,7 @@ static void i915_save_display(struct drm_device *dev)
                        dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
                        dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
                        dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
        -       } else if (!IS_VALLEYVIEW(dev)) {
        +       } else if (!IS_VALLEYVIEW(dev) && !IS_BROXTON(dev)) {
                        dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
                        dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
                        dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);

I'm not really familiar with the panel power sequencing stuff, so I'll let you
and Jani make the call there.  I'm just looking at S3 on Broxton (which has all
kinds of problems right now), and figured I'd try to clear up a couple of the
unclaimed register warnings to make it more clear where the real S3 problems
lie.

Thanks.


Matt

> 
> On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> > Changes based on future platform readiness patches related to
> > HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
> >
> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> >  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
> > b/drivers/gpu/drm/i915/i915_suspend.c
> > index cf67f82..e91d637 100644
> > --- a/drivers/gpu/drm/i915/i915_suspend.c
> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
> > @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
> >  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
> >
> >  	/* Panel power sequencer */
> > -	if (HAS_PCH_SPLIT(dev)) {
> > +	if (!HAS_GMCH_DISPLAY(dev)) {
> >  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
> >  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
> >  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> > @@ -79,7 +79,7 @@ static void i915_restore_display(struct
> drm_device *dev)
> >  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
> >
> >  	/* Panel power sequencer */
> > -	if (HAS_PCH_SPLIT(dev)) {
> > +	if (!HAS_GMCH_DISPLAY(dev)) {
> >  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> 
> I don't think you should touch i915_suspend.c at all. We're trying
> to get rid of this blind register save/restore, and make everything
> work in the encoder/connector code. Do note that we already skip
> this for vlv/chv power sequencer registers.
> 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 937ba31..68e10c1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
> > {
> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > -	if (HAS_PCH_SPLIT(dev))
> > +	if (!HAS_GMCH_DISPLAY(dev))
> >  		return PCH_PP_CONTROL;
> >  	else
> >  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> > @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
> > {
> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > -	if (HAS_PCH_SPLIT(dev))
> > +	if (!HAS_GMCH_DISPLAY(dev))
> >  		return PCH_PP_STATUS;
> >  	else
> >  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> > @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct
> drm_device *dev,
> >  	if (final->t11_t12 != 0)
> >  		return;
> >
> > -	if (HAS_PCH_SPLIT(dev)) {
> > +	if (!HAS_GMCH_DISPLAY(dev)) {
> >  		pp_ctrl_reg = PCH_PP_CONTROL;
> >  		pp_on_reg = PCH_PP_ON_DELAYS;
> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
> > @@ -5063,7 +5063,7 @@
> > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> >
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > -	if (HAS_PCH_SPLIT(dev)) {
> > +	if (!HAS_GMCH_DISPLAY(dev)) {
> >  		pp_on_reg = PCH_PP_ON_DELAYS;
> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
> >  		pp_div_reg = PCH_PP_DIVISOR;
> 
> For the rest, I'd like you to do what I suggested in my reply to
> patch 3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of
> these, with
> BXT_PP_WHATEVER(0) as the register. Later on, we can add code to
> choose the power sequencer, a bit similar to what vlv/chv do now
> (except there it's based on pipe).
> 
> BR,
> Jani.
> 
> > --
> > 2.0.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Jani Nikula, Intel Open Source Technology Center
> 
> - Vandana
> 
> 
> On 12/31/2015 4:26 AM, Matt Roper wrote:
> >Broxton-specific panel power sequencing was added in commit
> >
> >         commit b0a08bec96318be54db97c3f0b9e37b52561f9ea
> >         Author: Vandana Kannan <vandana.kannan@intel.com>
> >         Date:   Thu Jun 18 11:00:55 2015 +0530
> >
> >             drm/i915/bxt: eDP Panel Power sequencing
> >
> >As noted in that commit, Broxton has two sets of registers (and we're
> >supposed to read which to use from the VBT, although we're punting on
> >that part at the moment) and also doesn't have a divisor register.
> >Although we take the BXT-specific details into account at initial system
> >init, we fail to save/restore the appropriate registers around system
> >suspend/resume, which leads to unclaimed register warnings at the
> >moment, and may also fail to save the proper registers once we start
> >actually paying attention to the VBT.
> >
> >Cc: Vandana Kannan <vandana.kannan@intel.com>
> >Cc: drm-intel-fixes@lists.freedesktop.org
> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_suspend.c | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> >index bf582fe..61bec8e 100644
> >--- a/drivers/gpu/drm/i915/i915_suspend.c
> >+++ b/drivers/gpu/drm/i915/i915_suspend.c
> >@@ -44,7 +44,18 @@ static void i915_save_display(struct drm_device *dev)
> >  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
> >
> >  	/* Panel power sequencer */
> >-	if (HAS_PCH_SPLIT(dev)) {
> >+	if (IS_BROXTON(dev)) {
> >+		/*
> >+		 * TODO: BXT has 2 sets of PPS registers.
> >+		 * Correct Register for Broxton need to be identified
> >+		 * using VBT. hardcoding for now.
> >+		 *
> >+		 * There's also no divisor register on Broxton.
> >+		 */
> >+		dev_priv->regfile.savePP_CONTROL = I915_READ(BXT_PP_CONTROL(0));
> >+		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(BXT_PP_ON_DELAYS(0));
> >+		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(BXT_PP_OFF_DELAYS(0));
> >+	} else if (HAS_PCH_SPLIT(dev)) {
> >  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
> >  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
> >  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> >@@ -83,7 +94,16 @@ static void i915_restore_display(struct drm_device *dev)
> >  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
> >
> >  	/* Panel power sequencer */
> >-	if (HAS_PCH_SPLIT(dev)) {
> >+	if (IS_BROXTON(dev)) {
> >+		/*
> >+		 * TODO: BXT has 2 sets of PPS registers.
> >+		 * Correct Register for Broxton need to be identified
> >+		 * using VBT. hardcoding for now
> >+		 */
> >+		I915_WRITE(BXT_PP_ON_DELAYS(0), dev_priv->regfile.savePP_ON_DELAYS);
> >+		I915_WRITE(BXT_PP_OFF_DELAYS(0), dev_priv->regfile.savePP_OFF_DELAYS);
> >+		I915_WRITE(BXT_PP_CONTROL(0), dev_priv->regfile.savePP_CONTROL);
> >+	} else if (HAS_PCH_SPLIT(dev)) {
> >  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> >

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
  2015-12-31 16:51   ` Matt Roper
@ 2016-01-02 13:41     ` Jani Nikula
  2016-01-06  8:12       ` Daniel Vetter
  0 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2016-01-02 13:41 UTC (permalink / raw)
  To: Matt Roper, Kannan, Vandana; +Cc: intel-gfx, drm-intel-fixes

On Thu, 31 Dec 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Thu, Dec 31, 2015 at 08:31:45AM +0530, Kannan, Vandana wrote:
>> When I submitted the PPS patch in April, I got an input from Jani to
>> not make changes in i915_suspend.c as it was to become obsolete.
>> Below mail for your reference.
>> 
>> Jani,
>> Does your initial comment still hold good?
>
> I don't think not touching i915_suspend.c at all is an option since that
> means that the current code will try to read/write registers that simply
> don't exist on the platform.  Based on Jani's comment, maybe we should
> just be adding BXT to the list of platforms we don't do any PP
> save/restore on?

Not touching the registers in i915_suspend.c for BXT was what I meant,
but probably failed at expressing it. The encoder/connector hooks should
cover this, and if not, should be updated to cover this. The big hammer
suspend/resume in i915_suspend.c is oblivious to most modeset state and
is unable to do anything clever.

BR,
Jani.


>
>         diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>         index 61bec8e..69d5965 100644
>         --- a/drivers/gpu/drm/i915/i915_suspend.c
>         +++ b/drivers/gpu/drm/i915/i915_suspend.c
>         @@ -60,7 +60,7 @@ static void i915_save_display(struct drm_device *dev)
>                         dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>                         dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
>                         dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
>         -       } else if (!IS_VALLEYVIEW(dev)) {
>         +       } else if (!IS_VALLEYVIEW(dev) && !IS_BROXTON(dev)) {
>                         dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
>                         dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
>                         dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
>
> I'm not really familiar with the panel power sequencing stuff, so I'll let you
> and Jani make the call there.  I'm just looking at S3 on Broxton (which has all
> kinds of problems right now), and figured I'd try to clear up a couple of the
> unclaimed register warnings to make it more clear where the real S3 problems
> lie.
>
> Thanks.
>
>
> Matt
>
>> 
>> On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
>> > Changes based on future platform readiness patches related to
>> > HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>> >
>> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>> >  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>> >  2 files changed, 6 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
>> > b/drivers/gpu/drm/i915/i915_suspend.c
>> > index cf67f82..e91d637 100644
>> > --- a/drivers/gpu/drm/i915/i915_suspend.c
>> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
>> > @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>> >  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>> >
>> >  	/* Panel power sequencer */
>> > -	if (HAS_PCH_SPLIT(dev)) {
>> > +	if (!HAS_GMCH_DISPLAY(dev)) {
>> >  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>> >  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>> >  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
>> > @@ -79,7 +79,7 @@ static void i915_restore_display(struct
>> drm_device *dev)
>> >  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>> >
>> >  	/* Panel power sequencer */
>> > -	if (HAS_PCH_SPLIT(dev)) {
>> > +	if (!HAS_GMCH_DISPLAY(dev)) {
>> >  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>> >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>> >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>> 
>> I don't think you should touch i915_suspend.c at all. We're trying
>> to get rid of this blind register save/restore, and make everything
>> work in the encoder/connector code. Do note that we already skip
>> this for vlv/chv power sequencer registers.
>> 
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > b/drivers/gpu/drm/i915/intel_dp.c index 937ba31..68e10c1 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>> > {
>> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> >
>> > -	if (HAS_PCH_SPLIT(dev))
>> > +	if (!HAS_GMCH_DISPLAY(dev))
>> >  		return PCH_PP_CONTROL;
>> >  	else
>> >  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> > @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>> > {
>> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> >
>> > -	if (HAS_PCH_SPLIT(dev))
>> > +	if (!HAS_GMCH_DISPLAY(dev))
>> >  		return PCH_PP_STATUS;
>> >  	else
>> >  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> > @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct
>> drm_device *dev,
>> >  	if (final->t11_t12 != 0)
>> >  		return;
>> >
>> > -	if (HAS_PCH_SPLIT(dev)) {
>> > +	if (!HAS_GMCH_DISPLAY(dev)) {
>> >  		pp_ctrl_reg = PCH_PP_CONTROL;
>> >  		pp_on_reg = PCH_PP_ON_DELAYS;
>> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
>> > @@ -5063,7 +5063,7 @@
>> > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>> >
>> >  	lockdep_assert_held(&dev_priv->pps_mutex);
>> >
>> > -	if (HAS_PCH_SPLIT(dev)) {
>> > +	if (!HAS_GMCH_DISPLAY(dev)) {
>> >  		pp_on_reg = PCH_PP_ON_DELAYS;
>> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
>> >  		pp_div_reg = PCH_PP_DIVISOR;
>> 
>> For the rest, I'd like you to do what I suggested in my reply to
>> patch 3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of
>> these, with
>> BXT_PP_WHATEVER(0) as the register. Later on, we can add code to
>> choose the power sequencer, a bit similar to what vlv/chv do now
>> (except there it's based on pipe).
>> 
>> BR,
>> Jani.
>> 
>> > --
>> > 2.0.1
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center
>> 
>> - Vandana
>> 
>> 
>> On 12/31/2015 4:26 AM, Matt Roper wrote:
>> >Broxton-specific panel power sequencing was added in commit
>> >
>> >         commit b0a08bec96318be54db97c3f0b9e37b52561f9ea
>> >         Author: Vandana Kannan <vandana.kannan@intel.com>
>> >         Date:   Thu Jun 18 11:00:55 2015 +0530
>> >
>> >             drm/i915/bxt: eDP Panel Power sequencing
>> >
>> >As noted in that commit, Broxton has two sets of registers (and we're
>> >supposed to read which to use from the VBT, although we're punting on
>> >that part at the moment) and also doesn't have a divisor register.
>> >Although we take the BXT-specific details into account at initial system
>> >init, we fail to save/restore the appropriate registers around system
>> >suspend/resume, which leads to unclaimed register warnings at the
>> >moment, and may also fail to save the proper registers once we start
>> >actually paying attention to the VBT.
>> >
>> >Cc: Vandana Kannan <vandana.kannan@intel.com>
>> >Cc: drm-intel-fixes@lists.freedesktop.org
>> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> >---
>> >  drivers/gpu/drm/i915/i915_suspend.c | 24 ++++++++++++++++++++++--
>> >  1 file changed, 22 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>> >index bf582fe..61bec8e 100644
>> >--- a/drivers/gpu/drm/i915/i915_suspend.c
>> >+++ b/drivers/gpu/drm/i915/i915_suspend.c
>> >@@ -44,7 +44,18 @@ static void i915_save_display(struct drm_device *dev)
>> >  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>> >
>> >  	/* Panel power sequencer */
>> >-	if (HAS_PCH_SPLIT(dev)) {
>> >+	if (IS_BROXTON(dev)) {
>> >+		/*
>> >+		 * TODO: BXT has 2 sets of PPS registers.
>> >+		 * Correct Register for Broxton need to be identified
>> >+		 * using VBT. hardcoding for now.
>> >+		 *
>> >+		 * There's also no divisor register on Broxton.
>> >+		 */
>> >+		dev_priv->regfile.savePP_CONTROL = I915_READ(BXT_PP_CONTROL(0));
>> >+		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(BXT_PP_ON_DELAYS(0));
>> >+		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(BXT_PP_OFF_DELAYS(0));
>> >+	} else if (HAS_PCH_SPLIT(dev)) {
>> >  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>> >  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>> >  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
>> >@@ -83,7 +94,16 @@ static void i915_restore_display(struct drm_device *dev)
>> >  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>> >
>> >  	/* Panel power sequencer */
>> >-	if (HAS_PCH_SPLIT(dev)) {
>> >+	if (IS_BROXTON(dev)) {
>> >+		/*
>> >+		 * TODO: BXT has 2 sets of PPS registers.
>> >+		 * Correct Register for Broxton need to be identified
>> >+		 * using VBT. hardcoding for now
>> >+		 */
>> >+		I915_WRITE(BXT_PP_ON_DELAYS(0), dev_priv->regfile.savePP_ON_DELAYS);
>> >+		I915_WRITE(BXT_PP_OFF_DELAYS(0), dev_priv->regfile.savePP_OFF_DELAYS);
>> >+		I915_WRITE(BXT_PP_CONTROL(0), dev_priv->regfile.savePP_CONTROL);
>> >+	} else if (HAS_PCH_SPLIT(dev)) {
>> >  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>> >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>> >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>> >

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
  2016-01-02 13:41     ` Jani Nikula
@ 2016-01-06  8:12       ` Daniel Vetter
  2016-01-07  8:37         ` Jani Nikula
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2016-01-06  8:12 UTC (permalink / raw)
  To: Jani Nikula; +Cc: drm-intel-fixes, intel-gfx

On Sat, Jan 02, 2016 at 03:41:29PM +0200, Jani Nikula wrote:
> On Thu, 31 Dec 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > On Thu, Dec 31, 2015 at 08:31:45AM +0530, Kannan, Vandana wrote:
> >> When I submitted the PPS patch in April, I got an input from Jani to
> >> not make changes in i915_suspend.c as it was to become obsolete.
> >> Below mail for your reference.
> >> 
> >> Jani,
> >> Does your initial comment still hold good?
> >
> > I don't think not touching i915_suspend.c at all is an option since that
> > means that the current code will try to read/write registers that simply
> > don't exist on the platform.  Based on Jani's comment, maybe we should
> > just be adding BXT to the list of platforms we don't do any PP
> > save/restore on?
> 
> Not touching the registers in i915_suspend.c for BXT was what I meant,
> but probably failed at expressing it. The encoder/connector hooks should
> cover this, and if not, should be updated to cover this. The big hammer
> suspend/resume in i915_suspend.c is oblivious to most modeset state and
> is unable to do anything clever.

Hm, I thought it died already? What's keeping the pp code in
i915_suspend.c on life-support?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
  2016-01-06  8:12       ` Daniel Vetter
@ 2016-01-07  8:37         ` Jani Nikula
  0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2016-01-07  8:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: drm-intel-fixes, intel-gfx

On Wed, 06 Jan 2016, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Jan 02, 2016 at 03:41:29PM +0200, Jani Nikula wrote:
>> On Thu, 31 Dec 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
>> > On Thu, Dec 31, 2015 at 08:31:45AM +0530, Kannan, Vandana wrote:
>> >> When I submitted the PPS patch in April, I got an input from Jani to
>> >> not make changes in i915_suspend.c as it was to become obsolete.
>> >> Below mail for your reference.
>> >> 
>> >> Jani,
>> >> Does your initial comment still hold good?
>> >
>> > I don't think not touching i915_suspend.c at all is an option since that
>> > means that the current code will try to read/write registers that simply
>> > don't exist on the platform.  Based on Jani's comment, maybe we should
>> > just be adding BXT to the list of platforms we don't do any PP
>> > save/restore on?
>> 
>> Not touching the registers in i915_suspend.c for BXT was what I meant,
>> but probably failed at expressing it. The encoder/connector hooks should
>> cover this, and if not, should be updated to cover this. The big hammer
>> suspend/resume in i915_suspend.c is oblivious to most modeset state and
>> is unable to do anything clever.
>
> Hm, I thought it died already? What's keeping the pp code in
> i915_suspend.c on life-support?

We just haven't added stuff for new platforms, but the old cruft is
still there for old platforms. Haven't been bold enought to nuke it.

BR,
Jani.


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

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-07  8:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-30 22:56 [PATCH] drm/i915/bxt: Fix eDP panel power save/restore Matt Roper
2015-12-31  3:01 ` Kannan, Vandana
2015-12-31 16:51   ` Matt Roper
2016-01-02 13:41     ` Jani Nikula
2016-01-06  8:12       ` Daniel Vetter
2016-01-07  8:37         ` Jani Nikula
2015-12-31  7:43 ` ✗ failure: Fi.CI.BAT Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.