* [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend
@ 2016-10-13 11:34 Imre Deak
2016-10-13 12:21 ` Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Imre Deak @ 2016-10-13 11:34 UTC (permalink / raw)
To: intel-gfx; +Cc: David Weinehall
Currently the display INIT power domain disabling/enabling happens in a
mismatched way in the suspend/resume_early hooks respectively. This can
leave display power wells incorrectly disabled in the resume hook if the
suspend sequence is aborted for some reason resulting in the
suspend/resume hooks getting called but the suspend_late/resume_early
hooks being skipped. In particular this change fixes "Unclaimed read
from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
to devices.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e9b3bfc..5adabac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device *dev)
dev_priv->suspend_count++;
- intel_display_set_init_power(dev_priv, false);
-
intel_csr_ucode_suspend(dev_priv);
out:
@@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
disable_rpm_wakeref_asserts(dev_priv);
+ intel_display_set_init_power(dev_priv, false);
+
fw_csr = !IS_BROXTON(dev_priv) &&
suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
/*
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-13 11:34 [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend Imre Deak
@ 2016-10-13 12:21 ` Jani Nikula
2016-10-13 16:59 ` Imre Deak
2016-10-13 12:43 ` Ville Syrjälä
2016-10-14 11:01 ` ✗ Fi.CI.BAT: warning for " Patchwork
2 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2016-10-13 12:21 UTC (permalink / raw)
To: Imre Deak, intel-gfx; +Cc: David Weinehall
On Thu, 13 Oct 2016, Imre Deak <imre.deak@intel.com> wrote:
> Currently the display INIT power domain disabling/enabling happens in a
> mismatched way in the suspend/resume_early hooks respectively. This can
> leave display power wells incorrectly disabled in the resume hook if the
> suspend sequence is aborted for some reason resulting in the
> suspend/resume hooks getting called but the suspend_late/resume_early
> hooks being skipped. In particular this change fixes "Unclaimed read
> from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
> intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
> to devices.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Fixes: ?
Cc: stable@vger.kernel.org ?
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9b3bfc..5adabac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>
> dev_priv->suspend_count++;
>
> - intel_display_set_init_power(dev_priv, false);
> -
> intel_csr_ucode_suspend(dev_priv);
>
> out:
> @@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>
> disable_rpm_wakeref_asserts(dev_priv);
>
> + intel_display_set_init_power(dev_priv, false);
> +
> fw_csr = !IS_BROXTON(dev_priv) &&
> suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
> /*
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-13 12:21 ` Jani Nikula
@ 2016-10-13 16:59 ` Imre Deak
0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-10-13 16:59 UTC (permalink / raw)
To: Jani Nikula, intel-gfx; +Cc: David Weinehall
On to, 2016-10-13 at 15:21 +0300, Jani Nikula wrote:
> On Thu, 13 Oct 2016, Imre Deak <imre.deak@intel.com> wrote:
> > Currently the display INIT power domain disabling/enabling happens in a
> > mismatched way in the suspend/resume_early hooks respectively. This can
> > leave display power wells incorrectly disabled in the resume hook if the
> > suspend sequence is aborted for some reason resulting in the
> > suspend/resume hooks getting called but the suspend_late/resume_early
> > hooks being skipped. In particular this change fixes "Unclaimed read
> > from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
> > intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
> > to devices.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: David Weinehall <david.weinehall@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> Fixes: ?
Fixes: 85e90679335f ("drm/i915: disable power wells on suspend")
> Cc: stable@vger.kernel.org ?
It matters only for an error path which is rare (another device needs
to fail suspending between its suspend and suspend_late steps) and
pm_test. So I'd say this isn't for fixes or stable.
--Imre
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index e9b3bfc..5adabac 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device
> > *dev)
> >
> > dev_priv->suspend_count++;
> >
> > - intel_display_set_init_power(dev_priv, false);
> > -
> > intel_csr_ucode_suspend(dev_priv);
> >
> > out:
> > @@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct
> > drm_device *dev, bool hibernation)
> >
> > disable_rpm_wakeref_asserts(dev_priv);
> >
> > + intel_display_set_init_power(dev_priv, false);
> > +
> > fw_csr = !IS_BROXTON(dev_priv) &&
> > suspend_to_idle(dev_priv) && dev_priv-
> > >csr.dmc_payload;
> > /*
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-13 11:34 [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend Imre Deak
2016-10-13 12:21 ` Jani Nikula
@ 2016-10-13 12:43 ` Ville Syrjälä
2016-10-13 13:18 ` Imre Deak
2016-10-14 11:01 ` ✗ Fi.CI.BAT: warning for " Patchwork
2 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2016-10-13 12:43 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx, David Weinehall
On Thu, Oct 13, 2016 at 02:34:06PM +0300, Imre Deak wrote:
> Currently the display INIT power domain disabling/enabling happens in a
> mismatched way in the suspend/resume_early hooks respectively. This can
> leave display power wells incorrectly disabled in the resume hook if the
> suspend sequence is aborted for some reason resulting in the
> suspend/resume hooks getting called but the suspend_late/resume_early
> hooks being skipped. In particular this change fixes "Unclaimed read
> from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
> intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
> to devices.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9b3bfc..5adabac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>
> dev_priv->suspend_count++;
>
> - intel_display_set_init_power(dev_priv, false);
> -
> intel_csr_ucode_suspend(dev_priv);
>
> out:
> @@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>
> disable_rpm_wakeref_asserts(dev_priv);
>
> + intel_display_set_init_power(dev_priv, false);
> +
Makes sense in most cases.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> fw_csr = !IS_BROXTON(dev_priv) &&
> suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
However this part is quite confusing. We do the init_power_put
unconditionally, but the init_power_get is done from the init_hw()
which is conditional on this supend_to_idle special casing. So it seems
quite assymmetrical, and I can no longer claim to understand it.
> /*
> --
> 2.5.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-13 12:43 ` Ville Syrjälä
@ 2016-10-13 13:18 ` Imre Deak
0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-10-13 13:18 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, David Weinehall
On to, 2016-10-13 at 15:43 +0300, Ville Syrjälä wrote:
> On Thu, Oct 13, 2016 at 02:34:06PM +0300, Imre Deak wrote:
> > Currently the display INIT power domain disabling/enabling happens in a
> > mismatched way in the suspend/resume_early hooks respectively. This can
> > leave display power wells incorrectly disabled in the resume hook if the
> > suspend sequence is aborted for some reason resulting in the
> > suspend/resume hooks getting called but the suspend_late/resume_early
> > hooks being skipped. In particular this change fixes "Unclaimed read
> > from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
> > intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
> > to devices.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: David Weinehall <david.weinehall@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index e9b3bfc..5adabac 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device *dev)
> >
> > dev_priv->suspend_count++;
> >
> > - intel_display_set_init_power(dev_priv, false);
> > -
> > intel_csr_ucode_suspend(dev_priv);
> >
> > out:
> > @@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> >
> > disable_rpm_wakeref_asserts(dev_priv);
> >
> > + intel_display_set_init_power(dev_priv, false);
> > +
>
> Makes sense in most cases.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> > fw_csr = !IS_BROXTON(dev_priv) &&
> > suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
>
> However this part is quite confusing. We do the init_power_put
> unconditionally, but the init_power_get is done from the init_hw()
> which is conditional on this supend_to_idle special casing. So it seems
> quite assymmetrical, and I can no longer claim to understand it.
Yep, that's a problem of its own for the suspend-to-idle case, enabling the
INIT domain should be also unconditional and happen from this function.
I suppose this happened to work so far since it only applies to SKL where
nothing depended on the INIT domain, everything takes its own domain ref.
Will look into fixing this.
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-13 11:34 [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend Imre Deak
2016-10-13 12:21 ` Jani Nikula
2016-10-13 12:43 ` Ville Syrjälä
@ 2016-10-14 11:01 ` Patchwork
2016-10-14 11:41 ` Imre Deak
2 siblings, 1 reply; 7+ messages in thread
From: Patchwork @ 2016-10-14 11:01 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix mismatched INIT power domain disabling during suspend
URL : https://patchwork.freedesktop.org/series/13723/
State : warning
== Summary ==
Series 13723v1 drm/i915: Fix mismatched INIT power domain disabling during suspend
https://patchwork.freedesktop.org/api/1.0/series/13723/revisions/1/mbox/
Test drv_module_reload_basic:
skip -> PASS (fi-skl-6770hq)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-a:
pass -> DMESG-WARN (fi-ilk-650)
fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-j1900 total:246 pass:212 dwarn:2 dfail:0 fail:1 skip:31
fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:1 skip:35
fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-hsw-4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-ilk-650 total:246 pass:183 dwarn:1 dfail:0 fail:2 skip:60
fi-ivb-3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-6700k total:246 pass:221 dwarn:1 dfail:0 fail:0 skip:24
fi-skl-6770hq total:246 pass:230 dwarn:1 dfail:0 fail:1 skip:14
fi-snb-2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
Results at /archive/results/CI_IGT_test/Patchwork_2715/
fc23e80caa1a394f92ca132b2d19ac926bd2c803 drm-intel-nightly: 2016y-10m-14d-09h-02m-51s UTC integration manifest
6a2e371 drm/i915: Fix mismatched INIT power domain disabling during suspend
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: ✗ Fi.CI.BAT: warning for drm/i915: Fix mismatched INIT power domain disabling during suspend
2016-10-14 11:01 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2016-10-14 11:41 ` Imre Deak
0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-10-14 11:41 UTC (permalink / raw)
To: intel-gfx, Ville Syrjälä
On pe, 2016-10-14 at 11:01 +0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: Fix mismatched INIT power domain disabling during
> suspend
> URL : https://patchwork.freedesktop.org/series/13723/
> State : warning
>
> == Summary ==
>
> Series 13723v1 drm/i915: Fix mismatched INIT power domain disabling
> during suspend
> https://patchwork.freedesktop.org/api/1.0/series/13723/revisions/1/mb
> ox/
>
> Test drv_module_reload_basic:
> skip -> PASS (fi-skl-6770hq)
> Test kms_pipe_crc_basic:
> Subgroup nonblocking-crc-pipe-a:
> pass -> DMESG-WARN (fi-ilk-650)
[ 469.277956] [drm:intel_dp_check_link_status [i915]] DP C: channel EQ not ok, retraining
[ 469.278371] [drm:intel_dp_set_signal_levels [i915]] Using signal levels 00000000
[ 469.278414] [drm:intel_dp_set_signal_levels [i915]] Using vswing level 0
[ 469.278455] [drm:intel_dp_set_signal_levels [i915]] Using pre-emphasis level 0
[ 469.278499] [drm:intel_dp_program_link_training_pattern [i915]] Using DP training pattern TPS1
[ 469.279490] [drm:intel_dp_set_signal_levels [i915]] Using signal levels 02000000
[ 469.279533] [drm:intel_dp_set_signal_levels [i915]] Using vswing level 1
[ 469.279575] [drm:intel_dp_set_signal_levels [i915]] Using pre-emphasis level 0
[ 469.280337] [drm:intel_dp_start_link_train [i915]] clock recovery OK
[ 469.280354] [drm:intel_dp_program_link_training_pattern [i915]] Using DP training pattern TPS2
[ 469.281372] [drm:intel_dp_start_link_train [i915]] Channel EQ done. DP Training successful
[ 469.281869] [drm:drm_edid_to_eld] ELD monitor LEN LT2452pwC
[ 469.281871] [drm:drm_edid_to_eld] ELD size 36, SAD count 1
[ 469.281915] [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
[ 469.281979] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun
This happened earlier on the same machine, Ville is looking into
suppressing the errors during link retraining.
Thanks for the review, pushed the patch to -dinq.
>
> fi-bdw-
> 5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15
> fi-bsw-
> n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
> fi-bxt-
> t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
> fi-byt-
> j1900 total:246 pass:212 dwarn:2 dfail:0 fail:1 skip:31
> fi-byt-
> n2820 total:246 pass:210 dwarn:0 dfail:0 fail:1 skip:35
> fi-hsw-
> 4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
> fi-hsw-
> 4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
> fi-ilk-
> 650 total:246 pass:183 dwarn:1 dfail:0 fail:2 skip:60
> fi-ivb-
> 3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
> fi-ivb-
> 3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
> fi-kbl-
> 7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
> fi-skl-
> 6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
> fi-skl-
> 6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
> fi-skl-
> 6700k total:246 pass:221 dwarn:1 dfail:0 fail:0 skip:24
> fi-skl-
> 6770hq total:246 pass:230 dwarn:1 dfail:0 fail:1 skip:14
> fi-snb-
> 2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
> fi-snb-
> 2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
>
> Results at /archive/results/CI_IGT_test/Patchwork_2715/
>
> fc23e80caa1a394f92ca132b2d19ac926bd2c803 drm-intel-nightly: 2016y-
> 10m-14d-09h-02m-51s UTC integration manifest
> 6a2e371 drm/i915: Fix mismatched INIT power domain disabling during
> suspend
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-10-14 11:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-13 11:34 [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend Imre Deak
2016-10-13 12:21 ` Jani Nikula
2016-10-13 16:59 ` Imre Deak
2016-10-13 12:43 ` Ville Syrjälä
2016-10-13 13:18 ` Imre Deak
2016-10-14 11:01 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-10-14 11:41 ` Imre Deak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox